From f92d51b3feed35f746143977d785ca099531323e Mon Sep 17 00:00:00 2001 From: Wouter Wijngaards Date: Tue, 23 Oct 2007 08:30:21 +0000 Subject: [PATCH] dnssec lameness detection improved to disable detection when not sure. git-svn-id: file:///svn/unbound/trunk@715 be551aaa-1e26-0410-a405-d3ace91eadb9 --- doc/Changelog | 6 +++++ doc/requirements.txt | 19 +++++++++++++ iterator/iter_utils.c | 48 ++++++++++++++++++++++++++++++--- iterator/iter_utils.h | 19 ++++++++++++- iterator/iterator.c | 18 ++++++++----- testdata/iter_dnsseclame_ds.rpl | 5 +++- testdata/iter_dnsseclame_ta.rpl | 6 +++++ 7 files changed, 110 insertions(+), 11 deletions(-) diff --git a/doc/Changelog b/doc/Changelog index 1e1d53ee8..eab2cc201 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,9 @@ +23 October 2007: Wouter + - fixup (grand-)parent problem for dnssec-lameness detection. + - fixup tests to do additional section processing for lame replies, + since the detection needs that. + - no longer trust in query section in reply during dnssec lame detect. + 22 October 2007: Wouter - added donotquerylocalhost config option. Can be turned off for out test cases. diff --git a/doc/requirements.txt b/doc/requirements.txt index 6fb99dff1..5d50647cc 100644 --- a/doc/requirements.txt +++ b/doc/requirements.txt @@ -140,3 +140,22 @@ o The method by which dnssec-lameness is detected is not secure. DNSSEC lame Also for zones for which no chain of trust exists, but a DS is given by the parent, dnssec-lameness detection enables. This delivers dnnsec to our clients when possible (for client validators). + + The following issue needs to be resolved: + a server that serves both a parent and child zone, where + parent is signed, but child is not. The server must not be marked + lame for the parent zone, because the child answer is not signed. + Instead of a false positive, we want false negatives; failure to + detect dnssec-lameness is less of a problem than marking honest + servers lame. dnssec-lameness is a config error and deserves the trouble. + So, only messages that identify the zone are used to mark the zone + lame. The zone is identified by SOA or NS RRsets in the answer/auth. + That includes almost all negative responses and also A, AAAA qtypes. + That would be most responses from servers. + For referrals, delegations that add a single label can be checked to be + from their zone, this covers most delegation-centric zones. + + So possibly, for complicated setups, with multiple (parent-child) zones + on a server, dnssec-lameness detection does not work - no dnssec-lameness + is detected. Instead the zone that is dnssec-lame becomes bogus. + diff --git a/iterator/iter_utils.c b/iterator/iter_utils.c index 3787a9c0e..589da242f 100644 --- a/iterator/iter_utils.c +++ b/iterator/iter_utils.c @@ -371,18 +371,18 @@ iter_dp_is_useless(struct module_qstate* qstate, struct delegpt* dp) int iter_indicates_dnssec(struct module_env* env, struct delegpt* dp, - struct dns_msg* msg) + struct dns_msg* msg, uint16_t dclass) { /* information not available, !env->anchors can be common */ if(!env || !env->anchors || !dp || !dp->name || !msg || !msg->rep) return 0; /* a trust anchor exists with this name, RRSIGs expected */ if(anchor_find(env->anchors, dp->name, dp->namelabs, dp->namelen, - msg->qinfo.qclass)) + dclass)) return 1; /* see if DS rrset was given, in AUTH section */ if(reply_find_rrset_section_ns(msg->rep, dp->name, dp->namelen, - LDNS_RR_TYPE_DS, msg->qinfo.qclass)) + LDNS_RR_TYPE_DS, dclass)) return 1; return 0; } @@ -402,3 +402,45 @@ iter_msg_has_dnssec(struct dns_msg* msg) * not empty (NSEC) */ return 0; } + +int iter_msg_from_zone(struct dns_msg* msg, struct delegpt* dp, + enum response_type type, uint16_t dclass) +{ + if(!msg || !dp || !msg->rep || !dp->name) + return 0; + /* SOA RRset - always from reply zone */ + if(reply_find_rrset_section_an(msg->rep, dp->name, dp->namelen, + LDNS_RR_TYPE_SOA, dclass) || + reply_find_rrset_section_ns(msg->rep, dp->name, dp->namelen, + LDNS_RR_TYPE_SOA, dclass)) + return 1; + if(type == RESPONSE_TYPE_REFERRAL) { + size_t i; + /* if it adds a single label, i.e. we expect .com, + * and referral to example.com. NS ... , then origin zone + * is .com. For a referral to sub.example.com. NS ... then + * we do not know, since example.com. may be in between. */ + for(i=0; irep->an_numrrsets+msg->rep->ns_numrrsets; + i++) { + struct ub_packed_rrset_key* s = msg->rep->rrsets[i]; + if(ntohs(s->rk.type) == LDNS_RR_TYPE_NS && + ntohs(s->rk.rrset_class) == dclass) { + int l = dname_count_labels(s->rk.dname); + if(l == dp->namelabs + 1 && + dname_strict_subdomain(s->rk.dname, + l, dp->name, dp->namelabs)) + return 1; + } + } + return 0; + } + log_assert(type==RESPONSE_TYPE_ANSWER || type==RESPONSE_TYPE_CNAME); + /* not a referral, and not lame delegation (upwards), so, + * any NS rrset must be from the zone itself */ + if(reply_find_rrset_section_an(msg->rep, dp->name, dp->namelen, + LDNS_RR_TYPE_NS, dclass) || + reply_find_rrset_section_ns(msg->rep, dp->name, dp->namelen, + LDNS_RR_TYPE_NS, dclass)) + return 1; + return 0; +} diff --git a/iterator/iter_utils.h b/iterator/iter_utils.h index 1ef4c42dc..4999df98b 100644 --- a/iterator/iter_utils.h +++ b/iterator/iter_utils.h @@ -42,6 +42,7 @@ #ifndef ITERATOR_ITER_UTILS_H #define ITERATOR_ITER_UTILS_H +#include "iterator/iter_resptype.h" struct iter_env; struct config_file; struct module_env; @@ -143,10 +144,11 @@ int iter_dp_is_useless(struct module_qstate* qstate, struct delegpt* dp); * @param env: module env with trust anchors. * @param dp: delegation point. * @param msg: delegation message, with DS if a secure referral. + * @param dclass: class of query. * @return 1 if dnssec is expected, 0 if not. */ int iter_indicates_dnssec(struct module_env* env, struct delegpt* dp, - struct dns_msg* msg); + struct dns_msg* msg, uint16_t dclass); /** * See if a message contains DNSSEC. @@ -158,4 +160,19 @@ int iter_indicates_dnssec(struct module_env* env, struct delegpt* dp, */ int iter_msg_has_dnssec(struct dns_msg* msg); +/** + * See if a message is known to be from a certain zone. + * This looks for SOA or NS rrsets, for answers. + * For referrals, when one label is delegated, the zone is detected. + * Does not look at signatures. + * @param msg: the message to inspect. + * @param dp: delegation point with zone name to look for. + * @param type: type of message. + * @param dclass: class of query. + * @return true if message is certain to be from zone in dp->name. + * false if not sure (empty msg), or not from the zone. + */ +int iter_msg_from_zone(struct dns_msg* msg, struct delegpt* dp, + enum response_type type, uint16_t dclass); + #endif /* ITERATOR_ITER_UTILS_H */ diff --git a/iterator/iterator.c b/iterator/iterator.c index cd8e3a3a9..07ca5a69a 100644 --- a/iterator/iterator.c +++ b/iterator/iterator.c @@ -804,7 +804,7 @@ processInitRequest(struct module_qstate* qstate, struct iter_qstate* iq, /* if the cache reply dp equals a validation anchor or msg has DS, * then DNSSEC RRSIGs are expected in the reply */ iq->dnssec_expected = iter_indicates_dnssec(qstate->env, iq->dp, - iq->deleg_msg); + iq->deleg_msg, iq->qchase.qclass); /* Reset the RD flag. If this is a query restart, then the RD * will have been turned off. */ @@ -1192,12 +1192,18 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq, * differently. No queries should be sent elsewhere */ type = RESPONSE_TYPE_ANSWER; } - if(!(iq->chase_flags&BIT_RD) && type != RESPONSE_TYPE_LAME && - type != RESPONSE_TYPE_THROWAWAY && - type != RESPONSE_TYPE_UNTYPED && iq->dnssec_expected) { + if(iq->dnssec_expected && !(iq->chase_flags&BIT_RD) + && type != RESPONSE_TYPE_LAME + && type != RESPONSE_TYPE_THROWAWAY + && type != RESPONSE_TYPE_UNTYPED) { /* a possible answer, see if it is missing DNSSEC */ /* but not when forwarding, so we dont mark fwder lame */ - if(!iter_msg_has_dnssec(iq->response)) + /* also make sure the answer is from the zone we expected, + * otherwise, (due to parent,child on same server), we + * might mark the server,zone lame inappropriately */ + if(!iter_msg_has_dnssec(iq->response) && + iter_msg_from_zone(iq->response, iq->dp, type, + iq->qchase.qclass)) type = RESPONSE_TYPE_LAME; } @@ -1242,7 +1248,7 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq, /* see if the next dp is a trust anchor, or a DS was sent * along, indicating dnssec is expected for next zone */ iq->dnssec_expected = iter_indicates_dnssec(qstate->env, - iq->dp, iq->response); + iq->dp, iq->response, iq->qchase.qclass); /* stop current outstanding queries. * FIXME: should the outstanding queries be waited for and diff --git a/testdata/iter_dnsseclame_ds.rpl b/testdata/iter_dnsseclame_ds.rpl index cf4ad3d05..6c81aa756 100644 --- a/testdata/iter_dnsseclame_ds.rpl +++ b/testdata/iter_dnsseclame_ds.rpl @@ -270,12 +270,15 @@ www.sub.example.com. IN A SECTION ANSWER www.sub.example.com. IN A 11.11.11.11 SECTION AUTHORITY +; dnssec-lameness detection depends on this information +sub.example.com. IN NS ns.sub.example.com. +sub.example.com. IN NS ns.example.net. SECTION ADDITIONAL +ns.sub.example.com. IN A 1.2.3.6 ENTRY_END RANGE_END - STEP 1 QUERY ENTRY_BEGIN REPLY RD DO diff --git a/testdata/iter_dnsseclame_ta.rpl b/testdata/iter_dnsseclame_ta.rpl index 3e578f462..9bc1675c4 100644 --- a/testdata/iter_dnsseclame_ta.rpl +++ b/testdata/iter_dnsseclame_ta.rpl @@ -204,6 +204,12 @@ www.example.com. IN A SECTION ANSWER ; the wrong answer. www.example.com. IN A 10.20.30.40 +SECTION AUTHORITY +; dnssec-lameness detection depends on this information +example.com. IN NS ns.example.com. +example.com. IN NS ns.example.net. +SECTION ADDITIONAL +ns.example.com. IN A 1.2.3.55 ENTRY_END RANGE_END -- 2.47.2