use a different mechanism for AD flag

To make this work, do not use KR_VLDRANK_SECURE as the default value.
It's just too dangerous, and here it complicated determining the
appropriate value for the AD flag.

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index ebb709aa2e6fca4258a31c03c5bf28755b47b8ae..04b52468273df5ac11616af30aa8c43d080f88c1 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -270,12 +270,26 @@ static int update_cut(knot_pkt_t *pkt, const knot_rrset_t *rr,
        return state;
 }
 
+/** Compute rank appropriate for RRs present in the packet. */
+static inline kr_validation_rank_t get_initial_rank(const struct kr_request *req)
+{
+       const uint32_t qflags = req->current_query->flags;
+       if ((qflags & QUERY_CACHED)
+           && !knot_wire_get_cd(req->answer->wire)
+           && (qflags & QUERY_DNSSEC_WANT)
+           && !(qflags & (QUERY_DNSSEC_INSECURE|QUERY_DNSSEC_BOGUS)))
+               return KR_VLDRANK_SECURE;
+               /* TODO: make cache better signal real ranks of individual RRs. */
+       return KR_VLDRANK_INITIAL;
+}
+
 static int pick_authority(knot_pkt_t *pkt, struct kr_request *req, bool to_wire)
 {
        struct kr_query *qry = req->current_query;
        const knot_pktsection_t *ns = knot_pkt_section(pkt, KNOT_AUTHORITY);
-       uint8_t rank = !(qry->flags & QUERY_DNSSEC_WANT) || (qry->flags & QUERY_CACHED) ?
-                       KR_VLDRANK_SECURE : KR_VLDRANK_INITIAL;
+
+       kr_validation_rank_t rank = get_initial_rank(req);
+
        const knot_dname_t *zonecut_name = qry->zone_cut.name;
        bool referral = !knot_wire_get_aa(pkt->wire);
        if (referral) {
@@ -375,8 +389,7 @@ static int unroll_cname(knot_pkt_t *pkt, struct kr_request *req, bool referral,
        const knot_dname_t *cname = NULL;
        const knot_dname_t *pending_cname = query->sname;
        unsigned cname_chain_len = 0;
-       uint8_t rank = !(query->flags & QUERY_DNSSEC_WANT) || (query->flags & QUERY_CACHED) ?
-                       KR_VLDRANK_SECURE : KR_VLDRANK_INITIAL;
+       kr_validation_rank_t rank = get_initial_rank(req);
        bool is_final = (query->parent == NULL);
        uint32_t iter_count = 0;
        bool strict_mode = (query->flags & QUERY_STRICT);

diff --git a/lib/layer/validate.c b/lib/layer/validate.c
index 01140aa43337468d7c8c150dbf1fad8131b7c2b3..d43b255edfdffb5cc0dc3797b38902bdaebb493a 100644 (file)
--- a/lib/layer/validate.c
+++ b/lib/layer/validate.c
@@ -102,11 +102,11 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, knot_mm_t *pool)
                                vctx->err_cnt += 1;
                                break;
                        }
-                       entry->rank = KR_VLDRANK_SECURE;
+                       entry->rank = KR_VLDRANK_SECURE; /* TODO: not good semantically for RRSIGs */
                        continue;
                }
                if ((rr->type == KNOT_RRTYPE_NS) && (vctx->section_id == KNOT_AUTHORITY)) {
-                       entry->rank = KR_VLDRANK_SECURE;
+                       entry->rank = KR_VLDRANK_SECURE; /* FIXME: dangerous, most likely */
                        continue;
                }
                validation_result = kr_rrset_validate(vctx, rr);

diff --git a/lib/resolve.c b/lib/resolve.c
index bde7ccb994b8f2690bfee5a80819fd4c27d59010..3c3f0efb5079beb5d0b7a9265272de51e9b77764 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -417,7 +417,7 @@ static int answer_prepare(knot_pkt_t *answer, knot_pkt_t *query, struct kr_reque
 }
 
 /** @return error code, ignoring if forced to truncate the packet. */
-static int write_extra_records(rr_array_t *arr, knot_pkt_t *answer)
+static int write_extra_records(const rr_array_t *arr, knot_pkt_t *answer)
 {
        for (size_t i = 0; i < arr->len; ++i) {
                int err = knot_pkt_put(answer, 0, arr->at[i], 0);
@@ -428,10 +428,18 @@ static int write_extra_records(rr_array_t *arr, knot_pkt_t *answer)
        return kr_ok();
 }
 
-/** @return error code, ignoring if forced to truncate the packet. */
-static int write_extra_ranked_records(ranked_rr_array_t *arr, knot_pkt_t *answer)
+/**
+ * \param all_secure optionally &&-combine security of written RRs into its value.
+ *                  (i.e. if you pass reference to false, it will always remain)
+ * @return error code, ignoring if forced to truncate the packet.
+ */
+static int write_extra_ranked_records(const ranked_rr_array_t *arr, knot_pkt_t *answer,
+                                     bool *all_secure)
 {
        const bool has_dnssec = knot_pkt_has_dnssec(answer);
+       bool all_sec = true;
+       int err = kr_ok();
+
        for (size_t i = 0; i < arr->len; ++i) {
                ranked_rr_array_entry_t * entry = arr->at[i];
                if (!entry->to_wire) {
@@ -445,10 +453,21 @@ static int write_extra_ranked_records(ranked_rr_array_t *arr, knot_pkt_t *answer
                }
                int err = knot_pkt_put(answer, 0, rr, 0);
                if (err != KNOT_EOK) {
-                       return err == KNOT_ESPACE ? kr_ok() : kr_error(err);
+                       if (err == KNOT_ESPACE) {
+                               err = kr_ok();
+                       }
+                       break;
+               }
+
+               if (rr->type != KNOT_RRTYPE_RRSIG) {
+                       all_sec = all_sec && entry->rank == KR_VLDRANK_SECURE;
                }
        }
-       return kr_ok();
+
+       if (all_secure) {
+               *all_secure = *all_secure && all_sec;
+       }
+       return err;
 }
 
 /** @internal Add an EDNS padding RR into the answer if requested and required. */
@@ -512,13 +531,27 @@ static int answer_finalize(struct kr_request *request, int state)
                }
        }
 
+       struct kr_query *last = rplan->resolved.len > 0 ? array_tail(rplan->resolved) : NULL;
+               /* TODO  ^^^^ this is slightly fragile */
+       bool secure = (last != NULL); /* suspicious otherwise */
+       if (last && (last->flags & QUERY_STUB)) {
+               secure = false; /* don't trust forwarding for now */
+       }
+       if (last && (last->flags & QUERY_CACHED)) {
+               secure = secure && last->flags & QUERY_DNSSEC_WANT
+                        && !(last->flags & (QUERY_DNSSEC_INSECURE|QUERY_DNSSEC_BOGUS));
+       }
+       if (last && (last->flags & QUERY_DNSSEC_OPTOUT)) {
+               secure = false; /* the last answer is insecure due to opt-out */
+       }
+
        if (request->answ_selected.len > 0) {
                assert(answer->current <= KNOT_ANSWER);
                /* Write answer records. */
                if (answer->current < KNOT_ANSWER) {
                        knot_pkt_begin(answer, KNOT_ANSWER);
                }
-               if (write_extra_ranked_records(&request->answ_selected, answer)) {
+               if (write_extra_ranked_records(&request->answ_selected, answer, &secure)) {
                        return answer_fail(request);
                }
        }
@@ -527,7 +560,7 @@ static int answer_finalize(struct kr_request *request, int state)
        if (answer->current < KNOT_AUTHORITY) {
                knot_pkt_begin(answer, KNOT_AUTHORITY);
        }
-       if (write_extra_ranked_records(&request->auth_selected, answer)) {
+       if (write_extra_ranked_records(&request->auth_selected, answer, &secure)) {
                return answer_fail(request);
        }
        /* Write additional records. */
@@ -547,20 +580,10 @@ static int answer_finalize(struct kr_request *request, int state)
                ret = edns_put(answer);
        }
 
-       /* Set AD=1 if succeeded and requested secured answer. */
-       const bool has_ad = knot_wire_get_ad(answer->wire);
-       knot_wire_clear_ad(answer->wire);
-       if (state == KR_STATE_DONE && rplan->resolved.len > 0) {
-               struct kr_query *last = array_tail(rplan->resolved);
-               /* Do not set AD for RRSIG query, as we can't validate it. */
-               const bool secure = (last->flags & QUERY_DNSSEC_WANT) &&
-                                   !(last->flags & QUERY_DNSSEC_INSECURE) &&
-                                   !(last->flags & QUERY_DNSSEC_OPTOUT);
-               if (!(last->flags & QUERY_STUB) /* Never set AD if forwarding. */
-                   && has_ad && secure
-                   && knot_pkt_qtype(answer) != KNOT_RRTYPE_RRSIG) {
-                       knot_wire_set_ad(answer->wire);
-               }
+       /* Clear AD if not secure.  ATM answer has AD=1 if requested secured answer. */
+       if (!secure || state != KR_STATE_DONE
+           || knot_pkt_qtype(answer) == KNOT_RRTYPE_RRSIG) {
+               knot_wire_clear_ad(answer->wire);
        }
 
        return ret;

diff --git a/lib/resolve.h b/lib/resolve.h
index b0dd2d01d3bab451e3097faff496ee565de67a55..b31f35098a0f2bbad5cb45a230371c0eb6cb117b 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -74,12 +74,13 @@
 
 /** Validation rank */
 typedef enum kr_validation_rank {
-       KR_VLDRANK_INITIAL   = 0,   /* Entry was just added; not validated yet. */
+       KR_VLDRANK_INITIAL   = 0,   /* No validated yet or no information about it. */
        KR_VLDRANK_INSECURE  = 1,   /* Entry is DNSSEC insecure (e.g. RRSIG not exists). */
-       KR_VLDRANK_BAD       = 2,   /* Matching RRSIG found, but validation fails. */
+       KR_VLDRANK_BAD       = 2,   /* Matching RRSIG found, but validation fails.  Unused?! */
        KR_VLDRANK_MISMATCH  = 3,   /* RRSIG signer name is */
        KR_VLDRANK_UNKNOWN   = 4,   /* Unknown */
-       KR_VLDRANK_SECURE    = 5    /* Entry is DNSSEC valid (e.g. RRSIG exists). */
+       KR_VLDRANK_SECURE    = 5    /* Entry is DNSSEC valid (e.g. RRSIG exists).
+                                    * Note: it's also used for RRSIGs currently. */
 } kr_validation_rank_t;
 
 /** @cond internal Array of modules. */

diff --git a/lib/utils.h b/lib/utils.h
index 5100882b73b6a4556d5fd9ee34303e3cf94df2df..2676bfabd163f34c2a5b51efa081a5b30ac1bacb 100644 (file)
--- a/lib/utils.h
+++ b/lib/utils.h
@@ -93,7 +93,7 @@ struct kr_context;
 typedef array_t(knot_rrset_t *) rr_array_t;
 struct ranked_rr_array_entry {
        uint32_t qry_uid;
-       uint8_t rank;
+       uint8_t rank; /* kr_validation_rank_t */
        uint8_t revalidation_cnt;
        bool cached;
        bool yielded;