prevent TSIG keys from being added to multiple rings

it was possible to add a TSIG key to more than one TSIG
keyring at a time, and this was in fact happening with the
session key, which was generated once and then added to the
keyrings for each view as it was configured.

this has been corrected and a REQUIRE added to dns_tsigkeyring_add()
to prevent it from happening again.
This commit is contained in:
Evan Hunt 2023-05-21 12:59:38 -07:00
parent 911f3af2fb
commit afae1b65e8
4 changed files with 63 additions and 34 deletions

View file

@ -100,7 +100,7 @@ struct named_server {
named_statschannellist_t statschannels;
dns_tsigkey_t *sessionkey;
dst_key_t *sessionkey;
char *session_keyfile;
dns_name_t *session_keyname;
unsigned int session_keyalg;

View file

@ -4044,6 +4044,28 @@ 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'.
@ -5076,8 +5098,18 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
*/
CHECK(named_tsigkeyring_fromconfig(config, vconfig, view->mctx, &ring));
if (named_g_server->sessionkey != NULL) {
CHECK(dns_tsigkeyring_add(ring, named_g_server->session_keyname,
named_g_server->sessionkey));
dns_tsigkey_t *tsigkey = NULL;
result = dns_tsigkey_createfromkey(
named_g_server->session_keyname,
algorithm_name(named_g_server->session_keyalg),
named_g_server->sessionkey, false, NULL, 0, 0, mctx,
NULL, &tsigkey);
if (result == ISC_R_SUCCESS) {
result = dns_tsigkeyring_add(
ring, named_g_server->session_keyname, tsigkey);
dns_tsigkey_detach(&tsigkey);
}
CHECK(result);
}
dns_view_setkeyring(view, ring);
dns_tsigkeyring_detach(&ring);
@ -7535,7 +7567,7 @@ cleanup_session_key(named_server_t *server, isc_mem_t *mctx) {
}
if (server->sessionkey != NULL) {
dns_tsigkey_detach(&server->sessionkey);
dst_key_free(&server->sessionkey);
}
server->session_keyalg = DST_ALG_UNKNOWN;
@ -7545,9 +7577,8 @@ 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,
const dns_name_t *algname, unsigned int algtype,
uint16_t bits, isc_mem_t *mctx, bool first_time,
dns_tsigkey_t **tsigkeyp) {
unsigned int algtype, 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;
@ -7555,8 +7586,6 @@ generate_session_key(const char *filename, const char *keynamestr,
char key_txtsecret[256];
char key_rawsecret[64];
isc_region_t key_rawregion;
isc_stdtime_t now;
dns_tsigkey_t *tsigkey = NULL;
FILE *fp = NULL;
isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
@ -7572,8 +7601,7 @@ generate_session_key(const char *filename, const char *keynamestr,
}
/*
* Dump the key to the buffer for later use. Should be done before
* we transfer the ownership of key to tsigkey.
* Dump the key to the buffer for later use.
*/
isc_buffer_init(&key_rawbuffer, &key_rawsecret, sizeof(key_rawsecret));
CHECK(dst_key_tobuffer(key, &key_rawbuffer));
@ -7582,11 +7610,6 @@ generate_session_key(const char *filename, const char *keynamestr,
isc_buffer_init(&key_txtbuffer, &key_txtsecret, sizeof(key_txtsecret));
CHECK(isc_base64_totext(&key_rawregion, -1, "", &key_txtbuffer));
/* Store the key in tsigkey. */
isc_stdtime_get(&now);
CHECK(dns_tsigkey_createfromkey(dst_key_name(key), algname, key, false,
NULL, now, now, mctx, NULL, &tsigkey));
/* Dump the key to the key file. */
fp = named_os_openfile(filename, S_IRUSR | S_IWUSR, first_time);
if (fp == NULL) {
@ -7611,10 +7634,7 @@ generate_session_key(const char *filename, const char *keynamestr,
goto cleanup;
}
dst_key_free(&key);
*tsigkeyp = tsigkey;
*keyp = key;
return (ISC_R_SUCCESS);
cleanup:
@ -7627,9 +7647,6 @@ cleanup:
(void)isc_stdio_close(fp);
(void)isc_file_remove(filename);
}
if (tsigkey != NULL) {
dns_tsigkey_detach(&tsigkey);
}
if (key != NULL) {
dst_key_free(&key);
}
@ -7734,8 +7751,8 @@ configure_session_key(const cfg_obj_t **maps, named_server_t *server,
server->session_keybits = bits;
CHECK(generate_session_key(keyfile, keynamestr, keyname, algstr,
algname, algtype, bits, mctx,
first_time, &server->sessionkey));
algtype, bits, mctx, first_time,
&server->sessionkey));
}
return (result);

View file

@ -264,7 +264,9 @@ dns_tsigkeyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name,
* Place a TSIG key onto a key ring.
*
* Requires:
*\li 'ring', 'name' and 'tkey' are not NULL
*\li 'name' and 'ring' are not NULL
*\li 'tkey' is a valid TSIG key, which has not been
* added to any other keyrings
*
* Returns:
*\li #ISC_R_SUCCESS

View file

@ -223,16 +223,22 @@ keyring_add(dns_tsig_keyring_t *ring, const dns_name_t *name,
}
result = dns_rbt_addname(ring->keys, name, tkey);
if (result == ISC_R_SUCCESS && 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));
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);
@ -1857,6 +1863,10 @@ dns_tsigkeyring_add(dns_tsig_keyring_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(name != NULL);
result = keyring_add(ring, name, tkey);
if (result == ISC_R_SUCCESS) {
isc_refcount_increment(&tkey->refs);