From b1db1c14751919b3fb3c447b1d0716cfcec2af41 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 10 Apr 2023 23:46:10 -0700 Subject: [PATCH 01/11] minor tsig.c cleanups - style cleanups. - simplify the function parameters to dns_tsigkey_create(): + remove 'restored' and 'generated', they're only ever set to false. + remove 'creator' because it's only ever set to NULL. + remove 'inception' and 'expiry' because they're only ever set to (0, 0) or (now, now), and either way, this means "never expire". + remove 'ring' because we can just use dns_tsigkeyring_add() instead. - rename dns_keyring_restore() to dns_tsigkeyring_restore() to match the rest of the functions operating on dns_tsigkeyring objects. --- bin/dig/dighost.c | 3 +- bin/named/tsigconf.c | 28 ++++++------ bin/nsupdate/nsupdate.c | 6 +-- fuzz/dns_message_checksig.c | 9 +++- lib/dns/include/dns/tsig.h | 7 +-- lib/dns/tsig.c | 90 ++++++++++++++----------------------- lib/dns/view.c | 2 +- tests/dns/tsig_test.c | 5 ++- 8 files changed, 65 insertions(+), 85 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index d8323c6f66..c8db355a26 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -885,8 +885,7 @@ setup_text_key(void) { } result = dns_tsigkey_create(&keyname, hmacname, secretstore, - (int)secretsize, false, false, NULL, 0, 0, - mctx, NULL, &tsigkey); + (int)secretsize, mctx, &tsigkey); failure: if (result != ISC_R_SUCCESS) { printf(";; Couldn't create key %s: %s\n", keynametext, diff --git a/bin/named/tsigconf.c b/bin/named/tsigconf.c index 3333283bc7..550046275e 100644 --- a/bin/named/tsigconf.c +++ b/bin/named/tsigconf.c @@ -39,10 +39,7 @@ add_initial_keys(const cfg_obj_t *list, dns_tsig_keyring_t *ring, const char *keyid = NULL; unsigned char *secret = NULL; int secretalloc = 0; - int secretlen = 0; isc_result_t ret; - isc_stdtime_t now; - uint16_t bits; for (element = cfg_list_first(list); element != NULL; element = cfg_list_next(element)) @@ -50,12 +47,14 @@ add_initial_keys(const cfg_obj_t *list, dns_tsig_keyring_t *ring, const cfg_obj_t *algobj = NULL; const cfg_obj_t *secretobj = NULL; dns_name_t keyname; - const dns_name_t *alg; - const char *algstr; + const dns_name_t *alg = NULL; + const char *algstr = NULL; char keynamedata[1024]; isc_buffer_t keynamesrc, keynamebuf; - const char *secretstr; + const char *secretstr = NULL; isc_buffer_t secretbuf; + int secretlen = 0; + uint16_t bits; key = cfg_listelt_value(element); keyid = cfg_obj_asstring(cfg_map_getname(key)); @@ -104,13 +103,17 @@ add_initial_keys(const cfg_obj_t *list, dns_tsig_keyring_t *ring, } secretlen = isc_buffer_usedlength(&secretbuf); - now = isc_stdtime_now(); - ret = dns_tsigkey_create(&keyname, alg, secret, secretlen, - false, false, NULL, now, now, mctx, - ring, &tsigkey); + ret = dns_tsigkey_create(&keyname, alg, secret, secretlen, mctx, + &tsigkey); isc_mem_put(mctx, secret, secretalloc); secret = NULL; + if (ret == ISC_R_SUCCESS) { + ret = dns_tsigkeyring_add(ring, &keyname, tsigkey); + } if (ret != ISC_R_SUCCESS) { + if (tsigkey != NULL) { + dns_tsigkey_detach(&tsigkey); + } goto failure; } /* @@ -123,12 +126,11 @@ add_initial_keys(const cfg_obj_t *list, dns_tsig_keyring_t *ring, return (ISC_R_SUCCESS); failure: - cfg_obj_log(key, named_g_lctx, ISC_LOG_ERROR, - "configuring key '%s': %s", keyid, isc_result_totext(ret)); - if (secret != NULL) { isc_mem_put(mctx, secret, secretalloc); } + cfg_obj_log(key, named_g_lctx, ISC_LOG_ERROR, + "configuring key '%s': %s", keyid, isc_result_totext(ret)); return (ret); } diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 87affae84e..c539f10399 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -539,8 +539,7 @@ setup_keystr(void) { debug("keycreate"); result = dns_tsigkey_create(mykeyname, hmacname, secret, secretlen, - false, false, NULL, 0, 0, gmctx, NULL, - &tsigkey); + gmctx, &tsigkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "could not create key from %s: %s\n", keystr, isc_result_totext(result)); @@ -1703,8 +1702,7 @@ evaluate_key(char *cmdline) { dns_tsigkey_detach(&tsigkey); } result = dns_tsigkey_create(mykeyname, hmacname, secret, secretlen, - false, false, NULL, 0, 0, gmctx, NULL, - &tsigkey); + gmctx, &tsigkey); isc_mem_free(gmctx, secret); if (result != ISC_R_SUCCESS) { fprintf(stderr, "could not create key from %s %s: %s\n", diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index a4b9dd15ab..f60154dea4 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -265,13 +265,18 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { } result = dns_tsigkey_create(name, dns_tsig_hmacsha256_name, secret, - sizeof(secret), false, false, NULL, 0, 0, - mctx, ring, &tsigkey); + sizeof(secret), mctx, &tsigkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_tsigkey_create failed: %s\n", isc_result_totext(result)); return (1); } + result = dns_tsigkeyring_add(ring, name, tsigkey); + if (result != ISC_R_SUCCESS) { + fprintf(stderr, "dns_tsigkeyring_add failed: %s\n", + isc_result_totext(result)); + return (1); + } result = dns_name_fromstring(name, "sig0key", 0, NULL); if (result != ISC_R_SUCCESS) { diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index e8b10b3053..c9329c67d7 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -100,10 +100,7 @@ dns_tsigkey_identity(const dns_tsigkey_t *tsigkey); isc_result_t dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, - unsigned char *secret, int length, bool generated, - bool restored, const dns_name_t *creator, - isc_stdtime_t inception, isc_stdtime_t expire, - isc_mem_t *mctx, dns_tsig_keyring_t *ring, + unsigned char *secret, int length, isc_mem_t *mctx, dns_tsigkey_t **key); isc_result_t @@ -290,6 +287,6 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp); */ void -dns_keyring_restore(dns_tsig_keyring_t *ring, FILE *fp); +dns_tsigkeyring_restore(dns_tsig_keyring_t *ring, FILE *fp); ISC_LANG_ENDDECLS diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 15358fe834..cb453c5665 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -382,10 +382,8 @@ cleanup_ring(dns_tsig_keyring_t *ring) { dns_rbtnodechain_t chain; dns_name_t foundname; dns_fixedname_t fixedorigin; - dns_name_t *origin; + dns_name_t *origin = NULL; isc_stdtime_t now = isc_stdtime_now(); - dns_rbtnode_t *node; - dns_tsigkey_t *tkey; /* * Start up a new iterator each time. @@ -402,7 +400,9 @@ again: } for (;;) { - node = NULL; + dns_rbtnode_t *node = NULL; + dns_tsigkey_t *tkey = NULL; + dns_rbtnodechain_current(&chain, &foundname, origin, &node); tkey = node->data; if (tkey != NULL) { @@ -498,7 +498,7 @@ restore_key(dns_tsig_keyring_t *ring, isc_stdtime_t now, FILE *fp) { unsigned int inception, expire; int n; isc_buffer_t b; - dns_name_t *name, *creator, *algorithm; + dns_name_t *name = NULL, *creator = NULL, *algorithm = NULL; dns_fixedname_t fname, fcreator, falgorithm; isc_result_t result; unsigned int dstalg; @@ -593,11 +593,9 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp) { dns_rbtnodechain_t chain; dns_name_t foundname; dns_fixedname_t fixedorigin; - dns_name_t *origin; + dns_name_t *origin = NULL; isc_stdtime_t now = isc_stdtime_now(); - dns_rbtnode_t *node; - dns_tsigkey_t *tkey; - dns_tsig_keyring_t *ring; + dns_tsig_keyring_t *ring = NULL; REQUIRE(ringp != NULL && *ringp != NULL); @@ -618,7 +616,9 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp) { } for (;;) { - node = NULL; + dns_rbtnode_t *node = NULL; + dns_tsigkey_t *tkey = NULL; + dns_rbtnodechain_current(&chain, &foundname, origin, &node); tkey = node->data; if (tkey != NULL && tkey->generated && tkey->expire >= now) { @@ -655,10 +655,7 @@ dns_tsigkey_identity(const dns_tsigkey_t *tsigkey) { isc_result_t dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, - unsigned char *secret, int length, bool generated, - bool restored, const dns_name_t *creator, - isc_stdtime_t inception, isc_stdtime_t expire, - isc_mem_t *mctx, dns_tsig_keyring_t *ring, + unsigned char *secret, int length, isc_mem_t *mctx, dns_tsigkey_t **key) { dst_key_t *dstkey = NULL; isc_result_t result; @@ -688,9 +685,8 @@ dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, return (DNS_R_BADALG); } - result = dns_tsigkey_createfromkey(name, algorithm, dstkey, generated, - restored, creator, inception, expire, - mctx, ring, key); + result = dns_tsigkey_createfromkey(name, algorithm, dstkey, false, + false, NULL, 0, 0, mctx, NULL, key); if (dstkey != NULL) { dst_key_free(&dstkey); } @@ -762,7 +758,7 @@ dns_tsig_sign(dns_message_t *msg) { dns_rdataset_t *dataset = NULL; isc_region_t r; isc_stdtime_t now; - isc_mem_t *mctx; + isc_mem_t *mctx = NULL; dst_context_t *ctx = NULL; isc_result_t ret; unsigned char badtimedata[BADTIMELEN]; @@ -1054,15 +1050,15 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, isc_region_t r, source_r, header_r, sig_r; isc_buffer_t databuf; unsigned char data[32]; - dns_name_t *keyname; + dns_name_t *keyname = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; isc_stdtime_t now; isc_result_t ret; - dns_tsigkey_t *tsigkey; + dns_tsigkey_t *tsigkey = NULL; dst_key_t *key = NULL; unsigned char header[DNS_MESSAGE_HEADERLEN]; dst_context_t *ctx = NULL; - isc_mem_t *mctx; + isc_mem_t *mctx = NULL; uint16_t addcount, id; unsigned int siglen; unsigned int alg; @@ -1168,9 +1164,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, } if (ret != ISC_R_SUCCESS) { msg->tsigstatus = dns_tsigerror_badkey; - ret = dns_tsigkey_create( - keyname, &tsig.algorithm, NULL, 0, false, false, - NULL, now, now, mctx, NULL, &msg->tsigkey); + ret = dns_tsigkey_create(keyname, &tsig.algorithm, NULL, + 0, mctx, &msg->tsigkey); if (ret != ISC_R_SUCCESS) { return (ret); } @@ -1410,16 +1405,16 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { isc_region_t r, source_r, header_r, sig_r; isc_buffer_t databuf; unsigned char data[32]; - dns_name_t *keyname; + dns_name_t *keyname = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; isc_stdtime_t now; isc_result_t ret; - dns_tsigkey_t *tsigkey; + dns_tsigkey_t *tsigkey = NULL; dst_key_t *key = NULL; unsigned char header[DNS_MESSAGE_HEADERLEN]; uint16_t addcount, id; bool has_tsig = false; - isc_mem_t *mctx; + isc_mem_t *mctx = NULL; unsigned int siglen; unsigned int alg; @@ -1728,7 +1723,7 @@ cleanup_querystruct: isc_result_t dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, const dns_name_t *algorithm, dns_tsig_keyring_t *ring) { - dns_tsigkey_t *key; + dns_tsigkey_t *key = NULL; isc_stdtime_t now = isc_stdtime_now(); isc_result_t result; @@ -1742,7 +1737,6 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); RWLOCK(&ring->lock, isc_rwlocktype_read); - key = NULL; result = dns_rbt_findname(ring->keys, name, 0, NULL, (void *)&key); if (result == DNS_R_PARTIALMATCH || result == ISC_R_NOTFOUND) { RWUNLOCK(&ring->lock, isc_rwlocktype_read); @@ -1762,17 +1756,6 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); return (ISC_R_NOTFOUND); } -#if 0 - /* - * MPAXXX We really should look at the inception time. - */ - if (key->inception != key->expire && - isc_serial_lt(key->inception, now)) { - RWUNLOCK(&ring->lock, isc_rwlocktype_read); - adjust_lru(key); - return (ISC_R_NOTFOUND); - } -#endif /* if 0 */ isc_refcount_increment(&key->refs); RWUNLOCK(&ring->lock, isc_rwlocktype_read); adjust_lru(key); @@ -1781,14 +1764,11 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, } static void -free_tsignode(void *node, void *_unused) { - dns_tsigkey_t *key; +free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { + dns_tsigkey_t *key = node; - REQUIRE(node != NULL); + REQUIRE(key != NULL); - UNUSED(_unused); - - key = node; if (key->generated) { if (ISC_LINK_LINKED(key, link)) { ISC_LIST_UNLINK(key->ring->lru, key, link); @@ -1800,16 +1780,18 @@ free_tsignode(void *node, void *_unused) { isc_result_t dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { isc_result_t result; - dns_tsig_keyring_t *ring; + dns_tsig_keyring_t *ring = NULL; REQUIRE(mctx != NULL); REQUIRE(ringp != NULL); REQUIRE(*ringp == NULL); ring = isc_mem_get(mctx, sizeof(dns_tsig_keyring_t)); + *ring = (dns_tsig_keyring_t){ + .maxgenerated = DNS_TSIG_MAXGENERATEDKEYS, + .lru = ISC_LIST_INITIALIZER, + }; - isc_rwlock_init(&ring->lock); - ring->keys = NULL; result = dns_rbt_create(mctx, free_tsignode, NULL, &ring->keys); if (result != ISC_R_SUCCESS) { isc_rwlock_destroy(&ring->lock); @@ -1817,11 +1799,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { return (result); } - ring->writecount = 0; - ring->mctx = NULL; - ring->generated = 0; - ring->maxgenerated = DNS_TSIG_MAXGENERATEDKEYS; - ISC_LIST_INIT(ring->lru); + isc_rwlock_init(&ring->lock); isc_mem_attach(mctx, &ring->mctx); isc_refcount_init(&ring->references, 1); @@ -1859,7 +1837,7 @@ dns_tsigkeyring_attach(dns_tsig_keyring_t *source, void dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp) { - dns_tsig_keyring_t *ring; + dns_tsig_keyring_t *ring = NULL; REQUIRE(ringp != NULL); REQUIRE(*ringp != NULL); @@ -1873,7 +1851,7 @@ dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp) { } void -dns_keyring_restore(dns_tsig_keyring_t *ring, FILE *fp) { +dns_tsigkeyring_restore(dns_tsig_keyring_t *ring, FILE *fp) { isc_stdtime_t now = isc_stdtime_now(); isc_result_t result; diff --git a/lib/dns/view.c b/lib/dns/view.c index 49fc8a8d80..754c9974aa 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -734,7 +734,7 @@ dns_view_restorekeyring(dns_view_t *view) { if (result == ISC_R_SUCCESS) { fp = fopen(keyfile, "r"); if (fp != NULL) { - dns_keyring_restore(view->dynamickeys, fp); + dns_tsigkeyring_restore(view->dynamickeys, fp); (void)fclose(fp); } } diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index 6380e3e09b..743c5def1e 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -294,8 +294,9 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { assert_int_equal(result, ISC_R_SUCCESS); result = dns_tsigkey_create(keyname, dns_tsig_hmacsha256_name, secret, - sizeof(secret), false, false, NULL, 0, 0, - mctx, ring, &key); + sizeof(secret), mctx, &key); + assert_int_equal(result, ISC_R_SUCCESS); + result = dns_tsigkeyring_add(ring, keyname, key); assert_int_equal(result, ISC_R_SUCCESS); assert_non_null(key); From 6fa8524bbabe7e3cb3cb426dda38ab5d60707ce2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 11 Apr 2023 11:35:01 -0700 Subject: [PATCH 02/11] use ISC_REFCOUNT_IMPL for dns_tsigkey and dns_tsigkeyring use the ISC_REFCOUNT attach/detach implementation in dns/tsig.c so that detailed tracing can be used during refactoring. dns_tsig_keyring_t has been renamed dns_tsigkeyring_t so the type and the attach/detach function names will match. --- bin/named/include/named/tsigconf.h | 2 +- bin/named/server.c | 2 +- bin/named/tsigconf.c | 6 +- bin/nsupdate/nsupdate.c | 2 +- fuzz/dns_message_checksig.c | 4 +- lib/dns/include/dns/tkey.h | 4 +- lib/dns/include/dns/tsig.h | 102 +++++++++++++------------- lib/dns/include/dns/types.h | 2 +- lib/dns/include/dns/view.h | 10 +-- lib/dns/tkey.c | 10 +-- lib/dns/tsig.c | 111 ++++++++++------------------- lib/dns/view.c | 6 +- tests/dns/tsig_test.c | 2 +- 13 files changed, 113 insertions(+), 150 deletions(-) diff --git a/bin/named/include/named/tsigconf.h b/bin/named/include/named/tsigconf.h index 32a0120083..240eebe130 100644 --- a/bin/named/include/named/tsigconf.h +++ b/bin/named/include/named/tsigconf.h @@ -22,7 +22,7 @@ ISC_LANG_BEGINDECLS isc_result_t named_tsigkeyring_fromconfig(const cfg_obj_t *config, const cfg_obj_t *vconfig, - isc_mem_t *mctx, dns_tsig_keyring_t **ringp); + isc_mem_t *mctx, dns_tsigkeyring_t **ringp); /*%< * Create a TSIG key ring and configure it according to the 'key' * statements in the global and view configuration objects. diff --git a/bin/named/server.c b/bin/named/server.c index 4456cfb12c..058a85f6a3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4087,7 +4087,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, uint32_t lame_ttl, fail_ttl; uint32_t max_stale_ttl = 0; uint32_t stale_refresh_time = 0; - dns_tsig_keyring_t *ring = NULL; + dns_tsigkeyring_t *ring = NULL; dns_transport_list_t *transports = NULL; dns_view_t *pview = NULL; /* Production view */ dns_dispatch_t *dispatch4 = NULL; diff --git a/bin/named/tsigconf.c b/bin/named/tsigconf.c index 550046275e..cfa934f0e1 100644 --- a/bin/named/tsigconf.c +++ b/bin/named/tsigconf.c @@ -31,7 +31,7 @@ #include static isc_result_t -add_initial_keys(const cfg_obj_t *list, dns_tsig_keyring_t *ring, +add_initial_keys(const cfg_obj_t *list, dns_tsigkeyring_t *ring, isc_mem_t *mctx) { dns_tsigkey_t *tsigkey = NULL; const cfg_listelt_t *element; @@ -136,10 +136,10 @@ failure: isc_result_t named_tsigkeyring_fromconfig(const cfg_obj_t *config, const cfg_obj_t *vconfig, - isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { + isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { const cfg_obj_t *maps[3]; const cfg_obj_t *keylist; - dns_tsig_keyring_t *ring = NULL; + dns_tsigkeyring_t *ring = NULL; isc_result_t result; int i; diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index c539f10399..da91da6ab4 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -135,7 +135,7 @@ static dns_name_t *userzone = NULL; static dns_name_t *zname = NULL; static dns_name_t tmpzonename = DNS_NAME_INITEMPTY; static dns_name_t restart_primary = DNS_NAME_INITEMPTY; -static dns_tsig_keyring_t *gssring = NULL; +static dns_tsigkeyring_t *gssring = NULL; static dns_tsigkey_t *tsigkey = NULL; static dst_key_t *sig0key = NULL; static isc_sockaddr_t *servers = NULL; diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index f60154dea4..72f50cc24a 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -89,8 +89,8 @@ static isc_stdtime_t fuzztime = 0x622acce1; static isc_loopmgr_t *loopmgr = NULL; static dns_view_t *view = NULL; static dns_tsigkey_t *tsigkey = NULL; -static dns_tsig_keyring_t *ring = NULL; -static dns_tsig_keyring_t *emptyring = NULL; +static dns_tsigkeyring_t *ring = NULL; +static dns_tsigkeyring_t *emptyring = NULL; static char *wd = NULL; static char template[] = "/tmp/dns-message-checksig-XXXXXX"; diff --git a/lib/dns/include/dns/tkey.h b/lib/dns/include/dns/tkey.h index ac65603b03..7d8f2dc735 100644 --- a/lib/dns/include/dns/tkey.h +++ b/lib/dns/include/dns/tkey.h @@ -69,7 +69,7 @@ dns_tkeyctx_destroy(dns_tkeyctx_t **tctxp); isc_result_t dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, - dns_tsig_keyring_t *ring); + dns_tsigkeyring_t *ring); /*%< * Processes a query containing a TKEY record, adding or deleting TSIG * keys if necessary, and modifies the message to contain the response. @@ -113,7 +113,7 @@ dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name, isc_result_t 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_tsig_keyring_t *ring, + dns_tsigkey_t **outkey, dns_tsigkeyring_t *ring, char **err_message); /*%< * Client side negotiation of GSS-TSIG. Process the response diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index c9329c67d7..acf4fb0d9e 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -28,6 +28,9 @@ #include +/* Define to 1 for detailed reference tracing */ +#undef DNS_TSIG_TRACE + /* * Algorithms. */ @@ -51,7 +54,7 @@ extern const dns_name_t *dns_tsig_hmacsha512_name; */ #define DNS_TSIG_FUDGE 300 -struct dns_tsig_keyring { +struct dns_tsigkeyring { dns_rbt_t *keys; unsigned int writecount; isc_rwlock_t lock; @@ -68,18 +71,18 @@ struct dns_tsig_keyring { struct dns_tsigkey { /* Unlocked */ - unsigned int magic; /*%< Magic number. */ - isc_mem_t *mctx; - dst_key_t *key; /*%< Key */ - dns_name_t name; /*%< Key name */ - const dns_name_t *algorithm; /*%< Algorithm name */ - dns_name_t *creator; /*%< name that created secret */ - bool generated : 1; /*%< key was auto-generated */ - bool restored : 1; /*%< key was restored at startup */ - isc_stdtime_t inception; /*%< start of validity period */ - isc_stdtime_t expire; /*%< end of validity period */ - dns_tsig_keyring_t *ring; /*%< the enclosing keyring */ - isc_refcount_t refs; /*%< reference counter */ + unsigned int magic; /*%< Magic number. */ + isc_mem_t *mctx; + dst_key_t *key; /*%< Key */ + dns_name_t name; /*%< Key name */ + const dns_name_t *algorithm; /*%< Algorithm name */ + dns_name_t *creator; /*%< name that created secret */ + bool generated : 1; /*%< key was auto-generated */ + bool restored : 1; /*%< key was restored at startup */ + isc_stdtime_t inception; /*%< start of validity period */ + isc_stdtime_t expire; /*%< end of validity period */ + dns_tsigkeyring_t *ring; /*%< the enclosing keyring */ + isc_refcount_t references; /*%< reference counter */ ISC_LINK(dns_tsigkey_t) link; }; @@ -108,7 +111,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, - dns_tsig_keyring_t *ring, dns_tsigkey_t **key); + dns_tsigkeyring_t *ring, dns_tsigkey_t **key); /*%< * Creates a tsig key structure and saves it in the keyring. If key is * not NULL, *key will contain a copy of the key. The keys validity @@ -141,31 +144,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, */ void -dns_tsigkey_attach(dns_tsigkey_t *source, dns_tsigkey_t **targetp); -/*%< - * Attach '*targetp' to 'source'. - * - * Requires: - *\li 'key' is a valid TSIG key - * - * Ensures: - *\li *targetp is attached to source. - */ - -void -dns_tsigkey_detach(dns_tsigkey_t **keyp); -/*%< - * Detaches from the tsig key structure pointed to by '*key'. - * - * Requires: - *\li 'keyp' is not NULL and '*keyp' is a valid TSIG key - * - * Ensures: - *\li 'keyp' points to NULL - */ - -void -dns_tsigkey_setdeleted(dns_tsigkey_t *key); +dns_tsigkey_delete(dns_tsigkey_t *key); /*%< * Prevents this key from being used again. It will be deleted when * no references exist. @@ -194,7 +173,7 @@ dns_tsig_sign(dns_message_t *msg); isc_result_t dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, - dns_tsig_keyring_t *ring1, dns_tsig_keyring_t *ring2); + dns_tsigkeyring_t *ring1, dns_tsigkeyring_t *ring2); /*%< * Verifies the TSIG record in this message * @@ -223,7 +202,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, isc_result_t dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, - const dns_name_t *algorithm, dns_tsig_keyring_t *ring); + const dns_name_t *algorithm, dns_tsigkeyring_t *ring); /*%< * Returns the TSIG key corresponding to this name and (possibly) * algorithm. Also increments the key's reference counter. @@ -241,7 +220,7 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, */ isc_result_t -dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp); +dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp); /*%< * Create an empty TSIG key ring. * @@ -255,7 +234,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp); */ isc_result_t -dns_tsigkeyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name, +dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, dns_tsigkey_t *tkey); /*%< * Place a TSIG key onto a key ring. @@ -270,14 +249,8 @@ dns_tsigkeyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name, *\li Any other value indicates failure. */ -void -dns_tsigkeyring_attach(dns_tsig_keyring_t *source, dns_tsig_keyring_t **target); - -void -dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp); - isc_result_t -dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp); +dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp); /*%< * Destroy a TSIG key ring. @@ -287,6 +260,33 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp); */ void -dns_tsigkeyring_restore(dns_tsig_keyring_t *ring, FILE *fp); +dns_tsigkeyring_restore(dns_tsigkeyring_t *ring, FILE *fp); +/*%< + * Restore a TSIG keyring from a dump file 'fp'. + */ + +#if DNS_TSIG_TRACE +#define dns_tsigkey_ref(ptr) dns_tsigkey__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_tsigkey_unref(ptr) \ + dns_tsigkey__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_tsigkey_attach(ptr, ptrp) \ + dns_tsigkey__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_tsigkey_detach(ptrp) \ + dns_tsigkey__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_tsigkey); + +#define dns_tsigkeyring_ref(ptr) \ + dns_tsigkeyring__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_tsigkeyring_unref(ptr) \ + dns_tsigkeyring__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_tsigkeyring_attach(ptr, ptrp) \ + dns_tsigkeyring__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_tsigkeyring_detach(ptrp) \ + dns_tsigkeyring__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_tsigkeyring); +#else +ISC_REFCOUNT_DECL(dns_tsigkey); +ISC_REFCOUNT_DECL(dns_tsigkeyring); +#endif ISC_LANG_ENDDECLS diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h index 96297e8f6a..e0a0b27f78 100644 --- a/lib/dns/include/dns/types.h +++ b/lib/dns/include/dns/types.h @@ -147,7 +147,7 @@ typedef struct dns_tkeyctx dns_tkeyctx_t; typedef struct dns_transport dns_transport_t; typedef struct dns_transport_list dns_transport_list_t; typedef uint16_t dns_trust_t; -typedef struct dns_tsig_keyring dns_tsig_keyring_t; +typedef struct dns_tsigkeyring dns_tsigkeyring_t; typedef struct dns_tsigkey dns_tsigkey_t; typedef uint32_t dns_ttl_t; typedef struct dns_update_state dns_update_state_t; diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h index adb8dc7612..b28ececce6 100644 --- a/lib/dns/include/dns/view.h +++ b/lib/dns/include/dns/view.h @@ -108,8 +108,8 @@ struct dns_view { /* Configurable data. */ dns_transport_list_t *transports; - dns_tsig_keyring_t *statickeys; - dns_tsig_keyring_t *dynamickeys; + dns_tsigkeyring_t *statickeys; + dns_tsigkeyring_t *dynamickeys; dns_peerlist_t *peers; dns_order_t *order; dns_fwdtable_t *fwdtable; @@ -433,9 +433,9 @@ void dns_view_settransports(dns_view_t *view, dns_transport_list_t *list); void -dns_view_setkeyring(dns_view_t *view, dns_tsig_keyring_t *ring); +dns_view_setkeyring(dns_view_t *view, dns_tsigkeyring_t *ring); void -dns_view_setdynamickeyring(dns_view_t *view, dns_tsig_keyring_t *ring); +dns_view_setdynamickeyring(dns_view_t *view, dns_tsigkeyring_t *ring); /*%< * Set the view's static TSIG keys * @@ -452,7 +452,7 @@ dns_view_setdynamickeyring(dns_view_t *view, dns_tsig_keyring_t *ring); */ void -dns_view_getdynamickeyring(dns_view_t *view, dns_tsig_keyring_t **ringp); +dns_view_getdynamickeyring(dns_view_t *view, dns_tsigkeyring_t **ringp); /*%< * Return the views dynamic keys. * diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 37e283c50e..00efdd7091 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -174,7 +174,7 @@ free_namelist(dns_message_t *msg, dns_namelist_t *namelist) { static isc_result_t process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, dns_tkeyctx_t *tctx, dns_rdata_tkey_t *tkeyout, - dns_tsig_keyring_t *ring) { + dns_tsigkeyring_t *ring) { isc_result_t result = ISC_R_SUCCESS; dst_key_t *dstkey = NULL; dns_tsigkey_t *tsigkey = NULL; @@ -327,7 +327,7 @@ failure: static isc_result_t process_deletetkey(dns_name_t *signer, dns_name_t *name, dns_rdata_tkey_t *tkeyin, dns_rdata_tkey_t *tkeyout, - dns_tsig_keyring_t *ring) { + dns_tsigkeyring_t *ring) { isc_result_t result; dns_tsigkey_t *tsigkey = NULL; const dns_name_t *identity; @@ -353,7 +353,7 @@ process_deletetkey(dns_name_t *signer, dns_name_t *name, * was not generated with TKEY and is in the config file, it may be * reloaded later. */ - dns_tsigkey_setdeleted(tsigkey); + dns_tsigkey_delete(tsigkey); /* Release the reference */ dns_tsigkey_detach(&tsigkey); @@ -363,7 +363,7 @@ process_deletetkey(dns_name_t *signer, dns_name_t *name, isc_result_t dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, - dns_tsig_keyring_t *ring) { + dns_tsigkeyring_t *ring) { isc_result_t result = ISC_R_SUCCESS; dns_rdata_tkey_t tkeyin, tkeyout; bool freetkeyin = false; @@ -729,7 +729,7 @@ find_tkey(dns_message_t *msg, dns_name_t **name, dns_rdata_t *rdata, isc_result_t 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_tsig_keyring_t *ring, + dns_tsigkey_t **outkey, dns_tsigkeyring_t *ring, char **err_message) { dns_rdata_t rtkeyrdata = DNS_RDATA_INIT, qtkeyrdata = DNS_RDATA_INIT; dns_name_t *tkeyname; diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index cb453c5665..cfd32f954a 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -112,9 +112,7 @@ tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); static void -cleanup_ring(dns_tsig_keyring_t *ring); -static void -tsigkey_free(dns_tsigkey_t *key); +cleanup_ring(dns_tsigkeyring_t *ring); bool dns__tsig_algvalid(unsigned int alg) { @@ -194,7 +192,7 @@ adjust_lru(dns_tsigkey_t *tkey) { * counter: it's protected by a separate lock. */ static isc_result_t -keyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name, +keyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, dns_tsigkey_t *tkey) { isc_result_t result; @@ -237,7 +235,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, - dns_tsig_keyring_t *ring, dns_tsigkey_t **key) { + dns_tsigkeyring_t *ring, dns_tsigkey_t **key) { dns_tsigkey_t *tkey = NULL; isc_result_t ret; unsigned int refs = 0; @@ -305,7 +303,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, refs++; } - isc_refcount_init(&tkey->refs, refs); + isc_refcount_init(&tkey->references, refs); isc_mem_attach(mctx, &tkey->mctx); tkey->magic = TSIG_MAGIC; @@ -348,9 +346,9 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, cleanup_refs: tkey->magic = 0; while (refs-- > 0) { - isc_refcount_decrement0(&tkey->refs); + isc_refcount_decrement0(&tkey->references); } - isc_refcount_destroy(&tkey->refs); + isc_refcount_destroy(&tkey->references); if (tkey->key != NULL) { dst_key_free(&tkey->key); @@ -377,7 +375,7 @@ cleanup_name: * Find a few nodes to destroy if possible. */ static void -cleanup_ring(dns_tsig_keyring_t *ring) { +cleanup_ring(dns_tsigkeyring_t *ring) { isc_result_t result; dns_rbtnodechain_t chain; dns_name_t foundname; @@ -407,7 +405,7 @@ again: tkey = node->data; if (tkey != NULL) { if (tkey->generated && - isc_refcount_current(&tkey->refs) == 1 && + isc_refcount_current(&tkey->references) == 1 && tkey->inception != tkey->expire && tkey->expire < now) { @@ -427,13 +425,19 @@ again: } static void -destroyring(dns_tsig_keyring_t *ring) { +destroyring(dns_tsigkeyring_t *ring) { isc_refcount_destroy(&ring->references); dns_rbt_destroy(&ring->keys); isc_rwlock_destroy(&ring->lock); - isc_mem_putanddetach(&ring->mctx, ring, sizeof(dns_tsig_keyring_t)); + isc_mem_putanddetach(&ring->mctx, ring, sizeof(dns_tsigkeyring_t)); } +#if DNS_TSIG_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_tsigkeyring, destroyring); +#else +ISC_REFCOUNT_IMPL(dns_tsigkeyring, destroyring); +#endif + /* * Look up the DST_ALG_ constant for a given name. */ @@ -489,7 +493,7 @@ dns__tsig_algallocated(const dns_name_t *algorithm) { } static isc_result_t -restore_key(dns_tsig_keyring_t *ring, isc_stdtime_t now, FILE *fp) { +restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { dst_key_t *dstkey = NULL; char namestr[1024]; char creatorstr[1024]; @@ -588,14 +592,14 @@ dump_key(dns_tsigkey_t *tkey, FILE *fp) { } isc_result_t -dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp) { +dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp) { isc_result_t result; dns_rbtnodechain_t chain; dns_name_t foundname; dns_fixedname_t fixedorigin; dns_name_t *origin = NULL; isc_stdtime_t now = isc_stdtime_now(); - dns_tsig_keyring_t *ring = NULL; + dns_tsigkeyring_t *ring = NULL; REQUIRE(ringp != NULL && *ringp != NULL); @@ -693,17 +697,8 @@ dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, return (result); } -void -dns_tsigkey_attach(dns_tsigkey_t *source, dns_tsigkey_t **targetp) { - REQUIRE(VALID_TSIG_KEY(source)); - REQUIRE(targetp != NULL && *targetp == NULL); - - isc_refcount_increment(&source->refs); - *targetp = source; -} - static void -tsigkey_free(dns_tsigkey_t *key) { +destroy_tsigkey(dns_tsigkey_t *key) { REQUIRE(VALID_TSIG_KEY(key)); key->magic = 0; @@ -723,20 +718,14 @@ tsigkey_free(dns_tsigkey_t *key) { isc_mem_putanddetach(&key->mctx, key, sizeof(dns_tsigkey_t)); } -void -dns_tsigkey_detach(dns_tsigkey_t **keyp) { - REQUIRE(keyp != NULL && VALID_TSIG_KEY(*keyp)); - dns_tsigkey_t *key = *keyp; - *keyp = NULL; - - if (isc_refcount_decrement(&key->refs) == 1) { - isc_refcount_destroy(&key->refs); - tsigkey_free(key); - } -} +#if DNS_TSIG_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_tsigkey, destroy_tsigkey); +#else +ISC_REFCOUNT_IMPL(dns_tsigkey, destroy_tsigkey); +#endif void -dns_tsigkey_setdeleted(dns_tsigkey_t *key) { +dns_tsigkey_delete(dns_tsigkey_t *key) { REQUIRE(VALID_TSIG_KEY(key)); REQUIRE(key->ring != NULL); @@ -1045,7 +1034,7 @@ cleanup_context: isc_result_t dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, - dns_tsig_keyring_t *ring1, dns_tsig_keyring_t *ring2) { + dns_tsigkeyring_t *ring1, dns_tsigkeyring_t *ring2) { dns_rdata_any_tsig_t tsig, querytsig; isc_region_t r, source_r, header_r, sig_r; isc_buffer_t databuf; @@ -1722,7 +1711,7 @@ cleanup_querystruct: isc_result_t dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, - const dns_name_t *algorithm, dns_tsig_keyring_t *ring) { + const dns_name_t *algorithm, dns_tsigkeyring_t *ring) { dns_tsigkey_t *key = NULL; isc_stdtime_t now = isc_stdtime_now(); isc_result_t result; @@ -1756,7 +1745,7 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); return (ISC_R_NOTFOUND); } - isc_refcount_increment(&key->refs); + isc_refcount_increment(&key->references); RWUNLOCK(&ring->lock, isc_rwlocktype_read); adjust_lru(key); *tsigkey = key; @@ -1778,16 +1767,16 @@ free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { } isc_result_t -dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { +dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { isc_result_t result; - dns_tsig_keyring_t *ring = NULL; + dns_tsigkeyring_t *ring = NULL; REQUIRE(mctx != NULL); REQUIRE(ringp != NULL); REQUIRE(*ringp == NULL); - ring = isc_mem_get(mctx, sizeof(dns_tsig_keyring_t)); - *ring = (dns_tsig_keyring_t){ + ring = isc_mem_get(mctx, sizeof(dns_tsigkeyring_t)); + *ring = (dns_tsigkeyring_t){ .maxgenerated = DNS_TSIG_MAXGENERATEDKEYS, .lru = ISC_LIST_INITIALIZER, }; @@ -1795,7 +1784,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { result = dns_rbt_create(mctx, free_tsignode, NULL, &ring->keys); if (result != ISC_R_SUCCESS) { isc_rwlock_destroy(&ring->lock); - isc_mem_put(mctx, ring, sizeof(dns_tsig_keyring_t)); + isc_mem_put(mctx, ring, sizeof(dns_tsigkeyring_t)); return (result); } @@ -1808,7 +1797,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) { } isc_result_t -dns_tsigkeyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name, +dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, dns_tsigkey_t *tkey) { isc_result_t result; @@ -1818,40 +1807,14 @@ dns_tsigkeyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name, result = keyring_add(ring, name, tkey); if (result == ISC_R_SUCCESS) { - isc_refcount_increment(&tkey->refs); + isc_refcount_increment(&tkey->references); } return (result); } void -dns_tsigkeyring_attach(dns_tsig_keyring_t *source, - dns_tsig_keyring_t **target) { - REQUIRE(source != NULL); - REQUIRE(target != NULL && *target == NULL); - - isc_refcount_increment(&source->references); - - *target = source; -} - -void -dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp) { - dns_tsig_keyring_t *ring = NULL; - - REQUIRE(ringp != NULL); - REQUIRE(*ringp != NULL); - - ring = *ringp; - *ringp = NULL; - - if (isc_refcount_decrement(&ring->references) == 1) { - destroyring(ring); - } -} - -void -dns_tsigkeyring_restore(dns_tsig_keyring_t *ring, FILE *fp) { +dns_tsigkeyring_restore(dns_tsigkeyring_t *ring, FILE *fp) { isc_stdtime_t now = isc_stdtime_now(); isc_result_t result; diff --git a/lib/dns/view.c b/lib/dns/view.c index 754c9974aa..5b3ea09e0d 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -692,7 +692,7 @@ dns_view_settransports(dns_view_t *view, dns_transport_list_t *list) { } void -dns_view_setkeyring(dns_view_t *view, dns_tsig_keyring_t *ring) { +dns_view_setkeyring(dns_view_t *view, dns_tsigkeyring_t *ring) { REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(ring != NULL); if (view->statickeys != NULL) { @@ -702,7 +702,7 @@ dns_view_setkeyring(dns_view_t *view, dns_tsig_keyring_t *ring) { } void -dns_view_setdynamickeyring(dns_view_t *view, dns_tsig_keyring_t *ring) { +dns_view_setdynamickeyring(dns_view_t *view, dns_tsigkeyring_t *ring) { REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(ring != NULL); if (view->dynamickeys != NULL) { @@ -712,7 +712,7 @@ dns_view_setdynamickeyring(dns_view_t *view, dns_tsig_keyring_t *ring) { } void -dns_view_getdynamickeyring(dns_view_t *view, dns_tsig_keyring_t **ringp) { +dns_view_getdynamickeyring(dns_view_t *view, dns_tsigkeyring_t **ringp) { REQUIRE(DNS_VIEW_VALID(view)); REQUIRE(ringp != NULL && *ringp == NULL); if (view->dynamickeys != NULL) { diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index 743c5def1e..65837fc39b 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -271,7 +271,7 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { dns_fixedname_t fkeyname; dns_message_t *msg = NULL; dns_name_t *keyname; - dns_tsig_keyring_t *ring = NULL; + dns_tsigkeyring_t *ring = NULL; dns_tsigkey_t *key = NULL; isc_buffer_t *buf = NULL; isc_buffer_t *querytsig = NULL; From 404a13b4ddbd498529857fdd980b1629e58b6e36 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 11 Apr 2023 12:56:57 -0700 Subject: [PATCH 03/11] clean up reference counting in dns_tsigkey the reference counter in dns_tsigkey was being computed differently depending on whether there was a keyring or not. this is prone to error. --- lib/dns/include/dns/tsig.h | 3 +- lib/dns/tsig.c | 131 ++++++++++++++++--------------------- 2 files changed, 56 insertions(+), 78 deletions(-) diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index acf4fb0d9e..a573673749 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -251,9 +251,8 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, isc_result_t dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp); - /*%< - * Destroy a TSIG key ring. + * Dump a TSIG key ring to 'fp' and destroy it. * * Requires: *\li 'ringp' is not NULL diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index cfd32f954a..f011f57690 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -160,9 +160,10 @@ tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) { static void remove_fromring(dns_tsigkey_t *tkey) { - if (tkey->generated) { + if (tkey->generated && ISC_LINK_LINKED(tkey, link)) { ISC_LIST_UNLINK(tkey->ring->lru, tkey, link); tkey->ring->generated--; + dns_tsigkey_unref(tkey); } (void)dns_rbt_deletename(tkey->ring->keys, &tkey->name, false); } @@ -184,68 +185,21 @@ adjust_lru(dns_tsigkey_t *tkey) { } } -/* - * A supplemental routine just to add a key to ring. Note that reference - * counter should be counted separately because we may be adding the key - * as part of creation of the key, in which case the reference counter was - * already initialized. Also note we don't need RWLOCK for the reference - * counter: it's protected by a separate lock. - */ -static isc_result_t -keyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, - dns_tsigkey_t *tkey) { - isc_result_t result; - - RWLOCK(&ring->lock, isc_rwlocktype_write); - ring->writecount++; - - /* - * Do on the fly cleaning. Find some nodes we might not - * want around any more. - */ - if (ring->writecount > 10) { - cleanup_ring(ring); - ring->writecount = 0; - } - - result = dns_rbt_addname(ring->keys, name, tkey); - if (result == ISC_R_SUCCESS) { - if (tkey->generated) { - /* - * Add the new key to the LRU list and remove the - * least recently used key if there are too many - * keys on the list. - */ - ISC_LIST_APPEND(ring->lru, tkey, link); - if (ring->generated++ > ring->maxgenerated) { - remove_fromring(ISC_LIST_HEAD(ring->lru)); - } - } - - tkey->ring = ring; - } - - RWUNLOCK(&ring->lock, isc_rwlocktype_write); - - return (result); -} - isc_result_t dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, - dns_tsigkeyring_t *ring, dns_tsigkey_t **key) { + dns_tsigkeyring_t *ring, dns_tsigkey_t **keyp) { dns_tsigkey_t *tkey = NULL; isc_result_t ret; - unsigned int refs = 0; unsigned int dstalg = 0; - REQUIRE(key == NULL || *key == NULL); + REQUIRE(keyp == NULL || *keyp == NULL); REQUIRE(name != NULL); REQUIRE(algorithm != NULL); REQUIRE(mctx != NULL); - REQUIRE(key != NULL || ring != NULL); + REQUIRE(keyp != NULL || ring != NULL); tkey = isc_mem_get(mctx, sizeof(dns_tsigkey_t)); *tkey = (dns_tsigkey_t){ @@ -296,23 +250,22 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_attach(dstkey, &tkey->key); } - if (key != NULL) { - refs = 1; - } - if (ring != NULL) { - refs++; - } - - isc_refcount_init(&tkey->references, refs); + isc_refcount_init(&tkey->references, 1); isc_mem_attach(mctx, &tkey->mctx); - tkey->magic = TSIG_MAGIC; - if (ring != NULL) { - ret = keyring_add(ring, name, tkey); + ret = dns_tsigkeyring_add(ring, name, tkey); if (ret != ISC_R_SUCCESS) { goto cleanup_refs; } + + /* + * If we won't be passing the key back to the caller + * then we we can release a reference now. + */ + if (keyp == NULL) { + dns_tsigkey_unref(tkey); + } } /* @@ -329,9 +282,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, namestr); } - if (key != NULL) { - *key = tkey; - } + tkey->magic = TSIG_MAGIC; if (tkey->restored) { tsig_log(tkey, ISC_LOG_DEBUG(3), "restored from file"); @@ -341,13 +292,13 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, tsig_log(tkey, ISC_LOG_DEBUG(3), "statically configured"); } + if (keyp != NULL) { + *keyp = tkey; + } return (ISC_R_SUCCESS); cleanup_refs: tkey->magic = 0; - while (refs-- > 0) { - isc_refcount_decrement0(&tkey->references); - } isc_refcount_destroy(&tkey->references); if (tkey->key != NULL) { @@ -1745,9 +1696,9 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, RWUNLOCK(&ring->lock, isc_rwlocktype_write); return (ISC_R_NOTFOUND); } - isc_refcount_increment(&key->references); RWUNLOCK(&ring->lock, isc_rwlocktype_read); adjust_lru(key); + dns_tsigkey_ref(key); *tsigkey = key; return (ISC_R_SUCCESS); } @@ -1758,10 +1709,9 @@ free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { REQUIRE(key != NULL); - if (key->generated) { - if (ISC_LINK_LINKED(key, link)) { - ISC_LIST_UNLINK(key->ring->lru, key, link); - } + if (key->generated && ISC_LINK_LINKED(key, link)) { + ISC_LIST_UNLINK(key->ring->lru, key, link); + dns_tsigkey_unref(key); } dns_tsigkey_detach(&key); } @@ -1805,11 +1755,40 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, REQUIRE(tkey->ring == NULL); REQUIRE(name != NULL); - result = keyring_add(ring, name, tkey); - if (result == ISC_R_SUCCESS) { - isc_refcount_increment(&tkey->references); + RWLOCK(&ring->lock, isc_rwlocktype_write); + ring->writecount++; + + /* + * Do on the fly cleaning. Find some nodes we might not + * want around any more. + */ + if (ring->writecount > 10) { + cleanup_ring(ring); + ring->writecount = 0; } + result = dns_rbt_addname(ring->keys, name, tkey); + if (result == ISC_R_SUCCESS) { + dns_tsigkey_ref(tkey); + + /* + * If this is a TKEY-generated key, add it to the LRU list, + * and if we've exceeded the quota for generated keys, + * remove the least recently used one from the both the + * list and the RBT. + */ + if (tkey->generated) { + ISC_LIST_APPEND(ring->lru, tkey, link); + dns_tsigkey_ref(tkey); + if (ring->generated++ > ring->maxgenerated) { + remove_fromring(ISC_LIST_HEAD(ring->lru)); + } + } + + tkey->ring = ring; + } + RWUNLOCK(&ring->lock, isc_rwlocktype_write); + return (result); } From a6e187a8d514fe7977331a540f1ecacddcfec59b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 11 Apr 2023 16:10:07 -0700 Subject: [PATCH 04/11] further dns_tsigkey API tweaks - remove the 'ring' parameter from dns_tsigkey_createfromkey(), and use dns_tsigkeyring_add() to add key objects to a keyring instead. - add a magic number to dns_tsigkeyring_t - change dns_tsigkeyring_dumpanddetach() to dns_tsigkeyring_dump(); we now call dns_tsigkeyring_detach() separately. - remove 'maxgenerated' from dns_tsigkeyring_t since it never changes. --- bin/dig/dighost.c | 2 +- bin/nsupdate/nsupdate.c | 18 +------ lib/dns/include/dns/tsig.h | 60 ++++++++++++++------- lib/dns/tkey.c | 22 ++++++-- lib/dns/tsig.c | 104 ++++++++++++------------------------- lib/dns/view.c | 8 ++- 6 files changed, 98 insertions(+), 116 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index c8db355a26..beac0c0c1b 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1187,7 +1187,7 @@ setup_file_key(void) { if (hmacname != NULL) { result = dns_tsigkey_createfromkey( dst_key_name(dstkey), hmacname, dstkey, false, false, - NULL, 0, 0, mctx, NULL, &tsigkey); + NULL, 0, 0, mctx, &tsigkey); if (result != ISC_R_SUCCESS) { printf(";; Couldn't create key %s: %s\n", keynametext, isc_result_totext(result)); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index da91da6ab4..424dfce293 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -674,7 +674,7 @@ setup_keyfile(isc_mem_t *mctx, isc_log_t *lctx) { if (hmacname != NULL) { result = dns_tsigkey_createfromkey( dst_key_name(dstkey), hmacname, dstkey, false, false, - NULL, 0, 0, mctx, NULL, &tsigkey); + NULL, 0, 0, mctx, &tsigkey); dst_key_free(&dstkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "could not create key from %s: %s\n", @@ -3038,9 +3038,8 @@ start_gssrequest(dns_name_t *primary) { if (gssring != NULL) { dns_tsigkeyring_detach(&gssring); } - gssring = NULL; - result = dns_tsigkeyring_create(gmctx, &gssring); + result = dns_tsigkeyring_create(gmctx, &gssring); if (result != ISC_R_SUCCESS) { fatal("dns_tsigkeyring_create failed: %s", isc_result_totext(result)); @@ -3234,7 +3233,6 @@ recvgss(void *arg) { result = dns_name_fromtext(servname, &buf, dns_rootname, 0, NULL); check_result(result, "dns_name_fromtext"); - tsigkey = NULL; result = dns_tkey_gssnegotiate(tsigquery, rcvmsg, servname, &context, &tsigkey, gssring, &err_message); switch (result) { @@ -3262,18 +3260,6 @@ recvgss(void *arg) { * the TSIG -- this too is a spec violation, but it's * the least insane thing to do. */ -#if 0 - /* - * Verify the signature. - */ - rcvmsg->state = DNS_SECTION_ANY; - dns_message_setquerytsig(rcvmsg, NULL); - result = dns_message_settsigkey(rcvmsg, tsigkey); - check_result(result, "dns_message_settsigkey"); - result = dns_message_checksig(rcvmsg, NULL); - ddebug("tsig verification: %s", isc_result_totext(result)); - check_result(result, "dns_message_checksig"); -#endif /* 0 */ send_update(&tmpzonename, &primary_servers[primary_inuse]); setzoneclass(dns_rdataclass_none); diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index a573673749..4c7068acf9 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -54,7 +54,15 @@ extern const dns_name_t *dns_tsig_hmacsha512_name; */ #define DNS_TSIG_FUDGE 300 +/*% + * Default maximum quota for generated keys. + */ +#ifndef DNS_TSIG_MAXGENERATEDKEYS +#define DNS_TSIG_MAXGENERATEDKEYS 4096 +#endif /* ifndef DNS_TSIG_MAXGENERATEDKEYS */ + struct dns_tsigkeyring { + unsigned int magic; /*%< Magic number. */ dns_rbt_t *keys; unsigned int writecount; isc_rwlock_t lock; @@ -64,7 +72,6 @@ struct dns_tsigkeyring { * list and a maximum size. */ unsigned int generated; - unsigned int maxgenerated; ISC_LIST(dns_tsigkey_t) lru; isc_refcount_t references; }; @@ -111,19 +118,31 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, - dns_tsigkeyring_t *ring, dns_tsigkey_t **key); + dns_tsigkey_t **key); /*%< - * Creates a tsig key structure and saves it in the keyring. If key is - * not NULL, *key will contain a copy of the key. The keys validity - * period is specified by (inception, expire), and will not expire if - * inception == expire. If the key was generated, the creating identity, - * if there is one, should be in the creator parameter. Specifying an - * unimplemented algorithm will cause failure only if dstkey != NULL; this - * allows a transient key with an invalid algorithm to exist long enough - * to generate a BADKEY response. + * Creates a tsig key structure and stores it in *keyp. + * The key's validity period is specified by (inception, expire), + * and will not expire if inception == expire. * - * If dns_tsigkey_createfromkey is successful a new reference to 'dstkey' - * will have been made. + * If generated is true (meaning the key was generated + * via TKEY negotation), the creating identity (if any), should + * be specified in the creator parameter. + * + * If restored is true, this indicates the key was restored from + * a dump file created by dns_tsigkeyring_dumpanddetach(). This is + * used only for logging purposes and doesn't affect the key any + * other way. + * + * Specifying an unimplemented algorithm will cause failure only if + * dstkey != NULL; this allows a transient key with an invalid + * algorithm to exist long enough to generate a BADKEY response. + * + * If dns_tsigkey_createfromkey() is successful, a new reference to + * 'dstkey' will have been made. + * + * dns_tsigkey_create() is a simplified interface that omits + * dstkey, generated, restored, inception, and expired (defaulting + * to NULL, false, false, 0, and 0). * * Requires: *\li 'name' is a valid dns_name_t @@ -201,15 +220,15 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, */ isc_result_t -dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, +dns_tsigkey_find(dns_tsigkey_t **tsigkeyp, const dns_name_t *name, const dns_name_t *algorithm, dns_tsigkeyring_t *ring); /*%< * Returns the TSIG key corresponding to this name and (possibly) * algorithm. Also increments the key's reference counter. * * Requires: - *\li 'tsigkey' is not NULL - *\li '*tsigkey' is NULL + *\li 'tsigkeyp' is not NULL + *\li '*tsigkeyp' is NULL *\li 'name' is a valid dns_name_t *\li 'algorithm' is a valid dns_name_t or NULL *\li 'ring' is a valid keyring @@ -239,6 +258,11 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, /*%< * Place a TSIG key onto a key ring. * + * If the key is generated, it is also placed into an LRU queue. + * There is a maximum quota of 4096 generated keys per keyring; + * if this quota is exceeded, the oldest key in the LRU queue is + * deleted. + * * Requires: *\li 'name' and 'ring' are not NULL *\li 'tkey' is a valid TSIG key, which has not been @@ -250,12 +274,12 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, */ isc_result_t -dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp); +dns_tsigkeyring_dump(dns_tsigkeyring_t *ring, FILE *fp); /*%< - * Dump a TSIG key ring to 'fp' and destroy it. + * Dump a TSIG key ring to 'fp'. * * Requires: - *\li 'ringp' is not NULL + *\li 'ring' is a valid keyring. */ void diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 00efdd7091..f1426d40eb 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -265,7 +265,8 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, #endif /* HAVE_GSSAPI */ RETERR(dns_tsigkey_createfromkey( name, &tkeyin->algorithm, dstkey, true, false, - principal, now, expire, ring->mctx, ring, &tsigkey)); + principal, now, expire, ring->mctx, &tsigkey)); + RETERR(dns_tsigkeyring_add(ring, name, tsigkey)); dst_key_free(&dstkey); tkeyout->inception = now; tkeyout->expire = expire; @@ -732,13 +733,14 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, dns_tsigkey_t **outkey, dns_tsigkeyring_t *ring, char **err_message) { dns_rdata_t rtkeyrdata = DNS_RDATA_INIT, qtkeyrdata = DNS_RDATA_INIT; - dns_name_t *tkeyname; + 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); @@ -814,9 +816,16 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, * anything yet. */ - RETERR(dns_tsigkey_createfromkey( - tkeyname, DNS_TSIG_GSSAPI_NAME, dstkey, true, false, NULL, - rtkey.inception, rtkey.expire, ring->mctx, ring, outkey)); + RETERR(dns_tsigkey_createfromkey(tkeyname, DNS_TSIG_GSSAPI_NAME, dstkey, + true, false, NULL, rtkey.inception, + rtkey.expire, ring->mctx, &tsigkey)); + RETERR(dns_tsigkeyring_add(ring, tkeyname, tsigkey)); + if (outkey == NULL) { + dns_tsigkey_detach(&tsigkey); + } else { + *outkey = tsigkey; + } + dst_key_free(&dstkey); dns_rdata_freestruct(&rtkey); return (result); @@ -825,6 +834,9 @@ failure: /* * XXXSRA This probably leaks memory from qtkey. */ + if (tsigkey != NULL) { + dns_tsigkey_detach(&tsigkey); + } if (freertkey) { dns_rdata_freestruct(&rtkey); } diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index f011f57690..f5c4a149d5 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -39,12 +39,11 @@ #include "tsig_p.h" -#define TSIG_MAGIC ISC_MAGIC('T', 'S', 'I', 'G') -#define VALID_TSIG_KEY(x) ISC_MAGIC_VALID(x, TSIG_MAGIC) +#define TSIGKEYRING_MAGIC ISC_MAGIC('T', 'K', 'R', 'g') +#define VALID_TSIGKEYRING(x) ISC_MAGIC_VALID(x, TSIGKEYRING_MAGIC) -#ifndef DNS_TSIG_MAXGENERATEDKEYS -#define DNS_TSIG_MAXGENERATEDKEYS 4096 -#endif /* ifndef DNS_TSIG_MAXGENERATEDKEYS */ +#define TSIG_MAGIC ISC_MAGIC('T', 'S', 'I', 'G') +#define VALID_TSIGKEY(x) ISC_MAGIC_VALID(x, TSIG_MAGIC) #define is_response(msg) ((msg->flags & DNS_MESSAGEFLAG_QR) != 0) @@ -160,6 +159,9 @@ tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) { static void remove_fromring(dns_tsigkey_t *tkey) { + REQUIRE(VALID_TSIGKEY(tkey)); + REQUIRE(VALID_TSIGKEYRING(tkey->ring)); + if (tkey->generated && ISC_LINK_LINKED(tkey, link)) { ISC_LIST_UNLINK(tkey->ring->lru, tkey, link); tkey->ring->generated--; @@ -190,22 +192,20 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, - dns_tsigkeyring_t *ring, dns_tsigkey_t **keyp) { + dns_tsigkey_t **keyp) { dns_tsigkey_t *tkey = NULL; isc_result_t ret; unsigned int dstalg = 0; - REQUIRE(keyp == NULL || *keyp == NULL); + REQUIRE(keyp != NULL && *keyp == NULL); REQUIRE(name != NULL); REQUIRE(algorithm != NULL); REQUIRE(mctx != NULL); - REQUIRE(keyp != NULL || ring != NULL); tkey = isc_mem_get(mctx, sizeof(dns_tsigkey_t)); *tkey = (dns_tsigkey_t){ .generated = generated, .restored = restored, - .ring = ring, .inception = inception, .expire = expire, .name = DNS_NAME_INITEMPTY, @@ -253,21 +253,6 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, isc_refcount_init(&tkey->references, 1); isc_mem_attach(mctx, &tkey->mctx); - if (ring != NULL) { - ret = dns_tsigkeyring_add(ring, name, tkey); - if (ret != ISC_R_SUCCESS) { - goto cleanup_refs; - } - - /* - * If we won't be passing the key back to the caller - * then we we can release a reference now. - */ - if (keyp == NULL) { - dns_tsigkey_unref(tkey); - } - } - /* * Ignore this if it's a GSS key, since the key size is meaningless. */ @@ -297,24 +282,6 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, } return (ISC_R_SUCCESS); -cleanup_refs: - tkey->magic = 0; - isc_refcount_destroy(&tkey->references); - - if (tkey->key != NULL) { - dst_key_free(&tkey->key); - } - if (tkey->creator != NULL) { - dns_name_free(tkey->creator, mctx); - isc_mem_put(mctx, tkey->creator, sizeof(dns_name_t)); - } - if (dns__tsig_algallocated(tkey->algorithm)) { - dns_name_t *tmpname = UNCONST(tkey->algorithm); - if (dns_name_dynamic(tmpname)) { - dns_name_free(tmpname, mctx); - } - isc_mem_put(mctx, tmpname, sizeof(dns_name_t)); - } cleanup_name: dns_name_free(&tkey->name, mctx); isc_mem_put(mctx, tkey, sizeof(dns_tsigkey_t)); @@ -377,6 +344,7 @@ again: static void destroyring(dns_tsigkeyring_t *ring) { + ring->magic = 0; isc_refcount_destroy(&ring->references); dns_rbt_destroy(&ring->keys); isc_rwlock_destroy(&ring->lock); @@ -457,6 +425,7 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { dns_fixedname_t fname, fcreator, falgorithm; isc_result_t result; unsigned int dstalg; + dns_tsigkey_t *tkey = NULL; n = fscanf(fp, "%1023s %1023s %u %u %1023s %4095s\n", namestr, creatorstr, &inception, &expire, algorithmstr, keystr); @@ -509,7 +478,11 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { result = dns_tsigkey_createfromkey(name, algorithm, dstkey, true, true, creator, inception, expire, - ring->mctx, ring, NULL); + ring->mctx, &tkey); + if (result == ISC_R_SUCCESS) { + result = dns_tsigkeyring_add(ring, name, tkey); + } + dns_tsigkey_detach(&tkey); if (dstkey != NULL) { dst_key_free(&dstkey); } @@ -543,23 +516,15 @@ dump_key(dns_tsigkey_t *tkey, FILE *fp) { } isc_result_t -dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp) { +dns_tsigkeyring_dump(dns_tsigkeyring_t *ring, FILE *fp) { isc_result_t result; dns_rbtnodechain_t chain; dns_name_t foundname; dns_fixedname_t fixedorigin; dns_name_t *origin = NULL; isc_stdtime_t now = isc_stdtime_now(); - dns_tsigkeyring_t *ring = NULL; - REQUIRE(ringp != NULL && *ringp != NULL); - - ring = *ringp; - *ringp = NULL; - - if (isc_refcount_decrement(&ring->references) > 1) { - return (DNS_R_CONTINUE); - } + REQUIRE(VALID_TSIGKEYRING(ring)); dns_name_init(&foundname, NULL); origin = dns_fixedname_initname(&fixedorigin); @@ -590,13 +555,12 @@ dns_tsigkeyring_dumpanddetach(dns_tsigkeyring_t **ringp, FILE *fp) { } destroy: - destroyring(ring); return (result); } const dns_name_t * dns_tsigkey_identity(const dns_tsigkey_t *tsigkey) { - REQUIRE(tsigkey == NULL || VALID_TSIG_KEY(tsigkey)); + REQUIRE(tsigkey == NULL || VALID_TSIGKEY(tsigkey)); if (tsigkey == NULL) { return (NULL); @@ -641,7 +605,7 @@ dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, } result = dns_tsigkey_createfromkey(name, algorithm, dstkey, false, - false, NULL, 0, 0, mctx, NULL, key); + false, NULL, 0, 0, mctx, key); if (dstkey != NULL) { dst_key_free(&dstkey); } @@ -650,7 +614,7 @@ dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, static void destroy_tsigkey(dns_tsigkey_t *key) { - REQUIRE(VALID_TSIG_KEY(key)); + REQUIRE(VALID_TSIGKEY(key)); key->magic = 0; dns_name_free(&key->name, key->mctx); @@ -677,8 +641,7 @@ ISC_REFCOUNT_IMPL(dns_tsigkey, destroy_tsigkey); void dns_tsigkey_delete(dns_tsigkey_t *key) { - REQUIRE(VALID_TSIG_KEY(key)); - REQUIRE(key->ring != NULL); + REQUIRE(VALID_TSIGKEY(key)); RWLOCK(&key->ring->lock, isc_rwlocktype_write); remove_fromring(key); @@ -707,7 +670,7 @@ dns_tsig_sign(dns_message_t *msg) { REQUIRE(msg != NULL); key = dns_message_gettsigkey(msg); - REQUIRE(VALID_TSIG_KEY(key)); + REQUIRE(VALID_TSIGKEY(key)); /* * If this is a response, there should be a TSIG in the query with the @@ -1009,7 +972,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, tsigkey = dns_message_gettsigkey(msg); response = is_response(msg); - REQUIRE(tsigkey == NULL || VALID_TSIG_KEY(tsigkey)); + REQUIRE(tsigkey == NULL || VALID_TSIGKEY(tsigkey)); msg->verify_attempted = 1; msg->verified_sig = 0; @@ -1667,10 +1630,9 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, isc_stdtime_t now = isc_stdtime_now(); isc_result_t result; - REQUIRE(tsigkey != NULL); - REQUIRE(*tsigkey == NULL); REQUIRE(name != NULL); - REQUIRE(ring != NULL); + REQUIRE(VALID_TSIGKEYRING(ring)); + REQUIRE(tsigkey != NULL && *tsigkey == NULL); RWLOCK(&ring->lock, isc_rwlocktype_write); cleanup_ring(ring); @@ -1709,7 +1671,7 @@ free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { REQUIRE(key != NULL); - if (key->generated && ISC_LINK_LINKED(key, link)) { + if (key->ring != NULL && key->generated && ISC_LINK_LINKED(key, link)) { ISC_LIST_UNLINK(key->ring->lru, key, link); dns_tsigkey_unref(key); } @@ -1722,12 +1684,10 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { dns_tsigkeyring_t *ring = NULL; REQUIRE(mctx != NULL); - REQUIRE(ringp != NULL); - REQUIRE(*ringp == NULL); + REQUIRE(ringp != NULL && *ringp == NULL); ring = isc_mem_get(mctx, sizeof(dns_tsigkeyring_t)); *ring = (dns_tsigkeyring_t){ - .maxgenerated = DNS_TSIG_MAXGENERATEDKEYS, .lru = ISC_LIST_INITIALIZER, }; @@ -1741,6 +1701,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { isc_rwlock_init(&ring->lock); isc_mem_attach(mctx, &ring->mctx); isc_refcount_init(&ring->references, 1); + ring->magic = TSIGKEYRING_MAGIC; *ringp = ring; return (ISC_R_SUCCESS); @@ -1751,8 +1712,8 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, dns_tsigkey_t *tkey) { isc_result_t result; - REQUIRE(VALID_TSIG_KEY(tkey)); - REQUIRE(tkey->ring == NULL); + REQUIRE(VALID_TSIGKEY(tkey)); + REQUIRE(VALID_TSIGKEYRING(ring)); REQUIRE(name != NULL); RWLOCK(&ring->lock, isc_rwlocktype_write); @@ -1770,6 +1731,7 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, result = dns_rbt_addname(ring->keys, name, tkey); if (result == ISC_R_SUCCESS) { dns_tsigkey_ref(tkey); + tkey->ring = ring; /* * If this is a TKEY-generated key, add it to the LRU list, @@ -1780,7 +1742,7 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, if (tkey->generated) { ISC_LIST_APPEND(ring->lru, tkey, link); dns_tsigkey_ref(tkey); - if (ring->generated++ > ring->maxgenerated) { + if (ring->generated++ > DNS_TSIG_MAXGENERATEDKEYS) { remove_fromring(ISC_LIST_HEAD(ring->lru)); } } diff --git a/lib/dns/view.c b/lib/dns/view.c index 5b3ea09e0d..7f1cc7ce8c 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -250,11 +250,8 @@ destroy(dns_view_t *view) { if (result == ISC_R_SUCCESS) { (void)isc_file_openuniqueprivate(template, &fp); } - if (fp == NULL) { - dns_tsigkeyring_detach(&view->dynamickeys); - } else { - result = dns_tsigkeyring_dumpanddetach( - &view->dynamickeys, fp); + if (fp != NULL) { + result = dns_tsigkeyring_dump(view->dynamickeys, fp); if (result == ISC_R_SUCCESS) { if (fclose(fp) == 0) { result = isc_file_sanitize( @@ -273,6 +270,7 @@ destroy(dns_view_t *view) { (void)remove(template); } } + dns_tsigkeyring_detach(&view->dynamickeys); } if (view->transports != NULL) { dns_transport_list_detach(&view->transports); From ffacf0aec6ac265bba307f4ef5f4915406607b7a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 11 Apr 2023 19:01:31 -0700 Subject: [PATCH 05/11] use algorithm number instead of name to create TSIG keys the prior practice of passing a dns_name containing the expanded name of an algorithm to dns_tsigkey_create() and dns_tsigkey_createfromkey() is unnecessarily cumbersome; we can now pass the algorithm number instead. --- bin/dig/dig.c | 2 +- bin/dig/dighost.c | 69 +++++++--------- bin/dig/dighost.h | 4 +- bin/named/config.c | 33 +------- bin/named/controlconf.c | 6 +- bin/named/include/named/config.h | 5 +- bin/named/server.c | 54 ++++-------- bin/named/tsigconf.c | 2 +- bin/nsupdate/nsupdate.c | 93 ++++++++++----------- fuzz/dns_message_checksig.c | 2 +- lib/dns/include/dns/tsig.h | 4 +- lib/dns/tkey.c | 32 ++++---- lib/dns/tsig.c | 136 +++++++++++++------------------ lib/dns/tsig_p.h | 4 +- lib/ns/client.c | 27 +++--- tests/dns/tsig_test.c | 61 +------------- 16 files changed, 181 insertions(+), 353 deletions(-) diff --git a/bin/dig/dig.c b/bin/dig/dig.c index 06940123dc..9f5742a2d8 100644 --- a/bin/dig/dig.c +++ b/bin/dig/dig.c @@ -2457,7 +2457,7 @@ dash_option(char *option, char *next, dig_lookup_t **lookup, ptr = ptr2; ptr2 = ptr3; } else { - hmacname = DNS_TSIG_HMACMD5_NAME; + hmac = DST_ALG_HMACMD5; digestbits = 0; } /* XXXONDREJ: FIXME */ diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index beac0c0c1b..206c166a6b 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -138,7 +138,7 @@ char keyfile[MXNAME] = ""; char keysecret[MXNAME] = ""; unsigned char cookie_secret[33]; unsigned char cookie[8]; -const dns_name_t *hmacname = NULL; +dst_algorithm_t hmac = DST_ALG_UNKNOWN; unsigned int digestbits = 0; isc_buffer_t *namebuf = NULL; dns_tsigkey_t *tsigkey = NULL; @@ -874,7 +874,7 @@ setup_text_key(void) { secretsize = isc_buffer_usedlength(&secretbuf); - if (hmacname == NULL) { + if (hmac == DST_ALG_UNKNOWN) { result = DST_R_UNSUPPORTEDALG; goto failure; } @@ -884,7 +884,7 @@ setup_text_key(void) { goto failure; } - result = dns_tsigkey_create(&keyname, hmacname, secretstore, + result = dns_tsigkey_create(&keyname, hmac, secretstore, (int)secretsize, mctx, &tsigkey); failure: if (result != ISC_R_SUCCESS) { @@ -1021,50 +1021,50 @@ done: * Parse HMAC algorithm specification */ void -parse_hmac(const char *hmac) { +parse_hmac(const char *algname) { char buf[20]; size_t len; - REQUIRE(hmac != NULL); + REQUIRE(algname != NULL); - len = strlen(hmac); + len = strlen(algname); if (len >= sizeof(buf)) { - fatal("unknown key type '%.*s'", (int)len, hmac); + fatal("unknown key type '%.*s'", (int)len, algname); } - strlcpy(buf, hmac, sizeof(buf)); + strlcpy(buf, algname, sizeof(buf)); digestbits = 0; if (strcasecmp(buf, "hmac-md5") == 0) { - hmacname = DNS_TSIG_HMACMD5_NAME; + hmac = DST_ALG_HMACMD5; } else if (strncasecmp(buf, "hmac-md5-", 9) == 0) { - hmacname = DNS_TSIG_HMACMD5_NAME; + hmac = DST_ALG_HMACMD5; digestbits = parse_bits(&buf[9], "digest-bits [0..128]", 128); } else if (strcasecmp(buf, "hmac-sha1") == 0) { - hmacname = DNS_TSIG_HMACSHA1_NAME; + hmac = DST_ALG_HMACSHA1; digestbits = 0; } else if (strncasecmp(buf, "hmac-sha1-", 10) == 0) { - hmacname = DNS_TSIG_HMACSHA1_NAME; + hmac = DST_ALG_HMACSHA1; digestbits = parse_bits(&buf[10], "digest-bits [0..160]", 160); } else if (strcasecmp(buf, "hmac-sha224") == 0) { - hmacname = DNS_TSIG_HMACSHA224_NAME; + hmac = DST_ALG_HMACSHA224; } else if (strncasecmp(buf, "hmac-sha224-", 12) == 0) { - hmacname = DNS_TSIG_HMACSHA224_NAME; + hmac = DST_ALG_HMACSHA224; digestbits = parse_bits(&buf[12], "digest-bits [0..224]", 224); } else if (strcasecmp(buf, "hmac-sha256") == 0) { - hmacname = DNS_TSIG_HMACSHA256_NAME; + hmac = DST_ALG_HMACSHA256; } else if (strncasecmp(buf, "hmac-sha256-", 12) == 0) { - hmacname = DNS_TSIG_HMACSHA256_NAME; + hmac = DST_ALG_HMACSHA256; digestbits = parse_bits(&buf[12], "digest-bits [0..256]", 256); } else if (strcasecmp(buf, "hmac-sha384") == 0) { - hmacname = DNS_TSIG_HMACSHA384_NAME; + hmac = DST_ALG_HMACSHA384; } else if (strncasecmp(buf, "hmac-sha384-", 12) == 0) { - hmacname = DNS_TSIG_HMACSHA384_NAME; + hmac = DST_ALG_HMACSHA384; digestbits = parse_bits(&buf[12], "digest-bits [0..384]", 384); } else if (strcasecmp(buf, "hmac-sha512") == 0) { - hmacname = DNS_TSIG_HMACSHA512_NAME; + hmac = DST_ALG_HMACSHA512; } else if (strncasecmp(buf, "hmac-sha512-", 12) == 0) { - hmacname = DNS_TSIG_HMACSHA512_NAME; + hmac = DST_ALG_HMACSHA512; digestbits = parse_bits(&buf[12], "digest-bits [0..512]", 512); } else { fprintf(stderr, @@ -1165,38 +1165,29 @@ setup_file_key(void) { switch (dst_key_alg(dstkey)) { case DST_ALG_HMACMD5: - hmacname = DNS_TSIG_HMACMD5_NAME; - break; case DST_ALG_HMACSHA1: - hmacname = DNS_TSIG_HMACSHA1_NAME; - break; case DST_ALG_HMACSHA224: - hmacname = DNS_TSIG_HMACSHA224_NAME; - break; case DST_ALG_HMACSHA256: - hmacname = DNS_TSIG_HMACSHA256_NAME; - break; case DST_ALG_HMACSHA384: - hmacname = DNS_TSIG_HMACSHA384_NAME; - break; case DST_ALG_HMACSHA512: - hmacname = DNS_TSIG_HMACSHA512_NAME; + hmac = dst_key_alg(dstkey); break; + default: + dst_key_attach(dstkey, &sig0key); + dst_key_free(&dstkey); + return; } - if (hmacname != NULL) { - result = dns_tsigkey_createfromkey( - dst_key_name(dstkey), hmacname, dstkey, false, false, - NULL, 0, 0, mctx, &tsigkey); + if (dstkey != NULL) { + result = dns_tsigkey_createfromkey(dst_key_name(dstkey), hmac, + dstkey, false, false, NULL, + 0, 0, mctx, &tsigkey); if (result != ISC_R_SUCCESS) { printf(";; Couldn't create key %s: %s\n", keynametext, isc_result_totext(result)); - goto failure; } - } else { - dst_key_attach(dstkey, &sig0key); - dst_key_free(&dstkey); } + failure: if (dstkey != NULL) { dst_key_free(&dstkey); diff --git a/bin/dig/dighost.h b/bin/dig/dighost.h index 5daffbc500..af4379ce4c 100644 --- a/bin/dig/dighost.h +++ b/bin/dig/dighost.h @@ -259,7 +259,7 @@ extern isc_sockaddr_t localaddr; extern char keynametext[MXNAME]; extern char keyfile[MXNAME]; extern char keysecret[MXNAME]; -extern const dns_name_t *hmacname; +extern dst_algorithm_t hmac; extern unsigned int digestbits; extern dns_tsigkey_t *tsigkey; extern bool validated; @@ -341,7 +341,7 @@ isc_result_t parse_netprefix(isc_sockaddr_t **sap, const char *value); void -parse_hmac(const char *hmacstr); +parse_hmac(const char *algname); dig_lookup_t * requeue_lookup(dig_lookup_t *lookold, bool servers); diff --git a/bin/named/config.c b/bin/named/config.c index 12b00891ca..7e981f85d1 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -922,14 +922,8 @@ struct keyalgorithms { { NULL, hmacnone, DST_ALG_UNKNOWN, 0 } }; isc_result_t -named_config_getkeyalgorithm(const char *str, const dns_name_t **name, +named_config_getkeyalgorithm(const char *str, unsigned int *typep, uint16_t *digestbits) { - return (named_config_getkeyalgorithm2(str, name, NULL, digestbits)); -} - -isc_result_t -named_config_getkeyalgorithm2(const char *str, const dns_name_t **name, - unsigned int *typep, uint16_t *digestbits) { int i; size_t len = 0; uint16_t bits; @@ -960,31 +954,6 @@ named_config_getkeyalgorithm2(const char *str, const dns_name_t **name, } else { bits = algorithms[i].size; } - - if (name != NULL) { - switch (algorithms[i].hmac) { - case hmacmd5: - *name = dns_tsig_hmacmd5_name; - break; - case hmacsha1: - *name = dns_tsig_hmacsha1_name; - break; - case hmacsha224: - *name = dns_tsig_hmacsha224_name; - break; - case hmacsha256: - *name = dns_tsig_hmacsha256_name; - break; - case hmacsha384: - *name = dns_tsig_hmacsha384_name; - break; - case hmacsha512: - *name = dns_tsig_hmacsha512_name; - break; - default: - UNREACHABLE(); - } - } if (typep != NULL) { *typep = algorithms[i].type; } diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index fd1201c41f..57783d19af 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -742,8 +742,8 @@ register_keys(const cfg_obj_t *control, const cfg_obj_t *keylist, algstr = cfg_obj_asstring(algobj); secretstr = cfg_obj_asstring(secretobj); - result = named_config_getkeyalgorithm2(algstr, NULL, - &algtype, NULL); + result = named_config_getkeyalgorithm(algstr, &algtype, + NULL); if (result != ISC_R_SUCCESS) { cfg_obj_log(control, named_g_lctx, ISC_LOG_WARNING, @@ -836,7 +836,7 @@ get_rndckey(isc_mem_t *mctx, controlkeylist_t *keyids) { algstr = cfg_obj_asstring(algobj); secretstr = cfg_obj_asstring(secretobj); - result = named_config_getkeyalgorithm2(algstr, NULL, &algtype, NULL); + result = named_config_getkeyalgorithm(algstr, &algtype, NULL); if (result != ISC_R_SUCCESS) { cfg_obj_log(key, named_g_lctx, ISC_LOG_WARNING, "unsupported algorithm '%s' in " diff --git a/bin/named/include/named/config.h b/bin/named/include/named/config.h index 076a67f71f..c6ba4eae96 100644 --- a/bin/named/include/named/config.h +++ b/bin/named/include/named/config.h @@ -66,8 +66,5 @@ named_config_getport(const cfg_obj_t *config, const char *type, in_port_t *portp); isc_result_t -named_config_getkeyalgorithm(const char *str, const dns_name_t **name, +named_config_getkeyalgorithm(const char *str, unsigned int *typep, uint16_t *digestbits); -isc_result_t -named_config_getkeyalgorithm2(const char *str, const dns_name_t **name, - unsigned int *typep, uint16_t *digestbits); diff --git a/bin/named/server.c b/bin/named/server.c index 058a85f6a3..ea45def9d4 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -4024,28 +4024,6 @@ minimal_cache_allowed(const cfg_obj_t *maps[4], static const char *const response_synonyms[] = { "response", NULL }; -static const dns_name_t * -algorithm_name(unsigned int alg) { - switch (alg) { - case DST_ALG_HMACMD5: - return (dns_tsig_hmacmd5_name); - case DST_ALG_HMACSHA1: - return (dns_tsig_hmacsha1_name); - case DST_ALG_HMACSHA224: - return (dns_tsig_hmacsha224_name); - case DST_ALG_HMACSHA256: - return (dns_tsig_hmacsha256_name); - case DST_ALG_HMACSHA384: - return (dns_tsig_hmacsha384_name); - case DST_ALG_HMACSHA512: - return (dns_tsig_hmacsha512_name); - case DST_ALG_GSSAPI: - return (dns_tsig_gssapi_name); - default: - UNREACHABLE(); - } -} - /* * Configure 'view' according to 'vconfig', taking defaults from * 'config' where values are missing in 'vconfig'. @@ -5067,9 +5045,9 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, dns_tsigkey_t *tsigkey = NULL; result = dns_tsigkey_createfromkey( named_g_server->session_keyname, - algorithm_name(named_g_server->session_keyalg), + named_g_server->session_keyalg, named_g_server->sessionkey, false, false, NULL, 0, 0, - mctx, NULL, &tsigkey); + mctx, &tsigkey); if (result == ISC_R_SUCCESS) { result = dns_tsigkeyring_add( ring, named_g_server->session_keyname, tsigkey); @@ -7480,9 +7458,9 @@ cleanup_session_key(named_server_t *server, isc_mem_t *mctx) { static isc_result_t generate_session_key(const char *filename, const char *keynamestr, - const dns_name_t *keyname, const char *algstr, - unsigned int algtype, uint16_t bits, isc_mem_t *mctx, - bool first_time, dst_key_t **keyp) { + const dns_name_t *keyname, dst_algorithm_t alg, + uint16_t bits, isc_mem_t *mctx, bool first_time, + dst_key_t **keyp) { isc_result_t result = ISC_R_SUCCESS; dst_key_t *key = NULL; isc_buffer_t key_txtbuffer; @@ -7497,9 +7475,8 @@ generate_session_key(const char *filename, const char *keynamestr, "generating session key for dynamic DNS"); /* generate key */ - result = dst_key_generate(keyname, algtype, bits, 1, 0, - DNS_KEYPROTO_ANY, dns_rdataclass_in, mctx, - &key, NULL); + result = dst_key_generate(keyname, alg, bits, 1, 0, DNS_KEYPROTO_ANY, + dns_rdataclass_in, mctx, &key, NULL); if (result != ISC_R_SUCCESS) { return (result); } @@ -7528,12 +7505,12 @@ generate_session_key(const char *filename, const char *keynamestr, "key \"%s\" {\n" "\talgorithm %s;\n" "\tsecret \"%.*s\";\n};\n", - keynamestr, algstr, (int)isc_buffer_usedlength(&key_txtbuffer), + keynamestr, dst_hmac_algorithm_totext(alg), + (int)isc_buffer_usedlength(&key_txtbuffer), (char *)isc_buffer_base(&key_txtbuffer)); CHECK(isc_stdio_flush(fp)); result = isc_stdio_close(fp); - fp = NULL; if (result != ISC_R_SUCCESS) { goto cleanup; } @@ -7561,14 +7538,13 @@ cleanup: static isc_result_t configure_session_key(const cfg_obj_t **maps, named_server_t *server, isc_mem_t *mctx, bool first_time) { - const char *keyfile, *keynamestr, *algstr; + const char *keyfile = NULL, *keynamestr = NULL, *algstr = NULL; unsigned int algtype; dns_fixedname_t fname; - dns_name_t *keyname; - const dns_name_t *algname; + dns_name_t *keyname = NULL; isc_buffer_t buffer; uint16_t bits; - const cfg_obj_t *obj; + const cfg_obj_t *obj = NULL; bool need_deleteold = false; bool need_createnew = false; isc_result_t result; @@ -7601,9 +7577,7 @@ configure_session_key(const cfg_obj_t **maps, named_server_t *server, result = named_config_get(maps, "session-keyalg", &obj); INSIST(result == ISC_R_SUCCESS); algstr = cfg_obj_asstring(obj); - algname = NULL; - result = named_config_getkeyalgorithm2(algstr, &algname, &algtype, - &bits); + result = named_config_getkeyalgorithm(algstr, &algtype, &bits); if (result != ISC_R_SUCCESS) { const char *s = " (keeping current key)"; @@ -7654,7 +7628,7 @@ configure_session_key(const cfg_obj_t **maps, named_server_t *server, server->session_keyalg = algtype; server->session_keybits = bits; - CHECK(generate_session_key(keyfile, keynamestr, keyname, algstr, + CHECK(generate_session_key(keyfile, keynamestr, keyname, algtype, bits, mctx, first_time, &server->sessionkey)); } diff --git a/bin/named/tsigconf.c b/bin/named/tsigconf.c index cfa934f0e1..a1ed24ef93 100644 --- a/bin/named/tsigconf.c +++ b/bin/named/tsigconf.c @@ -47,7 +47,7 @@ add_initial_keys(const cfg_obj_t *list, dns_tsigkeyring_t *ring, const cfg_obj_t *algobj = NULL; const cfg_obj_t *secretobj = NULL; dns_name_t keyname; - const dns_name_t *alg = NULL; + dst_algorithm_t alg = DST_ALG_UNKNOWN; const char *algstr = NULL; char keynamedata[1024]; isc_buffer_t keynamesrc, keynamebuf; diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 424dfce293..eaf74b39a4 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -378,13 +378,13 @@ reset_system(void) { } static bool -parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, +parse_hmac(const char *hmacstr, size_t len, dst_algorithm_t *hmac, uint16_t *digestbitsp) { uint16_t digestbits = 0; isc_result_t result; char buf[20]; - REQUIRE(hmac != NULL && *hmac == NULL); + REQUIRE(hmac != NULL); REQUIRE(hmacstr != NULL); if (len >= sizeof(buf)) { @@ -396,9 +396,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, strlcpy(buf, hmacstr, ISC_MIN(len + 1, sizeof(buf))); if (strcasecmp(buf, "hmac-md5") == 0) { - *hmac = DNS_TSIG_HMACMD5_NAME; + *hmac = DST_ALG_HMACMD5; } else if (strncasecmp(buf, "hmac-md5-", 9) == 0) { - *hmac = DNS_TSIG_HMACMD5_NAME; + *hmac = DST_ALG_HMACMD5; result = isc_parse_uint16(&digestbits, &buf[9], 10); if (result != ISC_R_SUCCESS || digestbits > 128) { error("digest-bits out of range [0..128]"); @@ -406,9 +406,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, } *digestbitsp = (digestbits + 7) & ~0x7U; } else if (strcasecmp(buf, "hmac-sha1") == 0) { - *hmac = DNS_TSIG_HMACSHA1_NAME; + *hmac = DST_ALG_HMACSHA1; } else if (strncasecmp(buf, "hmac-sha1-", 10) == 0) { - *hmac = DNS_TSIG_HMACSHA1_NAME; + *hmac = DST_ALG_HMACSHA1; result = isc_parse_uint16(&digestbits, &buf[10], 10); if (result != ISC_R_SUCCESS || digestbits > 160) { error("digest-bits out of range [0..160]"); @@ -416,9 +416,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, } *digestbitsp = (digestbits + 7) & ~0x7U; } else if (strcasecmp(buf, "hmac-sha224") == 0) { - *hmac = DNS_TSIG_HMACSHA224_NAME; + *hmac = DST_ALG_HMACSHA224; } else if (strncasecmp(buf, "hmac-sha224-", 12) == 0) { - *hmac = DNS_TSIG_HMACSHA224_NAME; + *hmac = DST_ALG_HMACSHA224; result = isc_parse_uint16(&digestbits, &buf[12], 10); if (result != ISC_R_SUCCESS || digestbits > 224) { error("digest-bits out of range [0..224]"); @@ -426,9 +426,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, } *digestbitsp = (digestbits + 7) & ~0x7U; } else if (strcasecmp(buf, "hmac-sha256") == 0) { - *hmac = DNS_TSIG_HMACSHA256_NAME; + *hmac = DST_ALG_HMACSHA256; } else if (strncasecmp(buf, "hmac-sha256-", 12) == 0) { - *hmac = DNS_TSIG_HMACSHA256_NAME; + *hmac = DST_ALG_HMACSHA256; result = isc_parse_uint16(&digestbits, &buf[12], 10); if (result != ISC_R_SUCCESS || digestbits > 256) { error("digest-bits out of range [0..256]"); @@ -436,9 +436,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, } *digestbitsp = (digestbits + 7) & ~0x7U; } else if (strcasecmp(buf, "hmac-sha384") == 0) { - *hmac = DNS_TSIG_HMACSHA384_NAME; + *hmac = DST_ALG_HMACSHA384; } else if (strncasecmp(buf, "hmac-sha384-", 12) == 0) { - *hmac = DNS_TSIG_HMACSHA384_NAME; + *hmac = DST_ALG_HMACSHA384; result = isc_parse_uint16(&digestbits, &buf[12], 10); if (result != ISC_R_SUCCESS || digestbits > 384) { error("digest-bits out of range [0..384]"); @@ -446,9 +446,9 @@ parse_hmac(const dns_name_t **hmac, const char *hmacstr, size_t len, } *digestbitsp = (digestbits + 7) & ~0x7U; } else if (strcasecmp(buf, "hmac-sha512") == 0) { - *hmac = DNS_TSIG_HMACSHA512_NAME; + *hmac = DST_ALG_HMACSHA512; } else if (strncasecmp(buf, "hmac-sha512-", 12) == 0) { - *hmac = DNS_TSIG_HMACSHA512_NAME; + *hmac = DST_ALG_HMACSHA512; result = isc_parse_uint16(&digestbits, &buf[12], 10); if (result != ISC_R_SUCCESS || digestbits > 512) { error("digest-bits out of range [0..512]"); @@ -483,12 +483,12 @@ setup_keystr(void) { isc_buffer_t secretbuf; isc_result_t result; isc_buffer_t keynamesrc; - char *secretstr; - char *s, *n; + char *secretstr = NULL; + char *s = NULL, *n = NULL; dns_fixedname_t fkeyname; - dns_name_t *mykeyname; - char *name; - const dns_name_t *hmacname = NULL; + dns_name_t *mykeyname = NULL; + char *name = NULL; + dst_algorithm_t hmac; uint16_t digestbits = 0; mykeyname = dns_fixedname_initname(&fkeyname); @@ -507,11 +507,11 @@ setup_keystr(void) { } name = secretstr; secretstr = n + 1; - if (!parse_hmac(&hmacname, keystr, s - keystr, &digestbits)) { + if (!parse_hmac(keystr, s - keystr, &hmac, &digestbits)) { exit(1); } } else { - hmacname = DNS_TSIG_HMACMD5_NAME; + hmac = DST_ALG_HMACMD5; name = keystr; n = s; } @@ -538,8 +538,8 @@ setup_keystr(void) { secretlen = isc_buffer_usedlength(&secretbuf); debug("keycreate"); - result = dns_tsigkey_create(mykeyname, hmacname, secret, secretlen, - gmctx, &tsigkey); + result = dns_tsigkey_create(mykeyname, hmac, secret, secretlen, gmctx, + &tsigkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "could not create key from %s: %s\n", keystr, isc_result_totext(result)); @@ -622,7 +622,7 @@ static void setup_keyfile(isc_mem_t *mctx, isc_log_t *lctx) { dst_key_t *dstkey = NULL; isc_result_t result; - const dns_name_t *hmacname = NULL; + dst_algorithm_t hmac = DST_ALG_UNKNOWN; debug("Creating key..."); @@ -653,37 +653,26 @@ setup_keyfile(isc_mem_t *mctx, isc_log_t *lctx) { switch (dst_key_alg(dstkey)) { case DST_ALG_HMACMD5: - hmacname = DNS_TSIG_HMACMD5_NAME; - break; case DST_ALG_HMACSHA1: - hmacname = DNS_TSIG_HMACSHA1_NAME; - break; case DST_ALG_HMACSHA224: - hmacname = DNS_TSIG_HMACSHA224_NAME; - break; case DST_ALG_HMACSHA256: - hmacname = DNS_TSIG_HMACSHA256_NAME; - break; case DST_ALG_HMACSHA384: - hmacname = DNS_TSIG_HMACSHA384_NAME; - break; case DST_ALG_HMACSHA512: - hmacname = DNS_TSIG_HMACSHA512_NAME; + hmac = dst_key_alg(dstkey); break; - } - if (hmacname != NULL) { - result = dns_tsigkey_createfromkey( - dst_key_name(dstkey), hmacname, dstkey, false, false, - NULL, 0, 0, mctx, &tsigkey); - dst_key_free(&dstkey); - if (result != ISC_R_SUCCESS) { - fprintf(stderr, "could not create key from %s: %s\n", - keyfile, isc_result_totext(result)); - return; - } - } else { + default: dst_key_attach(dstkey, &sig0key); dst_key_free(&dstkey); + return; + } + + result = dns_tsigkey_createfromkey(dst_key_name(dstkey), hmac, dstkey, + false, false, NULL, 0, 0, mctx, + &tsigkey); + dst_key_free(&dstkey); + if (result != ISC_R_SUCCESS) { + fprintf(stderr, "could not create key from %s: %s\n", keyfile, + isc_result_totext(result)); } } @@ -1650,7 +1639,7 @@ evaluate_key(char *cmdline) { int secretlen; unsigned char *secret = NULL; isc_buffer_t secretbuf; - const dns_name_t *hmacname = NULL; + dst_algorithm_t hmac = DST_ALG_UNKNOWN; uint16_t digestbits = 0; char *n; @@ -1664,12 +1653,12 @@ evaluate_key(char *cmdline) { n = strchr(namestr, ':'); if (n != NULL) { - if (!parse_hmac(&hmacname, namestr, n - namestr, &digestbits)) { + if (!parse_hmac(namestr, n - namestr, &hmac, &digestbits)) { return (STATUS_SYNTAX); } namestr = n + 1; } else { - hmacname = DNS_TSIG_HMACMD5_NAME; + hmac = DST_ALG_HMACMD5; } isc_buffer_init(&b, namestr, strlen(namestr)); @@ -1701,8 +1690,8 @@ evaluate_key(char *cmdline) { if (tsigkey != NULL) { dns_tsigkey_detach(&tsigkey); } - result = dns_tsigkey_create(mykeyname, hmacname, secret, secretlen, - gmctx, &tsigkey); + result = dns_tsigkey_create(mykeyname, hmac, secret, secretlen, gmctx, + &tsigkey); isc_mem_free(gmctx, secret); if (result != ISC_R_SUCCESS) { fprintf(stderr, "could not create key from %s %s: %s\n", diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index 72f50cc24a..1f4417eb77 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -264,7 +264,7 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { return (1); } - result = dns_tsigkey_create(name, dns_tsig_hmacsha256_name, secret, + result = dns_tsigkey_create(name, DST_ALG_HMACSHA256, secret, sizeof(secret), mctx, &tsigkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_tsigkey_create failed: %s\n", diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index 4c7068acf9..c975c236ed 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -109,12 +109,12 @@ dns_tsigkey_identity(const dns_tsigkey_t *tsigkey); */ isc_result_t -dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, +dns_tsigkey_create(const dns_name_t *name, dst_algorithm_t algorithm, unsigned char *secret, int length, isc_mem_t *mctx, dns_tsigkey_t **key); isc_result_t -dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, +dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index f1426d40eb..1ec1898e7a 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -49,6 +49,7 @@ #include #include "dst_internal.h" +#include "tsig_p.h" #define TEMP_BUFFER_SZ 8192 #define TKEY_RANDOM_AMOUNT 16 @@ -264,8 +265,9 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, } #endif /* HAVE_GSSAPI */ RETERR(dns_tsigkey_createfromkey( - name, &tkeyin->algorithm, dstkey, true, false, - principal, now, expire, ring->mctx, &tsigkey)); + name, dns__tsig_algfromname(&tkeyin->algorithm), dstkey, + true, false, principal, now, expire, ring->mctx, + &tsigkey)); RETERR(dns_tsigkeyring_add(ring, name, tsigkey)); dst_key_free(&dstkey); tkeyout->inception = now; @@ -680,20 +682,18 @@ dns_tkey_buildgssquery(dns_message_t *msg, const dns_name_t *name, return (result); } - tkey.common.rdclass = dns_rdataclass_any; - tkey.common.rdtype = dns_rdatatype_tkey; - ISC_LINK_INIT(&tkey.common, link); - tkey.mctx = NULL; + tkey = (dns_rdata_tkey_t){ + .common.rdclass = dns_rdataclass_any, + .common.rdtype = dns_rdatatype_tkey, + .common.link = ISC_LINK_INITIALIZER, + .inception = now, + .expire = now + lifetime, + .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); - tkey.inception = now; - tkey.expire = now + lifetime; - tkey.mode = DNS_TKEYMODE_GSSAPI; - tkey.error = 0; - tkey.key = isc_buffer_base(&token); - tkey.keylen = isc_buffer_usedlength(&token); - tkey.other = NULL; - tkey.otherlen = 0; return (buildquery(msg, name, &tkey)); } @@ -816,8 +816,8 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, * anything yet. */ - RETERR(dns_tsigkey_createfromkey(tkeyname, DNS_TSIG_GSSAPI_NAME, dstkey, - true, false, NULL, rtkey.inception, + RETERR(dns_tsigkey_createfromkey(tkeyname, DST_ALG_GSSAPI, dstkey, true, + false, NULL, rtkey.inception, rtkey.expire, ring->mctx, &tsigkey)); RETERR(dns_tsigkeyring_add(ring, tkeyname, tsigkey)); if (outkey == NULL) { diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index f5c4a149d5..4ae37e764a 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -187,19 +187,39 @@ adjust_lru(dns_tsigkey_t *tkey) { } } +static const dns_name_t * +namefromalg(dst_algorithm_t alg) { + switch (alg) { + case DST_ALG_HMACMD5: + return (dns_tsig_hmacmd5_name); + case DST_ALG_HMACSHA1: + return (dns_tsig_hmacsha1_name); + case DST_ALG_HMACSHA224: + return (dns_tsig_hmacsha224_name); + case DST_ALG_HMACSHA256: + return (dns_tsig_hmacsha256_name); + case DST_ALG_HMACSHA384: + return (dns_tsig_hmacsha384_name); + case DST_ALG_HMACSHA512: + return (dns_tsig_hmacsha512_name); + case DST_ALG_GSSAPI: + return (dns_tsig_gssapi_name); + default: + return (NULL); + } +} + isc_result_t -dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, +dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, dst_key_t *dstkey, bool generated, bool restored, const dns_name_t *creator, isc_stdtime_t inception, isc_stdtime_t expire, isc_mem_t *mctx, dns_tsigkey_t **keyp) { dns_tsigkey_t *tkey = NULL; isc_result_t ret; - unsigned int dstalg = 0; REQUIRE(keyp != NULL && *keyp == NULL); REQUIRE(name != NULL); - REQUIRE(algorithm != NULL); REQUIRE(mctx != NULL); tkey = isc_mem_get(mctx, sizeof(dns_tsigkey_t)); @@ -215,31 +235,18 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, dns_name_dup(name, mctx, &tkey->name); (void)dns_name_downcase(&tkey->name, &tkey->name, NULL); - /* Check against known algorithm names */ - dstalg = dns__tsig_algfromname(algorithm); - if (dstalg != 0) { - /* - * 'algorithm' must be set to a static pointer - * so that dns__tsig_algallocated() can compare them. - */ - tkey->algorithm = dns__tsig_algnamefromname(algorithm); - if (dstkey != NULL && dst_key_alg(dstkey) != dstalg) { + if (algorithm != DST_ALG_UNKNOWN) { + if (dstkey != NULL && dst_key_alg(dstkey) != algorithm) { ret = DNS_R_BADALG; goto cleanup_name; } - } else { - dns_name_t *tmpname = NULL; - if (dstkey != NULL) { - ret = DNS_R_BADALG; - goto cleanup_name; - } - tmpname = isc_mem_get(mctx, sizeof(dns_name_t)); - dns_name_init(tmpname, NULL); - dns_name_dup(algorithm, mctx, tmpname); - (void)dns_name_downcase(tmpname, tmpname, NULL); - tkey->algorithm = tmpname; + } else if (dstkey != NULL) { + ret = DNS_R_BADALG; + goto cleanup_name; } + tkey->algorithm = namefromalg(algorithm); + if (creator != NULL) { tkey->creator = isc_mem_get(mctx, sizeof(dns_name_t)); dns_name_init(tkey->creator, NULL); @@ -257,7 +264,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm, * Ignore this if it's a GSS key, since the key size is meaningless. */ if (dstkey != NULL && dst_key_size(dstkey) < 64 && - dstalg != DST_ALG_GSSAPI) + algorithm != DST_ALG_GSSAPI) { char namestr[DNS_NAME_FORMATSIZE]; dns_name_format(name, namestr, sizeof(namestr)); @@ -360,34 +367,15 @@ ISC_REFCOUNT_IMPL(dns_tsigkeyring, destroyring); /* * Look up the DST_ALG_ constant for a given name. */ -unsigned int +dst_algorithm_t dns__tsig_algfromname(const dns_name_t *algorithm) { - int i; - int n = sizeof(known_algs) / sizeof(*known_algs); - for (i = 0; i < n; ++i) { + for (size_t i = 0; i < ARRAY_SIZE(known_algs); ++i) { const dns_name_t *name = known_algs[i].name; if (algorithm == name || dns_name_equal(algorithm, name)) { return (known_algs[i].dstalg); } } - return (0); -} - -/* - * Convert an algorithm name into a pointer to the - * corresponding pre-defined dns_name_t structure. - */ -const dns_name_t * -dns__tsig_algnamefromname(const dns_name_t *algorithm) { - int i; - int n = sizeof(known_algs) / sizeof(*known_algs); - for (i = 0; i < n; ++i) { - const dns_name_t *name = known_algs[i].name; - if (algorithm == name || dns_name_equal(algorithm, name)) { - return (name); - } - } - return (NULL); + return (DST_ALG_UNKNOWN); } /* @@ -465,7 +453,7 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { } dstalg = dns__tsig_algfromname(algorithm); - if (dstalg == 0) { + if (dstalg == DST_ALG_UNKNOWN) { return (DNS_R_BADALG); } @@ -476,7 +464,7 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { return (result); } - result = dns_tsigkey_createfromkey(name, algorithm, dstkey, true, true, + result = dns_tsigkey_createfromkey(name, dstalg, dstkey, true, true, creator, inception, expire, ring->mctx, &tkey); if (result == ISC_R_SUCCESS) { @@ -573,27 +561,25 @@ dns_tsigkey_identity(const dns_tsigkey_t *tsigkey) { } isc_result_t -dns_tsigkey_create(const dns_name_t *name, const dns_name_t *algorithm, +dns_tsigkey_create(const dns_name_t *name, dst_algorithm_t algorithm, unsigned char *secret, int length, isc_mem_t *mctx, dns_tsigkey_t **key) { dst_key_t *dstkey = NULL; isc_result_t result; - unsigned int dstalg = 0; REQUIRE(length >= 0); if (length > 0) { REQUIRE(secret != NULL); } - dstalg = dns__tsig_algfromname(algorithm); - if (dns__tsig_algvalid(dstalg)) { + if (dns__tsig_algvalid(algorithm)) { if (secret != NULL) { isc_buffer_t b; isc_buffer_init(&b, secret, length); isc_buffer_add(&b, length); result = dst_key_frombuffer( - name, dstalg, DNS_KEYOWNER_ENTITY, + name, algorithm, DNS_KEYOWNER_ENTITY, DNS_KEYPROTO_DNSSEC, dns_rdataclass_in, &b, mctx, &dstkey); if (result != ISC_R_SUCCESS) { @@ -685,36 +671,24 @@ dns_tsig_sign(dns_message_t *msg) { mctx = msg->mctx; - tsig.mctx = mctx; - tsig.common.rdclass = dns_rdataclass_any; - tsig.common.rdtype = dns_rdatatype_tsig; - ISC_LINK_INIT(&tsig.common, link); + now = msg->fuzzing ? msg->fuzztime : isc_stdtime_now(); + tsig = (dns_rdata_any_tsig_t){ + .mctx = mctx, + .common.rdclass = dns_rdataclass_any, + .common.rdtype = dns_rdatatype_tsig, + .common.link = ISC_LINK_INITIALIZER, + .timesigned = now + msg->timeadjust, + .fudge = DNS_TSIG_FUDGE, + .originalid = msg->id, + .error = response ? msg->querytsigstatus : dns_rcode_noerror, + }; + dns_name_init(&tsig.algorithm, NULL); dns_name_clone(key->algorithm, &tsig.algorithm); - if (msg->fuzzing) { - now = msg->fuzztime; - } else { - now = isc_stdtime_now(); - } - - tsig.timesigned = now + msg->timeadjust; - tsig.fudge = DNS_TSIG_FUDGE; - - tsig.originalid = msg->id; - isc_buffer_init(&databuf, data, sizeof(data)); - if (response) { - tsig.error = msg->querytsigstatus; - } else { - tsig.error = dns_rcode_noerror; - } - - if (tsig.error != dns_tsigerror_badtime) { - tsig.otherlen = 0; - tsig.other = NULL; - } else { + if (tsig.error == dns_tsigerror_badtime) { isc_buffer_t otherbuf; tsig.otherlen = BADTIMELEN; @@ -1067,8 +1041,9 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, } if (ret != ISC_R_SUCCESS) { msg->tsigstatus = dns_tsigerror_badkey; - ret = dns_tsigkey_create(keyname, &tsig.algorithm, NULL, - 0, mctx, &msg->tsigkey); + ret = dns_tsigkey_create( + keyname, dns__tsig_algfromname(&tsig.algorithm), + NULL, 0, mctx, &msg->tsigkey); if (ret != ISC_R_SUCCESS) { return (ret); } @@ -1714,6 +1689,7 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, REQUIRE(VALID_TSIGKEY(tkey)); REQUIRE(VALID_TSIGKEYRING(ring)); + REQUIRE(tkey->ring == NULL); REQUIRE(name != NULL); RWLOCK(&ring->lock, isc_rwlocktype_write); diff --git a/lib/dns/tsig_p.h b/lib/dns/tsig_p.h index a19688c54e..80e7323736 100644 --- a/lib/dns/tsig_p.h +++ b/lib/dns/tsig_p.h @@ -30,10 +30,8 @@ ISC_LANG_BEGINDECLS bool dns__tsig_algvalid(unsigned int alg); -unsigned int +dst_algorithm_t dns__tsig_algfromname(const dns_name_t *algorithm); -const dns_name_t * -dns__tsig_algnamefromname(const dns_name_t *algorithm); bool dns__tsig_algallocated(const dns_name_t *algorithm); diff --git a/lib/ns/client.c b/lib/ns/client.c index 395ebde6bf..d4bd311766 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -2095,8 +2095,7 @@ ns_client_request(isc_nmhandle_t *handle, isc_result_t eresult, signame = NULL; if (dns_message_gettsig(client->message, &signame) != NULL) { char namebuf[DNS_NAME_FORMATSIZE]; - char cnamebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(signame, namebuf, sizeof(namebuf)); + status = client->message->tsigstatus; isc_buffer_init(&b, tsigrcode, sizeof(tsigrcode) - 1); tresult = dns_tsigrcode_totext(status, &b); @@ -2105,23 +2104,17 @@ ns_client_request(isc_nmhandle_t *handle, isc_result_t eresult, if (client->message->tsigkey->generated) { dns_name_format( client->message->tsigkey->creator, - cnamebuf, sizeof(cnamebuf)); - ns_client_log( - client, DNS_LOGCATEGORY_SECURITY, - NS_LOGMODULE_CLIENT, ISC_LOG_ERROR, - "request has invalid signature: " - "TSIG %s (%s): %s (%s)", - namebuf, cnamebuf, - isc_result_totext(result), tsigrcode); + namebuf, sizeof(namebuf)); } else { - ns_client_log( - client, DNS_LOGCATEGORY_SECURITY, - NS_LOGMODULE_CLIENT, ISC_LOG_ERROR, - "request has invalid signature: " - "TSIG %s: %s (%s)", - namebuf, isc_result_totext(result), - tsigrcode); + dns_name_format(signame, namebuf, + sizeof(namebuf)); } + ns_client_log(client, DNS_LOGCATEGORY_SECURITY, + NS_LOGMODULE_CLIENT, ISC_LOG_ERROR, + "request has invalid signature: " + "TSIG %s: %s (%s)", + namebuf, isc_result_totext(result), + tsigrcode); } else { status = client->message->sig0status; isc_buffer_init(&b, tsigrcode, sizeof(tsigrcode) - 1); diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index 65837fc39b..fdc9ed3989 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -293,7 +293,7 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { result = dns_tsigkeyring_create(mctx, &ring); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_tsigkey_create(keyname, dns_tsig_hmacsha256_name, secret, + result = dns_tsigkey_create(keyname, DST_ALG_HMACSHA256, secret, sizeof(secret), mctx, &key); assert_int_equal(result, ISC_R_SUCCESS); result = dns_tsigkeyring_add(ring, keyname, key); @@ -485,63 +485,6 @@ ISC_RUN_TEST_IMPL(algvalid) { assert_false(dns__tsig_algvalid(DST_ALG_GSSAPI)); } -/* Tests the dns__tsig_algfromname function */ -ISC_RUN_TEST_IMPL(algfromname) { - UNUSED(state); - - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACMD5_NAME), - DST_ALG_HMACMD5); - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACSHA1_NAME), - DST_ALG_HMACSHA1); - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACSHA224_NAME), - DST_ALG_HMACSHA224); - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACSHA256_NAME), - DST_ALG_HMACSHA256); - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACSHA384_NAME), - DST_ALG_HMACSHA384); - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_HMACSHA512_NAME), - DST_ALG_HMACSHA512); - - assert_int_equal(dns__tsig_algfromname(DNS_TSIG_GSSAPI_NAME), - DST_ALG_GSSAPI); - - assert_int_equal(dns__tsig_algfromname(dns_rootname), 0); -} - -/* Tests the dns__tsig_algnamefromname function */ - -/* - * Helper function to create a dns_name_t from a string and see if - * the dns__tsig_algnamefromname function can correctly match it against the - * static table of known algorithms. - */ -static void -test_name(const char *name_string, const dns_name_t *expected) { - dns_name_t name; - dns_name_init(&name, NULL); - assert_int_equal(dns_name_fromstring(&name, name_string, 0, mctx), - ISC_R_SUCCESS); - assert_ptr_equal(dns__tsig_algnamefromname(&name), expected); - dns_name_free(&name, mctx); -} - -ISC_RUN_TEST_IMPL(algnamefromname) { - UNUSED(state); - - /* test the standard algorithms */ - test_name("hmac-md5.sig-alg.reg.int", DNS_TSIG_HMACMD5_NAME); - test_name("hmac-sha1", DNS_TSIG_HMACSHA1_NAME); - test_name("hmac-sha224", DNS_TSIG_HMACSHA224_NAME); - test_name("hmac-sha256", DNS_TSIG_HMACSHA256_NAME); - test_name("hmac-sha384", DNS_TSIG_HMACSHA384_NAME); - test_name("hmac-sha512", DNS_TSIG_HMACSHA512_NAME); - - test_name("gss-tsig", DNS_TSIG_GSSAPI_NAME); - - /* try another name that isn't a standard algorithm name */ - assert_null(dns__tsig_algnamefromname(dns_rootname)); -} - /* Tests the dns__tsig_algallocated function */ ISC_RUN_TEST_IMPL(algallocated) { UNUSED(state); @@ -564,8 +507,6 @@ ISC_RUN_TEST_IMPL(algallocated) { ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(tsig_tcp, setup_test, teardown_test) ISC_TEST_ENTRY(algvalid) -ISC_TEST_ENTRY(algfromname) -ISC_TEST_ENTRY_CUSTOM(algnamefromname, setup_test, teardown_test) ISC_TEST_ENTRY(algallocated) ISC_TEST_LIST_END From f2d5782e89e2c9ea28141842bda53126bc4bc3a8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Tue, 11 Apr 2023 23:43:19 -0700 Subject: [PATCH 06/11] get_key_struct() can no longer fail remove checks for NULL return values. --- lib/dns/dst_api.c | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 21f0f56f73..d01062781a 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -651,9 +651,6 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type, pubkey->key_flags, pubkey->key_proto, pubkey->key_size, pubkey->key_class, pubkey->key_ttl, mctx); - if (key == NULL) { - RETERR(ISC_R_NOMEMORY); - } if (key->func->parse == NULL) { RETERR(DST_R_UNSUPPORTEDALG); @@ -876,9 +873,6 @@ dst_key_fromgssapi(const dns_name_t *name, dns_gss_ctx_id_t gssctx, key = get_key_struct(name, DST_ALG_GSSAPI, 0, DNS_KEYPROTO_DNSSEC, 0, dns_rdataclass_in, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } if (intoken != NULL) { /* @@ -978,9 +972,6 @@ dst_key_buildinternal(const dns_name_t *name, unsigned int alg, key = get_key_struct(name, alg, flags, protocol, bits, rdclass, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } key->keydata.generic = data; @@ -1011,9 +1002,6 @@ dst_key_fromlabel(const dns_name_t *name, int alg, unsigned int flags, CHECKALG(alg); key = get_key_struct(name, alg, flags, protocol, 0, rdclass, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } if (key->func->fromlabel == NULL) { dst_key_free(&key); @@ -1053,9 +1041,6 @@ dst_key_generate(const dns_name_t *name, unsigned int alg, unsigned int bits, key = get_key_struct(name, alg, flags, protocol, bits, rdclass, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } if (bits == 0) { /*%< NULL KEY */ key->key_flags |= DNS_KEYTYPE_NOKEY; @@ -1546,9 +1531,6 @@ dst_key_restore(dns_name_t *name, unsigned int alg, unsigned int flags, } key = get_key_struct(name, alg, flags, protocol, 0, rdclass, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } result = (dst_t_func[alg]->restore)(key, keystr); if (result == ISC_R_SUCCESS) { @@ -2330,9 +2312,6 @@ frombuffer(const dns_name_t *name, unsigned int alg, unsigned int flags, REQUIRE(keyp != NULL && *keyp == NULL); key = get_key_struct(name, alg, flags, protocol, 0, rdclass, 0, mctx); - if (key == NULL) { - return (ISC_R_NOMEMORY); - } if (isc_buffer_remaininglength(source) > 0) { ret = algorithm_status(alg); From e64b44a5cb98ad8c191a82f85944cf25381cb3d1 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 12 Apr 2023 00:17:30 -0700 Subject: [PATCH 07/11] remove dns__tsig_algallocated() this function was no longer needed, because the algorithm name is no longer copied into the tsigkey object by dns_tsigkey_createfromkey(); it's always just a pointer to a statically defined name. --- lib/dns/tsig.c | 26 -------------------------- lib/dns/tsig_p.h | 2 -- tests/dns/tsig_test.c | 20 -------------------- 3 files changed, 48 deletions(-) diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 4ae37e764a..c75caff365 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -378,27 +378,6 @@ dns__tsig_algfromname(const dns_name_t *algorithm) { return (DST_ALG_UNKNOWN); } -/* - * Test whether the passed algorithm is NOT a pointer to one of the - * pre-defined known algorithms (and therefore one that has been - * dynamically allocated). - * - * This will return an incorrect result if passed a dynamically allocated - * dns_name_t that happens to match one of the pre-defined names. - */ -bool -dns__tsig_algallocated(const dns_name_t *algorithm) { - int i; - int n = sizeof(known_algs) / sizeof(*known_algs); - for (i = 0; i < n; ++i) { - const dns_name_t *name = known_algs[i].name; - if (algorithm == name) { - return (false); - } - } - return (true); -} - static isc_result_t restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { dst_key_t *dstkey = NULL; @@ -604,11 +583,6 @@ destroy_tsigkey(dns_tsigkey_t *key) { key->magic = 0; dns_name_free(&key->name, key->mctx); - if (dns__tsig_algallocated(key->algorithm)) { - dns_name_t *name = UNCONST(key->algorithm); - dns_name_free(name, key->mctx); - isc_mem_put(key->mctx, name, sizeof(dns_name_t)); - } if (key->key != NULL) { dst_key_free(&key->key); } diff --git a/lib/dns/tsig_p.h b/lib/dns/tsig_p.h index 80e7323736..42316a0608 100644 --- a/lib/dns/tsig_p.h +++ b/lib/dns/tsig_p.h @@ -32,7 +32,5 @@ bool dns__tsig_algvalid(unsigned int alg); dst_algorithm_t dns__tsig_algfromname(const dns_name_t *algorithm); -bool -dns__tsig_algallocated(const dns_name_t *algorithm); ISC_LANG_ENDDECLS diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index fdc9ed3989..88632b921a 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -485,29 +485,9 @@ ISC_RUN_TEST_IMPL(algvalid) { assert_false(dns__tsig_algvalid(DST_ALG_GSSAPI)); } -/* Tests the dns__tsig_algallocated function */ -ISC_RUN_TEST_IMPL(algallocated) { - UNUSED(state); - - /* test the standard algorithms */ - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACMD5_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA1_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA224_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA256_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA384_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA512_NAME)); - - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA512_NAME)); - assert_false(dns__tsig_algallocated(DNS_TSIG_HMACSHA512_NAME)); - - /* try another name that isn't a standard algorithm name */ - assert_true(dns__tsig_algallocated(dns_rootname)); -} - ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(tsig_tcp, setup_test, teardown_test) ISC_TEST_ENTRY(algvalid) -ISC_TEST_ENTRY(algallocated) ISC_TEST_LIST_END ISC_TEST_MAIN From 6105a7d360ac473e13941df57cdab731b66f8b79 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 12 Apr 2023 00:14:04 -0700 Subject: [PATCH 08/11] convert TSIG keyring storage from RBT to hash table since it is not necessary to find partial matches when looking up names in a TSIG keyring, we can use a hash table instead of an RBT to store them. the tsigkey object now stores the key name as a dns_fixedname rather than allocating memory for it. the `name` parameter to dns_tsigkeyring_add() has been removed; it was unneeded since the tsigkey object already contains a copy of the name. the opportunistic cleanup_ring() function has been removed; it was only slowing down lookups. --- bin/named/server.c | 3 +- bin/named/tsigconf.c | 7 +- bin/named/zoneconf.c | 2 +- bin/nsupdate/nsupdate.c | 6 +- fuzz/dns_message_checksig.c | 17 +-- lib/dns/include/dns/tsig.h | 26 ++--- lib/dns/message.c | 4 +- lib/dns/tkey.c | 4 +- lib/dns/tsig.c | 219 ++++++++++++------------------------ lib/dns/view.c | 6 +- lib/dns/zone.c | 6 +- lib/ns/client.c | 2 +- lib/ns/notify.c | 2 +- lib/ns/xfrout.c | 2 +- tests/dns/tsig_test.c | 8 +- 15 files changed, 104 insertions(+), 210 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index ea45def9d4..a4dc7215fb 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -5049,8 +5049,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, named_g_server->sessionkey, false, false, NULL, 0, 0, mctx, &tsigkey); if (result == ISC_R_SUCCESS) { - result = dns_tsigkeyring_add( - ring, named_g_server->session_keyname, tsigkey); + result = dns_tsigkeyring_add(ring, tsigkey); dns_tsigkey_detach(&tsigkey); } CHECK(result); diff --git a/bin/named/tsigconf.c b/bin/named/tsigconf.c index a1ed24ef93..dfeadfd99f 100644 --- a/bin/named/tsigconf.c +++ b/bin/named/tsigconf.c @@ -108,7 +108,7 @@ add_initial_keys(const cfg_obj_t *list, dns_tsigkeyring_t *ring, isc_mem_put(mctx, secret, secretalloc); secret = NULL; if (ret == ISC_R_SUCCESS) { - ret = dns_tsigkeyring_add(ring, &keyname, tsigkey); + ret = dns_tsigkeyring_add(ring, tsigkey); } if (ret != ISC_R_SUCCESS) { if (tsigkey != NULL) { @@ -154,10 +154,7 @@ named_tsigkeyring_fromconfig(const cfg_obj_t *config, const cfg_obj_t *vconfig, } maps[i] = NULL; - result = dns_tsigkeyring_create(mctx, &ring); - if (result != ISC_R_SUCCESS) { - return (result); - } + dns_tsigkeyring_create(mctx, &ring); for (i = 0;; i++) { if (maps[i] == NULL) { diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 8b1cd97f8d..867be2369c 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -809,7 +809,7 @@ isself(dns_view_t *myview, dns_tsigkey_t *mykey, const isc_sockaddr_t *srcaddr, bool match; isc_result_t result; - result = dns_view_gettsig(view, &mykey->name, &key); + result = dns_view_gettsig(view, mykey->name, &key); if (result != ISC_R_SUCCESS) { continue; } diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index eaf74b39a4..f733682c16 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -3028,11 +3028,7 @@ start_gssrequest(dns_name_t *primary) { dns_tsigkeyring_detach(&gssring); } - result = dns_tsigkeyring_create(gmctx, &gssring); - if (result != ISC_R_SUCCESS) { - fatal("dns_tsigkeyring_create failed: %s", - isc_result_totext(result)); - } + dns_tsigkeyring_create(gmctx, &gssring); dns_name_format(primary, namestr, sizeof(namestr)); if (kserver == NULL) { diff --git a/fuzz/dns_message_checksig.c b/fuzz/dns_message_checksig.c index 1f4417eb77..15a61995e1 100644 --- a/fuzz/dns_message_checksig.c +++ b/fuzz/dns_message_checksig.c @@ -243,19 +243,8 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { return (1); } - result = dns_tsigkeyring_create(mctx, &ring); - if (result != ISC_R_SUCCESS) { - fprintf(stderr, "dns_tsigkeyring_create failed: %s\n", - isc_result_totext(result)); - return (1); - } - - result = dns_tsigkeyring_create(mctx, &emptyring); - if (result != ISC_R_SUCCESS) { - fprintf(stderr, "dns_tsigkeyring_create failed: %s\n", - isc_result_totext(result)); - return (1); - } + dns_tsigkeyring_create(mctx, &ring); + dns_tsigkeyring_create(mctx, &emptyring); result = dns_name_fromstring(name, "tsig-key", 0, NULL); if (result != ISC_R_SUCCESS) { @@ -271,7 +260,7 @@ LLVMFuzzerInitialize(int *argc ISC_ATTR_UNUSED, char ***argv ISC_ATTR_UNUSED) { isc_result_totext(result)); return (1); } - result = dns_tsigkeyring_add(ring, name, tsigkey); + result = dns_tsigkeyring_add(ring, tsigkey); if (result != ISC_R_SUCCESS) { fprintf(stderr, "dns_tsigkeyring_add failed: %s\n", isc_result_totext(result)); diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index c975c236ed..c4bf46a622 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -17,12 +17,14 @@ #include +#include #include #include #include #include #include +#include #include #include @@ -62,11 +64,11 @@ extern const dns_name_t *dns_tsig_hmacsha512_name; #endif /* ifndef DNS_TSIG_MAXGENERATEDKEYS */ struct dns_tsigkeyring { - unsigned int magic; /*%< Magic number. */ - dns_rbt_t *keys; - unsigned int writecount; - isc_rwlock_t lock; - isc_mem_t *mctx; + unsigned int magic; /*%< Magic number. */ + isc_hashmap_t *keys; + unsigned int writecount; + isc_rwlock_t lock; + isc_mem_t *mctx; /* * LRU list of generated key along with a count of the keys on the * list and a maximum size. @@ -80,8 +82,9 @@ struct dns_tsigkey { /* Unlocked */ unsigned int magic; /*%< Magic number. */ isc_mem_t *mctx; - dst_key_t *key; /*%< Key */ - dns_name_t name; /*%< Key name */ + dst_key_t *key; /*%< Key */ + dns_fixedname_t fn; + dns_name_t *name; /*%< Key name */ const dns_name_t *algorithm; /*%< Algorithm name */ dns_name_t *creator; /*%< name that created secret */ bool generated : 1; /*%< key was auto-generated */ @@ -238,7 +241,7 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkeyp, const dns_name_t *name, *\li #ISC_R_NOTFOUND */ -isc_result_t +void dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp); /*%< * Create an empty TSIG key ring. @@ -246,15 +249,10 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp); * Requires: *\li 'mctx' is not NULL *\li 'ringp' is not NULL, and '*ringp' is NULL - * - * Returns: - *\li #ISC_R_SUCCESS - *\li #ISC_R_NOMEMORY */ isc_result_t -dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, - dns_tsigkey_t *tkey); +dns_tsigkeyring_add(dns_tsigkeyring_t *ring, dns_tsigkey_t *tkey); /*%< * Place a TSIG key onto a key ring. * diff --git a/lib/dns/message.c b/lib/dns/message.c index 120e3680e9..a86724a074 100644 --- a/lib/dns/message.c +++ b/lib/dns/message.c @@ -689,7 +689,7 @@ spacefortsig(dns_tsigkey_t *key, int otherlen) { * 26 + n1 + n2 + x + y bytes */ - dns_name_toregion(&key->name, &r1); + dns_name_toregion(key->name, &r1); dns_name_toregion(key->algorithm, &r2); if (key->key == NULL) { x = 0; @@ -3074,7 +3074,7 @@ dns_message_signer(dns_message_t *msg, dns_name_t *signer) { if (result == ISC_R_SUCCESS) { result = DNS_R_NOIDENTITY; } - identity = &msg->tsigkey->name; + identity = msg->tsigkey->name; } dns_name_clone(identity, signer); } diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 1ec1898e7a..83eafa71ff 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -268,7 +268,7 @@ process_gsstkey(dns_message_t *msg, dns_name_t *name, dns_rdata_tkey_t *tkeyin, name, dns__tsig_algfromname(&tkeyin->algorithm), dstkey, true, false, principal, now, expire, ring->mctx, &tsigkey)); - RETERR(dns_tsigkeyring_add(ring, name, tsigkey)); + RETERR(dns_tsigkeyring_add(ring, tsigkey)); dst_key_free(&dstkey); tkeyout->inception = now; tkeyout->expire = expire; @@ -819,7 +819,7 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, RETERR(dns_tsigkey_createfromkey(tkeyname, DST_ALG_GSSAPI, dstkey, true, false, NULL, rtkey.inception, rtkey.expire, ring->mctx, &tsigkey)); - RETERR(dns_tsigkeyring_add(ring, tkeyname, tsigkey)); + RETERR(dns_tsigkeyring_add(ring, tsigkey)); if (outkey == NULL) { dns_tsigkey_detach(&tsigkey); } else { diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index c75caff365..01e22a45c4 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -30,7 +31,6 @@ #include #include #include -#include #include #include #include @@ -110,9 +110,6 @@ static void tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) ISC_FORMAT_PRINTF(3, 4); -static void -cleanup_ring(dns_tsigkeyring_t *ring); - bool dns__tsig_algvalid(unsigned int alg) { return (alg == DST_ALG_HMACMD5 || alg == DST_ALG_HMACSHA1 || @@ -131,7 +128,7 @@ tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) { return; } if (key != NULL) { - dns_name_format(&key->name, namestr, sizeof(namestr)); + dns_name_format(key->name, namestr, sizeof(namestr)); } else { strlcpy(namestr, "", sizeof(namestr)); } @@ -158,7 +155,17 @@ tsig_log(dns_tsigkey_t *key, int level, const char *fmt, ...) { } static void -remove_fromring(dns_tsigkey_t *tkey) { +rm_hashmap(dns_tsigkey_t *tkey) { + REQUIRE(VALID_TSIGKEY(tkey)); + REQUIRE(VALID_TSIGKEYRING(tkey->ring)); + + (void)isc_hashmap_delete(tkey->ring->keys, NULL, tkey->name->ndata, + tkey->name->length); + dns_tsigkey_detach(&tkey); +} + +static void +rm_lru(dns_tsigkey_t *tkey) { REQUIRE(VALID_TSIGKEY(tkey)); REQUIRE(VALID_TSIGKEYRING(tkey->ring)); @@ -167,7 +174,6 @@ remove_fromring(dns_tsigkey_t *tkey) { tkey->ring->generated--; dns_tsigkey_unref(tkey); } - (void)dns_rbt_deletename(tkey->ring->keys, &tkey->name, false); } static void @@ -228,12 +234,12 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, .restored = restored, .inception = inception, .expire = expire, - .name = DNS_NAME_INITEMPTY, .link = ISC_LINK_INITIALIZER, }; - dns_name_dup(name, mctx, &tkey->name); - (void)dns_name_downcase(&tkey->name, &tkey->name, NULL); + tkey->name = dns_fixedname_initname(&tkey->fn); + dns_name_copy(name, tkey->name); + (void)dns_name_downcase(tkey->name, tkey->name, NULL); if (algorithm != DST_ALG_UNKNOWN) { if (dstkey != NULL && dst_key_alg(dstkey) != algorithm) { @@ -290,70 +296,33 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, return (ISC_R_SUCCESS); cleanup_name: - dns_name_free(&tkey->name, mctx); isc_mem_put(mctx, tkey, sizeof(dns_tsigkey_t)); return (ret); } -/* - * Find a few nodes to destroy if possible. - */ -static void -cleanup_ring(dns_tsigkeyring_t *ring) { - isc_result_t result; - dns_rbtnodechain_t chain; - dns_name_t foundname; - dns_fixedname_t fixedorigin; - dns_name_t *origin = NULL; - isc_stdtime_t now = isc_stdtime_now(); - - /* - * Start up a new iterator each time. - */ - dns_name_init(&foundname, NULL); - origin = dns_fixedname_initname(&fixedorigin); - -again: - dns_rbtnodechain_init(&chain); - result = dns_rbtnodechain_first(&chain, ring->keys, &foundname, origin); - if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { - dns_rbtnodechain_invalidate(&chain); - return; - } - - for (;;) { - dns_rbtnode_t *node = NULL; - dns_tsigkey_t *tkey = NULL; - - dns_rbtnodechain_current(&chain, &foundname, origin, &node); - tkey = node->data; - if (tkey != NULL) { - if (tkey->generated && - isc_refcount_current(&tkey->references) == 1 && - tkey->inception != tkey->expire && - tkey->expire < now) - { - tsig_log(tkey, 2, "tsig expire: deleting"); - /* delete the key */ - dns_rbtnodechain_invalidate(&chain); - remove_fromring(tkey); - goto again; - } - } - result = dns_rbtnodechain_next(&chain, &foundname, origin); - if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { - dns_rbtnodechain_invalidate(&chain); - return; - } - } -} - static void destroyring(dns_tsigkeyring_t *ring) { + isc_result_t result; + isc_hashmap_iter_t *it = NULL; + + RWLOCK(&ring->lock, isc_rwlocktype_write); + isc_hashmap_iter_create(ring->keys, &it); + for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS; + result = isc_hashmap_iter_delcurrent_next(it)) + { + dns_tsigkey_t *tkey = NULL; + isc_hashmap_iter_current(it, (void **)&tkey); + rm_lru(tkey); + dns_tsigkey_detach(&tkey); + } + isc_hashmap_iter_destroy(&it); + isc_hashmap_destroy(&ring->keys); + RWUNLOCK(&ring->lock, isc_rwlocktype_write); + ring->magic = 0; + isc_refcount_destroy(&ring->references); - dns_rbt_destroy(&ring->keys); isc_rwlock_destroy(&ring->lock); isc_mem_putanddetach(&ring->mctx, ring, sizeof(dns_tsigkeyring_t)); } @@ -447,7 +416,7 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) { creator, inception, expire, ring->mctx, &tkey); if (result == ISC_R_SUCCESS) { - result = dns_tsigkeyring_add(ring, name, tkey); + result = dns_tsigkeyring_add(ring, tkey); } dns_tsigkey_detach(&tkey); if (dstkey != NULL) { @@ -468,7 +437,7 @@ dump_key(dns_tsigkey_t *tkey, FILE *fp) { REQUIRE(tkey != NULL); REQUIRE(fp != NULL); - dns_name_format(&tkey->name, namestr, sizeof(namestr)); + dns_name_format(tkey->name, namestr, sizeof(namestr)); dns_name_format(tkey->creator, creatorstr, sizeof(creatorstr)); dns_name_format(tkey->algorithm, algorithmstr, sizeof(algorithmstr)); result = dst_key_dump(tkey->key, tkey->mctx, &buffer, &length); @@ -485,44 +454,27 @@ dump_key(dns_tsigkey_t *tkey, FILE *fp) { isc_result_t dns_tsigkeyring_dump(dns_tsigkeyring_t *ring, FILE *fp) { isc_result_t result; - dns_rbtnodechain_t chain; - dns_name_t foundname; - dns_fixedname_t fixedorigin; - dns_name_t *origin = NULL; isc_stdtime_t now = isc_stdtime_now(); + isc_hashmap_iter_t *it = NULL; + bool found = false; REQUIRE(VALID_TSIGKEYRING(ring)); - dns_name_init(&foundname, NULL); - origin = dns_fixedname_initname(&fixedorigin); - dns_rbtnodechain_init(&chain); - result = dns_rbtnodechain_first(&chain, ring->keys, &foundname, origin); - if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { - dns_rbtnodechain_invalidate(&chain); - goto destroy; - } - - for (;;) { - dns_rbtnode_t *node = NULL; + isc_hashmap_iter_create(ring->keys, &it); + for (result = isc_hashmap_iter_first(it); result == ISC_R_SUCCESS; + result = isc_hashmap_iter_next(it)) + { dns_tsigkey_t *tkey = NULL; + isc_hashmap_iter_current(it, (void **)&tkey); - dns_rbtnodechain_current(&chain, &foundname, origin, &node); - tkey = node->data; - if (tkey != NULL && tkey->generated && tkey->expire >= now) { + if (tkey->generated && tkey->expire >= now) { dump_key(tkey, fp); - } - result = dns_rbtnodechain_next(&chain, &foundname, origin); - if (result != ISC_R_SUCCESS && result != DNS_R_NEWORIGIN) { - dns_rbtnodechain_invalidate(&chain); - if (result == ISC_R_NOMORE) { - result = ISC_R_SUCCESS; - } - goto destroy; + found = true; } } + isc_hashmap_iter_destroy(&it); -destroy: - return (result); + return (found ? ISC_R_SUCCESS : ISC_R_NOTFOUND); } const dns_name_t * @@ -535,7 +487,7 @@ dns_tsigkey_identity(const dns_tsigkey_t *tsigkey) { if (tsigkey->generated) { return (tsigkey->creator); } else { - return (&tsigkey->name); + return (tsigkey->name); } } @@ -582,7 +534,6 @@ destroy_tsigkey(dns_tsigkey_t *key) { REQUIRE(VALID_TSIGKEY(key)); key->magic = 0; - dns_name_free(&key->name, key->mctx); if (key->key != NULL) { dst_key_free(&key->key); } @@ -604,7 +555,8 @@ dns_tsigkey_delete(dns_tsigkey_t *key) { REQUIRE(VALID_TSIGKEY(key)); RWLOCK(&key->ring->lock, isc_rwlocktype_write); - remove_fromring(key); + rm_lru(key); + rm_hashmap(key); RWUNLOCK(&key->ring->lock, isc_rwlocktype_write); } @@ -756,7 +708,7 @@ dns_tsig_sign(dns_message_t *msg) { /* * Digest the name, class, ttl, alg. */ - dns_name_toregion(&key->name, &r); + dns_name_toregion(key->name, &r); ret = dst_context_adddata(ctx, &r); if (ret != ISC_R_SUCCESS) { goto cleanup_context; @@ -863,7 +815,7 @@ dns_tsig_sign(dns_message_t *msg) { } dns_message_gettempname(msg, &owner); - dns_name_copy(&key->name, owner); + dns_name_copy(key->name, owner); dns_message_gettemprdatalist(msg, &datalist); @@ -982,7 +934,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, * Do the key name and algorithm match that of the query? */ if (response && - (!dns_name_equal(keyname, &tsigkey->name) || + (!dns_name_equal(keyname, tsigkey->name) || !dns_name_equal(&tsig.algorithm, &querytsig.algorithm))) { msg->tsigstatus = dns_tsigerror_badkey; @@ -1126,7 +1078,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, /* * Digest the key name. */ - dns_name_toregion(&tsigkey->name, &r); + dns_name_toregion(tsigkey->name, &r); ret = dst_context_adddata(ctx, &r); if (ret != ISC_R_SUCCESS) { goto cleanup_context; @@ -1322,7 +1274,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { /* * Do the key name and algorithm match that of the query? */ - if (!dns_name_equal(keyname, &tsigkey->name) || + if (!dns_name_equal(keyname, tsigkey->name) || !dns_name_equal(&tsig.algorithm, &querytsig.algorithm)) { msg->tsigstatus = dns_tsigerror_badkey; @@ -1583,15 +1535,12 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, REQUIRE(VALID_TSIGKEYRING(ring)); REQUIRE(tsigkey != NULL && *tsigkey == NULL); - RWLOCK(&ring->lock, isc_rwlocktype_write); - cleanup_ring(ring); - RWUNLOCK(&ring->lock, isc_rwlocktype_write); - RWLOCK(&ring->lock, isc_rwlocktype_read); - result = dns_rbt_findname(ring->keys, name, 0, NULL, (void *)&key); - if (result == DNS_R_PARTIALMATCH || result == ISC_R_NOTFOUND) { + result = isc_hashmap_find(ring->keys, NULL, name->ndata, name->length, + (void **)&key); + if (result == ISC_R_NOTFOUND) { RWUNLOCK(&ring->lock, isc_rwlocktype_read); - return (ISC_R_NOTFOUND); + return (result); } if (algorithm != NULL && !dns_name_equal(key->algorithm, algorithm)) { RWUNLOCK(&ring->lock, isc_rwlocktype_read); @@ -1603,7 +1552,8 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, */ RWUNLOCK(&ring->lock, isc_rwlocktype_read); RWLOCK(&ring->lock, isc_rwlocktype_write); - remove_fromring(key); + rm_lru(key); + rm_hashmap(key); RWUNLOCK(&ring->lock, isc_rwlocktype_write); return (ISC_R_NOTFOUND); } @@ -1614,22 +1564,8 @@ dns_tsigkey_find(dns_tsigkey_t **tsigkey, const dns_name_t *name, return (ISC_R_SUCCESS); } -static void -free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { - dns_tsigkey_t *key = node; - - REQUIRE(key != NULL); - - if (key->ring != NULL && key->generated && ISC_LINK_LINKED(key, link)) { - ISC_LIST_UNLINK(key->ring->lru, key, link); - dns_tsigkey_unref(key); - } - dns_tsigkey_detach(&key); -} - -isc_result_t +void dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { - isc_result_t result; dns_tsigkeyring_t *ring = NULL; REQUIRE(mctx != NULL); @@ -1640,45 +1576,26 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsigkeyring_t **ringp) { .lru = ISC_LIST_INITIALIZER, }; - result = dns_rbt_create(mctx, free_tsignode, NULL, &ring->keys); - if (result != ISC_R_SUCCESS) { - isc_rwlock_destroy(&ring->lock); - isc_mem_put(mctx, ring, sizeof(dns_tsigkeyring_t)); - return (result); - } - + isc_hashmap_create(mctx, 12, ISC_HASHMAP_CASE_INSENSITIVE, &ring->keys); isc_rwlock_init(&ring->lock); isc_mem_attach(mctx, &ring->mctx); isc_refcount_init(&ring->references, 1); ring->magic = TSIGKEYRING_MAGIC; *ringp = ring; - return (ISC_R_SUCCESS); } isc_result_t -dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, - dns_tsigkey_t *tkey) { +dns_tsigkeyring_add(dns_tsigkeyring_t *ring, dns_tsigkey_t *tkey) { isc_result_t result; REQUIRE(VALID_TSIGKEY(tkey)); REQUIRE(VALID_TSIGKEYRING(ring)); REQUIRE(tkey->ring == NULL); - REQUIRE(name != NULL); RWLOCK(&ring->lock, isc_rwlocktype_write); - ring->writecount++; - - /* - * Do on the fly cleaning. Find some nodes we might not - * want around any more. - */ - if (ring->writecount > 10) { - cleanup_ring(ring); - ring->writecount = 0; - } - - result = dns_rbt_addname(ring->keys, name, tkey); + result = isc_hashmap_add(ring->keys, NULL, tkey->name->ndata, + tkey->name->length, tkey); if (result == ISC_R_SUCCESS) { dns_tsigkey_ref(tkey); tkey->ring = ring; @@ -1693,7 +1610,9 @@ dns_tsigkeyring_add(dns_tsigkeyring_t *ring, const dns_name_t *name, ISC_LIST_APPEND(ring->lru, tkey, link); dns_tsigkey_ref(tkey); if (ring->generated++ > DNS_TSIG_MAXGENERATEDKEYS) { - remove_fromring(ISC_LIST_HEAD(ring->lru)); + dns_tsigkey_t *key = ISC_LIST_HEAD(ring->lru); + rm_lru(key); + rm_hashmap(key); } } diff --git a/lib/dns/view.c b/lib/dns/view.c index 7f1cc7ce8c..35c788e82b 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -144,10 +144,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass, const char *name, goto cleanup_zt; } - result = dns_tsigkeyring_create(view->mctx, &view->dynamickeys); - if (result != ISC_R_SUCCESS) { - goto cleanup_weakrefs; - } + dns_tsigkeyring_create(view->mctx, &view->dynamickeys); result = dns_badcache_init(view->mctx, DNS_VIEW_FAILCACHESIZE, &view->failcache); @@ -196,7 +193,6 @@ cleanup_dynkeys: dns_tsigkeyring_detach(&view->dynamickeys); } -cleanup_weakrefs: isc_refcount_decrementz(&view->weakrefs); isc_refcount_destroy(&view->weakrefs); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index d448fe3ffe..6be05191ed 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -12273,7 +12273,7 @@ notify_send_toaddr(void *arg) { if (key != NULL) { char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(&key->name, namebuf, sizeof(namebuf)); + dns_name_format(key->name, namebuf, sizeof(namebuf)); notify_log(notify->zone, ISC_LOG_INFO, "sending notify to %s : TSIG (%s)", addrbuf, namebuf); @@ -17350,7 +17350,7 @@ again: char buf[DNS_NAME_FORMATSIZE + sizeof(": TSIG ''")]; if (zone->tsigkey != NULL) { char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(&zone->tsigkey->name, namebuf, + dns_name_format(zone->tsigkey->name, namebuf, sizeof(namebuf)); snprintf(buf, sizeof(buf), ": TSIG '%s'", namebuf); @@ -20484,7 +20484,7 @@ checkds_send_toaddr(void *arg) { if (key != NULL) { char namebuf[DNS_NAME_FORMATSIZE]; - dns_name_format(&key->name, namebuf, sizeof(namebuf)); + dns_name_format(key->name, namebuf, sizeof(namebuf)); dns_zone_log(checkds->zone, ISC_LOG_DEBUG(3), "checkds: sending DS query to %s : TSIG (%s)", addrbuf, namebuf); diff --git a/lib/ns/client.c b/lib/ns/client.c index d4bd311766..7c56c5f4a4 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -533,7 +533,7 @@ ns_client_send(ns_client_t *client) { isc_netaddr_fromsockaddr(&netaddr, &client->peeraddr); if (client->message->tsigkey != NULL) { - name = &client->message->tsigkey->name; + name = client->message->tsigkey->name; } if (client->view->nocasecompress == NULL || diff --git a/lib/ns/notify.c b/lib/ns/notify.c index 633ed96b8b..f2ae6e5f84 100644 --- a/lib/ns/notify.c +++ b/lib/ns/notify.c @@ -129,7 +129,7 @@ ns_notify_start(ns_client_t *client, isc_nmhandle_t *handle) { tsigkey = dns_message_gettsigkey(request); if (tsigkey != NULL) { - dns_name_format(&tsigkey->name, namebuf, sizeof(namebuf)); + dns_name_format(tsigkey->name, namebuf, sizeof(namebuf)); if (tsigkey->generated) { char cnamebuf[DNS_NAME_FORMATSIZE]; diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index cad24d2170..b83a6dcec1 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -1105,7 +1105,7 @@ have_stream: CHECK(xfr->stream->methods->first(xfr->stream)); if (xfr->tsigkey != NULL) { - dns_name_format(&xfr->tsigkey->name, keyname, sizeof(keyname)); + dns_name_format(xfr->tsigkey->name, keyname, sizeof(keyname)); } else { keyname[0] = '\0'; } diff --git a/tests/dns/tsig_test.c b/tests/dns/tsig_test.c index 88632b921a..6c6935e7c9 100644 --- a/tests/dns/tsig_test.c +++ b/tests/dns/tsig_test.c @@ -148,7 +148,7 @@ add_tsig(dst_context_t *tsigctx, dns_tsigkey_t *key, isc_buffer_t *target) { ISC_LIST_APPEND(rdatalist.rdata, &rdata, link); dns_rdataset_init(&rdataset); dns_rdatalist_tordataset(&rdatalist, &rdataset); - CHECK(dns_rdataset_towire(&rdataset, &key->name, &cctx, target, 0, + CHECK(dns_rdataset_towire(&rdataset, key->name, &cctx, target, 0, &count)); /* @@ -290,13 +290,13 @@ ISC_RUN_TEST_IMPL(tsig_tcp) { result = dns_name_fromstring(keyname, "test", 0, NULL); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_tsigkeyring_create(mctx, &ring); - assert_int_equal(result, ISC_R_SUCCESS); + dns_tsigkeyring_create(mctx, &ring); + assert_non_null(ring); result = dns_tsigkey_create(keyname, DST_ALG_HMACSHA256, secret, sizeof(secret), mctx, &key); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_tsigkeyring_add(ring, keyname, key); + result = dns_tsigkeyring_add(ring, key); assert_int_equal(result, ISC_R_SUCCESS); assert_non_null(key); From 96e8b0e78234ddfcf6843da7c98dfd623de3236b Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Apr 2023 09:25:13 -0700 Subject: [PATCH 09/11] rename 'ret' to 'result' purely to assuage my desire for consistency across modules, result variables have been renamed to 'result' as they are throughout most of BIND. there are no other changes. --- lib/dns/tsig.c | 296 ++++++++++++++++++++++++------------------------- 1 file changed, 148 insertions(+), 148 deletions(-) diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index 01e22a45c4..78e7328335 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -222,7 +222,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, isc_stdtime_t expire, isc_mem_t *mctx, dns_tsigkey_t **keyp) { dns_tsigkey_t *tkey = NULL; - isc_result_t ret; + isc_result_t result; REQUIRE(keyp != NULL && *keyp == NULL); REQUIRE(name != NULL); @@ -243,11 +243,11 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, if (algorithm != DST_ALG_UNKNOWN) { if (dstkey != NULL && dst_key_alg(dstkey) != algorithm) { - ret = DNS_R_BADALG; + result = DNS_R_BADALG; goto cleanup_name; } } else if (dstkey != NULL) { - ret = DNS_R_BADALG; + result = DNS_R_BADALG; goto cleanup_name; } @@ -298,7 +298,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, dst_algorithm_t algorithm, cleanup_name: isc_mem_put(mctx, tkey, sizeof(dns_tsigkey_t)); - return (ret); + return (result); } static void @@ -575,7 +575,7 @@ dns_tsig_sign(dns_message_t *msg) { isc_stdtime_t now; isc_mem_t *mctx = NULL; dst_context_t *ctx = NULL; - isc_result_t ret; + isc_result_t result; unsigned char badtimedata[BADTIMELEN]; unsigned int sigsize = 0; bool response; @@ -636,10 +636,10 @@ dns_tsig_sign(dns_message_t *msg) { * has validated at this point. This is why we include a * MAC length > 0 in the reply. */ - ret = dst_context_create(key->key, mctx, DNS_LOGCATEGORY_DNSSEC, - true, 0, &ctx); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dst_context_create( + key->key, mctx, DNS_LOGCATEGORY_DNSSEC, true, 0, &ctx); + if (result != ISC_R_SUCCESS) { + return (result); } /* @@ -656,28 +656,28 @@ dns_tsig_sign(dns_message_t *msg) { INSIST(msg->verified_sig); - ret = dns_rdataset_first(msg->querytsig); - if (ret != ISC_R_SUCCESS) { + result = dns_rdataset_first(msg->querytsig); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } dns_rdataset_current(msg->querytsig, &querytsigrdata); - ret = dns_rdata_tostruct(&querytsigrdata, &querytsig, - NULL); - if (ret != ISC_R_SUCCESS) { + result = dns_rdata_tostruct(&querytsigrdata, &querytsig, + NULL); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } isc_buffer_putuint16(&databuf, querytsig.siglen); if (isc_buffer_availablelength(&databuf) < querytsig.siglen) { - ret = ISC_R_NOSPACE; + result = ISC_R_NOSPACE; goto cleanup_context; } isc_buffer_putmem(&databuf, querytsig.signature, querytsig.siglen); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } querytsig_ok = true; @@ -689,8 +689,8 @@ dns_tsig_sign(dns_message_t *msg) { isc_buffer_init(&headerbuf, header, sizeof(header)); dns_message_renderheader(msg, &headerbuf); isc_buffer_usedregion(&headerbuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -699,8 +699,8 @@ dns_tsig_sign(dns_message_t *msg) { */ isc_buffer_usedregion(msg->buffer, &r); isc_region_consume(&r, DNS_MESSAGE_HEADERLEN); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -709,8 +709,8 @@ dns_tsig_sign(dns_message_t *msg) { * Digest the name, class, ttl, alg. */ dns_name_toregion(key->name, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -718,14 +718,14 @@ dns_tsig_sign(dns_message_t *msg) { isc_buffer_putuint16(&databuf, dns_rdataclass_any); isc_buffer_putuint32(&databuf, 0); /* ttl */ isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } dns_name_toregion(&tsig.algorithm, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } } @@ -737,8 +737,8 @@ dns_tsig_sign(dns_message_t *msg) { isc_buffer_putuint48(&databuf, tsig.timesigned); isc_buffer_putuint16(&databuf, tsig.fudge); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -751,8 +751,8 @@ dns_tsig_sign(dns_message_t *msg) { isc_buffer_putuint16(&databuf, tsig.otherlen); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -762,22 +762,22 @@ dns_tsig_sign(dns_message_t *msg) { if (tsig.otherlen > 0) { r.length = tsig.otherlen; r.base = tsig.other; - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } } } - ret = dst_key_sigsize(key->key, &sigsize); - if (ret != ISC_R_SUCCESS) { + result = dst_key_sigsize(key->key, &sigsize); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } tsig.signature = isc_mem_get(mctx, sigsize); isc_buffer_init(&sigbuf, tsig.signature, sigsize); - ret = dst_context_sign(ctx, &sigbuf); - if (ret != ISC_R_SUCCESS) { + result = dst_context_sign(ctx, &sigbuf); + if (result != ISC_R_SUCCESS) { goto cleanup_signature; } dst_context_destroy(&ctx); @@ -801,9 +801,9 @@ dns_tsig_sign(dns_message_t *msg) { dns_message_gettemprdata(msg, &rdata); isc_buffer_allocate(msg->mctx, &dynbuf, 512); - ret = dns_rdata_fromstruct(rdata, dns_rdataclass_any, - dns_rdatatype_tsig, &tsig, dynbuf); - if (ret != ISC_R_SUCCESS) { + result = dns_rdata_fromstruct(rdata, dns_rdataclass_any, + dns_rdatatype_tsig, &tsig, dynbuf); + if (result != ISC_R_SUCCESS) { goto cleanup_dynbuf; } @@ -843,7 +843,7 @@ cleanup_context: if (ctx != NULL) { dst_context_destroy(&ctx); } - return (ret); + return (result); } isc_result_t @@ -856,7 +856,7 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, dns_name_t *keyname = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; isc_stdtime_t now; - isc_result_t ret; + isc_result_t result; dns_tsigkey_t *tsigkey = NULL; dst_key_t *key = NULL; unsigned char header[DNS_MESSAGE_HEADERLEN]; @@ -908,25 +908,25 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, */ keyname = msg->tsigname; - ret = dns_rdataset_first(msg->tsig); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdataset_first(msg->tsig); + if (result != ISC_R_SUCCESS) { + return (result); } dns_rdataset_current(msg->tsig, &rdata); - ret = dns_rdata_tostruct(&rdata, &tsig, NULL); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdata_tostruct(&rdata, &tsig, NULL); + if (result != ISC_R_SUCCESS) { + return (result); } dns_rdata_reset(&rdata); if (response) { - ret = dns_rdataset_first(msg->querytsig); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdataset_first(msg->querytsig); + if (result != ISC_R_SUCCESS) { + return (result); } dns_rdataset_current(msg->querytsig, &rdata); - ret = dns_rdata_tostruct(&rdata, &querytsig, NULL); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdata_tostruct(&rdata, &querytsig, NULL); + if (result != ISC_R_SUCCESS) { + return (result); } } @@ -956,22 +956,22 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, * Find dns_tsigkey_t based on keyname. */ if (tsigkey == NULL) { - ret = ISC_R_NOTFOUND; + result = ISC_R_NOTFOUND; if (ring1 != NULL) { - ret = dns_tsigkey_find(&tsigkey, keyname, - &tsig.algorithm, ring1); + result = dns_tsigkey_find(&tsigkey, keyname, + &tsig.algorithm, ring1); } - if (ret == ISC_R_NOTFOUND && ring2 != NULL) { - ret = dns_tsigkey_find(&tsigkey, keyname, - &tsig.algorithm, ring2); + if (result == ISC_R_NOTFOUND && ring2 != NULL) { + result = dns_tsigkey_find(&tsigkey, keyname, + &tsig.algorithm, ring2); } - if (ret != ISC_R_SUCCESS) { + if (result != ISC_R_SUCCESS) { msg->tsigstatus = dns_tsigerror_badkey; - ret = dns_tsigkey_create( + result = dns_tsigkey_create( keyname, dns__tsig_algfromname(&tsig.algorithm), NULL, 0, mctx, &msg->tsigkey); - if (ret != ISC_R_SUCCESS) { - return (ret); + if (result != ISC_R_SUCCESS) { + return (result); } tsig_log(msg->tsigkey, 2, "unknown key"); return (DNS_R_TSIGVERIFYFAILURE); @@ -985,9 +985,9 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, * Check digest length. */ alg = dst_key_alg(key); - ret = dst_key_sigsize(key, &siglen); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dst_key_sigsize(key, &siglen); + if (result != ISC_R_SUCCESS) { + return (result); } if (dns__tsig_algvalid(alg)) { if (tsig.siglen > siglen) { @@ -1009,25 +1009,25 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, sig_r.base = tsig.signature; sig_r.length = tsig.siglen; - ret = dst_context_create(key, mctx, DNS_LOGCATEGORY_DNSSEC, - false, 0, &ctx); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dst_context_create(key, mctx, DNS_LOGCATEGORY_DNSSEC, + false, 0, &ctx); + if (result != ISC_R_SUCCESS) { + return (result); } if (response) { isc_buffer_init(&databuf, data, sizeof(data)); isc_buffer_putuint16(&databuf, querytsig.siglen); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } if (querytsig.siglen > 0) { r.length = querytsig.siglen; r.base = querytsig.signature; - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } } @@ -1059,8 +1059,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, */ header_r.base = (unsigned char *)header; header_r.length = DNS_MESSAGE_HEADERLEN; - ret = dst_context_adddata(ctx, &header_r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &header_r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1070,8 +1070,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, isc_buffer_usedregion(source, &source_r); r.base = source_r.base + DNS_MESSAGE_HEADERLEN; r.length = msg->sigstart - DNS_MESSAGE_HEADERLEN; - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1079,8 +1079,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, * Digest the key name. */ dns_name_toregion(tsigkey->name, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1088,8 +1088,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, isc_buffer_putuint16(&databuf, tsig.common.rdclass); isc_buffer_putuint32(&databuf, msg->tsig->ttl); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1097,8 +1097,8 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, * Digest the key algorithm. */ dns_name_toregion(tsigkey->algorithm, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1108,27 +1108,27 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, isc_buffer_putuint16(&databuf, tsig.error); isc_buffer_putuint16(&databuf, tsig.otherlen); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } if (tsig.otherlen > 0) { r.base = tsig.other; r.length = tsig.otherlen; - ret = dst_context_adddata(ctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(ctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } } - ret = dst_context_verify(ctx, &sig_r); - if (ret == DST_R_VERIFYFAILURE) { - ret = DNS_R_TSIGVERIFYFAILURE; + result = dst_context_verify(ctx, &sig_r); + if (result == DST_R_VERIFYFAILURE) { + result = DNS_R_TSIGVERIFYFAILURE; tsig_log(msg->tsigkey, 2, "signature failed to verify(1)"); goto cleanup_context; - } else if (ret != ISC_R_SUCCESS) { + } else if (result != ISC_R_SUCCESS) { goto cleanup_context; } msg->verified_sig = 1; @@ -1152,12 +1152,12 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { msg->tsigstatus = dns_tsigerror_badtime; tsig_log(msg->tsigkey, 2, "signature has expired"); - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; goto cleanup_context; } else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) { msg->tsigstatus = dns_tsigerror_badtime; tsig_log(msg->tsigkey, 2, "signature is in the future"); - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; goto cleanup_context; } @@ -1170,14 +1170,14 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, msg->tsigstatus = dns_tsigerror_badtrunc; tsig_log(msg->tsigkey, 2, "truncated signature length too small"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; goto cleanup_context; } if (tsig.siglen > 0 && digestbits == 0 && tsig.siglen < siglen) { msg->tsigstatus = dns_tsigerror_badtrunc; tsig_log(msg->tsigkey, 2, "signature length too small"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; goto cleanup_context; } } @@ -1185,22 +1185,22 @@ dns_tsig_verify(isc_buffer_t *source, dns_message_t *msg, if (response && tsig.error != dns_rcode_noerror) { msg->tsigstatus = tsig.error; if (tsig.error == dns_tsigerror_badtime) { - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; } else { - ret = DNS_R_TSIGERRORSET; + result = DNS_R_TSIGERRORSET; } goto cleanup_context; } msg->tsigstatus = dns_rcode_noerror; - ret = ISC_R_SUCCESS; + result = ISC_R_SUCCESS; cleanup_context: if (ctx != NULL) { dst_context_destroy(&ctx); } - return (ret); + return (result); } static isc_result_t @@ -1212,7 +1212,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { dns_name_t *keyname = NULL; dns_rdata_t rdata = DNS_RDATA_INIT; isc_stdtime_t now; - isc_result_t ret; + isc_result_t result; dns_tsigkey_t *tsigkey = NULL; dst_key_t *key = NULL; unsigned char header[DNS_MESSAGE_HEADERLEN]; @@ -1243,14 +1243,14 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { /* * Extract and parse the previous TSIG */ - ret = dns_rdataset_first(msg->querytsig); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdataset_first(msg->querytsig); + if (result != ISC_R_SUCCESS) { + return (result); } dns_rdataset_current(msg->querytsig, &rdata); - ret = dns_rdata_tostruct(&rdata, &querytsig, NULL); - if (ret != ISC_R_SUCCESS) { - return (ret); + result = dns_rdata_tostruct(&rdata, &querytsig, NULL); + if (result != ISC_R_SUCCESS) { + return (result); } dns_rdata_reset(&rdata); @@ -1261,13 +1261,13 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { has_tsig = true; keyname = msg->tsigname; - ret = dns_rdataset_first(msg->tsig); - if (ret != ISC_R_SUCCESS) { + result = dns_rdataset_first(msg->tsig); + if (result != ISC_R_SUCCESS) { goto cleanup_querystruct; } dns_rdataset_current(msg->tsig, &rdata); - ret = dns_rdata_tostruct(&rdata, &tsig, NULL); - if (ret != ISC_R_SUCCESS) { + result = dns_rdata_tostruct(&rdata, &tsig, NULL); + if (result != ISC_R_SUCCESS) { goto cleanup_querystruct; } @@ -1278,7 +1278,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { !dns_name_equal(&tsig.algorithm, &querytsig.algorithm)) { msg->tsigstatus = dns_tsigerror_badkey; - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; tsig_log(msg->tsigkey, 2, "key name and algorithm do not match"); goto cleanup_querystruct; @@ -1288,15 +1288,15 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { * Check digest length. */ alg = dst_key_alg(key); - ret = dst_key_sigsize(key, &siglen); - if (ret != ISC_R_SUCCESS) { + result = dst_key_sigsize(key, &siglen); + if (result != ISC_R_SUCCESS) { goto cleanup_querystruct; } if (dns__tsig_algvalid(alg)) { if (tsig.siglen > siglen) { tsig_log(tsigkey, 2, "signature length too big"); - ret = DNS_R_FORMERR; + result = DNS_R_FORMERR; goto cleanup_querystruct; } if (tsig.siglen > 0 && @@ -1305,16 +1305,16 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { { tsig_log(tsigkey, 2, "signature length below minimum"); - ret = DNS_R_FORMERR; + result = DNS_R_FORMERR; goto cleanup_querystruct; } } } if (msg->tsigctx == NULL) { - ret = dst_context_create(key, mctx, DNS_LOGCATEGORY_DNSSEC, - false, 0, &msg->tsigctx); - if (ret != ISC_R_SUCCESS) { + result = dst_context_create(key, mctx, DNS_LOGCATEGORY_DNSSEC, + false, 0, &msg->tsigctx); + if (result != ISC_R_SUCCESS) { goto cleanup_querystruct; } @@ -1324,8 +1324,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { isc_buffer_init(&databuf, data, sizeof(data)); isc_buffer_putuint16(&databuf, querytsig.siglen); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(msg->tsigctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(msg->tsigctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1335,8 +1335,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { if (querytsig.siglen > 0) { r.length = querytsig.siglen; r.base = querytsig.signature; - ret = dst_context_adddata(msg->tsigctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(msg->tsigctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } } @@ -1375,8 +1375,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { */ header_r.base = (unsigned char *)header; header_r.length = DNS_MESSAGE_HEADERLEN; - ret = dst_context_adddata(msg->tsigctx, &header_r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(msg->tsigctx, &header_r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1390,8 +1390,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { } else { r.length = source_r.length - DNS_MESSAGE_HEADERLEN; } - ret = dst_context_adddata(msg->tsigctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(msg->tsigctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1403,8 +1403,8 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { isc_buffer_putuint48(&databuf, tsig.timesigned); isc_buffer_putuint16(&databuf, tsig.fudge); isc_buffer_usedregion(&databuf, &r); - ret = dst_context_adddata(msg->tsigctx, &r); - if (ret != ISC_R_SUCCESS) { + result = dst_context_adddata(msg->tsigctx, &r); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } @@ -1414,24 +1414,24 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { if (tsig.error != dns_rcode_noerror) { msg->tsigstatus = tsig.error; if (tsig.error == dns_tsigerror_badtime) { - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; } else { - ret = DNS_R_TSIGERRORSET; + result = DNS_R_TSIGERRORSET; } } else { tsig_log(msg->tsigkey, 2, "signature is empty"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; } goto cleanup_context; } - ret = dst_context_verify(msg->tsigctx, &sig_r); - if (ret == DST_R_VERIFYFAILURE) { + result = dst_context_verify(msg->tsigctx, &sig_r); + if (result == DST_R_VERIFYFAILURE) { tsig_log(msg->tsigkey, 2, "signature failed to verify(2)"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; goto cleanup_context; - } else if (ret != ISC_R_SUCCESS) { + } else if (result != ISC_R_SUCCESS) { goto cleanup_context; } msg->verified_sig = 1; @@ -1455,19 +1455,19 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { if (now + msg->timeadjust > tsig.timesigned + tsig.fudge) { msg->tsigstatus = dns_tsigerror_badtime; tsig_log(msg->tsigkey, 2, "signature has expired"); - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; goto cleanup_context; } else if (now + msg->timeadjust < tsig.timesigned - tsig.fudge) { msg->tsigstatus = dns_tsigerror_badtime; tsig_log(msg->tsigkey, 2, "signature is in the future"); - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; goto cleanup_context; } alg = dst_key_alg(key); - ret = dst_key_sigsize(key, &siglen); - if (ret != ISC_R_SUCCESS) { + result = dst_key_sigsize(key, &siglen); + if (result != ISC_R_SUCCESS) { goto cleanup_context; } if (dns__tsig_algvalid(alg)) { @@ -1480,7 +1480,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { tsig_log(msg->tsigkey, 2, "truncated signature length " "too small"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; goto cleanup_context; } if (tsig.siglen > 0 && digestbits == 0 && @@ -1489,7 +1489,7 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { msg->tsigstatus = dns_tsigerror_badtrunc; tsig_log(msg->tsigkey, 2, "signature length too small"); - ret = DNS_R_TSIGVERIFYFAILURE; + result = DNS_R_TSIGVERIFYFAILURE; goto cleanup_context; } } @@ -1497,16 +1497,16 @@ tsig_verify_tcp(isc_buffer_t *source, dns_message_t *msg) { if (tsig.error != dns_rcode_noerror) { msg->tsigstatus = tsig.error; if (tsig.error == dns_tsigerror_badtime) { - ret = DNS_R_CLOCKSKEW; + result = DNS_R_CLOCKSKEW; } else { - ret = DNS_R_TSIGERRORSET; + result = DNS_R_TSIGERRORSET; } goto cleanup_context; } } msg->tsigstatus = dns_rcode_noerror; - ret = ISC_R_SUCCESS; + result = ISC_R_SUCCESS; cleanup_context: /* @@ -1514,14 +1514,14 @@ cleanup_context: * for unsigned messages; it is a running sum till the next * TSIG signed message. */ - if ((ret != ISC_R_SUCCESS || has_tsig) && msg->tsigctx != NULL) { + if ((result != ISC_R_SUCCESS || has_tsig) && msg->tsigctx != NULL) { dst_context_destroy(&msg->tsigctx); } cleanup_querystruct: dns_rdata_freestruct(&querytsig); - return (ret); + return (result); } isc_result_t From f4084ff54378170d0e130ab139a5c87de8180267 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Apr 2023 01:22:09 -0700 Subject: [PATCH 10/11] 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 f733682c16..724567ce41 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 7d8f2dc735..f1998beb1c 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 83eafa71ff..90b1876f47 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 857de2121b..f8f8a82c0e 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)); From 885c132f4ae6d895ee973648e15d9e61d01555a8 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Apr 2023 17:46:49 -0700 Subject: [PATCH 11/11] improve code flow the code in dns_tkey_processquery() was unnecessarily hard to follow. --- lib/dns/tkey.c | 129 ++++++++++++++++--------------------------------- 1 file changed, 42 insertions(+), 87 deletions(-) diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 90b1876f47..0f98820edf 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -23,6 +23,7 @@ #endif #include +#include #include #include #include @@ -155,13 +156,11 @@ 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 = NULL; - dns_rdataset_t *set = NULL; - while (!ISC_LIST_EMPTY(*namelist)) { - name = ISC_LIST_HEAD(*namelist); + while ((name = ISC_LIST_HEAD(*namelist)) != NULL) { + dns_rdataset_t *set = NULL; ISC_LIST_UNLINK(*namelist, name, link); - while (!ISC_LIST_EMPTY(name->list)) { - set = ISC_LIST_HEAD(name->list); + while ((set = ISC_LIST_HEAD(name->list)) != NULL) { ISC_LIST_UNLINK(name->list, set, link); if (dns_rdataset_isassociated(set)) { dns_rdataset_disassociate(set); @@ -364,6 +363,7 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, dns_namelist_t namelist = ISC_LIST_INITIALIZER; char tkeyoutdata[512]; isc_buffer_t tkeyoutbuf; + dns_tsigkey_t *tsigkey = NULL; REQUIRE(msg != NULL); REQUIRE(tctx != NULL); @@ -407,7 +407,8 @@ 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_TKEYMODE_GSSAPI doesn't require a signature, but other + * modes do. */ result = dns_message_signer(msg, &tsigner); if (result == ISC_R_SUCCESS) { @@ -429,20 +430,25 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, .algorithm = DNS_NAME_INITEMPTY, .mode = tkeyin.mode, }; - dns_name_clone(&tkeyin.algorithm, &tkeyout.algorithm); - /* - * A delete operation must have a fully specified key name. If this - * is not a delete, we do the following: - * if (qname != ".") - * keyname = qname + defaultdomain - * else - * keyname = + defaultdomain - */ - if (tkeyin.mode != DNS_TKEYMODE_DELETE) { - dns_tsigkey_t *tsigkey = NULL; - + switch (tkeyin.mode) { + case DNS_TKEYMODE_DELETE: + /* + * A delete operation uses the fully specified qname. + */ + RETERR(process_deletetkey(signer, qname, &tkeyin, &tkeyout, + ring)); + break; + case DNS_TKEYMODE_GSSAPI: + /* + * For non-delete operations we do this: + * + * if (qname != ".") + * keyname = qname + defaultdomain + * else + * keyname = + defaultdomain + */ if (tctx->domain == NULL && tkeyin.mode != DNS_TKEYMODE_GSSAPI) { tkey_log("dns_tkey_processquery: tkey-domain not set"); @@ -457,68 +463,33 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, dns_name_copy(qname, keyname); dns_name_getlabelsequence(keyname, 0, n - 1, keyname); } else { - static char hexdigits[16] = { '0', '1', '2', '3', - '4', '5', '6', '7', - '8', '9', 'A', 'B', - 'C', 'D', 'E', 'F' }; unsigned char randomdata[16]; char randomtext[32]; isc_buffer_t b; - unsigned int i, j; + isc_region_t r = { + .base = randomdata, + .length = sizeof(randomdata), + }; isc_nonce_buf(randomdata, sizeof(randomdata)); - - for (i = 0, j = 0; i < sizeof(randomdata); i++) { - unsigned char val = randomdata[i]; - randomtext[j++] = hexdigits[val >> 4]; - randomtext[j++] = hexdigits[val & 0xF]; - } isc_buffer_init(&b, randomtext, sizeof(randomtext)); - isc_buffer_add(&b, sizeof(randomtext)); - result = dns_name_fromtext(keyname, &b, NULL, 0, NULL); - if (result != ISC_R_SUCCESS) { - goto failure; - } - } - - if (tkeyin.mode == DNS_TKEYMODE_GSSAPI) { - /* Yup. This is a hack */ - result = dns_name_concatenate(keyname, dns_rootname, - keyname, NULL); - if (result != ISC_R_SUCCESS) { - goto failure; - } - } else { - result = dns_name_concatenate(keyname, tctx->domain, - keyname, NULL); - if (result != ISC_R_SUCCESS) { - goto failure; - } + RETERR(isc_hex_totext(&r, 2, "", &b)); + RETERR(dns_name_fromtext(keyname, &b, NULL, 0, NULL)); } + RETERR(dns_name_concatenate(keyname, dns_rootname, keyname, + NULL)); result = dns_tsigkey_find(&tsigkey, keyname, NULL, ring); if (result == ISC_R_SUCCESS) { tkeyout.error = dns_tsigerror_badname; dns_tsigkey_detach(&tsigkey); - goto failure_with_tkey; - } else if (result != ISC_R_NOTFOUND) { - goto failure; + break; + } else if (result == ISC_R_NOTFOUND) { + RETERR(process_gsstkey(msg, keyname, &tkeyin, tctx, + &tkeyout, ring)); + break; } - } else { - keyname = qname; - } - - switch (tkeyin.mode) { - case DNS_TKEYMODE_GSSAPI: - tkeyout.error = dns_rcode_noerror; - RETERR(process_gsstkey(msg, keyname, &tkeyin, tctx, &tkeyout, - ring)); - break; - case DNS_TKEYMODE_DELETE: - tkeyout.error = dns_rcode_noerror; - RETERR(process_deletetkey(signer, keyname, &tkeyin, &tkeyout, - ring)); - break; + goto failure; case DNS_TKEYMODE_SERVERASSIGNED: case DNS_TKEYMODE_RESOLVERASSIGNED: result = DNS_R_NOTIMP; @@ -527,41 +498,26 @@ dns_tkey_processquery(dns_message_t *msg, dns_tkeyctx_t *tctx, tkeyout.error = dns_tsigerror_badmode; } -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 (tkeyout.key != NULL) { isc_mem_put(tkeyout.mctx, tkeyout.key, tkeyout.keylen); } - if (tkeyout.other != NULL) { - isc_mem_put(tkeyout.mctx, tkeyout.other, tkeyout.otherlen); - } - if (result != ISC_R_SUCCESS) { - goto failure; - } - - add_rdata_to_list(msg, keyname, &rdata, 0, &namelist); + RETERR(result); RETERR(dns_message_reply(msg, true)); - - name = ISC_LIST_HEAD(namelist); - while (name != NULL) { - dns_name_t *next = ISC_LIST_NEXT(name, link); + add_rdata_to_list(msg, keyname, &rdata, 0, &namelist); + while ((name = ISC_LIST_HEAD(namelist)) != NULL) { ISC_LIST_UNLINK(namelist, name, link); dns_message_addname(msg, name, DNS_SECTION_ANSWER); - name = next; } - return (ISC_R_SUCCESS); failure: - if (!ISC_LIST_EMPTY(namelist)) { - free_namelist(msg, &namelist); - } + free_namelist(msg, &namelist); return (result); } @@ -765,7 +721,6 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, * the GSS negotiation hasn't completed yet, so we can't sign * anything yet. */ - RETERR(dns_tsigkey_createfromkey(tkeyname, DST_ALG_GSSAPI, dstkey, true, false, NULL, rtkey.inception, rtkey.expire, ring->mctx, &tsigkey));