From: Vladimír Čunát Date: Wed, 1 Nov 2017 15:36:34 +0000 (+0100) Subject: add KR_RANK_TRY X-Git-Tag: v1.5.0~1^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a9e4fb50a340836a7c493392cd50d2872808921;p=thirdparty%2Fknot-resolver.git add KR_RANK_TRY attempt validation for more records but require it for fewer of them (e.g. avoids SERVFAIL when server adds extra records but omits RRSIGs) --- diff --git a/NEWS b/NEWS index 08c426b3f..c1a71cad5 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -Knot Resolver 1.5.0 (2017-11-xx) +Knot Resolver 1.5.0 (2017-11-02) ================================ Bugfixes @@ -9,6 +9,8 @@ Improvements ------------ - new module ta_signal_query supporting Signaling Trust Anchor Knowledge using Keytag Query (RFC 8145 section 5) +- attempt validation for more records but require it for fewer of them + (e.g. avoids SERVFAIL when server adds extra records but omits RRSIGs) Knot Resolver 1.4.0 (2017-09-22) diff --git a/lib/cache.c b/lib/cache.c index 0cda5e719..3ba98f2f2 100644 --- a/lib/cache.c +++ b/lib/cache.c @@ -34,7 +34,7 @@ #include "lib/utils.h" /* Cache version */ -#define KEY_VERSION "V\x04" +#define KEY_VERSION "V\x05" /* Key size */ #define KEY_HSIZE (sizeof(uint8_t) + sizeof(uint16_t)) #define KEY_SIZE (KEY_HSIZE + KNOT_DNAME_MAXLEN) diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c index 52e0cec90..9d5ded5ce 100644 --- a/lib/layer/iterate.c +++ b/lib/layer/iterate.c @@ -306,22 +306,11 @@ static uint8_t get_initial_rank(const knot_rrset_t *rr, const struct kr_query *q } if (answer || type == KNOT_RRTYPE_DS || type == KNOT_RRTYPE_NSEC || type == KNOT_RRTYPE_NSEC3) { + /* We almost always want these validated, and it should be possible. */ return KR_RANK_INITIAL | KR_RANK_AUTH; } - if (type == KNOT_RRTYPE_NS) { - /* Some servers add extra NS RRset, which allows us to refresh - * cache "for free", potentially speeding up zone cut lookups - * in future. Still, it might theoretically cause some problems: - * https://mailarchive.ietf.org/arch/msg/dnsop/CYjPDlwtpxzdQV_qycB-WfnW6CI - */ - if (!is_nonauth && knot_dname_is_equal(qry->zone_cut.name, rr->owner)) { - return KR_RANK_INITIAL | KR_RANK_AUTH; - } else { - return KR_RANK_OMIT; - } - } - - return KR_RANK_INITIAL; + /* Be aggressive: try to validate anything else (almost never extra latency). */ + return KR_RANK_TRY; /* TODO: this classifier of authoritativity may not be perfect yet. */ } @@ -749,7 +738,7 @@ static int process_stub(knot_pkt_t *pkt, struct kr_request *req) for (unsigned i = 0; i < an->count; ++i) { const knot_rrset_t *rr = knot_pkt_rr(an, i); int err = kr_ranked_rrarray_add(&req->answ_selected, rr, - KR_RANK_INITIAL | KR_RANK_AUTH, true, query->uid, &req->pool); + KR_RANK_OMIT | KR_RANK_AUTH, true, query->uid, &req->pool); /* KR_RANK_AUTH: we don't have the records directly from * an authoritative source, but we do trust the server and it's * supposed to only send us authoritative records. */ diff --git a/lib/layer/validate.c b/lib/layer/validate.c index 6169d778e..1b6e9f37b 100644 --- a/lib/layer/validate.c +++ b/lib/layer/validate.c @@ -76,7 +76,8 @@ static bool pkt_has_type(const knot_pkt_t *pkt, uint16_t type) return section_has_type(knot_pkt_section(pkt, KNOT_ADDITIONAL), type); } -static int validate_section(kr_rrset_validation_ctx_t *vctx, knot_mm_t *pool) +static int validate_section(kr_rrset_validation_ctx_t *vctx, const struct kr_query *qry, + knot_mm_t *pool) { if (!vctx) { return kr_error(EINVAL); @@ -85,7 +86,7 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, knot_mm_t *pool) /* Can't use qry->zone_cut.name directly, as this name can * change when updating cut information before validation. */ - vctx->zone_name = vctx->keys ? vctx->keys->owner : NULL; + vctx->zone_name = vctx->keys ? vctx->keys->owner : NULL; int validation_result = 0; for (ssize_t i = 0; i < vctx->rrs->len; ++i) { @@ -112,13 +113,28 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, knot_mm_t *pool) continue; } + uint8_t rank_orig = entry->rank; validation_result = kr_rrset_validate(vctx, rr); if (validation_result == kr_ok()) { kr_rank_set(&entry->rank, KR_RANK_SECURE); + + } else if (kr_rank_test(rank_orig, KR_RANK_TRY)) { + WITH_VERBOSE { + VERBOSE_MSG(qry, ">< failed to validate but skipping: "); + kr_rrtype_print(rr->type, "", " "); + kr_dname_print(rr->owner, "", "\n"); + } + vctx->result = kr_ok(); + kr_rank_set(&entry->rank, KR_RANK_TRY); + /* ^^ BOGUS would be more accurate, but it might change + * to MISMATCH on revalidation, e.g. in test val_referral_nods :-/ + */ + } else if (validation_result == kr_error(ENOENT)) { /* no RRSIGs found */ kr_rank_set(&entry->rank, KR_RANK_MISSING); vctx->err_cnt += 1; + } else { kr_rank_set(&entry->rank, KR_RANK_BOGUS); vctx->err_cnt += 1; @@ -149,7 +165,7 @@ static int validate_records(struct kr_request *req, knot_pkt_t *answer, knot_mm_ .result = 0 }; - int ret = validate_section(&vctx, pool); + int ret = validate_section(&vctx, qry, pool); req->answ_validated = (vctx.err_cnt == 0); if (ret != kr_ok()) { return ret; @@ -162,7 +178,7 @@ static int validate_records(struct kr_request *req, knot_pkt_t *answer, knot_mm_ vctx.err_cnt = 0; vctx.result = 0; - ret = validate_section(&vctx, pool); + ret = validate_section(&vctx, qry, pool); req->auth_validated = (vctx.err_cnt == 0); if (ret != kr_ok()) { return ret; @@ -420,7 +436,8 @@ static const knot_dname_t *find_first_signer(ranked_rr_array_t *arr) const knot_rrset_t *rr = entry->rr; if (entry->yielded || (!kr_rank_test(entry->rank, KR_RANK_INITIAL) && - !kr_rank_test(entry->rank, KR_RANK_MISMATCH))) { + !kr_rank_test(entry->rank, KR_RANK_TRY) && + !kr_rank_test(entry->rank, KR_RANK_MISMATCH))) { continue; } if (rr->type == KNOT_RRTYPE_RRSIG) { @@ -744,6 +761,7 @@ static void rank_records(kr_layer_t *ctx, enum kr_rank rank_to_set) continue; } if (kr_rank_test(entry->rank, KR_RANK_INITIAL) + || kr_rank_test(entry->rank, KR_RANK_TRY) || kr_rank_test(entry->rank, KR_RANK_MISSING)) { kr_rank_set(&entry->rank, rank_to_set); } diff --git a/lib/resolve.c b/lib/resolve.c index 4bd652081..3ecf5aa2c 100644 --- a/lib/resolve.c +++ b/lib/resolve.c @@ -43,6 +43,7 @@ bool kr_rank_check(uint8_t rank) switch (rank & ~KR_RANK_AUTH) { case KR_RANK_INITIAL: case KR_RANK_OMIT: + case KR_RANK_TRY: case KR_RANK_INDET: case KR_RANK_BOGUS: case KR_RANK_MISMATCH: diff --git a/lib/resolve.h b/lib/resolve.h index 43b5f9f3d..ab7b53e19 100644 --- a/lib/resolve.h +++ b/lib/resolve.h @@ -91,9 +91,13 @@ * https://tools.ietf.org/html/rfc4035#section-4.3 */ enum kr_rank { - KR_RANK_INITIAL = 0, /**< Did not attempt to validate. */ - KR_RANK_OMIT = 1, /**< Do not attempt to validate. (And don't consider it a validation failure.) */ - KR_RANK_INDET, /**< Unable to determine whether it should be secure. */ + KR_RANK_INITIAL = 0, /**< Did not attempt to validate. It's assumed + compulsory to validate (or prove insecure). */ + KR_RANK_OMIT, /**< Do not attempt to validate. + (And don't consider it a validation failure.) */ + KR_RANK_TRY, /**< Attempt to validate, but failures are non-fatal. */ + + KR_RANK_INDET = 4, /**< Unable to determine whether it should be secure. */ KR_RANK_BOGUS, /**< Ought to be secure but isn't. */ KR_RANK_MISMATCH, KR_RANK_MISSING, /**< Unable to obtain a good signature. */