Commit 43828818 authored by Mark Andrews's avatar Mark Andrews
Browse files

Merge branch '899-fromwire-check-flags-for-nokey' into 'master'

Check KEY flags for empty key in fromwire method

See merge request !1574
parents 629b978f f78c688c
Pipeline #13139 passed with stages
in 57 seconds
5203. [bug] Enforce whether key rdata exists or not in KEY,
DNSKEY, CDNSKEY and RKEY. [GL #899]
5202. [bug] <dns/ecs.h> was missing ISC_LANG_ENDDECLS. [GL #976]
 
5201. [bug] Fix a possible deadlock in RPZ update code. [GL #973]
......
......@@ -337,7 +337,7 @@ ninfo14 NINFO "foo\;"
ninfo15 NINFO "bar\\;"
; type 57
rkey01 RKEY 512 ( 255 1 AQMFD5raczCJHViKtLYhWGz8hMY
rkey01 RKEY 0 ( 255 1 AQMFD5raczCJHViKtLYhWGz8hMY
9UGRuniJDBzC7w0aRyzWZriO6i2odGWWQVucZqKV
sENW91IOW4vqudngPZsY3GvQ/xVA8/7pyFj6b7Esg
a60zyGW6LFe9r8n6paHrlG5ojqf0BaqHT+8= )
......
......@@ -121,7 +121,7 @@ openpgpkey.example. 3600 IN OPENPGPKEY AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7
ptr01.example. 3600 IN PTR example.
px01.example. 3600 IN PX 65535 foo. bar.
px02.example. 3600 IN PX 65535 . .
rkey01.example. 3600 IN RKEY 512 255 1 AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7w0aRyzWZriO6i2od GWWQVucZqKVsENW91IOW4vqudngPZsY3GvQ/xVA8/7pyFj6b7Esga60z yGW6LFe9r8n6paHrlG5ojqf0BaqHT+8=
rkey01.example. 3600 IN RKEY 0 255 1 AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7w0aRyzWZriO6i2od GWWQVucZqKVsENW91IOW4vqudngPZsY3GvQ/xVA8/7pyFj6b7Esga60z yGW6LFe9r8n6paHrlG5ojqf0BaqHT+8=
rp01.example. 3600 IN RP mbox-dname.example. txt-dname.example.
rp02.example. 3600 IN RP . .
rt01.example. 3600 IN RT 0 intermediate-host.example.
......
......@@ -121,7 +121,7 @@ openpgpkey.example. 3600 IN OPENPGPKEY AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7
ptr01.example. 3600 IN PTR example.
px01.example. 3600 IN PX 65535 foo. bar.
px02.example. 3600 IN PX 65535 . .
rkey01.example. 3600 IN RKEY 512 255 1 AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7w0aRyzWZriO6i2od GWWQVucZqKVsENW91IOW4vqudngPZsY3GvQ/xVA8/7pyFj6b7Esga60z yGW6LFe9r8n6paHrlG5ojqf0BaqHT+8=
rkey01.example. 3600 IN RKEY 0 255 1 AQMFD5raczCJHViKtLYhWGz8hMY9UGRuniJDBzC7w0aRyzWZriO6i2od GWWQVucZqKVsENW91IOW4vqudngPZsY3GvQ/xVA8/7pyFj6b7Esga60z yGW6LFe9r8n6paHrlG5ojqf0BaqHT+8=
rp01.example. 3600 IN RP mbox-dname.example. txt-dname.example.
rp02.example. 3600 IN RP . .
rt01.example. 3600 IN RT 0 intermediate-host.example.
......
......@@ -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_token_t token;
......@@ -26,7 +49,6 @@ generic_fromtext_key(ARGS_FROMTEXT) {
dns_secproto_t proto;
dns_keyflags_t flags;
UNUSED(type);
UNUSED(rdclass);
UNUSED(origin);
UNUSED(options);
......@@ -36,6 +58,9 @@ generic_fromtext_key(ARGS_FROMTEXT) {
RETERR(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string,
false));
RETTOK(dns_keyflags_fromtext(&flags, &token.value.as_textregion));
if (type == dns_rdatatype_rkey && flags != 0U) {
RETTOK(DNS_R_FORMERR);
}
RETERR(uint16_tobuffer(flags, target));
/* protocol */
......@@ -51,8 +76,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);
}
return (isc_base64_tobuffer(lexer, target, -2));
}
......@@ -99,8 +125,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) {
......@@ -160,22 +187,35 @@ 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];
if (type == dns_rdatatype_rkey && flags != 0U) {
return (DNS_R_FORMERR);
}
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);
......@@ -258,6 +298,10 @@ generic_fromstruct_key(ARGS_FROMSTRUCT) {
UNUSED(type);
UNUSED(rdclass);
if (type == dns_rdatatype_rkey) {
INSIST(key->flags == 0U);
}
/* Flags */
RETERR(uint16_tobuffer(key->flags, target));
......
......@@ -305,10 +305,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) {
......@@ -318,7 +320,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);
}
/*
......@@ -358,7 +360,7 @@ master_includelist_test(void **state) {
assert_int_equal(result, DNS_R_SEENINCLUDE);
assert_non_null(filename);
if (filename != NULL) {
assert_string_equal(filename, "testdata/master/master7.data");
assert_string_equal(filename, "testdata/master/master6.data");
isc_mem_free(mctx, filename);
}
}
......
......@@ -396,6 +396,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) {
......@@ -654,6 +691,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.
*
......@@ -782,6 +824,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.
*
......@@ -1327,6 +1374,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
*
......@@ -1507,6 +1589,43 @@ nxt(void **state) {
dns_rdatatype_nxt, sizeof(dns_rdata_nxt_t));
}
static void
rkey(void **state) {
text_ok_t text_ok[] = {
/*
* Valid, flags set to 0 and a key is present.
*/
TEXT_VALID("0 0 0 aaaa"),
/*
* Invalid, non-zero flags.
*/
TEXT_INVALID("1 0 0 aaaa"),
TEXT_INVALID("65535 0 0 aaaa"),
/*
* Sentinel.
*/
TEXT_SENTINEL()
};
wire_ok_t wire_ok[] = {
/*
* Valid, flags set to 0 and a key is present.
*/
WIRE_VALID(0x00, 0x00, 0x00, 0x00, 0x00),
/*
* Invalid, non-zero flags.
*/
WIRE_INVALID(0x00, 0x01, 0x00, 0x00, 0x00),
WIRE_INVALID(0xff, 0xff, 0x00, 0x00, 0x00),
/*
* Sentinel.
*/
WIRE_SENTINEL()
};
key_required(state, dns_rdatatype_rkey, sizeof(dns_rdata_rkey_t));
check_rdata(text_ok, wire_ok, NULL, false, dns_rdataclass_in,
dns_rdatatype_rkey, sizeof(dns_rdata_rkey_t));
}
/*
* WKS tests.
*
......@@ -1828,18 +1947,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),
......
;
; master7.data contains a good zone file
; master6.data contains a good zone file
;
$include testdata/master/master7.data
$include testdata/master/master6.data
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment