validator: trim TTLs by RRSIG's expiration and original TTL

A down-side is that validation can now modify the validated RRset
on success.  I checked all transitive call sites that it's OK.
The change is pretty simple; I just hand-tested it a bit with faketime.

diff --git a/NEWS b/NEWS
index 42d710e7beb842a87b62eebc55341e37577ce38f..9b08ef41f494caa3afc232a756823795e8ab2b40 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Bugfixes
 - fix incorrect SERVFAIL on cached bogus answer for +cd request (!860)
   (regression since 4.1.0 release, in less common cases)
 - prefill module: allow a different module-loading style (#506)
+- validation: trim TTLs by RRSIG's expiration and original TTL (#319, #504)
 
 Improvements
 ------------

diff --git a/lib/dnssec.c b/lib/dnssec.c
index efcdb1fb87f8b6637ea97dee6773121351cf8120..da095dbba8991b961d39ef45fa0cfc286a77f42e 100644 (file)
--- a/lib/dnssec.c
+++ b/lib/dnssec.c
@@ -27,8 +27,9 @@
 #include <libknot/rrtype/dnskey.h>
 #include <libknot/rrtype/nsec.h>
 #include <libknot/rrtype/rrsig.h>
-#include <contrib/wire.h>
 
+#include "contrib/cleanup.h"
+#include "contrib/wire.h"
 #include "lib/defines.h"
 #include "lib/dnssec/nsec.h"
 #include "lib/dnssec/nsec3.h"
@@ -38,7 +39,7 @@
 
 /* forward */
 static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx,
-       const knot_rrset_t *covered, size_t key_pos, const struct dseckey *key);
+       knot_rrset_t *covered, size_t key_pos, const struct dseckey *key);
 
 void kr_crypto_init(void)
 {
@@ -138,7 +139,7 @@ static inline int wildcard_radix_len_diff(const knot_dname_t *expanded,
        return knot_dname_labels(expanded, NULL) - knot_rrsig_labels(rrsig);
 }
 
-int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *covered)
+int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, knot_rrset_t *covered)
 {
        if (!vctx) {
                return kr_error(EINVAL);
@@ -161,14 +162,15 @@ int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *cover
 /**
  * Validate RRSet using a specific key.
  * @param vctx    Pointer to validation context.
- * @param covered RRSet covered by a signature. It must be in canonical format.
+ * @param covered RRSet covered by a signature.  It must be in canonical format.
+ *               TTL may get lowered.
  * @param key_pos Position of the key to be validated with.
  * @param key     Key to be used to validate.
  *               If NULL, then key from DNSKEY RRSet is used.
  * @return        0 or error code, same as vctx->result.
  */
 static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx,
-                               const knot_rrset_t *covered,
+                               knot_rrset_t *covered,
                                size_t key_pos, const struct dseckey *key)
 {
        const knot_pkt_t *pkt         = vctx->pkt;
@@ -256,7 +258,23 @@ static int kr_rrset_validate_with_key(kr_rrset_validation_ctx_t *vctx,
                                }
                                vctx->flags |= KR_DNSSEC_VFLG_WEXPAND;
                        }
-                       /* Validated with current key, OK */
+                       /* Validated with current key, OK;
+                        * now just trim TTL in case it's over-extended. */
+                       const uint32_t ttl_max = MIN(knot_rrsig_original_ttl(rdata_j),
+                                       knot_rrsig_sig_expiration(rdata_j) - timestamp);
+                       if (unlikely(covered->ttl > ttl_max)) {
+                               if (VERBOSE_STATUS) {
+                                       auto_free char
+                                               *name_str = kr_dname_text(covered->owner),
+                                               *type_str = kr_rrtype_text(covered->type);
+                                       QRVERBOSE(NULL, "vldr",
+                                               "trimming TTL of %s %s: %d -> %d\n",
+                                               name_str, type_str,
+                                               (int)covered->ttl, (int)ttl_max);
+                               }
+                               covered->ttl = ttl_max;
+                       }
+
                        kr_dnssec_key_free(&created_key);
                        vctx->result = kr_ok();
                        return vctx->result;
@@ -286,7 +304,7 @@ bool kr_ds_algo_support(const knot_rrset_t *ta)
 int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta)
 {
        const knot_pkt_t *pkt         = vctx->pkt;
-       const knot_rrset_t *keys      = vctx->keys;
+       knot_rrset_t *keys            = vctx->keys;
 
        const bool ok = pkt && keys && ta && ta->rrs.count && ta->rrs.rdata
                        && ta->type == KNOT_RRTYPE_DS;
@@ -303,7 +321,7 @@ int kr_dnskeys_trusted(kr_rrset_validation_ctx_t *vctx, const knot_rrset_t *ta)
        for (uint16_t i = 0; i < keys->rrs.count; ++i) {
                /* RFC4035 5.3.1, bullet 8 */ /* ZSK */
                /* LATER(optim.): more efficient way to iterate than _at() */
-               const knot_rdata_t *krr = knot_rdataset_at(&keys->rrs, i);
+               knot_rdata_t *krr = knot_rdataset_at(&keys->rrs, i);
                if (!kr_dnssec_key_zsk(krr->data) || kr_dnssec_key_revoked(krr->data)) {
                        continue;
                }

diff --git a/lib/dnssec.h b/lib/dnssec.h
index 8383048b2fa0d5e0d9ab97c47acccfa4caab9f6d..89850a328d44b9f1672b70fc434d459d47b6d564 100644 (file)
--- a/lib/dnssec.h
+++ b/lib/dnssec.h
@@ -50,7 +50,7 @@ struct kr_rrset_validation_ctx {
        const knot_pkt_t *pkt;          /*!< Packet to be validated. */
        ranked_rr_array_t *rrs;         /*!< List of preselected RRs to be validated. */
        knot_section_t section_id;      /*!< Section to work with. */
-       const knot_rrset_t *keys;       /*!< DNSKEY RRSet. */
+       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. */
         bool has_nsec3;                        /*!< Whether to use NSEC3 validation. */
@@ -76,10 +76,10 @@ typedef struct kr_rrset_validation_ctx kr_rrset_validation_ctx_t;
  * Validate RRSet.
  * @param vctx    Pointer to validation context.
  * @param covered RRSet covered by a signature. It must be in canonical format.
+ *               Its TTL may get lowered.
  * @return        0 or error code, same as vctx->result.
  */
-int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx,
-                       const knot_rrset_t *covered);
+int kr_rrset_validate(kr_rrset_validation_ctx_t *vctx, knot_rrset_t *covered);
 
 /**
  * Return true iff the RRset contains at least one usable DS.  See RFC6840 5.2.
@@ -89,7 +89,7 @@ bool kr_ds_algo_support(const knot_rrset_t *ta);
 
 /**
  * Check whether the DNSKEY rrset matches the supplied trust anchor RRSet.
- * @param vctx  Pointer to validation context.
+ * @param vctx  Pointer to validation context.  Note that TTL of vctx->keys may get lowered.
  * @param ta    Trust anchor RRSet against which to validate the DNSKEY RRSet.
  * @return      0 or error code, same as vctx->result.
  */

diff --git a/lib/layer/validate.c b/lib/layer/validate.c
index 59634bc30f923880837233459209183533a80385..d2d399b5cbb21e99313b9939a916bdcf9c547507 100644 (file)
--- a/lib/layer/validate.c
+++ b/lib/layer/validate.c
@@ -109,7 +109,7 @@ static int validate_section(kr_rrset_validation_ctx_t *vctx, const struct kr_que
        int validation_result = 0;
        for (ssize_t i = 0; i < vctx->rrs->len; ++i) {
                ranked_rr_array_entry_t *entry = vctx->rrs->at[i];
-               const knot_rrset_t *rr = entry->rr;
+               knot_rrset_t * const rr = entry->rr;
 
                if (entry->yielded || vctx->qry_uid != entry->qry_uid) {
                        continue;
@@ -1053,7 +1053,8 @@ static int validate(kr_layer_t *ctx, knot_pkt_t *pkt)
        }
 
        /* Validate all records, fail as bogus if it doesn't match.
-        * Do not revalidate data from cache, as it's already trusted. */
+        * Do not revalidate data from cache, as it's already trusted.
+        * TTLs of RRsets may get lowered. */
        if (!(qry->flags.CACHED)) {
                ret = validate_records(req, pkt, req->rplan.pool, has_nsec3);
                if (ret != 0) {