From: Evan Hunt Date: Tue, 11 Apr 2023 06:46:10 +0000 (-0700) Subject: minor tsig.c cleanups X-Git-Tag: v9.19.15~31^2~10 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b1db1c14751919b3fb3c447b1d0716cfcec2af41;p=thirdparty%2Fbind9.git 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. --- diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index d8323c6f664..c8db355a264 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 3333283bc7c..550046275e5 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 87affae84e3..c539f103999 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 a4b9dd15ab1..f60154dea4f 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 e8b10b30539..c9329c67d75 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 15358fe834b..cb453c5665d 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; - - REQUIRE(node != NULL); +free_tsignode(void *node, void *arg ISC_ATTR_UNUSED) { + dns_tsigkey_t *key = node; - UNUSED(_unused); + REQUIRE(key != NULL); - 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 49fc8a8d80d..754c9974aa5 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 6380e3e09ba..743c5def1e9 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);