From 07b599748f847b9019676e525da16912da89c966 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Mar 2025 12:20:40 -0700 Subject: [PATCH 1/3] Tidy up keyvalue.h definitions Use enums for DNS_KEYFLAG_, DNS_KEYTYPE_, DNS_KEYOWNER_, DNS_KEYALG_, and DNS_KEYPROTO_ values. Remove values that are never used. Eliminate the obsolete DNS_KEYFLAG_SIGNATORYMASK. Instead, add three more RESERVED bits for the key flag values that it covered but which were never used. (cherry picked from commit fee1ba40df939f25fc9258b2681a1a2bd7965f5d) --- bin/dnssec/dnssec-keyfromlabel.c | 15 +--- bin/dnssec/dnssec-keygen.c | 5 +- lib/dns/include/dns/keyvalues.h | 141 +++++++++++++++---------------- 3 files changed, 70 insertions(+), 91 deletions(-) diff --git a/bin/dnssec/dnssec-keyfromlabel.c b/bin/dnssec/dnssec-keyfromlabel.c index cf45491f39..6cf9ec98ad 100644 --- a/bin/dnssec/dnssec-keyfromlabel.c +++ b/bin/dnssec/dnssec-keyfromlabel.c @@ -563,7 +563,7 @@ main(int argc, char **argv) { { flags |= DNS_KEYOWNER_ENTITY; } else if (strcasecmp(nametype, "user") == 0) { - flags |= DNS_KEYOWNER_USER; + /* no owner flags */ } else { fatal("invalid KEY nametype %s", nametype); } @@ -592,19 +592,6 @@ main(int argc, char **argv) { fatal("invalid DNSKEY protocol: %d", protocol); } - if ((flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY) { - if ((flags & DNS_KEYFLAG_SIGNATORYMASK) != 0) { - fatal("specified null key with signing authority"); - } - } - - if ((flags & DNS_KEYFLAG_OWNERMASK) == DNS_KEYOWNER_ZONE && - alg == DNS_KEYALG_DH) - { - fatal("a key with algorithm '%s' cannot be a zone key", - algname); - } - isc_buffer_init(&buf, filename, sizeof(filename) - 1); /* associate the key */ diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index a79623a1df..7e6d2d30dc 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -572,7 +572,7 @@ keygen(keygen_ctx_t *ctx, isc_mem_t *mctx, int argc, char **argv) { { flags |= DNS_KEYOWNER_ENTITY; } else if (strcasecmp(ctx->nametype, "user") == 0) { - flags |= DNS_KEYOWNER_USER; + /* no owner flags */ } else { fatal("invalid KEY nametype %s", ctx->nametype); } @@ -603,9 +603,6 @@ keygen(keygen_ctx_t *ctx, isc_mem_t *mctx, int argc, char **argv) { if (ctx->size > 0) { fatal("specified null key with non-zero size"); } - if ((flags & DNS_KEYFLAG_SIGNATORYMASK) != 0) { - fatal("specified null key with signing authority"); - } } if ((flags & DNS_KEYFLAG_OWNERMASK) == DNS_KEYOWNER_ZONE && diff --git a/lib/dns/include/dns/keyvalues.h b/lib/dns/include/dns/keyvalues.h index 21552661bc..5211a9ea75 100644 --- a/lib/dns/include/dns/keyvalues.h +++ b/lib/dns/include/dns/keyvalues.h @@ -16,89 +16,84 @@ /*! \file dns/keyvalues.h */ /* - * Flags field of the KEY RR rdata + * 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. */ -#define DNS_KEYFLAG_TYPEMASK 0xC000 /*%< Mask for "type" bits */ -#define DNS_KEYTYPE_AUTHCONF 0x0000 /*%< Key usable for both */ -#define DNS_KEYTYPE_CONFONLY 0x8000 /*%< Key usable for confidentiality */ -#define DNS_KEYTYPE_AUTHONLY 0x4000 /*%< Key usable for authentication */ -#define DNS_KEYTYPE_NOKEY 0xC000 /*%< No key usable for either; no key */ -#define DNS_KEYTYPE_NOAUTH DNS_KEYTYPE_CONFONLY -#define DNS_KEYTYPE_NOCONF DNS_KEYTYPE_AUTHONLY +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. */ -#define DNS_KEYFLAG_RESERVED2 0x2000 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_EXTENDED 0x1000 /*%< key has extended flags */ -#define DNS_KEYFLAG_RESERVED4 0x0800 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_RESERVED5 0x0400 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_OWNERMASK 0x0300 /*%< these bits determine the type */ -#define DNS_KEYOWNER_USER 0x0000 /*%< key is assoc. with user */ -#define DNS_KEYOWNER_ENTITY 0x0200 /*%< key is assoc. with entity eg host */ -#define DNS_KEYOWNER_ZONE 0x0100 /*%< key is zone key */ -#define DNS_KEYOWNER_RESERVED 0x0300 /*%< reserved meaning */ -#define DNS_KEYFLAG_REVOKE 0x0080 /*%< key revoked (per rfc5011) */ -#define DNS_KEYFLAG_RESERVED9 0x0040 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_RESERVED10 0x0020 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_RESERVED11 0x0010 /*%< reserved - must be zero */ -#define DNS_KEYFLAG_SIGNATORYMASK \ - 0x000F /*%< key can sign RR's of same name \ - */ + DNS_KEYFLAG_RESERVED2 = 1 << 13, /* reserved: must be zero. */ -#define DNS_KEYFLAG_RESERVEDMASK \ - (DNS_KEYFLAG_RESERVED2 | DNS_KEYFLAG_RESERVED4 | \ - DNS_KEYFLAG_RESERVED5 | DNS_KEYFLAG_RESERVED9 | \ - DNS_KEYFLAG_RESERVED10 | DNS_KEYFLAG_RESERVED11) -#define DNS_KEYFLAG_KSK 0x0001 /*%< key signing key */ + 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.) */ -#define DNS_KEYFLAG_RESERVEDMASK2 0xFFFF /*%< no bits defined here */ + 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 */ +}; + +#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} */ -#define DNS_KEYALG_RSAMD5 1 /*%< RSA with MD5 */ -#define DNS_KEYALG_RSA 1 /*%< Used just for tagging */ -#define DNS_KEYALG_DH 2 /*%< Diffie Hellman KEY */ -#define DNS_KEYALG_DSA 3 /*%< DSA KEY */ -#define DNS_KEYALG_NSEC3DSA 6 -#define DNS_KEYALG_DSS DNS_ALG_DSA -#define DNS_KEYALG_ECC 4 -#define DNS_KEYALG_RSASHA1 5 -#define DNS_KEYALG_NSEC3RSASHA1 7 -#define DNS_KEYALG_RSASHA256 8 -#define DNS_KEYALG_RSASHA512 10 -#define DNS_KEYALG_ECCGOST 12 -#define DNS_KEYALG_ECDSA256 13 -#define DNS_KEYALG_ECDSA384 14 -#define DNS_KEYALG_ED25519 15 -#define DNS_KEYALG_ED448 16 -#define DNS_KEYALG_INDIRECT 252 -#define DNS_KEYALG_PRIVATEDNS 253 -#define DNS_KEYALG_PRIVATEOID 254 /*%< Key begins with OID giving alg */ -#define DNS_KEYALG_MAX 255 +enum { + DNS_KEYALG_RSAMD5 = 1, /*%< RSA with MD5 */ + DNS_KEYALG_DH = 2, /*%< Diffie Hellman KEY */ + DNS_KEYALG_DSA = 3, /*%< DSA KEY */ + DNS_KEYALG_RSASHA1 = 5, + DNS_KEYALG_NSEC3DSA = 6, + DNS_KEYALG_NSEC3RSASHA1 = 7, + DNS_KEYALG_RSASHA256 = 8, + DNS_KEYALG_RSASHA512 = 10, + DNS_KEYALG_ECCGOST = 12, + DNS_KEYALG_ECDSA256 = 13, + DNS_KEYALG_ECDSA384 = 14, + DNS_KEYALG_ED25519 = 15, + DNS_KEYALG_ED448 = 16, + DNS_KEYALG_INDIRECT = 252, + DNS_KEYALG_PRIVATEDNS = 253, + DNS_KEYALG_PRIVATEOID = 254, /*%< Key begins with OID giving alg */ + DNS_KEYALG_MAX = 255, +}; /* Protocol values */ -#define DNS_KEYPROTO_RESERVED 0 -#define DNS_KEYPROTO_TLS 1 -#define DNS_KEYPROTO_EMAIL 2 -#define DNS_KEYPROTO_DNSSEC 3 -#define DNS_KEYPROTO_IPSEC 4 -#define DNS_KEYPROTO_ANY 255 - -/* Signatures */ -#define DNS_SIG_RSAMINBITS 512 /*%< Size of a mod or exp in bits */ -#define DNS_SIG_RSAMAXBITS 2552 -/* Total of binary mod and exp */ -#define DNS_SIG_RSAMAXBYTES ((DNS_SIG_RSAMAXBITS + 7 / 8) * 2 + 3) -/*%< Max length of text sig block */ -#define DNS_SIG_RSAMAXBASE64 (((DNS_SIG_RSAMAXBYTES + 2) / 3) * 4) -#define DNS_SIG_RSAMINSIZE ((DNS_SIG_RSAMINBITS + 7) / 8) -#define DNS_SIG_RSAMAXSIZE ((DNS_SIG_RSAMAXBITS + 7) / 8) +enum { + DNS_KEYPROTO_RESERVED = 0, + DNS_KEYPROTO_DNSSEC = 3, + DNS_KEYPROTO_ANY = 255, +}; +/* Key and signature sizes */ +#define DNS_KEY_ECDSA256SIZE 64 #define DNS_SIG_ECDSA256SIZE 64 + +#define DNS_KEY_ECDSA384SIZE 96 #define DNS_SIG_ECDSA384SIZE 96 -#define DNS_KEY_ECDSA256SIZE 64 -#define DNS_KEY_ECDSA384SIZE 96 - -#define DNS_SIG_ED25519SIZE 64 -#define DNS_SIG_ED448SIZE 114 - #define DNS_KEY_ED25519SIZE 32 -#define DNS_KEY_ED448SIZE 57 +#define DNS_SIG_ED25519SIZE 64 + +#define DNS_KEY_ED448SIZE 57 +#define DNS_SIG_ED448SIZE 114 From 4b813335672e563bd719865f9eeef6b6e5d53b30 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 13 Mar 2025 17:44:49 -0700 Subject: [PATCH 2/3] Don't check DNS_KEYFLAG_NOAUTH All DNSKEY keys are able to authenticate. The DNS_KEYTYPE_NOAUTH (and DNS_KEYTYPE_NOCONF) flags were defined for the KEY rdata type, and are not applicable to DNSKEY. Previously, because the DNSKEY implementation was built on top of KEY, the NOAUTH flag prevented authentication in DNSKEYs as well. This has been corrected. (cherry picked from commit 5c21576f82f9f62c2e22aac920a37a4013ac3a80) --- lib/dns/dnssec.c | 35 +---------------------------------- lib/dns/key.c | 3 --- lib/dns/zone.c | 3 +-- 3 files changed, 2 insertions(+), 39 deletions(-) diff --git a/lib/dns/dnssec.c b/lib/dns/dnssec.c index 662136db87..3fc3988281 100644 --- a/lib/dns/dnssec.c +++ b/lib/dns/dnssec.c @@ -194,7 +194,6 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, isc_result_t ret; isc_buffer_t *databuf = NULL; char data[256 + 8]; - uint32_t flags; unsigned int sigsize; dns_fixedname_t fnewname; dns_fixedname_t fsigner; @@ -212,17 +211,6 @@ dns_dnssec_sign(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, return DNS_R_INVALIDTIME; } - /* - * Is the key allowed to sign data? - */ - flags = dst_key_flags(key); - if ((flags & DNS_KEYTYPE_NOAUTH) != 0) { - return DNS_R_KEYUNAUTHORIZED; - } - if ((flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE) { - return DNS_R_KEYUNAUTHORIZED; - } - sig.mctx = mctx; sig.common.rdclass = set->rdclass; sig.common.rdtype = dns_rdatatype_rrsig; @@ -385,7 +373,6 @@ dns_dnssec_verify(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, unsigned char data[300]; dst_context_t *ctx = NULL; int labels = 0; - uint32_t flags; bool downcase = false; REQUIRE(name != NULL); @@ -450,19 +437,6 @@ dns_dnssec_verify(const dns_name_t *name, dns_rdataset_t *set, dst_key_t *key, break; } - /* - * Is the key allowed to sign data? - */ - flags = dst_key_flags(key); - if ((flags & DNS_KEYTYPE_NOAUTH) != 0) { - inc_stat(dns_dnssecstats_fail); - return DNS_R_KEYUNAUTHORIZED; - } - if ((flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE) { - inc_stat(dns_dnssecstats_fail); - return DNS_R_KEYUNAUTHORIZED; - } - again: ret = dst_context_create(key, mctx, DNS_LOGCATEGORY_DNSSEC, false, maxbits, &ctx); @@ -1687,9 +1661,7 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, RETERR(dns_dnssec_keyfromrdata(origin, &rdata, mctx, &dnskey)); dst_key_setttl(dnskey, keys.ttl); - if (!is_zone_key(dnskey) || - (dst_key_flags(dnskey) & DNS_KEYTYPE_NOAUTH) != 0) - { + if (!is_zone_key(dnskey)) { goto skip; } @@ -1792,11 +1764,6 @@ dns_dnssec_keylistfromrdataset(const dns_name_t *origin, const char *directory, } RETERR(result); - /* This should never happen. */ - if ((dst_key_flags(privkey) & DNS_KEYTYPE_NOAUTH) != 0) { - goto skip; - } - /* * Whatever the key's default TTL may have * been, the rdataset TTL takes priority. diff --git a/lib/dns/key.c b/lib/dns/key.c index 0b4b3d261f..7f496ff086 100644 --- a/lib/dns/key.c +++ b/lib/dns/key.c @@ -127,9 +127,6 @@ bool dst_key_iszonekey(const dst_key_t *key) { REQUIRE(VALID_KEY(key)); - if ((key->key_flags & DNS_KEYTYPE_NOAUTH) != 0) { - return false; - } if ((key->key_flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE) { return false; } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index ac4263fb46..e91c2f6bde 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -20851,8 +20851,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_KEYTYPE_NOAUTH)) != DNS_KEYOWNER_ZONE) + if ((dnskey.flags & DNS_KEYFLAG_OWNERMASK) != DNS_KEYOWNER_ZONE) { ISC_LIST_UNLINK(diff->tuples, tuple, link); ISC_LIST_APPEND(tuples, tuple, link); From f095d22adf36121fbb8d8a523bedce739eaa9fe3 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 25 Mar 2025 14:15:37 +1100 Subject: [PATCH 3/3] DNS_KEYTYPE_NOKEY is only applicable to KEY (cherry picked from commit 53c6721abc49746d91e61a5bb2cbbea24d64dd72) --- bin/dnssec/dnssec-keygen.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index 7e6d2d30dc..44008bbce5 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -631,7 +631,9 @@ keygen(keygen_ctx_t *ctx, isc_mem_t *mctx, int argc, char **argv) { break; } - if ((flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY) { + if ((flags & DNS_KEYFLAG_TYPEMASK) == DNS_KEYTYPE_NOKEY && + (ctx->options & DST_TYPE_KEY) != 0) + { null_key = true; }