check flags for no key in fromwire for *KEY

(cherry picked from commit 2592e91516a44368ef86c17010eb56db017523ca)

diff --git a/lib/dns/rdata/generic/key_25.c b/lib/dns/rdata/generic/key_25.c
index 57fefd6cda9d0b11f0a6e814e7e4034b91ceb9af..77f8bf7a3d76b39e9fba7d6b4e9367a2760bb5f4 100644 (file)
--- 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 b1b797dd98a8b9361023141526471117826b1480..5d5f935f299885eb0a7d1c17a1d100078f17a2d2 100644 (file)
--- 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 28a8eca5ac403632e88ea65cd3c1fdea33eba1f9..ad4f14c0af6e1fc93e45d875f2f78f84d9bf1163 100644 (file)
--- 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 2210f42354327e4acac2744d50ce6019f35b370f..d16b6f3d6e670f576c299b2d6077e9334228b7b8 100644 (file)
--- 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