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 08c426b3f0ec8897d789b8800cca2ac1f9293c49..c1a71cad5df86628bcea426e9f99d5362da3e43f 100644 (file)
--- 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 0cda5e7190d5623aecc84adf5d0f7cdc99293b0d..3ba98f2f24b0c97868d44a6dadddb421aae2c764 100644 (file)
--- 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 52e0cec90daa261b035d224667b92af16438c41a..9d5ded5ceb8d8ae3dc8bd9c38dc4173e4328a204 100644 (file)
--- 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 6169d778e44ff657946c0409adddcc71d0427b4f..1b6e9f37bbdbd728ee51b4d9ceacaae4bef22cbd 100644 (file)
--- 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 4bd652081fe145ebb0b208d5a97384c375b471c3..3ecf5aa2c8fb4b6efeeab8339f089d087c5ab209 100644 (file)
--- 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 43b5f9f3d46775c5d5cf66df651968d6f47677d4..ab7b53e191159468de9e7ec930a3a97e9ceb38f8 100644 (file)
--- 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. */