diff --git a/lib/dns/rdata/generic/key_25.c b/lib/dns/rdata/generic/key_25.c index 57fefd6cda..77f8bf7a3d 100644 --- a/lib/dns/rdata/generic/key_25.c +++ b/lib/dns/rdata/generic/key_25.c @@ -19,6 +19,29 @@ #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 inline 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 inline isc_result_t generic_fromtext_key(ARGS_FROMTEXT) { isc_result_t result; @@ -27,7 +50,6 @@ generic_fromtext_key(ARGS_FROMTEXT) { dns_secproto_t proto; dns_keyflags_t flags; - UNUSED(type); UNUSED(rdclass); UNUSED(origin); UNUSED(options); @@ -52,8 +74,9 @@ generic_fromtext_key(ARGS_FROMTEXT) { RETERR(mem_tobuffer(target, &alg, 1)); /* No Key? */ - if ((flags & 0xc000) == 0xc000) + if (generic_key_nokey(type, flags)) { return (ISC_R_SUCCESS); + } result = isc_base64_tobuffer(lexer, target, -2); if (result != ISC_R_SUCCESS) @@ -108,8 +131,9 @@ generic_totext_key(ARGS_TOTEXT) { RETERR(str_totext(buf, target)); /* No Key? */ - if ((flags & 0xc000) == 0xc000) + if (generic_key_nokey(rdata->type, flags)) { return (ISC_R_SUCCESS); + } if ((tctx->flags & DNS_STYLEFLAG_RRCOMMENT) != 0 && algorithm == DNS_KEYALG_PRIVATEDNS) { @@ -169,22 +193,31 @@ generic_totext_key(ARGS_TOTEXT) { static inline isc_result_t generic_fromwire_key(ARGS_FROMWIRE) { unsigned char algorithm; + uint16_t flags; isc_region_t sr; - UNUSED(type); UNUSED(rdclass); UNUSED(dctx); UNUSED(options); isc_buffer_activeregion(source, &sr); - if (sr.length < 4) + if (sr.length < 4) { return (ISC_R_UNEXPECTEDEND); + } + flags = (sr.base[0] << 8) | sr.base[1]; algorithm = sr.base[3]; RETERR(mem_tobuffer(target, sr.base, 4)); 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); + } + if (algorithm == DNS_KEYALG_PRIVATEDNS) { dns_name_t name; dns_decompress_setmethods(dctx, DNS_COMPRESS_NONE); diff --git a/lib/dns/tests/master_test.c b/lib/dns/tests/master_test.c index b1b797dd98..5d5f935f29 100644 --- a/lib/dns/tests/master_test.c +++ b/lib/dns/tests/master_test.c @@ -311,10 +311,12 @@ dnskey_test(void **state) { assert_int_equal(result, ISC_R_SUCCESS); } - /* * DNSKEY with no key material test: * dns_master_loadfile() understands DNSKEY with no key material + * + * RFC 4034 removed the ability to signal NOKEY, so empty key material should + * be rejected. */ static void dnsnokey_test(void **state) { @@ -324,7 +326,7 @@ dnsnokey_test(void **state) { result = test_master("testdata/master/master7.data", dns_masterformat_text, nullmsg, nullmsg); - assert_int_equal(result, ISC_R_SUCCESS); + assert_int_equal(result, ISC_R_UNEXPECTEDEND); } /* @@ -363,9 +365,10 @@ master_includelist_test(void **state) { &filename, mctx, dns_masterformat_text); assert_int_equal(result, DNS_R_SEENINCLUDE); assert_non_null(filename); - - assert_string_equal(filename, "testdata/master/master7.data"); - isc_mem_free(mctx, filename); + if (filename != NULL) { + assert_string_equal(filename, "testdata/master/master6.data"); + isc_mem_free(mctx, filename); + } } /* diff --git a/lib/dns/tests/rdata_test.c b/lib/dns/tests/rdata_test.c index 28a8eca5ac..ad4f14c0af 100644 --- a/lib/dns/tests/rdata_test.c +++ b/lib/dns/tests/rdata_test.c @@ -398,6 +398,43 @@ check_rdata(const text_ok_t *text_ok, const wire_ok_t *wire_ok, } } +/* + * Common tests for RR types based on KEY that require key data: + * + * - CDNSKEY (RFC 7344) + * - DNSKEY (RFC 4034) + * - RKEY (draft-reid-dnsext-rkey-00) + */ +static void +key_required(void **state, dns_rdatatype_t type, size_t size) { + wire_ok_t wire_ok[] = { + /* + * RDATA must be at least 5 octets in size: + * + * - 2 octets for Flags, + * - 1 octet for Protocol, + * - 1 octet for Algorithm, + * - Public Key must not be empty. + * + * RFC 2535 section 3.1.2 allows the Public Key to be empty if + * bits 0-1 of Flags are both set, but that only applies to KEY + * records: for the RR types tested here, the Public Key must + * not be empty. + */ + WIRE_INVALID(0x00), + WIRE_INVALID(0x00, 0x00), + WIRE_INVALID(0x00, 0x00, 0x00), + WIRE_INVALID(0xc0, 0x00, 0x00, 0x00), + WIRE_INVALID(0x00, 0x00, 0x00, 0x00), + WIRE_VALID(0x00, 0x00, 0x00, 0x00, 0x00), + WIRE_SENTINEL() + }; + + UNUSED(state); + + check_rdata(NULL, wire_ok, NULL, false, dns_rdataclass_in, type, size); +} + /* APL RDATA manipulations */ static void apl(void **state) { @@ -656,6 +693,11 @@ amtrelay(void **state) { dns_rdatatype_amtrelay, sizeof(dns_rdata_amtrelay_t)); } +static void +cdnskey(void **state) { + key_required(state, dns_rdatatype_cdnskey, sizeof(dns_rdata_cdnskey_t)); +} + /* * CSYNC tests. * @@ -784,6 +826,11 @@ csync(void **state) { dns_rdatatype_csync, sizeof(dns_rdata_csync_t)); } +static void +dnskey(void **state) { + key_required(state, dns_rdatatype_dnskey, sizeof(dns_rdata_dnskey_t)); +} + /* * DOA tests. * @@ -1329,6 +1376,41 @@ isdn(void **state) { dns_rdatatype_isdn, sizeof(dns_rdata_isdn_t)); } +/* + * KEY tests. + */ +static void +key(void **state) { + wire_ok_t wire_ok[] = { + /* + * RDATA is comprised of: + * + * - 2 octets for Flags, + * - 1 octet for Protocol, + * - 1 octet for Algorithm, + * - variable number of octets for Public Key. + * + * RFC 2535 section 3.1.2 states 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. + */ + 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(0x00, 0x00, 0x00, 0x00), + WIRE_VALID(0x00, 0x00, 0x00, 0x00, 0x00), + WIRE_SENTINEL() + }; + + UNUSED(state); + + check_rdata(NULL, wire_ok, NULL, false, dns_rdataclass_in, + dns_rdatatype_key, sizeof(dns_rdata_key_t)); +} + /* * http://ana-3.lcs.mit.edu/~jnc/nimrod/dns.txt * @@ -1509,6 +1591,11 @@ nxt(void **state) { dns_rdatatype_nxt, sizeof(dns_rdata_nxt_t)); } +static void +rkey(void **state) { + key_required(state, dns_rdatatype_rkey, sizeof(dns_rdata_rkey_t)); +} + /* * WKS tests. * @@ -1816,18 +1903,22 @@ main(void) { cmocka_unit_test_setup_teardown(amtrelay, _setup, _teardown), cmocka_unit_test_setup_teardown(apl, _setup, _teardown), cmocka_unit_test_setup_teardown(atma, _setup, _teardown), + cmocka_unit_test_setup_teardown(cdnskey, _setup, _teardown), cmocka_unit_test_setup_teardown(csync, _setup, _teardown), cmocka_unit_test_setup_teardown(doa, _setup, _teardown), + cmocka_unit_test_setup_teardown(dnskey, _setup, _teardown), cmocka_unit_test_setup_teardown(eid, _setup, _teardown), cmocka_unit_test_setup_teardown(edns_client_subnet, _setup, _teardown), cmocka_unit_test_setup_teardown(hip, _setup, _teardown), cmocka_unit_test_setup_teardown(isdn, _setup, _teardown), + cmocka_unit_test_setup_teardown(key, _setup, _teardown), cmocka_unit_test_setup_teardown(nimloc, _setup, _teardown), cmocka_unit_test_setup_teardown(nsec, _setup, _teardown), cmocka_unit_test_setup_teardown(nsec3, _setup, _teardown), cmocka_unit_test_setup_teardown(nxt, _setup, _teardown), cmocka_unit_test_setup_teardown(wks, _setup, _teardown), + cmocka_unit_test_setup_teardown(rkey, _setup, _teardown), cmocka_unit_test_setup_teardown(zonemd, _setup, _teardown), cmocka_unit_test_setup_teardown(atcname, NULL, NULL), cmocka_unit_test_setup_teardown(atparent, NULL, NULL), diff --git a/lib/dns/tests/testdata/master/master8.data b/lib/dns/tests/testdata/master/master8.data index 2210f42354..d16b6f3d6e 100644 --- a/lib/dns/tests/testdata/master/master8.data +++ b/lib/dns/tests/testdata/master/master8.data @@ -1,4 +1,4 @@ ; -; master7.data contains a good zone file +; master6.data contains a good zone file ; -$include testdata/master/master7.data +$include testdata/master/master6.data