From f4084ff54378170d0e130ab139a5c87de8180267 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Apr 2023 01:22:09 -0700 Subject: [PATCH] minor tkey-related fixups - 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 | 4 +- lib/dns/include/dns/tkey.h | 6 +- lib/dns/tkey.c | 181 +++++++++++++------------------------ lib/ns/server.c | 2 - 4 files changed, 67 insertions(+), 126 deletions(-) diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index f733682c166..724567ce415 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -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"); diff --git a/lib/dns/include/dns/tkey.h b/lib/dns/include/dns/tkey.h index 7d8f2dc7357..f1998beb1c9 100644 --- a/lib/dns/include/dns/tkey.h +++ b/lib/dns/include/dns/tkey.h @@ -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). diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 83eafa71ff8..90b1876f47f 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -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); } diff --git a/lib/ns/server.c b/lib/ns/server.c index 857de2121bb..f8f8a82c0e6 100644 --- a/lib/ns/server.c +++ b/lib/ns/server.c @@ -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)); -- 2.47.3