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 d8323c6f6646b85db3a882e21cc986826eb9e18e..c8db355a264d97c869a4a52919e58899397d31c0 100644 (file)
--- 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 3333283bc7c038df9aad50d655e122678ab440fe..550046275e5780583f3e03c364662541fe46f7d2 100644 (file)
--- 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 87affae84e3d9d5f15c31ecccdbe806486fb189e..c539f103999384e9e71e9c4b9d0bc1cba261d659 100644 (file)
--- 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 a4b9dd15ab1fd1e5ca18d765660792160f0e3fde..f60154dea4f1b413442b417299f9337125df0075 100644 (file)
--- 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 e8b10b30539c8dee0258ec18fba0899ce98627d0..c9329c67d75666b1523d540b29d95fb8c02dfc7d 100644 (file)
--- 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 15358fe834bd2eaaae010bde065a9befddd06786..cb453c5665d98f13a986c709fce24079122acfd9 100644 (file)
--- 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 49fc8a8d80d0d116bb629abadbef8d63e77a51c8..754c9974aa57fdc435e981ad960a898d6391dfa0 100644 (file)
--- 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 6380e3e09ba736adb1350072e2d3f4037e413023..743c5def1e926b85736e56223544e9e0cdb84a45 100644 (file)
--- 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);