diff --git a/lib/layer/validate.c b/lib/layer/validate.c index 31c8da944b9655f74f3555b4ba26797c451648eb..8791e849ba3f35fad3e5b828fb76d600738454a7 100644 --- a/lib/layer/validate.c +++ b/lib/layer/validate.c @@ -191,8 +191,8 @@ static int validate_keyset(struct kr_query *qry, knot_pkt_t *answer, bool has_ns (knot_dname_cmp(rr->owner, qry->zone_cut.name) != 0)) { continue; } - /* Merge with zone cut. */ - if (!qry->zone_cut.key) { + /* Merge with zone cut (or replace ancestor key). */ + if (!qry->zone_cut.key || !knot_dname_is_equal(qry->zone_cut.key->owner, rr->owner)) { qry->zone_cut.key = knot_rrset_copy(rr, qry->zone_cut.pool); if (!qry->zone_cut.key) { return kr_error(ENOMEM); @@ -215,6 +215,8 @@ static int validate_keyset(struct kr_query *qry, knot_pkt_t *answer, bool has_ns #warning TODO: Ensure canonical format of the whole DNSKEY RRSet. (Also remove duplicities?) /* Check if there's a key for current TA. */ + /* @todo this is not going to work with cached DNSKEY, as the TA is not yet ready, + * must not check if the data comes from cache */ int ret = kr_dnskeys_trusted(answer, KNOT_ANSWER, qry->zone_cut.key, qry->zone_cut.trust_anchor, qry->zone_cut.name, qry->timestamp.tv_sec, has_nsec3); @@ -302,11 +304,10 @@ static int update_parent(struct kr_query *qry, uint16_t answer_type) break; case KNOT_RRTYPE_DS: DEBUG_MSG(qry, "<= parent: updating DS\n"); - parent->zone_cut.trust_anchor = knot_rrset_copy(qry->zone_cut.key, parent->zone_cut.pool); - if (!parent->zone_cut.key) { + parent->zone_cut.trust_anchor = knot_rrset_copy(qry->zone_cut.trust_anchor, parent->zone_cut.pool); + if (!parent->zone_cut.trust_anchor) { return KNOT_STATE_FAIL; } - parent->flags &= ~QUERY_AWAIT_DS; break; default: break; } @@ -354,8 +355,6 @@ static int update_delegation(struct kr_request *req, struct kr_query *qry, knot_ /* Extend trust anchor */ DEBUG_MSG(qry, "<= DS: OK\n"); - knot_rrset_free(&cut->trust_anchor, cut->pool); - knot_rrset_free(&cut->key, cut->pool); cut->trust_anchor = new_ds; return ret; } @@ -370,33 +369,43 @@ static int validate(knot_layer_t *ctx, knot_pkt_t *pkt) return ctx->state; } - /* Pass-through if user doesn't want secure answer, or cached. - * Since we let the data into cache, we're going to trust it. - */ - if (!(qry->flags & QUERY_DNSSEC_WANT) || (qry->flags & QUERY_CACHED)) { + /* Pass-through if user doesn't want secure answer. */ + if (!(qry->flags & QUERY_DNSSEC_WANT)) { return ctx->state; } - if (!knot_pkt_has_dnssec(pkt)) { + if (!(qry->flags & QUERY_CACHED) && !knot_pkt_has_dnssec(pkt)) { DEBUG_MSG(qry, "<= got insecure response\n"); qry->flags |= QUERY_DNSSEC_BOGUS; return KNOT_STATE_FAIL; } + /* Check if this is a DNSKEY answer, check trust chain and store. */ + uint16_t qtype = knot_pkt_qtype(pkt); + bool has_nsec3 = _knot_pkt_has_type(pkt, KNOT_RRTYPE_NSEC3); + if (qtype == KNOT_RRTYPE_DNSKEY) { + ret = validate_keyset(qry, pkt, has_nsec3); + if (ret != 0) { + DEBUG_MSG(qry, "<= bad keys, broken trust chain\n"); + qry->flags |= QUERY_DNSSEC_BOGUS; + return KNOT_STATE_FAIL; + } + } + /* Check whether the current zone cut holds keys that can be used * for validation (i.e. RRSIG signer name matches key owner). */ const knot_dname_t *key_own = qry->zone_cut.key ? qry->zone_cut.key->owner : NULL; const knot_dname_t *sig_name = first_rrsig_signer_name(pkt); if (key_own && sig_name && !knot_dname_is_equal(key_own, sig_name)) { - if (qry->flags & QUERY_AWAIT_DS) { - qry->flags |= QUERY_DNSSEC_BOGUS; - return KNOT_STATE_FAIL; /* This indicates a DS is not available. */ - } - qry->flags |= QUERY_AWAIT_DS; + /* @todo this sometimes causes duplicated data in answer, as the answer is + * fetched again after we have a valid DS/DNSKEY, fix this */ + /* @todo for non-existence proofs, there may be only SOA and we need to fetch the + * keys matching it instead of current cut */ + DEBUG_MSG(qry, ">< cut changed, needs revalidation\n"); + qry->flags &= ~QUERY_RESOLVED; return KNOT_STATE_CONSUME; } - bool has_nsec3 = _knot_pkt_has_type(pkt, KNOT_RRTYPE_NSEC3); uint8_t pkt_rcode = knot_wire_get_rcode(pkt->wire); /* Validate non-existence proof if not positive answer. */ @@ -450,25 +459,17 @@ static int validate(knot_layer_t *ctx, knot_pkt_t *pkt) } } - /* Check if this is a DNSKEY answer, check trust chain and store. */ - uint16_t qtype = knot_pkt_qtype(pkt); - if (qtype == KNOT_RRTYPE_DNSKEY) { - ret = validate_keyset(qry, pkt, has_nsec3); + /* Validate all records, fail as bogus if it doesn't match. + * Do not revalidate data from cache, as it's already trusted. */ + if (!(qry->flags & QUERY_CACHED)) { + ret = validate_records(qry, pkt, req->rplan.pool, has_nsec3); if (ret != 0) { - DEBUG_MSG(qry, "<= bad keys, broken trust chain\n"); + DEBUG_MSG(qry, "<= couldn't validate RRSIGs\n"); qry->flags |= QUERY_DNSSEC_BOGUS; return KNOT_STATE_FAIL; } } - /* Validate all records, fail as bogus if it doesn't match. */ - ret = validate_records(qry, pkt, req->rplan.pool, has_nsec3); - if (ret != 0) { - DEBUG_MSG(qry, "<= couldn't validate RRSIGs\n"); - qry->flags |= QUERY_DNSSEC_BOGUS; - return KNOT_STATE_FAIL; - } - /* Check and update current delegation point security status. */ ret = update_delegation(req, qry, pkt, has_nsec3); if (ret != 0) { diff --git a/lib/resolve.c b/lib/resolve.c index a9eea382ab3f38a26a670b0a63af2916430e2d39..2e80d9591eea558874e598d9f7d22cafadc46296 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -333,23 +333,23 @@ int kr_resolve_consume(struct kr_request *request, knot_pkt_t *packet) } /** @internal Spawn subrequest in current zone cut (no minimization or lookup). */ -static int zone_cut_subreq(struct kr_rplan *rplan, struct kr_query *parent, +static struct kr_query *zone_cut_subreq(struct kr_rplan *rplan, struct kr_query *parent, const knot_dname_t *qname, uint16_t qtype) { struct kr_query *next = kr_rplan_push(rplan, parent, qname, parent->sclass, qtype); if (!next) { - return kr_error(ENOMEM); + return NULL; } kr_zonecut_set(&next->zone_cut, parent->zone_cut.name); if (kr_zonecut_copy(&next->zone_cut, &parent->zone_cut) != 0 || kr_zonecut_copy_trust(&next->zone_cut, &parent->zone_cut) != 0) { - return kr_error(ENOMEM); + return NULL; } next->flags |= QUERY_NO_MINIMIZE; if (parent->flags & QUERY_DNSSEC_WANT) { next->flags |= QUERY_DNSSEC_WANT; } - return kr_ok(); + return next; } /** @internal Check current zone cut status and credibility, spawn subrequests if needed. */ @@ -399,18 +399,22 @@ static int zone_cut_check(struct kr_request *request, struct kr_query *qry, knot knot_rrset_t *ta_rr = kr_ta_get(trust_anchors, qry->zone_cut.name); qry->zone_cut.trust_anchor = knot_rrset_copy(ta_rr, qry->zone_cut.pool); } - /* Try to fetch missing DS. */ - if (want_secured && (qry->flags & QUERY_AWAIT_DS)) { - int ret = zone_cut_subreq(rplan, qry, qry->zone_cut.name, KNOT_RRTYPE_DS); - if (ret != 0) { + /* Try to fetch missing DS (from above the cut). */ + bool refetch_ta = !qry->zone_cut.trust_anchor || !knot_dname_is_equal(qry->zone_cut.name, qry->zone_cut.trust_anchor->owner); + if (want_secured && refetch_ta) { + /* @todo we could fetch the information from the parent cut, but we don't remember that now */ + struct kr_query *next = kr_rplan_push(rplan, qry, qry->zone_cut.name, qry->sclass, KNOT_RRTYPE_DS); + if (!next) { return KNOT_STATE_FAIL; } + next->flags |= QUERY_AWAIT_CUT|QUERY_DNSSEC_WANT; return KNOT_STATE_DONE; } - /* Try to fetch missing DNSKEY. */ - if (want_secured && qry->zone_cut.trust_anchor && !qry->zone_cut.key && qry->stype != KNOT_RRTYPE_DNSKEY) { - int ret = zone_cut_subreq(rplan, qry, qry->zone_cut.name, KNOT_RRTYPE_DNSKEY); - if (ret != 0) { + /* Try to fetch missing DNSKEY (either missing or above current cut). */ + bool refetch_key = !qry->zone_cut.key || !knot_dname_is_equal(qry->zone_cut.name, qry->zone_cut.key->owner); + if (want_secured && qry->zone_cut.trust_anchor && refetch_key && qry->stype != KNOT_RRTYPE_DNSKEY) { + struct kr_query *next = zone_cut_subreq(rplan, qry, qry->zone_cut.name, KNOT_RRTYPE_DNSKEY); + if (!next) { return KNOT_STATE_FAIL; } return KNOT_STATE_DONE; diff --git a/lib/rplan.h b/lib/rplan.h index 28968547886ee62325ddb9e66937e7f238c32440..cf165768f735d6f92194dfdc4cfaf8fe263bd002 100644 --- a/lib/rplan.h +++ b/lib/rplan.h @@ -41,7 +41,6 @@ X(ALLOW_LOCAL, 1 << 11) /**< Allow queries to local or private address ranges. */ \ X(DNSSEC_WANT , 1 << 12) /**< Want DNSSEC secured answer. */ \ X(DNSSEC_BOGUS , 1 << 13) /**< Query response is DNSSEC bogus. */ \ - X(AWAIT_DS , 1 << 14) /**< Query is waiting for DS lookup. */ /** Query flags */ enum kr_query_flag { diff --git a/tests/testdata/iter_validate_child_zone_noaddr.rpl b/tests/testdata/iter_validate_child_zone_noaddr.rpl index f0db972fa7672be3b1f86ed64d8a47e246f113df..408defde8247644d67bc9b591da89e08e25cfc3e 100644 --- a/tests/testdata/iter_validate_child_zone_noaddr.rpl +++ b/tests/testdata/iter_validate_child_zone_noaddr.rpl @@ -63,6 +63,21 @@ cz. 86400 IN RRSIG DS 8 1 86400 20150920050000 20150910040000 1518 . LRx9WQ8Kh SECTION ADDITIONAL a.ns.nic.cz. 172800 IN A 194.0.12.1 ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +ADJUST copy_id +REPLY QR NOERROR +SECTION QUESTION +nic.cz. IN DS +SECTION AUTHORITY +cz. 172800 IN NS a.ns.nic.cz. +cz. 86400 IN DS 54576 10 2 397E50C85EDE9CDE33F363A9E66FD1B216D788F8DD438A57A423A386 869C8F06 +cz. 86400 IN RRSIG DS 8 1 86400 20150920050000 20150910040000 1518 . LRx9WQ8KhcUHOCe+eY7jvw1QIm1aRrin02Qn9YtImOGf4V1MVhf1ZYoF mP7GOBDXAbAJhrb5fPKumLsuRLgmA+5VyFhBMmzgqwRjdec1Tu7mWHoQ EukoZp4y2Mmw4NuAs1pBJQOZzLxhYUk+vbjK9mZm5u+mTtt/EFUu8QfG bp8= +SECTION ADDITIONAL +a.ns.nic.cz. 172800 IN A 194.0.12.1 +ENTRY_END + RANGE_END ;a.ns.nic.cz.