]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
minor tkey-related fixups
authorEvan Hunt <each@isc.org>
Thu, 13 Apr 2023 08:22:09 +0000 (01:22 -0700)
committerEvan Hunt <each@isc.org>
Wed, 14 Jun 2023 08:14:38 +0000 (08:14 +0000)
- style fixes and general tidying-up in tkey.c
- remove the unused 'intoken' parameter from dns_tkey_buildgssquery()
- remove an unnecessary call to dns_tkeyctx_create() in ns_server_create()
  (the TKEY context that was created there would soon be destroyed and
  another one created when the configuration was loaded).

bin/nsupdate/nsupdate.c
lib/dns/include/dns/tkey.h
lib/dns/tkey.c
lib/ns/server.c

index f733682c16682e1a75eadecc74f44da0087202b0..724567ce4158411d080f68342e4c064b0720a8e2 100644 (file)
@@ -3080,8 +3080,8 @@ start_gssrequest(dns_name_t *primary) {
 
        /* Build first request. */
        context = GSS_C_NO_CONTEXT;
-       result = dns_tkey_buildgssquery(rmsg, keyname, servname, NULL, 0,
-                                       &context, gmctx, &err_message);
+       result = dns_tkey_buildgssquery(rmsg, keyname, servname, 0, &context,
+                                       gmctx, &err_message);
        if (result == ISC_R_FAILURE) {
                fprintf(stderr, "tkey query failed: %s\n",
                        err_message != NULL ? err_message : "unknown error");
index 7d8f2dc73577f9a0aeabae255543ca6e161a7f07..f1998beb1c9c5b1be69e9b9976fb71d16a8d1c53 100644 (file)
@@ -89,9 +89,9 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
 
 isc_result_t
 dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name,
-                      const dns_name_t *gname, isc_buffer_t *intoken,
-                      uint32_t lifetime, dns_gss_ctx_id_t *context,
-                      isc_mem_t *mctx, char **err_message);
+                      const dns_name_t *gname, uint32_t lifetime,
+                      dns_gss_ctx_id_t *context, isc_mem_t *mctx,
+                      char **err_message);
 /*%<
  *     Builds a query containing a TKEY that will generate a GSSAPI context.
  *     The key is requested to have the specified lifetime (in seconds).
index 83eafa71ff8a39e1e6949d1445a697f00c15b231..90b1876f47fc6d212b60ca8b831944997c812d22 100644 (file)
@@ -91,8 +91,8 @@ dns_tkeyctx_create(isc_mem_t *mctx, dns_tkeyctx_t **tctxp) {
 
 void
 dns_tkeyctx_destroy(dns_tkeyctx_t **tctxp) {
-       isc_mem_t *mctx;
-       dns_tkeyctx_t *tctx;
+       isc_mem_t *mctx = NULL;
+       dns_tkeyctx_t *tctx = NULL;
 
        REQUIRE(tctxp != NULL && *tctxp != NULL);
 
@@ -154,8 +154,8 @@ add_rdata_to_list(dns_message_t *msg, dns_name_t *name, dns_rdata_t *rdata,
 
 static void
 free_namelist(dns_message_t *msg, dns_namelist_t *namelist) {
-       dns_name_t *name;
-       dns_rdataset_t *set;
+       dns_name_t *name = NULL;
+       dns_rdataset_t *set = NULL;
 
        while (!ISC_LIST_EMPTY(*namelist)) {
                name = ISC_LIST_HEAD(*namelist);
@@ -179,8 +179,8 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
        isc_result_t result = ISC_R_SUCCESS;
        dst_key_t *dstkey = NULL;
        dns_tsigkey_t *tsigkey = NULL;
-       dns_fixedname_t fixed;
-       dns_name_t *principal;
+       dns_fixedname_t fprincipal;
+       dns_name_t *principal = dns_fixedname_initname(&fprincipal);
        isc_stdtime_t now = isc_stdtime_now();
        isc_region_t intoken;
        isc_buffer_t *outtoken = NULL;
@@ -208,20 +208,15 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
         * XXXDCL need to check for key expiry per 4.1.1
         * XXXDCL need a way to check fully established, perhaps w/key_flags
         */
-
-       intoken.base = tkeyin->key;
-       intoken.length = tkeyin->keylen;
-
        result = dns_tsigkey_find(&tsigkey, name, &tkeyin->algorithm, ring);
        if (result == ISC_R_SUCCESS) {
                gss_ctx = dst_key_getgssctx(tsigkey->key);
        }
 
-       principal = dns_fixedname_initname(&fixed);
-
        /*
         * Note that tctx->gsscred may be NULL if tctx->gssapi_keytab is set
         */
+       intoken = (isc_region_t){ tkeyin->key, tkeyin->keylen };
        result = dst_gssapi_acceptctx(tctx->gsscred, tctx->gssapi_keytab,
                                      &intoken, &outtoken, &gss_ctx, principal,
                                      tctx->mctx);
@@ -230,17 +225,16 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
                        dns_tsigkey_detach(&tsigkey);
                }
                tkeyout->error = dns_tsigerror_badkey;
-               tkey_log("process_gsstkey(): dns_tsigerror_badkey"); /* XXXSRA
-                                                                     */
+               tkey_log("process_gsstkey(): dns_tsigerror_badkey");
                return (ISC_R_SUCCESS);
        }
        if (result != DNS_R_CONTINUE && result != ISC_R_SUCCESS) {
                goto failure;
        }
+
        /*
         * XXXDCL Section 4.1.3: Limit GSS_S_CONTINUE_NEEDED to 10 times.
         */
-
        if (dns_name_countlabels(principal) == 0U) {
                if (tsigkey != NULL) {
                        dns_tsigkey_detach(&tsigkey);
@@ -277,7 +271,7 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
                tkeyout->expire = tsigkey->expire;
        }
 
-       if (outtoken) {
+       if (outtoken != NULL) {
                tkeyout->key = isc_mem_get(tkeyout->mctx,
                                           isc_buffer_usedlength(outtoken));
                tkeyout->keylen = isc_buffer_usedlength(outtoken);
@@ -290,10 +284,6 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin,
                memmove(tkeyout->key, tkeyin->key, tkeyin->keylen);
        }
 
-       tkeyout->error = dns_rcode_noerror;
-
-       tkey_log("process_gsstkey(): dns_tsigerror_noerror"); /* XXXSRA */
-
        /*
         * We found a TKEY to respond with.  If the request is not TSIG signed,
         * we need to make sure the response is signed (see RFC 3645, Section
@@ -312,18 +302,14 @@ failure:
        if (tsigkey != NULL) {
                dns_tsigkey_detach(&tsigkey);
        }
-
        if (dstkey != NULL) {
                dst_key_free(&dstkey);
        }
-
        if (outtoken != NULL) {
                isc_buffer_free(&outtoken);
        }
 
-       tkey_log("process_gsstkey(): %s", isc_result_totext(result)); /* XXXSRA
-                                                                      */
-
+       tkey_log("process_gsstkey(): %s", isc_result_totext(result));
        return (result);
 }
 
@@ -333,7 +319,7 @@ process_deletetkey(dns_name_t *signer, dns_name_t *name,
                   dns_tsigkeyring_t *ring) {
        isc_result_t result;
        dns_tsigkey_t *tsigkey = NULL;
-       const dns_name_t *identity;
+       const dns_name_t *identity = NULL;
 
        result = dns_tsigkey_find(&tsigkey, name, &tkeyin->algorithm, ring);
        if (result != ISC_R_SUCCESS) {
@@ -369,12 +355,13 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
                      dns_tsigkeyring_t *ring) {
        isc_result_t result = ISC_R_SUCCESS;
        dns_rdata_tkey_t tkeyin, tkeyout;
-       bool freetkeyin = false;
-       dns_name_t *qname, *name, *keyname, *signer, tsigner;
+       dns_name_t *qname = NULL, *name = NULL;
+       dns_name_t *keyname = NULL, *signer = NULL;
+       dns_name_t tsigner = DNS_NAME_INITEMPTY;
        dns_fixedname_t fkeyname;
-       dns_rdataset_t *tkeyset;
-       dns_rdata_t rdata;
-       dns_namelist_t namelist;
+       dns_rdataset_t *tkeyset = NULL;
+       dns_rdata_t rdata = DNS_RDATA_INIT;
+       dns_namelist_t namelist = ISC_LIST_INITIALIZER;
        char tkeyoutdata[512];
        isc_buffer_t tkeyoutbuf;
 
@@ -382,8 +369,6 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
        REQUIRE(tctx != NULL);
        REQUIRE(ring != NULL);
 
-       ISC_LIST_INIT(namelist);
-
        /*
         * Interpret the question section.
         */
@@ -392,14 +377,11 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
                return (DNS_R_FORMERR);
        }
 
-       qname = NULL;
        dns_message_currentname(msg, DNS_SECTION_QUESTION, &qname);
 
        /*
         * Look for a TKEY record that matches the question.
         */
-       tkeyset = NULL;
-       name = NULL;
        result = dns_message_findname(msg, DNS_SECTION_ADDITIONAL, qname,
                                      dns_rdatatype_tkey, 0, &name, &tkeyset);
        if (result != ISC_R_SUCCESS) {
@@ -408,16 +390,15 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
                         "matching the question");
                goto failure;
        }
+
        result = dns_rdataset_first(tkeyset);
        if (result != ISC_R_SUCCESS) {
                result = DNS_R_FORMERR;
                goto failure;
        }
-       dns_rdata_init(&rdata);
-       dns_rdataset_current(tkeyset, &rdata);
 
+       dns_rdataset_current(tkeyset, &rdata);
        RETERR(dns_rdata_tostruct(&rdata, &tkeyin, NULL));
-       freetkeyin = true;
 
        if (tkeyin.error != dns_rcode_noerror) {
                result = DNS_R_FORMERR;
@@ -428,37 +409,29 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
         * Before we go any farther, verify that the message was signed.
         * GSSAPI TKEY doesn't require a signature, the rest do.
         */
-       dns_name_init(&tsigner, NULL);
        result = dns_message_signer(msg, &tsigner);
-       if (result != ISC_R_SUCCESS) {
-               if (tkeyin.mode == DNS_TKEYMODE_GSSAPI &&
-                   result == ISC_R_NOTFOUND)
-               {
-                       signer = NULL;
-               } else {
-                       tkey_log("dns_tkey_processquery: query was not "
-                                "properly signed - rejecting");
-                       result = DNS_R_FORMERR;
-                       goto failure;
-               }
-       } else {
+       if (result == ISC_R_SUCCESS) {
                signer = &tsigner;
+       } else if (result != ISC_R_NOTFOUND ||
+                  tkeyin.mode != DNS_TKEYMODE_GSSAPI)
+       {
+               tkey_log("dns_tkey_processquery: query was not "
+                        "properly signed - rejecting");
+               result = DNS_R_FORMERR;
+               goto failure;
        }
 
-       tkeyout.common.rdclass = tkeyin.common.rdclass;
-       tkeyout.common.rdtype = tkeyin.common.rdtype;
-       ISC_LINK_INIT(&tkeyout.common, link);
-       tkeyout.mctx = msg->mctx;
+       tkeyout = (dns_rdata_tkey_t){
+               .common.rdclass = tkeyin.common.rdclass,
+               .common.rdtype = tkeyin.common.rdtype,
+               .common.link = ISC_LINK_INITIALIZER,
+               .mctx = msg->mctx,
+               .algorithm = DNS_NAME_INITEMPTY,
+               .mode = tkeyin.mode,
+       };
 
-       dns_name_init(&tkeyout.algorithm, NULL);
        dns_name_clone(&tkeyin.algorithm, &tkeyout.algorithm);
 
-       tkeyout.inception = tkeyout.expire = 0;
-       tkeyout.mode = tkeyin.mode;
-       tkeyout.error = 0;
-       tkeyout.keylen = tkeyout.otherlen = 0;
-       tkeyout.key = tkeyout.other = NULL;
-
        /*
         * A delete operation must have a fully specified key name.  If this
         * is not a delete, we do the following:
@@ -524,7 +497,6 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
                }
 
                result = dns_tsigkey_find(&tsigkey, keyname, NULL, ring);
-
                if (result == ISC_R_SUCCESS) {
                        tkeyout.error = dns_tsigerror_badname;
                        dns_tsigkey_detach(&tsigkey);
@@ -556,18 +528,12 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx,
        }
 
 failure_with_tkey:
-
        dns_rdata_init(&rdata);
        isc_buffer_init(&tkeyoutbuf, tkeyoutdata, sizeof(tkeyoutdata));
        result = dns_rdata_fromstruct(&rdata, tkeyout.common.rdclass,
                                      tkeyout.common.rdtype, &tkeyout,
                                      &tkeyoutbuf);
 
-       if (freetkeyin) {
-               dns_rdata_freestruct(&tkeyin);
-               freetkeyin = false;
-       }
-
        if (tkeyout.key != NULL) {
                isc_mem_put(tkeyout.mctx, tkeyout.key, tkeyout.keylen);
        }
@@ -593,10 +559,6 @@ failure_with_tkey:
        return (ISC_R_SUCCESS);
 
 failure:
-
-       if (freetkeyin) {
-               dns_rdata_freestruct(&tkeyin);
-       }
        if (!ISC_LIST_EMPTY(namelist)) {
                free_namelist(msg, &namelist);
        }
@@ -658,17 +620,15 @@ buildquery(dns_message_t *msg, const dns_name_t *name, dns_rdata_tkey_t *tkey) {
 
 isc_result_t
 dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name,
-                      const dns_name_t *gname, isc_buffer_t *intoken,
-                      uint32_t lifetime, dns_gss_ctx_id_t *context,
-                      isc_mem_t *mctx, char **err_message) {
+                      const dns_name_t *gname, uint32_t lifetime,
+                      dns_gss_ctx_id_t *context, isc_mem_t *mctx,
+                      char **err_message) {
        dns_rdata_tkey_t tkey;
        isc_result_t result;
        isc_stdtime_t now = isc_stdtime_now();
        isc_buffer_t token;
        unsigned char array[TEMP_BUFFER_SZ];
 
-       UNUSED(intoken);
-
        REQUIRE(msg != NULL);
        REQUIRE(name != NULL);
        REQUIRE(gname != NULL);
@@ -688,11 +648,11 @@ dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name,
                .common.link = ISC_LINK_INITIALIZER,
                .inception = now,
                .expire = now + lifetime,
+               .algorithm = DNS_NAME_INITEMPTY,
                .mode = DNS_TKEYMODE_GSSAPI,
                .key = isc_buffer_base(&token),
                .keylen = isc_buffer_usedlength(&token),
        };
-       dns_name_init(&tkey.algorithm, NULL);
        dns_name_clone(DNS_TSIG_GSSAPI_NAME, &tkey.algorithm);
 
        return (buildquery(msg, name, &tkey));
@@ -701,22 +661,24 @@ dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name,
 static isc_result_t
 find_tkey(dns_message_t *msg, dns_name_t **name, dns_rdata_t *rdata,
          int section) {
-       dns_rdataset_t *tkeyset;
        isc_result_t result;
 
        result = dns_message_firstname(msg, section);
        while (result == ISC_R_SUCCESS) {
-               *name = NULL;
-               dns_message_currentname(msg, section, name);
-               tkeyset = NULL;
-               result = dns_message_findtype(*name, dns_rdatatype_tkey, 0,
+               dns_rdataset_t *tkeyset = NULL;
+               dns_name_t *cur = NULL;
+
+               dns_message_currentname(msg, section, &cur);
+               result = dns_message_findtype(cur, dns_rdatatype_tkey, 0,
                                              &tkeyset);
                if (result == ISC_R_SUCCESS) {
                        result = dns_rdataset_first(tkeyset);
                        if (result != ISC_R_SUCCESS) {
-                               return (result);
+                               break;
                        }
+
                        dns_rdataset_current(tkeyset, rdata);
+                       *name = cur;
                        return (ISC_R_SUCCESS);
                }
                result = dns_message_nextname(msg, section);
@@ -732,22 +694,19 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
                      const dns_name_t *server, dns_gss_ctx_id_t *context,
                      dns_tsigkey_t **outkey, dns_tsigkeyring_t *ring,
                      char **err_message) {
+       isc_result_t result;
        dns_rdata_t rtkeyrdata = DNS_RDATA_INIT, qtkeyrdata = DNS_RDATA_INIT;
        dns_name_t *tkeyname = NULL;
        dns_rdata_tkey_t rtkey, qtkey, tkey;
        isc_buffer_t intoken, outtoken;
        dst_key_t *dstkey = NULL;
-       isc_result_t result;
        unsigned char array[TEMP_BUFFER_SZ];
-       bool freertkey = false;
        dns_tsigkey_t *tsigkey = NULL;
 
        REQUIRE(qmsg != NULL);
        REQUIRE(rmsg != NULL);
        REQUIRE(server != NULL);
-       if (outkey != NULL) {
-               REQUIRE(*outkey == NULL);
-       }
+       REQUIRE(outkey == NULL || *outkey == NULL);
 
        if (rmsg->rcode != dns_rcode_noerror) {
                return (dns_result_fromrcode(rmsg->rcode));
@@ -755,7 +714,6 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
 
        RETERR(find_tkey(rmsg, &tkeyname, &rtkeyrdata, DNS_SECTION_ANSWER));
        RETERR(dns_rdata_tostruct(&rtkeyrdata, &rtkey, NULL));
-       freertkey = true;
 
        RETERR(find_tkey(qmsg, &tkeyname, &qtkeyrdata, DNS_SECTION_ADDITIONAL));
        RETERR(dns_rdata_tostruct(&qtkeyrdata, &qtkey, NULL));
@@ -764,7 +722,7 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
            rtkey.mode != DNS_TKEYMODE_GSSAPI ||
            !dns_name_equal(&rtkey.algorithm, &qtkey.algorithm))
        {
-               tkey_log("dns_tkey_processdhresponse: tkey mode invalid "
+               tkey_log("dns_tkey_gssnegotiate: tkey mode invalid "
                         "or error set(4)");
                result = DNS_R_INVALIDTKEY;
                goto failure;
@@ -780,28 +738,20 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
        }
 
        if (result == DNS_R_CONTINUE) {
-               dns_fixedname_t fixed;
+               tkey = (dns_rdata_tkey_t){
+                       .common.rdclass = dns_rdataclass_any,
+                       .common.rdtype = dns_rdatatype_tkey,
+                       .common.link = ISC_LINK_INITIALIZER,
+                       .inception = qtkey.inception,
+                       .expire = qtkey.expire,
+                       .algorithm = DNS_NAME_INITEMPTY,
+                       .mode = DNS_TKEYMODE_GSSAPI,
+                       .key = isc_buffer_base(&outtoken),
+                       .keylen = isc_buffer_usedlength(&outtoken),
+               };
 
-               dns_fixedname_init(&fixed);
-               dns_name_copy(tkeyname, dns_fixedname_name(&fixed));
-               tkeyname = dns_fixedname_name(&fixed);
-
-               tkey.common.rdclass = dns_rdataclass_any;
-               tkey.common.rdtype = dns_rdatatype_tkey;
-               ISC_LINK_INIT(&tkey.common, link);
-               tkey.mctx = NULL;
-               dns_name_init(&tkey.algorithm, NULL);
                dns_name_clone(DNS_TSIG_GSSAPI_NAME, &tkey.algorithm);
 
-               tkey.inception = qtkey.inception;
-               tkey.expire = qtkey.expire;
-               tkey.mode = DNS_TKEYMODE_GSSAPI;
-               tkey.error = 0;
-               tkey.key = isc_buffer_base(&outtoken);
-               tkey.keylen = isc_buffer_usedlength(&outtoken);
-               tkey.other = NULL;
-               tkey.otherlen = 0;
-
                dns_message_reset(qmsg, DNS_MESSAGE_INTENTRENDER);
                RETERR(buildquery(qmsg, tkeyname, &tkey));
                return (DNS_R_CONTINUE);
@@ -827,19 +777,12 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg,
        }
 
        dst_key_free(&dstkey);
-       dns_rdata_freestruct(&rtkey);
        return (result);
 
 failure:
-       /*
-        * XXXSRA This probably leaks memory from qtkey.
-        */
        if (tsigkey != NULL) {
                dns_tsigkey_detach(&tsigkey);
        }
-       if (freertkey) {
-               dns_rdata_freestruct(&rtkey);
-       }
        if (dstkey != NULL) {
                dst_key_free(&dstkey);
        }
index 857de2121bb45641c114fb05e36cefa3b4d36b16..f8f8a82c0e6ba3def54dc12fea4135f058c792f3 100644 (file)
@@ -65,8 +65,6 @@ ns_server_create(isc_mem_t *mctx, ns_matchview_t matchingview,
        ISC_LIST_INIT(sctx->http_quotas);
        isc_mutex_init(&sctx->http_quotas_lock);
 
-       CHECKFATAL(dns_tkeyctx_create(mctx, &sctx->tkeyctx));
-
        CHECKFATAL(ns_stats_create(mctx, ns_statscounter_max, &sctx->nsstats));
 
        CHECKFATAL(dns_rdatatypestats_create(mctx, &sctx->rcvquerystats));