upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

rem: dev: Remove obsolete KEY record flags deprecated by RFC 3445

Browse source 
KEY resource records originally defined NOAUTH, NOCONF, EXTENDED, and
ENTITY flags that were removed by RFC 3445 back in 2002. BIND still
carried code to parse and emit them, including the additional two-octet
flags field that followed when the EXTENDED bit was set. That handling
has been removed and the affected bit positions are now reserved.

Dropping the extended-flags handling also eliminates a possible crash
that could be reached when signing a zone containing an invalid key.

Closes #5900

Merge branch '5900-remove-keyflag-extended' into 'main'

See merge request isc-projects/bind9!11961
This commit is contained in:
Ondřej Surý 2026-05-05 10:49:37 +02:00
commit 1535b32dab
10 changed files with 41 additions and 166 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
bin/dnssec/dnssec-keyfromlabel.c
View file

@ -543,8 +543,6 @@ main(int argc, char **argv) {
flags |= DNS_KEYOWNER_ZONE; /* DNSKEY: name type ZONE */
flags |= kskflag;
flags |= revflag;
} else {
flags |= DNS_KEYOWNER_ENTITY; /* KEY: name type HOST */
}
isc_buffer_init(&buf, filename, sizeof(filename) - 1);

19
bin/dnssec/dnssec-keygen.c
View file

@ -232,7 +232,6 @@ keygen(keygen_ctx_t *ctx, const char *keyname) {
char filename[255];
char algstr[DNS_SECALG_FORMATSIZE];
uint16_t flags = 0;
bool null_key = false;
bool conflict = false;
bool show_progress = false;
isc_buffer_t buf;
@ -497,8 +496,6 @@ keygen(keygen_ctx_t *ctx, const char *keyname) {
if ((ctx->options & DST_TYPE_KEY) == 0) {
flags |= DNS_KEYOWNER_ZONE; /* DNSKEY: name type ZONE */
} else {
flags |= DNS_KEYOWNER_ENTITY; /* KEY: name type HOST */
}
if (ctx->directory == NULL) {
@ -532,12 +529,6 @@ keygen(keygen_ctx_t *ctx, const char *keyname) {
break;
}
if ((flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY &&
(ctx->options & DST_TYPE_KEY) != 0)
{
null_key = true;
}
isc_buffer_init(&buf, filename, sizeof(filename) - 1);
do {
@ -668,11 +659,6 @@ keygen(keygen_ctx_t *ctx, const char *keyname) {
ctx->tag_min, ctx->tag_max, NULL))
{
conflict = true;
if (null_key) {
dst_key_free(&key);
break;
}
if (verbose > 0) {
isc_buffer_clear(&buf);
result = dst_key_buildfilename(
@ -692,11 +678,6 @@ keygen(keygen_ctx_t *ctx, const char *keyname) {
}
} while (conflict);
if (conflict) {
fatal("cannot generate a null key due to possible key ID "
"collision");
}
if (ctx->predecessor != NULL && prevkey != NULL) {
dst_key_setnum(prevkey, DST_NUM_SUCCESSOR, dst_key_id(key));
dst_key_setnum(key, DST_NUM_PREDECESSOR, dst_key_id(prevkey));

5
lib/dns/dnssec.c
View file

@ -707,8 +707,7 @@ syncdelete(dst_key_t *key, isc_stdtime_t now) {
return false;
}
#define is_zone_key(key) \
((dst_key_flags(key) & DNS_KEYFLAG_OWNERMASK) == DNS_KEYOWNER_ZONE)
#define is_zone_key(key) ((dst_key_flags(key) & DNS_KEYOWNER_ZONE) != 0)
isc_result_t
dns_dnssec_signmessage(dns_message_t *msg, dst_key_t *key) {
@ -1049,7 +1048,7 @@ dns_dnssec_signs(dns_rdata_t *rdata, const dns_name_t *name,
bool
dns_dnssec_iszonekey(dns_rdata_dnskey_t *key) {
return (key->flags & DNS_KEYFLAG_OWNERMASK) == DNS_KEYOWNER_ZONE &&
return (key->flags & DNS_KEYOWNER_ZONE) != 0 &&
(key->protocol == DNS_KEYPROTO_DNSSEC ||
key->protocol == DNS_KEYPROTO_ANY);
}

51
lib/dns/dst_api.c
View file

@ -385,9 +385,7 @@ dst_key_tofile(const dst_key_t *key, int type, const char *directory) {
RETERR(write_key_state(key, type, directory));
}
if (((type & DST_TYPE_PRIVATE) != 0) &&
(key->key_flags & DNS_KEYFLAG_TYPEMASK) != DNS_KEYTYPE_NOKEY)
{
if ((type & DST_TYPE_PRIVATE) != 0) {
return key->func->tofile(key, directory);
}
return ISC_R_SUCCESS;
@ -551,9 +549,7 @@ dst_key_fromnamedfile(const char *filename, const char *dirname, int type,
CHECK(result);
}
if ((type & (DST_TYPE_PRIVATE | DST_TYPE_PUBLIC)) == DST_TYPE_PUBLIC ||
(pubkey->key_flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY)
{
if ((type & (DST_TYPE_PRIVATE | DST_TYPE_PUBLIC)) == DST_TYPE_PUBLIC) {
CHECK(computeid(pubkey));
pubkey->modified = false;
*keyp = pubkey;
@ -652,15 +648,7 @@ dst_key_todns(const dst_key_t *key, isc_buffer_t *target) {
isc_buffer_putuint8(target,
(uint8_t)dst_algorithm_tosecalg(key->key_alg));
if ((key->key_flags & DNS_KEYFLAG_EXTENDED) != 0) {
if (isc_buffer_availablelength(target) < 2) {
return ISC_R_NOSPACE;
}
isc_buffer_putuint16(
target, (uint16_t)((key->key_flags >> 16) & 0xffff));
}
if (key->keydata.generic == NULL) { /*%< NULL KEY */
if (key->keydata.generic == NULL) {
return ISC_R_SUCCESS;
}
@ -671,7 +659,7 @@ isc_result_t
dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass,
isc_buffer_t *source, isc_mem_t *mctx, dst_key_t **keyp) {
uint8_t alg, proto;
uint32_t flags, extflags;
uint32_t flags;
dst_key_t *key = NULL;
dns_keytag_t id, rid;
isc_region_t r;
@ -688,14 +676,6 @@ dst_key_fromdns(const dns_name_t *name, dns_rdataclass_t rdclass,
id = dst_region_computeid(&r);
rid = dst_region_computerid(&r);
if ((flags & DNS_KEYFLAG_EXTENDED) != 0) {
if (isc_buffer_remaininglength(source) < 2) {
return DST_R_INVALIDPUBLICKEY;
}
extflags = isc_buffer_getuint16(source);
flags |= (extflags << 16);
}
RETERR(frombuffer(name, alg, flags, proto, rdclass, source, mctx,
&key));
key->key_id = id;
@ -927,12 +907,6 @@ dst_key_generate(const dns_name_t *name, unsigned int alg, unsigned int bits,
key->label = isc_mem_strdup(mctx, label);
}
if (bits == 0) { /*%< NULL KEY */
key->key_flags |= DNS_KEYTYPE_NOKEY;
*keyp = key;
return ISC_R_SUCCESS;
}
if (key->func->generate == NULL) {
dst_key_free(&key);
return DST_R_UNSUPPORTEDALG;
@ -1190,9 +1164,6 @@ pub_compare(const dst_key_t *key1, const dst_key_t *key2) {
}
/* Zero out flags. */
buf1[0] = buf1[1] = 0;
if ((key1->key_flags & DNS_KEYFLAG_EXTENDED) != 0) {
isc_buffer_subtract(&b1, 2);
}
isc_buffer_init(&b2, buf2, sizeof(buf2));
result = dst_key_todns(key2, &b2);
@ -1201,23 +1172,9 @@ pub_compare(const dst_key_t *key1, const dst_key_t *key2) {
}
/* Zero out flags. */
buf2[0] = buf2[1] = 0;
if ((key2->key_flags & DNS_KEYFLAG_EXTENDED) != 0) {
isc_buffer_subtract(&b2, 2);
}
isc_buffer_usedregion(&b1, &r1);
/* Remove extended flags. */
if ((key1->key_flags & DNS_KEYFLAG_EXTENDED) != 0) {
memmove(&buf1[4], &buf1[6], r1.length - 6);
r1.length -= 2;
}
isc_buffer_usedregion(&b2, &r2);
/* Remove extended flags. */
if ((key2->key_flags & DNS_KEYFLAG_EXTENDED) != 0) {
memmove(&buf2[4], &buf2[6], r2.length - 6);
r2.length -= 2;
}
return isc_region_compare(&r1, &r2) == 0;
}

53
lib/dns/include/dns/keyvalues.h
View file

@ -16,47 +16,26 @@
/*! \file dns/keyvalues.h */
/*
* Flags field of the KEY rdata. Also used by DNSKEY, CDNSKEY, RKEY,
* KEYDATA. Some values are only defined for KEY and not the others,
* and vice versa.
* Flags field of the DNSKEY rdata. Also used by KEY, CDNSKEY, RKEY,
* and KEYDATA, which share DNSKEY's wire format.
*
* The following flags were removed by RFC 3445 and MUST be zero.
* Any use of these flags will break backwards compatibility with
* old software. As long as they are zero they are safe:
* - 1 << 15: Formerly DNS_KEYTYPE_NOAUTH.
* - 1 << 14: Formerly DNS_KEYTYPE_NOCONF.
* - 1 << 12: Formerly DNS_KEYFLAG_EXTENDED.
* - 1 << 9: Formerly DNS_KEYOWNER_ENTITY.
*
* The following flags are reserved and MUST be zero.
* - 1 << 13, 1 << 11, 1 << 10, 1 << 6 through 1 << 2
*/
enum {
/* valid for KEY only. if both are set, there is no key data. */
DNS_KEYTYPE_NOAUTH = 1 << 15, /* cannot be used for authentication. */
DNS_KEYTYPE_NOCONF = 1 << 14, /* cannot be used for confidentiality. */
DNS_KEYFLAG_RESERVED2 = 1 << 13, /* reserved: must be zero. */
DNS_KEYFLAG_EXTENDED = 1 << 12, /* key has extended flags: if this is
* set, the first two octets of the
* key data are an additional flags
* field, at least one bit of which
* must be nonzero. (valid for KEY
* only.) */
DNS_KEYFLAG_RESERVED4 = 1 << 11, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED5 = 1 << 10, /* reserved: must be zero. */
/* if nether of these is set, this is a user key (valid for KEY only) */
DNS_KEYOWNER_ENTITY = 1 << 9, /* host key (valid for KEY only). */
DNS_KEYOWNER_ZONE = 1 << 8, /* zone key (mandatory for DNSKEY). */
DNS_KEYFLAG_REVOKE = 1 << 7, /* key revoked (per rfc5011) */
DNS_KEYFLAG_RESERVED9 = 1 << 6, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED10 = 1 << 5, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED11 = 1 << 4, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED12 = 1 << 3, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED13 = 1 << 4, /* reserved: must be zero. */
DNS_KEYFLAG_RESERVED14 = 1 << 2, /* reserved: must be zero. */
DNS_KEYFLAG_KSK = 1 << 0, /* key signing key */
DNS_KEYOWNER_ZONE = 1 << 8, /* zone key (mandatory for DNSKEY). */
DNS_KEYFLAG_REVOKE = 1 << 7, /* key revoked (per rfc5011) */
DNS_KEYFLAG_KSK = 1 << 0, /* key signing key */
};
#define DNS_KEYFLAG_OWNERMASK (DNS_KEYOWNER_ENTITY | DNS_KEYOWNER_ZONE)
#define DNS_KEYFLAG_TYPEMASK (DNS_KEYTYPE_NOAUTH | DNS_KEYTYPE_NOCONF)
#define DNS_KEYTYPE_NOKEY DNS_KEYFLAG_TYPEMASK
/* The Algorithm field of the KEY and SIG RR's is an integer, {1..254} */
enum {
DNS_KEYALG_RSAMD5 = 1, /*%< RSA with MD5 */

2
lib/dns/key.c
View file

@ -128,7 +128,7 @@ bool
dst_key_iszonekey(const dst_key_t *key) {
REQUIRE(VALID_KEY(key));
if ((key->key_flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE) {
if ((key->key_flags & DNS_KEYOWNER_ZONE) == 0) {
return false;
}
if (key->key_proto != DNS_KEYPROTO_DNSSEC &&

36
lib/dns/rdata/generic/key_25.c
View file

@ -21,29 +21,6 @@
#define RRTYPE_KEY_ATTRIBUTES \
(DNS_RDATATYPEATTR_ATCNAME | DNS_RDATATYPEATTR_ZONECUTAUTH)
/*
* RFC 2535 section 3.1.2 says that if bits 0-1 of the Flags field are
* both set, it means there is no key information and the RR stops after
* the algorithm octet. However, this only applies to KEY records, as
* indicated by the specifications of the RR types based on KEY:
*
* CDNSKEY - RFC 7344
* DNSKEY - RFC 4034
* RKEY - draft-reid-dnsext-rkey-00
*/
static bool
generic_key_nokey(dns_rdatatype_t type, unsigned int flags) {
switch (type) {
case dns_rdatatype_cdnskey:
case dns_rdatatype_dnskey:
case dns_rdatatype_rkey:
return false;
case dns_rdatatype_key:
default:
return (flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY;
}
}
static isc_result_t
generic_fromtext_key(ARGS_FROMTEXT) {
isc_token_t token;
@ -78,11 +55,6 @@ generic_fromtext_key(ARGS_FROMTEXT) {
RETTOK(dns_secalg_fromtext(&alg, &token.value.as_textregion));
RETERR(mem_tobuffer(target, &alg, 1));
/* No Key? */
if (generic_key_nokey(type, flags)) {
return ISC_R_SUCCESS;
}
/*
* Save the current used value. It will become the current
* value when we parse the keydata field.
@ -149,11 +121,6 @@ generic_totext_key(ARGS_TOTEXT) {
isc_region_consume(&sr, 1);
RETERR(str_totext(buf, target));
/* No Key? */
if (generic_key_nokey(rdata->type, flags)) {
return ISC_R_SUCCESS;
}
if ((tctx->flags & DNS_STYLEFLAG_RRCOMMENT) != 0 &&
algorithm == DNS_KEYALG_PRIVATEDNS)
{
@ -249,9 +216,6 @@ generic_fromwire_key(ARGS_FROMWIRE) {
isc_region_consume(&sr, 4);
isc_buffer_forward(source, 4);
if (generic_key_nokey(type, flags)) {
return ISC_R_SUCCESS;
}
if (sr.length == 0) {
return ISC_R_UNEXPECTEDEND;
}

10
lib/dns/tsig.c
View file

@ -367,9 +367,8 @@ restore_key(dns_tsigkeyring_t *ring, isc_stdtime_t now, FILE *fp) {
return DNS_R_BADALG;
}
RETERR(dst_key_restore(name, dstalg, DNS_KEYOWNER_ENTITY,
DNS_KEYPROTO_DNSSEC, dns_rdataclass_in,
ring->mctx, keystr, &dstkey));
RETERR(dst_key_restore(name, dstalg, 0, DNS_KEYPROTO_DNSSEC,
dns_rdataclass_in, ring->mctx, keystr, &dstkey));
result = dns_tsigkey_createfromkey(name, dstalg, dstkey, true, true,
creator, inception, expire,
@ -472,9 +471,8 @@ dns_tsigkey_create(const dns_name_t *name, dst_algorithm_t algorithm,
isc_buffer_init(&b, secret, length);
isc_buffer_add(&b, length);
RETERR(dst_key_frombuffer(
name, algorithm, DNS_KEYOWNER_ENTITY,
DNS_KEYPROTO_DNSSEC, dns_rdataclass_in, &b,
mctx, &dstkey));
name, algorithm, 0, DNS_KEYPROTO_DNSSEC,
dns_rdataclass_in, &b, mctx, &dstkey));
}
} else if (length > 0) {
return DNS_R_BADALG;

17
lib/dns/zone.c
View file

@ -114,10 +114,11 @@
/*%
* Key flags
*/
#define REVOKE(x) ((dst_key_flags(x) & DNS_KEYFLAG_REVOKE) != 0)
#define KSK(x) ((dst_key_flags(x) & DNS_KEYFLAG_KSK) != 0)
#define ID(x) dst_key_id(x)
#define ALG(x) dst_key_alg(x)
#define REVOKE(x) ((dst_key_flags(x) & DNS_KEYFLAG_REVOKE) != 0)
#define KSK(x) ((dst_key_flags(x) & DNS_KEYFLAG_KSK) != 0)
#define ZONEKEY(x) ((dst_key_flags(x) & DNS_KEYOWNER_ZONE) != 0)
#define ID(x) dst_key_id(x)
#define ALG(x) dst_key_alg(x)
/*%
* KASP flags
@ -5122,9 +5123,6 @@ keyfromfile(dns_zone_t *zone, dst_key_t *pubkey, isc_mem_t *mctx,
return result;
}
#define is_zone_key(key) \
((dst_key_flags(key) & DNS_KEYFLAG_OWNERMASK) == DNS_KEYOWNER_ZONE)
static isc_result_t
findzonekeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
dns_dbnode_t *node, const dns_name_t *name, isc_stdtime_t now,
@ -5148,7 +5146,7 @@ findzonekeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver,
CHECK(dns_dnssec_keyfromrdata(name, &rdata, mctx, &pubkey));
dst_key_setttl(pubkey, rdataset.ttl);
if (!is_zone_key(pubkey)) {
if (!ZONEKEY(pubkey)) {
goto next;
}
/* Corrupted .key file? */
@ -16498,8 +16496,7 @@ add_signing_records(dns_db_t *db, dns_rdatatype_t privatetype,
result = dns_rdata_tostruct(&tuple->rdata, &dnskey, NULL);
RUNTIME_CHECK(result == ISC_R_SUCCESS);
if ((dnskey.flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE)
{
if ((dnskey.flags & DNS_KEYOWNER_ZONE) == 0) {
ISC_LIST_UNLINK(diff->tuples, tuple, link);
ISC_LIST_APPEND(tuples, tuple, link);
continue;

12
tests/dns/rdata_test.c
View file

@ -2255,20 +2255,22 @@ ISC_RUN_TEST_IMPL(key) {
* - 1 octet for Algorithm,
* - variable number of octets for Public Key.
*
* RFC 2535 section 3.1.2 states that if bits
* RFC 2535 section 3.1.2 stated that if bits
* 0-1 of Flags are both set, the RR stops after
* the algorithm octet and thus its length must
* be 4 octets. In any other case, though, the
* Public Key part must not be empty.
* Public Key part must not be empty. This was
* modified by RFC 3445 which removed
* flags 0-1, and hence the concept of NOKEY.
*
* Algorithms PRIVATEDNS (253) and PRIVATEOID (254)
* have an algorithm identifier embedded and the start
* have an algorithm identifier embedded at the start
* of the public key.
*/
WIRE_INVALID(0x00), WIRE_INVALID(0x00, 0x00),
WIRE_INVALID(0x00, 0x00, 0x00),
WIRE_VALID(0xc0, 0x00, 0x00, 0x00),
WIRE_INVALID(0xc0, 0x00, 0x00, 0x00, 0x00),
WIRE_INVALID(0xc0, 0x00, 0x00, 0x00),
WIRE_VALID(0xc0, 0x00, 0x00, 0x00, 0x00),
WIRE_INVALID(0x00, 0x00, 0x00, 0x00),
WIRE_VALID(0x00, 0x00, 0x00, 0x00, 0x00),
/* PRIVATEDNS example. without key data */