upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-11 08:20:01 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Detect NSEC3 salt collisions

Browse source 
When generating a new salt, compare it with the previous NSEC3
paremeters to ensure the new parameters are different from the
previous ones.

This moves the salt generation call from 'bin/named/*.s' to
'lib/dns/zone.c'. When setting new NSEC3 parameters, you can set a new
function parameter 'resalt' to enforce a new salt to be generated. A
new salt will also be generated if 'salt' is set to NULL.

Logging salt with zone context can now be done with 'dnssec_log',
removing the need for 'dns_nsec3_log_salt'.
This commit is contained in:
Matthijs Mekking 2020-11-05 11:12:24 +01:00
parent 3b4c764b43
commit 6b5d7357df
9 changed files with 92 additions and 148 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGES
View file

@ -1,6 +1,8 @@
5538. [func] Add NSEC3 support for zones that manage DNSSEC with
the 'dnssec-policy' configuration. A new option
'nsec3param' can be used to set the NSEC3 parameters.
This now also detects salt collisions, and logs salt
generation messages with zone context.
[GL #1620]
5537. [func] The query plugin mechanism has been extended

21
bin/named/server.c
View file

@ -14369,7 +14369,7 @@ named_server_signing(named_server_t *server, isc_lex_t *lex,
bool list = false, clear = false;
bool chain = false;
bool setserial = false;
bool log_salt = false;
bool resalt = false;
uint32_t serial = 0;
char keystr[DNS_SECALG_FORMATSIZE + 7]; /* <5-digit keyid>/<alg> */
unsigned short hash = 0, flags = 0, iter = 0, saltlen = 0;
@ -14452,8 +14452,7 @@ named_server_signing(named_server_t *server, isc_lex_t *lex,
* configurable.
*/
saltlen = 8;
CHECK(dns_nsec3_generate_salt(salt, saltlen));
log_salt = true;
resalt = true;
} else if (strcmp(ptr, "-") != 0) {
isc_buffer_t buf;
@ -14491,19 +14490,9 @@ named_server_signing(named_server_t *server, isc_lex_t *lex,
(void)putstr(text, "request queued");
(void)putnull(text);
} else if (chain) {
if (log_salt) {
char zonetext[DNS_NAME_MAXTEXT + 32];
dns_zone_name(zone, zonetext, sizeof(zonetext));
dns_nsec3_log_salt(
named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, salt,
saltlen,
"generated salt for zone %s:", zonetext);
}
CHECK(dns_zone_setnsec3param(zone, (uint8_t)hash,
(uint8_t)flags, iter,
(uint8_t)saltlen, salt, true));
CHECK(dns_zone_setnsec3param(
zone, (uint8_t)hash, (uint8_t)flags, iter,
(uint8_t)saltlen, salt, true, resalt));
(void)putstr(text, "nsec3param request queued");
(void)putnull(text);
} else if (setserial) {

45
bin/named/zoneconf.c
View file

@ -1562,50 +1562,15 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
bool sigvalinsecs;
if (kasp != NULL) {
unsigned char saltbuf[255];
unsigned char *salt;
DE_CONST("-", salt);
if (dns_kasp_nsec3(kasp)) {
result = dns_zone_checknsec3param(
result = dns_zone_setnsec3param(
zone, 1, dns_kasp_nsec3flags(kasp),
dns_kasp_nsec3iter(kasp),
dns_kasp_nsec3saltlen(kasp), NULL);
if (result != ISC_R_SUCCESS) {
if (dns_kasp_nsec3saltlen(kasp) > 0) {
char zonetext[DNS_NAME_MAXTEXT +
32];
dns_zone_name(zone, zonetext,
sizeof(zonetext));
RETERR(dns_nsec3_generate_salt(
saltbuf,
dns_kasp_nsec3saltlen(
kasp)));
salt = saltbuf;
dns_nsec3_log_salt(
named_g_lctx,
NAMED_LOGCATEGORY_GENERAL,
NAMED_LOGMODULE_SERVER,
ISC_LOG_INFO, salt,
dns_kasp_nsec3saltlen(
kasp),
"generated salt for "
"zone %s:",
zonetext);
}
result = dns_zone_setnsec3param(
zone, 1,
dns_kasp_nsec3flags(kasp),
dns_kasp_nsec3iter(kasp),
dns_kasp_nsec3saltlen(kasp),
salt, true);
}
dns_kasp_nsec3saltlen(kasp), NULL, true,
false);
} else {
result = dns_zone_setnsec3param(zone, 0, 0, 0,
0, salt, true);
result = dns_zone_setnsec3param(
zone, 0, 0, 0, 0, NULL, true, false);
}
INSIST(result == ISC_R_SUCCESS);
}

2
doc/notes/notes-current.rst
View file

@ -62,7 +62,7 @@ Feature Changes
- Add NSEC3 support for zones that manage their DNSSEC with the `dnssec-policy`
configuration. A new option 'nsec3param' can be used to set the desired
NSEC3 parameters. [GL #1620].
NSEC3 parameters, and will detect collisions when resalting. [GL #1620].
Bug Fixes
~~~~~~~~~

8
lib/dns/include/dns/nsec3.h
View file

@ -79,14 +79,6 @@ dns_nsec3_generate_salt(unsigned char *salt, size_t saltlen);
* Generate a salt with the given salt length.
*/
void
dns_nsec3_log_salt(isc_log_t *lctx, isc_logcategory_t *category,
isc_logmodule_t *module, int level, unsigned char *salt,
size_t saltlen, const char *fmt, ...);
/*%<
* Utility to log the salt.
*/
isc_result_t
dns_nsec3_hashname(dns_fixedname_t *result,
unsigned char rethash[NSEC3_MAX_HASH_LENGTH],

22
lib/dns/include/dns/zone.h
View file

@ -2378,33 +2378,17 @@ dns_zone_getraw(dns_zone_t *zone, dns_zone_t **raw);
isc_result_t
dns_zone_keydone(dns_zone_t *zone, const char *data);
isc_result_t
dns_zone_checknsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
uint16_t iter, uint8_t saltlen, unsigned char *salt);
/*%
* Check if the NSEC3 parameters for the zone match the requested parameters.
*
* If 'salt' is NULL, a match is found if the salt has the requested length,
* otherwise the NSEC3 salt must match the requested salt value too.
*
* Requires:
* \li 'zone' to be valid.
*
* Returns:
* \li ISC_R_SUCCESS, if a match is found.
* \li Error, if no match is found, or if the db lookup failed.
*/
isc_result_t
dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
uint16_t iter, uint8_t saltlen, unsigned char *salt,
bool replace);
bool replace, bool resalt);
/*%
* Set the NSEC3 parameters for the zone.
*
* If 'replace' is true, then the existing NSEC3 chain, if any, will
* be replaced with the new one. If 'hash' is zero, then the replacement
* chain will be NSEC rather than NSEC3.
* chain will be NSEC rather than NSEC3. If 'resalt' is true, or if 'salt'
* is NULL, generate a new salt with the given salt length.
*
* Requires:
* \li 'zone' to be valid.

35
lib/dns/nsec3.c
View file

@ -235,41 +235,6 @@ dns_nsec3_generate_salt(unsigned char *salt, size_t saltlen) {
return (ISC_R_SUCCESS);
}
void
dns_nsec3_log_salt(isc_log_t *lctx, isc_logcategory_t *category,
isc_logmodule_t *module, int level, unsigned char *salt,
size_t saltlen, const char *fmt, ...) {
va_list ap;
char message[4096];
unsigned char text[255 * 2 + 1];
isc_region_t r;
isc_buffer_t buf;
isc_result_t result;
if (!isc_log_wouldlog(dns_lctx, level)) {
return;
}
va_start(ap, fmt);
vsnprintf(message, sizeof(message), fmt, ap);
r.base = salt;
r.length = (unsigned int)saltlen;
isc_buffer_init(&buf, text, sizeof(text));
result = isc_hex_totext(&r, 2, "", &buf);
if (result == ISC_R_SUCCESS) {
text[saltlen * 2] = 0;
} else {
text[0] = 0;
}
isc_log_write(lctx, category, module, level, "%s %s", message, text);
va_end(ap);
}
isc_result_t
dns_nsec3_hashname(dns_fixedname_t *result,
unsigned char rethash[NSEC3_MAX_HASH_LENGTH],

2
lib/dns/win32/libdns.def.in
View file

@ -654,7 +654,6 @@ dns_nsec3_delnsec3sx
dns_nsec3_generate_salt
dns_nsec3_hashlength
dns_nsec3_hashname
dns_nsec3_log_salt
dns_nsec3_maxiterations
dns_nsec3_noexistnodata
dns_nsec3_supportedhash
@ -1165,7 +1164,6 @@ dns_zone_catz_enable
dns_zone_catz_enable_db
dns_zone_cdscheck
dns_zone_checknames
dns_zone_checknsec3param
dns_zone_clearforwardacl
dns_zone_clearnotifyacl
dns_zone_clearqueryacl

103
lib/dns/zone.c
View file

@ -21054,10 +21054,16 @@ failure:
/*
* Check if zone has NSEC3PARAM (and thus a chain) with the right parameters.
*
* If 'salt' is NULL, a match is found if the salt has the requested length,
* otherwise the NSEC3 salt must match the requested salt value too.
*
* Returns ISC_R_SUCCESS, if a match is found, or an error if no match is
* found, or if the db lookup failed.
*/
isc_result_t
dns_zone_checknsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
uint16_t iter, uint8_t saltlen, unsigned char *salt) {
static isc_result_t
zone_has_nsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
uint16_t iter, uint8_t saltlen, unsigned char *salt) {
isc_result_t result = ISC_R_UNEXPECTED;
dns_dbnode_t *node = NULL;
dns_db_t *db = NULL;
@ -21146,8 +21152,28 @@ cleanup:
return (result);
}
static void
salt2text(unsigned char *salt, uint8_t saltlen, unsigned char *text,
unsigned int textlen) {
isc_region_t r;
isc_buffer_t buf;
isc_result_t result;
r.base = salt;
r.length = (unsigned int)saltlen;
isc_buffer_init(&buf, text, textlen);
result = isc_hex_totext(&r, 2, "", &buf);
if (result == ISC_R_SUCCESS) {
text[saltlen * 2] = 0;
} else {
text[0] = 0;
}
}
/*
* Called when an "rndc signing -nsec3param ..." command is received.
* Called when an "rndc signing -nsec3param ..." command is received, or the
* 'dnssec-policy' has changed.
*
* Allocate and prepare an nsec3param_t structure which holds information about
* the NSEC3 changes requested for the zone:
@ -21168,7 +21194,7 @@ cleanup:
isc_result_t
dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
uint16_t iter, uint8_t saltlen, unsigned char *salt,
bool replace) {
bool replace, bool resalt) {
isc_result_t result = ISC_R_SUCCESS;
dns_rdata_nsec3param_t param;
dns_rdata_t nrdata = DNS_RDATA_INIT;
@ -21178,13 +21204,51 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
nsec3param_t *np;
dns_zone_t *dummy = NULL;
isc_buffer_t b;
isc_event_t *e;
isc_event_t *e = NULL;
unsigned char saltbuf[255];
unsigned char salttext[255 * 2 + 1];
REQUIRE(DNS_ZONE_VALID(zone));
REQUIRE(salt != NULL);
LOCK_ZONE(zone);
result = zone_has_nsec3param(zone, hash, flags, iter, saltlen, salt);
if (result == ISC_R_SUCCESS) {
/*
* The right NSEC3 parameters are already set, no need to
* set again, unless resalting is enforced.
*/
if (!resalt) {
result = ISC_R_EXISTS;
goto failure;
}
}
if (saltlen == 0) {
DE_CONST("-", salt);
salttext[0] = '-';
salttext[1] = 0;
} else if (resalt || salt == NULL) {
do {
/* Generate a new salt. */
CHECK(dns_nsec3_generate_salt(saltbuf, saltlen));
if (result != ISC_R_SUCCESS) {
goto failure;
}
salt = saltbuf;
salt2text(salt, saltlen, salttext, sizeof(salttext));
dnssec_log(zone, ISC_LOG_INFO, "generated salt: %s",
salttext);
/*
* Check for NSEC3 param conflicts, this is done to
* avoid salt collision.
*/
result = zone_has_nsec3param(zone, hash, flags, iter,
saltlen, salt);
} while (result == ISC_R_SUCCESS);
}
INSIST(salt != NULL);
e = isc_event_allocate(zone->mctx, zone, DNS_EVENT_SETNSEC3PARAM,
setnsec3param, zone, sizeof(struct np3event));
@ -21216,28 +21280,10 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
np->nsec = false;
if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(3))) {
unsigned char text[255 * 2 + 1];
isc_buffer_t buf;
isc_result_t ret;
isc_region_t r;
r.base = salt;
r.length = (unsigned int)saltlen;
if (saltlen > 0) {
isc_buffer_init(&buf, text, sizeof(text));
ret = isc_hex_totext(&r, 2, "", &buf);
if (ret == ISC_R_SUCCESS) {
text[saltlen * 2] = 0;
} else {
text[0] = 0;
}
} else {
text[0] = '-';
text[1] = 0;
}
salt2text(salt, saltlen, salttext, sizeof(salttext));
dnssec_log(zone, ISC_LOG_DEBUG(3),
"setnsec3param:nsec3 %u %u %u %s", hash,
flags, iter, text);
flags, iter, salttext);
}
}
@ -21248,6 +21294,7 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
* receive_secure_db() if it ever gets called or simply freed by
* zone_free() otherwise.
*/
ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read);
if (zone->db != NULL) {
zone_iattach(zone, &dummy);
@ -21258,6 +21305,8 @@ dns_zone_setnsec3param(dns_zone_t *zone, uint8_t hash, uint8_t flags,
}
ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read);
result = ISC_R_SUCCESS;
failure:
if (e != NULL) {
isc_event_free(&e);