From 9f9b7f0e8d455b1c88e51ddcefdbf19b472e1ef2 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 9 Dec 2010 00:54:34 +0000 Subject: [PATCH] 2982. [bug] Reference count dst keys. dst_key_attach() can be used increment the reference count. Note: dns_tsigkey_createfromkey() callers should now always call dst_key_free() rather than setting it to NULL on success. [RT #22672] --- CHANGES | 7 +++++++ bin/dig/dighost.c | 4 ++-- bin/named/server.c | 6 ++++-- bin/nsupdate/nsupdate.c | 13 ++++++++----- bin/tests/system/tkey/keydelete.c | 5 +++-- lib/dns/dst_api.c | 28 +++++++++++++++++++++++++++- lib/dns/dst_internal.h | 4 +++- lib/dns/include/dns/tsec.h | 4 ++-- lib/dns/include/dns/tsig.h | 9 ++++++--- lib/dns/include/dst/dst.h | 12 +++++++++++- lib/dns/tkey.c | 11 +++++++---- lib/dns/tsec.c | 12 +++--------- lib/dns/tsig.c | 21 +++++++++------------ lib/export/samples/sample-update.c | 7 +++---- 14 files changed, 95 insertions(+), 48 deletions(-) diff --git a/CHANGES b/CHANGES index 3eba0cb3e7..604b335037 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +2982. [bug] Reference count dst keys. dst_key_attach() can be used + increment the reference count. + + Note: dns_tsigkey_createfromkey() callers should now + always call dst_key_free() rather than setting it + to NULL on success. [RT #22672] + 2981. [func] Partial DNS64 support (AAAA synthesis). [RT #21991] 2980. [bug] named didn't properly handle UPDATES that changed the diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 744579cdb6..fc5e491ef5 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: dighost.c,v 1.335 2010/12/02 23:22:41 marka Exp $ */ +/* $Id: dighost.c,v 1.336 2010/12/09 00:54:33 marka Exp $ */ /*! \file * \note @@ -1135,7 +1135,7 @@ setup_file_key(void) { goto failure; } result = dns_tsigkey_createfromkey(dst_key_name(dstkey), hmacname, - &dstkey, ISC_FALSE, NULL, 0, 0, + dstkey, ISC_FALSE, NULL, 0, 0, mctx, NULL, &key); if (result != ISC_R_SUCCESS) { printf(";; Couldn't create key %s: %s\n", diff --git a/bin/named/server.c b/bin/named/server.c index fe0c3f9f56..d0f61fa04e 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: server.c,v 1.589 2010/12/08 23:47:05 tbox Exp $ */ +/* $Id: server.c,v 1.590 2010/12/09 00:54:33 marka Exp $ */ /*! \file */ @@ -3658,7 +3658,7 @@ generate_session_key(const char *filename, const char *keynamestr, /* Store the key in tsigkey. */ isc_stdtime_get(&now); - CHECK(dns_tsigkey_createfromkey(dst_key_name(key), algname, &key, + CHECK(dns_tsigkey_createfromkey(dst_key_name(key), algname, key, ISC_FALSE, NULL, now, now, mctx, NULL, &tsigkey)); @@ -3681,6 +3681,8 @@ generate_session_key(const char *filename, const char *keynamestr, RUNTIME_CHECK(isc_stdio_flush(fp) == ISC_R_SUCCESS); RUNTIME_CHECK(isc_stdio_close(fp) == ISC_R_SUCCESS); + dst_key_free(&key); + *tsigkeyp = tsigkey; return (ISC_R_SUCCESS); diff --git a/bin/nsupdate/nsupdate.c b/bin/nsupdate/nsupdate.c index 6ba76d83f4..6435272834 100644 --- a/bin/nsupdate/nsupdate.c +++ b/bin/nsupdate/nsupdate.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: nsupdate.c,v 1.184 2010/12/03 00:37:33 marka Exp $ */ +/* $Id: nsupdate.c,v 1.185 2010/12/09 00:54:33 marka Exp $ */ /*! \file */ @@ -641,6 +641,9 @@ setup_keyfile(isc_mem_t *mctx, isc_log_t *lctx) { debug("Creating key..."); + if (sig0key != NULL) + dst_key_free(&sig0key); + /* Try reading the key from a K* pair */ result = dst_key_fromnamedfile(keyfile, NULL, DST_TYPE_PRIVATE | DST_TYPE_KEY, mctx, @@ -682,17 +685,17 @@ setup_keyfile(isc_mem_t *mctx, isc_log_t *lctx) { } if (hmacname != NULL) { result = dns_tsigkey_createfromkey(dst_key_name(dstkey), - hmacname, &dstkey, ISC_FALSE, + hmacname, dstkey, ISC_FALSE, NULL, 0, 0, mctx, NULL, &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)); - dst_key_free(&dstkey); return; } - } else - sig0key = dstkey; + } else + dst_key_attach(dstkey, &sig0key); } static void diff --git a/bin/tests/system/tkey/keydelete.c b/bin/tests/system/tkey/keydelete.c index 75e1273aff..ec989504bd 100644 --- a/bin/tests/system/tkey/keydelete.c +++ b/bin/tests/system/tkey/keydelete.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: keydelete.c,v 1.15 2010/12/02 23:46:56 tbox Exp $ */ +/* $Id: keydelete.c,v 1.16 2010/12/09 00:54:33 marka Exp $ */ #include @@ -230,8 +230,9 @@ main(int argc, char **argv) { CHECK("dst_key_fromnamedfile", result); result = dns_tsigkey_createfromkey(dst_key_name(dstkey), DNS_TSIG_HMACMD5_NAME, - &dstkey, ISC_TRUE, NULL, 0, 0, + dstkey, ISC_TRUE, NULL, 0, 0, mctx, ring, &tsigkey); + dst_key_free(&dstkey); CHECK("dns_tsigkey_createfromkey", result); (void)isc_app_run(); diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index bdb86ce601..81ed8bb486 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -31,7 +31,7 @@ /* * Principal Author: Brian Wellington - * $Id: dst_api.c,v 1.52 2010/12/02 23:22:42 marka Exp $ + * $Id: dst_api.c,v 1.53 2010/12/09 00:54:33 marka Exp $ */ /*! \file */ @@ -51,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -1015,10 +1016,22 @@ dst_key_paramcompare(const dst_key_t *key1, const dst_key_t *key2) { return (ISC_FALSE); } +void +dst_key_attach(dst_key_t *source, dst_key_t **target) { + + REQUIRE(dst_initialized == ISC_TRUE); + REQUIRE(target != NULL && *target == NULL); + REQUIRE(VALID_KEY(source)); + + isc_refcount_increment(&source->refs, NULL); + *target = source; +} + void dst_key_free(dst_key_t **keyp) { isc_mem_t *mctx; dst_key_t *key; + unsigned int refs; REQUIRE(dst_initialized == ISC_TRUE); REQUIRE(keyp != NULL && VALID_KEY(*keyp)); @@ -1026,6 +1039,11 @@ dst_key_free(dst_key_t **keyp) { key = *keyp; mctx = key->mctx; + isc_refcount_decrement(&key->refs, &refs); + if (refs != 0) + return; + + isc_refcount_destroy(&key->refs); if (key->keydata.generic != NULL) { INSIST(key->func->destroy != NULL); key->func->destroy(key); @@ -1165,14 +1183,22 @@ get_key_struct(dns_name_t *name, unsigned int alg, memset(key, 0, sizeof(dst_key_t)); key->magic = KEY_MAGIC; + result = isc_refcount_init(&key->refs, 1); + if (result != ISC_R_SUCCESS) { + isc_mem_put(mctx, key, sizeof(dst_key_t)); + return (NULL); + } + key->key_name = isc_mem_get(mctx, sizeof(dns_name_t)); if (key->key_name == NULL) { + isc_refcount_destroy(&key->refs); isc_mem_put(mctx, key, sizeof(dst_key_t)); return (NULL); } dns_name_init(key->key_name, NULL); result = dns_name_dup(name, mctx, key->key_name); if (result != ISC_R_SUCCESS) { + isc_refcount_destroy(&key->refs); isc_mem_put(mctx, key->key_name, sizeof(dns_name_t)); isc_mem_put(mctx, key, sizeof(dst_key_t)); return (NULL); diff --git a/lib/dns/dst_internal.h b/lib/dns/dst_internal.h index 84e461a70f..d958017cbd 100644 --- a/lib/dns/dst_internal.h +++ b/lib/dns/dst_internal.h @@ -29,7 +29,7 @@ * IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: dst_internal.h,v 1.23 2009/10/27 22:25:37 marka Exp $ */ +/* $Id: dst_internal.h,v 1.24 2010/12/09 00:54:33 marka Exp $ */ #ifndef DST_DST_INTERNAL_H #define DST_DST_INTERNAL_H 1 @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,7 @@ typedef struct dst_hmacsha512_key dst_hmacsha512_key_t; /*% DST Key Structure */ struct dst_key { unsigned int magic; + isc_refcount_t refs; dns_name_t * key_name; /*%< name of the key */ unsigned int key_size; /*%< size of the key in bits */ unsigned int key_proto; /*%< protocols this key is used for */ diff --git a/lib/dns/include/dns/tsec.h b/lib/dns/include/dns/tsec.h index bb7d24ece5..a9d60c21c6 100644 --- a/lib/dns/include/dns/tsec.h +++ b/lib/dns/include/dns/tsec.h @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: tsec.h,v 1.5 2010/12/02 23:46:56 tbox Exp $ */ +/* $Id: tsec.h,v 1.6 2010/12/09 00:54:34 marka Exp $ */ #ifndef DNS_TSEC_H #define DNS_TSEC_H 1 @@ -65,7 +65,7 @@ typedef enum { } dns_tsectype_t; isc_result_t -dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, +dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t *key, dns_tsec_t **tsecp); /*%< * Create a TSEC structure and stores a type-dependent key structure in it. diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h index 6a835ed72c..c5299c5341 100644 --- a/lib/dns/include/dns/tsig.h +++ b/lib/dns/include/dns/tsig.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: tsig.h,v 1.56 2010/12/02 23:22:42 marka Exp $ */ +/* $Id: tsig.h,v 1.57 2010/12/09 00:54:34 marka Exp $ */ #ifndef DNS_TSIG_H #define DNS_TSIG_H 1 @@ -103,7 +103,7 @@ dns_tsigkey_create(dns_name_t *name, dns_name_t *algorithm, isc_result_t dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, - dst_key_t **dstkeyp, isc_boolean_t generated, + dst_key_t *dstkey, isc_boolean_t generated, 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); @@ -117,12 +117,15 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, * 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. + * * Requires: *\li 'name' is a valid dns_name_t *\li 'algorithm' is a valid dns_name_t *\li 'secret' is a valid pointer *\li 'length' is an integer >= 0 - *\li 'key' is a valid dst key or NULL + *\li 'dstkey' is a valid dst key or NULL *\li 'creator' points to a valid dns_name_t or is NULL *\li 'mctx' is a valid memory context *\li 'ring' is a valid TSIG keyring or NULL diff --git a/lib/dns/include/dst/dst.h b/lib/dns/include/dst/dst.h index 1f6020a3bf..cb766f85b0 100644 --- a/lib/dns/include/dst/dst.h +++ b/lib/dns/include/dst/dst.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: dst.h,v 1.25 2009/10/26 21:18:24 each Exp $ */ +/* $Id: dst.h,v 1.26 2010/12/09 00:54:34 marka Exp $ */ #ifndef DST_DST_H #define DST_DST_H 1 @@ -591,6 +591,16 @@ dst_key_paramcompare(const dst_key_t *key1, const dst_key_t *key2); * \li ISC_FALSE */ +void +dst_key_attach(dst_key_t *source, dst_key_t **target); +/* + * Attach to a existing key increasing the reference count. + * + * Requires: + *\li 'source' to be a valid key. + *\li 'target' to be non-NULL and '*target' to be NULL. + */ + void dst_key_free(dst_key_t **keyp); /*%< diff --git a/lib/dns/tkey.c b/lib/dns/tkey.c index 6155498219..16f425f8c6 100644 --- a/lib/dns/tkey.c +++ b/lib/dns/tkey.c @@ -16,7 +16,7 @@ */ /* - * $Id: tkey.c,v 1.95 2010/12/02 23:22:42 marka Exp $ + * $Id: tkey.c,v 1.96 2010/12/09 00:54:34 marka Exp $ */ /*! \file */ #include @@ -491,10 +491,11 @@ process_gsstkey(dns_name_t *name, dns_rdata_tkey_t *tkeyin, expire = now + lifetime; #endif RETERR(dns_tsigkey_createfromkey(name, &tkeyin->algorithm, - &dstkey, ISC_TRUE, + dstkey, ISC_TRUE, dns_fixedname_name(&principal), now, expire, ring->mctx, ring, NULL)); + dst_key_free(&dstkey); tkeyout->inception = now; tkeyout->expire = expire; } else { @@ -1272,9 +1273,10 @@ dns_tkey_processgssresponse(dns_message_t *qmsg, dns_message_t *rmsg, &dstkey)); RETERR(dns_tsigkey_createfromkey(tkeyname, DNS_TSIG_GSSAPI_NAME, - &dstkey, ISC_FALSE, NULL, + dstkey, ISC_FALSE, NULL, rtkey.inception, rtkey.expire, ring->mctx, ring, outkey)); + dst_key_free(&dstkey); dns_rdata_freestruct(&rtkey); return (result); @@ -1407,9 +1409,10 @@ dns_tkey_gssnegotiate(dns_message_t *qmsg, dns_message_t *rmsg, (win2k ? DNS_TSIG_GSSAPIMS_NAME : DNS_TSIG_GSSAPI_NAME), - &dstkey, ISC_TRUE, NULL, + dstkey, ISC_TRUE, NULL, rtkey.inception, rtkey.expire, ring->mctx, ring, outkey)); + dst_key_free(&dstkey); dns_rdata_freestruct(&rtkey); return (result); diff --git a/lib/dns/tsec.c b/lib/dns/tsec.c index bbc12f51cd..bfa6195d0d 100644 --- a/lib/dns/tsec.c +++ b/lib/dns/tsec.c @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: tsec.c,v 1.6 2010/12/02 23:46:56 tbox Exp $ */ +/* $Id: tsec.c,v 1.7 2010/12/09 00:54:34 marka Exp $ */ #include @@ -44,16 +44,14 @@ struct dns_tsec { }; isc_result_t -dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, +dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t *key, dns_tsec_t **tsecp) { isc_result_t result; dns_tsec_t *tsec; dns_tsigkey_t *tsigkey = NULL; dns_name_t *algname; - dst_key_t *key; - REQUIRE(keyp != NULL && *keyp != NULL); REQUIRE(mctx != NULL); REQUIRE(tsecp != NULL && *tsecp == NULL); @@ -61,8 +59,6 @@ dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, if (tsec == NULL) return (ISC_R_NOMEMORY); - key = *keyp; - tsec->type = type; tsec->mctx = mctx; @@ -92,7 +88,7 @@ dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, return (DNS_R_BADALG); } result = dns_tsigkey_createfromkey(dst_key_name(key), - algname, keyp, ISC_FALSE, + algname, key, ISC_FALSE, NULL, 0, 0, mctx, NULL, &tsigkey); if (result != ISC_R_SUCCESS) { @@ -103,7 +99,6 @@ dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, break; case dns_tsectype_sig0: tsec->ukey.key = key; - *keyp = NULL; break; default: INSIST(0); @@ -112,7 +107,6 @@ dns_tsec_create(isc_mem_t *mctx, dns_tsectype_t type, dst_key_t **keyp, tsec->magic = DNS_TSEC_MAGIC; *tsecp = tsec; - ENSURE(*keyp == NULL); return (ISC_R_SUCCESS); } diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c index ab33ea3e65..117e0b8e72 100644 --- a/lib/dns/tsig.c +++ b/lib/dns/tsig.c @@ -16,7 +16,7 @@ */ /* - * $Id: tsig.c,v 1.142 2010/12/02 23:22:42 marka Exp $ + * $Id: tsig.c,v 1.143 2010/12/09 00:54:34 marka Exp $ */ /*! \file */ #include @@ -287,7 +287,7 @@ keyring_add(dns_tsig_keyring_t *ring, dns_name_t *name, isc_result_t dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, - dst_key_t **dstkeyp, isc_boolean_t generated, + dst_key_t *dstkey, isc_boolean_t generated, 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) @@ -295,7 +295,6 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, dns_tsigkey_t *tkey; isc_result_t ret; unsigned int refs = 0; - dst_key_t *dstkey; REQUIRE(key == NULL || *key == NULL); REQUIRE(name != NULL); @@ -303,10 +302,6 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, REQUIRE(mctx != NULL); REQUIRE(key != NULL || ring != NULL); - if (dstkeyp != NULL) - dstkey = *dstkeyp; - else - dstkey = NULL; tkey = (dns_tsigkey_t *) isc_mem_get(mctx, sizeof(dns_tsigkey_t)); if (tkey == NULL) return (ISC_R_NOMEMORY); @@ -402,7 +397,9 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, } else tkey->creator = NULL; - tkey->key = dstkey; + tkey->key = NULL; + if (dstkey != NULL) + dst_key_attach(dstkey, &tkey->key); tkey->ring = ring; if (key != NULL) @@ -441,8 +438,6 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, namestr); } - if (dstkeyp != NULL) - *dstkeyp = NULL; if (key != NULL) *key = tkey; @@ -454,6 +449,8 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm, isc_refcount_decrement(&tkey->refs, NULL); isc_refcount_destroy(&tkey->refs); cleanup_creator: + 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)); @@ -630,10 +627,10 @@ dns_tsigkey_create(dns_name_t *name, dns_name_t *algorithm, } else if (length > 0) return (DNS_R_BADALG); - result = dns_tsigkey_createfromkey(name, algorithm, &dstkey, + result = dns_tsigkey_createfromkey(name, algorithm, dstkey, generated, creator, inception, expire, mctx, ring, key); - if (result != ISC_R_SUCCESS && dstkey != NULL) + if (dstkey != NULL) dst_key_free(&dstkey); return (result); } diff --git a/lib/export/samples/sample-update.c b/lib/export/samples/sample-update.c index cfb73bae92..e54d154424 100644 --- a/lib/export/samples/sample-update.c +++ b/lib/export/samples/sample-update.c @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: sample-update.c,v 1.9 2010/12/03 21:47:19 marka Exp $ */ +/* $Id: sample-update.c,v 1.10 2010/12/09 00:54:34 marka Exp $ */ #include @@ -745,10 +745,9 @@ setup_tsec(char *keyfile, isc_mem_t *mctx) { else tsectype = dns_tsectype_sig0; - result = dns_tsec_create(mctx, tsectype, &dstkey, &tsec); + result = dns_tsec_create(mctx, tsectype, dstkey, &tsec); + dst_key_free(&dstkey); if (result != ISC_R_SUCCESS) { - if (dstkey != NULL) - dst_key_free(&dstkey); fprintf(stderr, "could not create tsec: %s\n", isc_result_totext(result)); exit(1);