2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 22:15:20 +00:00

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.
This commit is contained in:
Evan Hunt
2023-04-11 16:10:07 -07:00
parent 404a13b4dd
commit a6e187a8d5
6 changed files with 98 additions and 116 deletions

View File

@@ -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));

View File

@@ -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);

View File

@@ -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

View File

@@ -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);
}

View File

@@ -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));
}
}

View File

@@ -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);