From e08aa7c5e1a230d029c501b78acca2f2e249bccb Mon Sep 17 00:00:00 2001 From: Wouter Wijngaards Date: Mon, 9 Feb 2015 11:44:46 +0000 Subject: [PATCH] - Fix validation failure in case upstream forwarder (ISC BIND) does not have the same trust anchors and decides to insert unsigned NS record in authority section. git-svn-id: file:///svn/unbound/trunk@3329 be551aaa-1e26-0410-a405-d3ace91eadb9 --- doc/Changelog | 5 ++ testdata/val_spurious_ns.rpl | 151 +++++++++++++++++++++++++++++++++++ validator/val_utils.c | 12 +++ validator/val_utils.h | 7 ++ validator/validator.c | 57 +++++++++++++ 5 files changed, 232 insertions(+) create mode 100644 testdata/val_spurious_ns.rpl diff --git a/doc/Changelog b/doc/Changelog index 72a83a5b2..93e4e5fb8 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +9 February 2015: Wouter + - Fix validation failure in case upstream forwarder (ISC BIND) does + not have the same trust anchors and decides to insert unsigned NS + record in authority section. + 2 February 2015: Wouter - infra-cache-min-rtt patch from Florian Riehm, for expected long uplink roundtrip times. diff --git a/testdata/val_spurious_ns.rpl b/testdata/val_spurious_ns.rpl new file mode 100644 index 000000000..741fd1aff --- /dev/null +++ b/testdata/val_spurious_ns.rpl @@ -0,0 +1,151 @@ +; config options +; The island of trust is at example.com +server: + trust-anchor: "example.com. 3600 IN DS 2854 3 1 46e4ffc6e9a4793b488954bd3f0cc6af0dfb201b" + val-override-date: "20070916134226" + target-fetch-policy: "0 0 0 0 0" + +stub-zone: + name: "." + stub-addr: 193.0.14.129 # K.ROOT-SERVERS.NET. +CONFIG_END + +SCENARIO_BEGIN Test validator with spurious unsigned NS in auth section + +; K.ROOT-SERVERS.NET. +RANGE_BEGIN 0 100 + ADDRESS 193.0.14.129 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +. IN NS +SECTION ANSWER +. IN NS K.ROOT-SERVERS.NET. +SECTION ADDITIONAL +K.ROOT-SERVERS.NET. IN A 193.0.14.129 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION AUTHORITY +com. IN NS a.gtld-servers.net. +SECTION ADDITIONAL +a.gtld-servers.net. IN A 192.5.6.30 +ENTRY_END +RANGE_END + +; a.gtld-servers.net. +RANGE_BEGIN 0 100 + ADDRESS 192.5.6.30 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +com. IN NS +SECTION ANSWER +com. IN NS a.gtld-servers.net. +SECTION ADDITIONAL +a.gtld-servers.net. IN A 192.5.6.30 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION AUTHORITY +example.com. IN NS ns.example.com. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ENTRY_END +RANGE_END + +; ns.example.com. +RANGE_BEGIN 0 100 + ADDRESS 1.2.3.4 +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +example.com. IN NS +SECTION ANSWER +example.com. IN NS ns.example.com. +example.com. 3600 IN RRSIG NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854} +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 3 3 3600 20070926135752 20070829135752 2854 example.com. MC0CFQCMSWxVehgOQLoYclB9PIAbNP229AIUeH0vNNGJhjnZiqgIOKvs1EhzqAo= ;{id = 2854} +ENTRY_END + +; response to DNSKEY priming query +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +example.com. IN DNSKEY +SECTION ANSWER +example.com. 3600 IN DNSKEY 256 3 3 ALXLUsWqUrY3JYER3T4TBJII s70j+sDS/UT2QRp61SE7S3E EXopNXoFE73JLRmvpi/UrOO/Vz4Se 6wXv/CYCKjGw06U4WRgR YXcpEhJROyNapmdIKSx hOzfLVE1gqA0PweZR8d tY3aNQSRn3sPpwJr6Mi /PqQKAMMrZ9ckJpf1+b QMOOvxgzz2U1GS18b3y ZKcgTMEaJzd/GZYzi/B N2DzQ0MsrSwYXfsNLFO Bbs8PJMW4LYIxeeOe6rUgkWOF 7CC9Dh/dduQ1QrsJhmZAEFfd6ByYV+ ;{id = 2854 (zsk), size = 1688b} +example.com. 3600 IN RRSIG DNSKEY 3 2 3600 20070926134802 20070829134802 2854 example.com. MCwCFG1yhRNtTEa3Eno2zhVVuy2EJX3wAhQeLyUp6+UXcpC5qGNu9tkrTEgPUg== ;{id = 2854} +SECTION AUTHORITY +example.com. IN NS ns.example.com. +example.com. 3600 IN RRSIG NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854} +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 3 3 3600 20070926135752 20070829135752 2854 example.com. MC0CFQCMSWxVehgOQLoYclB9PIAbNP229AIUeH0vNNGJhjnZiqgIOKvs1EhzqAo= ;{id = 2854} +ENTRY_END + +; response to query of interest +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +ns.example.com. 3600 IN RRSIG A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCQMyTjn7WWwpwAR1LlVeLpRgZGuQIUCcJDEkwAuzytTDRlYK7nIMwH1CM= ;{id = 2854} +SECTION AUTHORITY +example.com. IN NS ns.example.com. +;example.com. 3600 IN RRSIG NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854} +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +www.example.com. 3600 IN RRSIG A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFC99iE9K5y2WNgI0gFvBWaTi9wm6AhUAoUqOpDtG5Zct+Qr9F3mSdnbc6V4= ;{id = 2854} +ENTRY_END +RANGE_END + +STEP 1 QUERY +ENTRY_BEGIN +REPLY RD DO +SECTION QUESTION +www.example.com. IN A +ENTRY_END + +; recursion happens here. +STEP 10 CHECK_ANSWER +ENTRY_BEGIN +MATCH all +REPLY QR RD RA AD DO NOERROR +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +www.example.com. 3600 IN RRSIG A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFC99iE9K5y2WNgI0gFvBWaTi9wm6AhUAoUqOpDtG5Zct+Qr9F3mSdnbc6V4= ;{id = 2854} +SECTION AUTHORITY +; removed by spurious NS record removal code +;;example.com. IN NS ns.example.com. +;;example.com. 3600 IN RRSIG NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854} +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.4 +ns.example.com. 3600 IN RRSIG A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCQMyTjn7WWwpwAR1LlVeLpRgZGuQIUCcJDEkwAuzytTDRlYK7nIMwH1CM= ;{id = 2854} +ENTRY_END + +SCENARIO_END diff --git a/validator/val_utils.c b/validator/val_utils.c index ecf2dfaf0..475b0c905 100644 --- a/validator/val_utils.c +++ b/validator/val_utils.c @@ -846,6 +846,18 @@ val_fill_reply(struct reply_info* chase, struct reply_info* orig, chase->ar_numrrsets; } +void val_reply_remove_auth(struct reply_info* rep, size_t index) +{ + log_assert(index < rep->rrset_count); + log_assert(index >= rep->an_numrrsets); + log_assert(index < rep->an_numrrsets+rep->ns_numrrsets); + memmove(rep->rrsets+index, rep->rrsets+index+1, + sizeof(struct ub_packed_rrset_key*)* + (rep->rrset_count - index - 1)); + rep->ns_numrrsets--; + rep->rrset_count--; +} + void val_check_nonsecure(struct val_env* ve, struct reply_info* rep) { diff --git a/validator/val_utils.h b/validator/val_utils.h index b0344eff7..cdb87697e 100644 --- a/validator/val_utils.h +++ b/validator/val_utils.h @@ -294,6 +294,13 @@ int val_chase_cname(struct query_info* qchase, struct reply_info* rep, void val_fill_reply(struct reply_info* chase, struct reply_info* orig, size_t cname_skip, uint8_t* name, size_t len, uint8_t* signer); +/** + * Remove rrset with index from reply, from the authority section. + * @param rep: reply to remove it from. + * @param index: rrset to remove, must be in the authority section. + */ +void val_reply_remove_auth(struct reply_info* rep, size_t index); + /** * Remove all unsigned or non-secure status rrsets from NS and AR sections. * So that unsigned data does not get let through to clients, when we have diff --git a/validator/validator.c b/validator/validator.c index 9d5d5c390..cc07cc2b1 100644 --- a/validator/validator.c +++ b/validator/validator.c @@ -574,6 +574,61 @@ detect_wrongly_truncated(struct reply_info* rep) return 1; } +/** + * For messages that are not referrals, if the chase reply contains an + * unsigned NS record in the authority section it could have been + * inserted by a (BIND) forwarder that thinks the zone is insecure, and + * that has an NS record without signatures in cache. Remove the NS + * record since the reply does not hinge on that record (in the authority + * section), but do not remove it if it removes the last record from the + * answer+authority sections. + * @param chase_reply: the chased reply, we have a key for this contents, + * so we should have signatures for these rrsets and not having + * signatures means it will be bogus. + * @param orig_reply: original reply, remove NS from there as well because + * we cannot mark the NS record as DNSSEC valid because it is not + * validated by signatures. + */ +static void +remove_spurious_authority(struct reply_info* chase_reply, + struct reply_info* orig_reply) +{ + size_t i, found = 0; + int remove = 0; + /* if no answer and only 1 auth RRset, do not remove that one */ + if(chase_reply->an_numrrsets == 0 && chase_reply->ns_numrrsets == 1) + return; + /* search authority section for unsigned NS records */ + for(i = chase_reply->an_numrrsets; + i < chase_reply->an_numrrsets+chase_reply->ns_numrrsets; i++) { + struct packed_rrset_data* d = (struct packed_rrset_data*) + chase_reply->rrsets[i]->entry.data; + if(ntohs(chase_reply->rrsets[i]->rk.type) == LDNS_RR_TYPE_NS + && d->rrsig_count == 0) { + found = i; + remove = 1; + break; + } + } + /* see if we found the entry */ + if(!remove) return; + log_rrset_key(VERB_ALGO, "Removing spurious unsigned NS record " + "(likely inserted by forwarder)", chase_reply->rrsets[found]); + + /* find rrset in orig_reply */ + for(i = orig_reply->an_numrrsets; + i < orig_reply->an_numrrsets+orig_reply->ns_numrrsets; i++) { + if(ntohs(orig_reply->rrsets[i]->rk.type) == LDNS_RR_TYPE_NS + && query_dname_compare(orig_reply->rrsets[i]->rk.dname, + chase_reply->rrsets[found]->rk.dname) == 0) { + /* remove from orig_msg */ + val_reply_remove_auth(orig_reply, i); + break; + } + } + /* remove rrset from chase_reply */ + val_reply_remove_auth(chase_reply, found); +} /** * Given a "positive" response -- a response that contains an answer to the @@ -1642,6 +1697,8 @@ processValidate(struct module_qstate* qstate, struct val_qstate* vq, } subtype = val_classify_response(qstate->query_flags, &qstate->qinfo, &vq->qchase, vq->orig_msg->rep, vq->rrset_skip); + if(subtype != VAL_CLASS_REFERRAL) + remove_spurious_authority(vq->chase_reply, vq->orig_msg->rep); /* check signatures in the message; * answer and authority must be valid, additional is only checked. */ -- 2.47.2