TTL bounds: improve the logic

- apply to first (uncached) answer already
- don't extend over signature validity

Nit: the tests were using too high TTL (RFCs disallow the "sign bit").
It was working because (manual) cache-insertion was applying bounds,
but now the bounds don't get applied anymore, so it would fail.

diff --git a/daemon/cache.test/insert_ns.test.integr/kresd_config.j2 b/daemon/cache.test/insert_ns.test.integr/kresd_config.j2
index ac83d20f5471d16b069196ae44c65280330335af..bf2165b81fba2d427198fecd3c28a10262f95555 100644 (file)
--- a/daemon/cache.test/insert_ns.test.integr/kresd_config.j2
+++ b/daemon/cache.test/insert_ns.test.integr/kresd_config.j2
@@ -14,7 +14,7 @@ local ffi = require('ffi')
 local c = kres.context().cache
 ns_name = todname('ns.example.com')
 local ns_addr = '\1\2\3\4'
-local rr = kres.rrset(ns_name, kres.type.A, kres.class.IN, 3600999999)
+local rr = kres.rrset(ns_name, kres.type.A, kres.class.IN, 2147483647)
 assert(rr:add_rdata(ns_addr, #ns_addr))
 assert(c:insert(rr, nil, ffi.C.KR_RANK_SECURE))
 

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 6a572b034fd147ed7837c9e7c71faca4abd9feff..530592aa689b6d29f414e5e912bf4d15d634dd48 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -597,14 +597,11 @@ static ssize_t stash_rrset(struct kr_cache *cache, const struct kr_query *qry,
        if (kr_fails_assert(val_new_entry.data))
                return kr_error(EFAULT);
 
-       const uint32_t ttl = rr->ttl;
-       /* FIXME: consider TTLs and expirations of RRSIGs as well, just in case. */
-
        /* Write the entry itself. */
        struct entry_h *eh = val_new_entry.data;
        memset(eh, 0, offsetof(struct entry_h, data));
        eh->time = timestamp;
-       eh->ttl  = MAX(MIN(ttl, cache->ttl_max), cache->ttl_min);
+       eh->ttl  = rr->ttl;
        eh->rank = rank;
        rdataset_dematerialize(&rr->rrs, eh->data);
        rdataset_dematerialize(rds_sigs, eh->data + rr_ssize);

diff --git a/lib/cache/api.h b/lib/cache/api.h
index 76cbebc1b9a5f87f2b0ba35424325e13567b6026..48cc0d2f2ad3238a2520042e3e133aaf987d4816 100644 (file)
--- a/lib/cache/api.h
+++ b/lib/cache/api.h
@@ -25,7 +25,7 @@ struct kr_cache
        kr_cdb_pt db;                 /**< Storage instance */
        const struct kr_cdb_api *api; /**< Storage engine */
        struct kr_cdb_stats stats;
-       uint32_t ttl_min, ttl_max; /**< TTL limits */
+       uint32_t ttl_min, ttl_max; /**< TTL limits; enforced primarily in iterator actually. */
 
        /* A pair of stamps for detection of real-time shifts during runtime. */
        struct timeval checkpoint_walltime; /**< Wall time on the last check-point. */

diff --git a/lib/dnssec.c b/lib/dnssec.c
index f56ab759f955fc5b275b6a89ca24573198eb8ce4..3eb6a2ada6fd7f2ff93773585c31305336c0991a 100644 (file)
--- a/lib/dnssec.c
+++ b/lib/dnssec.c
@@ -144,16 +144,24 @@ int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, knot_rrset_t *covered)
 
 /** Assuming `rrs` was validated with `sig`, trim its TTL in case it's over-extended. */
 static bool trim_ttl(knot_rrset_t *rrs, const knot_rdata_t *sig,
-                       uint32_t timestamp, const struct kr_query *log_qry)
+                       const kr_rrset_validation_ctx_t *vctx)
 {
-       const uint32_t ttl_max = MIN(knot_rrsig_original_ttl(sig),
-                       knot_rrsig_sig_expiration(sig) - timestamp);
+       /* The trimming logic is a bit complicated.
+        *
+        * We respect configured ttl_min over the (signed) original TTL,
+        * but we very much want to avoid TTLs over signature expiration,
+        * as that could cause serious issues with downstream validators.
+        */
+       const uint32_t ttl_max = MIN(
+                       MAX(knot_rrsig_original_ttl(sig), vctx->ttl_min),
+                       knot_rrsig_sig_expiration(sig) - vctx->timestamp
+       );
        if (likely(rrs->ttl <= ttl_max))
                return false;
-       if (kr_log_is_debug_qry(VALIDATOR, log_qry)) {
+       if (kr_log_is_debug_qry(VALIDATOR, vctx->log_qry)) {
                auto_free char *name_str = kr_dname_text(rrs->owner),
                                *type_str = kr_rrtype_text(rrs->type);
-               kr_log_q(log_qry, VALIDATOR, "trimming TTL of %s %s: %d -> %d\n",
+               kr_log_q(vctx->log_qry, VALIDATOR, "trimming TTL of %s %s: %d -> %d\n",
                        name_str, type_str, (int)rrs->ttl, (int)ttl_max);
        }
        rrs->ttl = ttl_max;
@@ -204,7 +212,7 @@ struct kr_svldr_ctx * kr_svldr_new_ctx(const knot_rrset_t *ds, knot_rrset_t *dns
        struct kr_svldr_ctx *ctx = calloc(1, sizeof(*ctx));
        if (unlikely(!ctx))
                return NULL;
-       ctx->vctx.timestamp = timestamp;
+       ctx->vctx.timestamp = timestamp; // .ttl_min is implicitly zero
        ctx->vctx.zone_name = knot_dname_copy(ds->owner, NULL);
        if (unlikely(!ctx->vctx.zone_name))
                goto fail;
@@ -254,7 +262,7 @@ static int kr_svldr_rrset_with_key(knot_rrset_t *rrs, const knot_rdataset_t *rrs
                // that also means we don't need to perform non-existence proofs.
                const int trim_labels = (val_flgs & FLG_WILDCARD_EXPANSION) ? 1 : 0;
                if (kr_check_signature(rdata_j, key->key, rrs, trim_labels) == 0) {
-                       trim_ttl(rrs, rdata_j, vctx->timestamp, vctx->log_qry);
+                       trim_ttl(rrs, rdata_j, vctx);
                        vctx->result = kr_ok();
                        return vctx->result;
                } else {
@@ -382,7 +390,7 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx,
                                vctx->flags |= KR_DNSSEC_VFLG_WEXPAND;
                        }
 
-                       trim_ttl(covered, rdata_j, vctx->timestamp, vctx->log_qry);
+                       trim_ttl(covered, rdata_j, vctx);
 
                        kr_dnssec_key_free(&created_key);
                        vctx->result = kr_ok();

diff --git a/lib/dnssec.h b/lib/dnssec.h
index 97c8831d4323d07536fa4f3539b148eaf10c0783..d9369978e4c3ae1f3f9cfbbb8cf32744f51f99c4 100644 (file)
--- a/lib/dnssec.h
+++ b/lib/dnssec.h
@@ -38,6 +38,7 @@ struct kr_rrset_validation_ctx {
        knot_rrset_t *keys;             /*!< DNSKEY RRSet; TTLs may get lowered when validating this set. */
         const knot_dname_t *zone_name; /*!< Name of the zone containing the RRSIG RRSet. */
        uint32_t timestamp;             /*!< Validation time. */
+       uint32_t ttl_min;               /*!< See trim_ttl() for details. */
         bool has_nsec3;                        /*!< Whether to use NSEC3 validation. */
        uint32_t qry_uid;               /*!< Current query uid. */
        uint32_t flags;                 /*!< Output - Flags. */

diff --git a/lib/layer/iterate.c b/lib/layer/iterate.c
index 03366acea34b6ddebfa72d5e9720cb3124b88f97..2a37e68565097c1e0b60f7ef6254e2851d5f306a 100644 (file)
--- a/lib/layer/iterate.c
+++ b/lib/layer/iterate.c
@@ -1010,6 +1010,22 @@ static bool satisfied_by_additional(const struct kr_query *qry)
        return false;
 }
 
+/** Restrict all RRset TTLs to the specified bounds (if matching qry_uid). */
+static void bound_ttls(ranked_rr_array_t *array, uint32_t qry_uid,
+                       uint32_t ttl_min, uint32_t ttl_max)
+{
+       for (ssize_t i = 0; i < array->len; ++i) {
+               if (array->at[i]->qry_uid != qry_uid)
+                       continue;
+               uint32_t *ttl = &array->at[i]->rr->ttl;
+               if (*ttl < ttl_min) {
+                       *ttl = ttl_min;
+               } else if (*ttl > ttl_max) {
+                       *ttl = ttl_max;
+               }
+       }
+}
+
 /** Resolve input query or continue resolution with followups.
  *
  *  This roughly corresponds to RFC1034, 5.3.3 4a-d.
@@ -1186,12 +1202,14 @@ rrarray_finalize:
        /* Finish construction of libknot-format RRsets.
         * We do this even if dropping the answer, though it's probably useless. */
        (void)0;
+       const struct kr_cache *cache = &req->ctx->cache;
        ranked_rr_array_t *selected[] = kr_request_selected(req);
        for (knot_section_t i = KNOT_ANSWER; i <= KNOT_ADDITIONAL; ++i) {
                ret = kr_ranked_rrarray_finalize(selected[i], query->uid, &req->pool);
-               if (unlikely(ret)) {
+               if (unlikely(ret))
                        return KR_STATE_FAIL;
-               }
+               if (!query->flags.CACHED)
+                       bound_ttls(selected[i], query->uid, cache->ttl_min, cache->ttl_max);
        }
 
        return state;

diff --git a/lib/layer/validate.c b/lib/layer/validate.c
index ef21e3010851c8c130f507a7fc923de9540b49f7..68e85659ce5077baca205815f410e44111548a05 100644 (file)
--- a/lib/layer/validate.c
+++ b/lib/layer/validate.c
@@ -268,6 +268,7 @@ static int validate_records(struct kr_request *req, knot_pkt_t *answer, knot_mm_
                .keys           = qry->zone_cut.key,
                .zone_name      = qry->zone_cut.name,
                .timestamp      = qry->timestamp.tv_sec,
+               .ttl_min        = req->ctx->cache.ttl_min,
                .qry_uid        = qry->uid,
                .has_nsec3      = has_nsec3,
                .flags          = 0,
@@ -377,6 +378,7 @@ static int validate_keyset(struct kr_request *req, knot_pkt_t *answer, bool has_
                        .keys           = qry->zone_cut.key,
                        .zone_name      = qry->zone_cut.name,
                        .timestamp      = qry->timestamp.tv_sec,
+                       .ttl_min        = req->ctx->cache.ttl_min,
                        .qry_uid        = qry->uid,
                        .has_nsec3      = has_nsec3,
                        .flags          = 0,

diff --git a/modules/policy/policy.rpz.test.lua b/modules/policy/policy.rpz.test.lua
index 70ef9fb6f7a6cbbc513ea48c3868de04bfc7ca99..94fb9ceb48b9a014e31ad5e4cdba1ba7d4518568 100644 (file)
--- a/modules/policy/policy.rpz.test.lua
+++ b/modules/policy/policy.rpz.test.lua
@@ -7,7 +7,7 @@ local function prepare_cache()
        local c = kres.context().cache
 
        local passthru_addr = '\127\0\0\9'
-       rr_passthru = kres.rrset(todname('rpzpassthru.'), kres.type.A, kres.class.IN, 3600999999)
+       rr_passthru = kres.rrset(todname('rpzpassthru.'), kres.type.A, kres.class.IN, 2147483647)
        assert(rr_passthru:add_rdata(passthru_addr, #passthru_addr))
        assert(c:insert(rr_passthru, nil, ffi.C.KR_RANK_SECURE + ffi.C.KR_RANK_AUTH))