From 2c2359af6123f898d2b5dd04d8d1c08a0eb94828 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 16 Dec 2015 10:47:15 +0100 Subject: [PATCH 1/3] Remove duplicate records in RRset before verifying As suggested in RFC4034 section 6.3 --- src/dnssec.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 3fdff91b..b31643b6 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -1629,6 +1629,10 @@ static int _getdns_verify_rrsig(struct mem_funcs *mf, if (!_dnssec_rdata_to_canonicalize(rrset->rr_type)) for (i = 0; i < n_rrs; i++) { + if (i && !_rr_iter_rdata_cmp( + &val_rrset[i], &val_rrset[i-1])) + continue; + gldns_buffer_write(&valbuf, owner, owner_len); gldns_buffer_write_u16(&valbuf, rrset->rr_type); gldns_buffer_write_u16(&valbuf, rrset->rr_class); @@ -1637,6 +1641,8 @@ static int _getdns_verify_rrsig(struct mem_funcs *mf, val_rrset[i].nxt - val_rrset[i].rr_type - 8); } else for (i = 0; i < n_rrs; i++) { + if (i && !_rr_iter_rdata_cmp(&val_rrset[i], &val_rrset[i-1])) + continue; gldns_buffer_write(&valbuf, owner, owner_len); gldns_buffer_write_u16(&valbuf, rrset->rr_type); gldns_buffer_write_u16(&valbuf, rrset->rr_class); @@ -1664,16 +1670,21 @@ static int _getdns_verify_rrsig(struct mem_funcs *mf, } DEBUG_SEC( "written to valbuf: %zu bytes\n" , gldns_buffer_position(&valbuf)); - assert(gldns_buffer_position(&valbuf) == valbuf_sz); + assert(gldns_buffer_position(&valbuf) <= valbuf_sz); + gldns_buffer_flip(&valbuf); r = _getdns_verify_canonrrset(&valbuf, key->rr_i.rr_type[13], signer->nxt, rrsig->rr_i.nxt - signer->nxt, key->rr_i.rr_type+14, key->rr_i.nxt - key->rr_i.rr_type-14, &reason); #if defined(SEC_DEBUG) && SEC_DEBUG - if (r == 0) + if (r == 0) { DEBUG_SEC("verification failed: %s\n", reason); + debug_sec_print_rrset("verification failed: ", rrset); + debug_sec_print_rr("verification failed: ", &rrsig->rr_i); + debug_sec_print_rr("verification failed: ", &key->rr_i); + } #endif if (val_rrset != val_rrset_spc) GETDNS_FREE(*mf, val_rrset); From d09e892285c2f445d31b57785051e5b6dd3a905e Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 16 Dec 2015 12:02:53 +0100 Subject: [PATCH 2/3] Convert rr_dict with missing rdata to wire format In wireformat this then means no rdata. This is needed with the zonecut indicating DSes returned in the validation chain. --- src/dnssec.c | 2 +- src/rr-dict.c | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index b31643b6..59537391 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -2777,7 +2777,7 @@ static int chain_head_validate_with_ta(struct mem_funcs *mf, if ((s = chain_node_get_trusted_keys( mf, now, skew, head->parent, ta, &keys)) != GETDNS_DNSSEC_SECURE) - return s; + return s; if (rrset_has_rrs(&head->rrset)) { if ((keytag = a_key_signed_rrset( diff --git a/src/rr-dict.c b/src/rr-dict.c index 4fe30794..fe4af1ca 100644 --- a/src/rr-dict.c +++ b/src/rr-dict.c @@ -762,11 +762,11 @@ _getdns_rr_dict2wire(getdns_dict *rr_dict, gldns_buffer *buf) assert(buf); if ((r = getdns_dict_get_bindata(rr_dict, "name", &name))) - goto error; + return r; gldns_buffer_write(buf, name->data, name->size); if ((r = getdns_dict_get_int(rr_dict, "type", &rr_type))) - goto error; + return r; gldns_buffer_write_u16(buf, (uint16_t)rr_type); (void) getdns_dict_get_int(rr_dict, "class", &rr_class); @@ -787,10 +787,13 @@ _getdns_rr_dict2wire(getdns_dict *rr_dict, gldns_buffer *buf) break; } - if ((r = getdns_dict_get_dict(rr_dict, "rdata", &rdata))) - goto error; + if ((r = getdns_dict_get_dict(rr_dict, "rdata", &rdata))) { + if (r == GETDNS_RETURN_NO_SUCH_DICT_NAME) { + gldns_buffer_write_u16(buf, 0); + r = GETDNS_RETURN_GOOD; + } - if (n_rdata_fields == 0 && GETDNS_RETURN_GOOD == + } else if (n_rdata_fields == 0 && GETDNS_RETURN_GOOD == (r = getdns_dict_get_bindata(rdata, "rdata_raw", &rdata_raw))) { gldns_buffer_write_u16(buf, (uint16_t)rdata_raw->size); @@ -829,7 +832,6 @@ _getdns_rr_dict2wire(getdns_dict *rr_dict, gldns_buffer *buf) gldns_buffer_write_u16_at(buf, rdata_size_mark, (uint16_t)(gldns_buffer_position(buf)-rdata_size_mark-2)); } -error: return r; } From 1ef4db8e9d5ebe4747c552a51d4921a17f054bc4 Mon Sep 17 00:00:00 2001 From: Willem Toorop Date: Wed, 16 Dec 2015 12:40:32 +0100 Subject: [PATCH 3/3] Unique NSEC and NSEC3 rrsets in "validation_chain" --- src/dnssec.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/src/dnssec.c b/src/dnssec.c index 59537391..c717d8bf 100644 --- a/src/dnssec.c +++ b/src/dnssec.c @@ -2991,6 +2991,26 @@ static size_t count_outstanding_requests(chain_head *head) return count + count_outstanding_requests(head->next); } +static int rrset_in_list(getdns_rrset *rrset, getdns_list *list) +{ + size_t i; + getdns_dict *rr_dict; + uint32_t rr_type; + uint32_t rr_class; + getdns_bindata *name; + + for (i = 0; !getdns_list_get_dict(list, i, &rr_dict); i++) { + if (!getdns_dict_get_int(rr_dict, "type", &rr_type) && + rrset->rr_type == rr_type && + !getdns_dict_get_int(rr_dict, "class", &rr_class) && + rrset->rr_class == rr_class && + !getdns_dict_get_bindata(rr_dict, "name", &name) && + dname_compare(rrset->name, name->data) == 0) + return 1; + } + return 0; +} + static void append_rrs2val_chain_list(getdns_context *ctxt, getdns_list *val_chain_list, getdns_network_req *netreq, int signer) { @@ -3006,10 +3026,14 @@ static void append_rrs2val_chain_list(getdns_context *ctxt, rrset = rrset_iter_value(i); - if (rrset->rr_type != GETDNS_RRTYPE_DNSKEY && - rrset->rr_type != GETDNS_RRTYPE_DS && - rrset->rr_type != GETDNS_RRTYPE_NSEC && - rrset->rr_type != GETDNS_RRTYPE_NSEC3) + if (rrset->rr_type == GETDNS_RRTYPE_NSEC || + rrset->rr_type == GETDNS_RRTYPE_NSEC3) { + + if (rrset_in_list(rrset, val_chain_list)) + continue; + + } else if (rrset->rr_type != GETDNS_RRTYPE_DNSKEY && + rrset->rr_type != GETDNS_RRTYPE_DS) continue; for ( rr = rrtype_iter_init(&rr_spc, rrset)