lua: ensure answer_clear() keeps original EDNS

Answers to EDNS requests from certain lua policies that use the
answer_clear() function would lack OPT RR and thus violate the MUST
condition in RFC6891.6.1.1.

diff --git a/NEWS b/NEWS
index 5abf1da0465d77a546a4243c26f5812e1177d3b5..12661505563b7778f8cc9e0aaf5b13f925d8e908 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@ Bugfixes
 --------
 - policy.rpz: improve logs, fix origin detection in files without $ORIGIN
 - lua: log() works again; broken in 5.4.2 (!1223)
+- policy: correctly include EDNS0 previously omitted by some actions (!1230)
 
 
 Knot Resolver 5.4.2 (2021-10-13)

diff --git a/daemon/lua/kres-gen-29.lua b/daemon/lua/kres-gen-29.lua
index 32795f2b442612922cbffbb6a5151c7e6bf40216..ded494651e3a5f5526c7c820c280f5e8630dcca2 100644 (file)
--- a/daemon/lua/kres-gen-29.lua
+++ b/daemon/lua/kres-gen-29.lua
@@ -378,6 +378,7 @@ int knot_pkt_put_rotate(knot_pkt_t *, uint16_t, const knot_rrset_t *, uint16_t,
 knot_pkt_t *knot_pkt_new(void *, uint16_t, knot_mm_t *);
 void knot_pkt_free(knot_pkt_t *);
 int knot_pkt_parse(knot_pkt_t *, unsigned int);
+knot_rrset_t *kr_request_ensure_edns(struct kr_request *);
 knot_pkt_t *kr_request_ensure_answer(struct kr_request *);
 struct kr_rplan *kr_resolve_plan(struct kr_request *);
 knot_mm_t *kr_resolve_pool(struct kr_request *);

diff --git a/daemon/lua/kres-gen-31.lua b/daemon/lua/kres-gen-31.lua
index f540ddf85e6c1dff9e8e45e94adf56799da1534b..dc7334728cce7a53f3d7baa8e576d622d6ee2a43 100644 (file)
--- a/daemon/lua/kres-gen-31.lua
+++ b/daemon/lua/kres-gen-31.lua
@@ -378,6 +378,7 @@ int knot_pkt_put_rotate(knot_pkt_t *, uint16_t, const knot_rrset_t *, uint16_t,
 knot_pkt_t *knot_pkt_new(void *, uint16_t, knot_mm_t *);
 void knot_pkt_free(knot_pkt_t *);
 int knot_pkt_parse(knot_pkt_t *, unsigned int);
+knot_rrset_t *kr_request_ensure_edns(struct kr_request *);
 knot_pkt_t *kr_request_ensure_answer(struct kr_request *);
 struct kr_rplan *kr_resolve_plan(struct kr_request *);
 knot_mm_t *kr_resolve_pool(struct kr_request *);

diff --git a/daemon/lua/kres-gen.sh b/daemon/lua/kres-gen.sh
index eded99ea97ac52c8b5342ff5d5bdfaf327554313..b4244505fc80287f737c53ad84aed5fb0d0e4cdf 100755 (executable)
--- a/daemon/lua/kres-gen.sh
+++ b/daemon/lua/kres-gen.sh
@@ -201,6 +201,7 @@ EOF
 ## libkres API
 ${CDEFS} ${LIBKRES} functions <<-EOF
 # Resolution request
+       kr_request_ensure_edns
        kr_request_ensure_answer
        kr_resolve_plan
        kr_resolve_pool

diff --git a/daemon/lua/kres.lua b/daemon/lua/kres.lua
index ce3556393ebb0bd97cb1ecc7e5acbe3296f7fb5c..140421660214b456b833121e80cd3746c1d03f8a 100644 (file)
--- a/daemon/lua/kres.lua
+++ b/daemon/lua/kres.lua
@@ -879,6 +879,11 @@ ffi.metatype( kr_request_t, {
                        req.vars_ref = ref
                        return var
                end,
+               -- Ensure that answer has EDNS if needed; can't fail.
+               ensure_edns = function (req)
+                       assert(ffi.istype(kr_request_t, req))
+                       return C.kr_request_ensure_edns(req)
+               end,
                -- Ensure that answer exists and return it; can't fail.
                ensure_answer = function (req)
                        assert(ffi.istype(kr_request_t, req))

diff --git a/lib/resolve.c b/lib/resolve.c
index d9a92ec1f396f735e8217b656ab48ccb043618b9..1ffd07ec72c1ca3d455c414c36f5ef0902c7f50b 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -699,6 +699,27 @@ static int resolve_query(struct kr_request *request, const knot_pkt_t *packet)
        return request->state;
 }
 
+knot_rrset_t* kr_request_ensure_edns(struct kr_request *request)
+{
+       kr_require(request && request->answer && request->qsource.packet && request->ctx);
+       knot_pkt_t* answer = request->answer;
+       bool want_edns = knot_pkt_has_edns(request->qsource.packet);
+       if (!want_edns) {
+               kr_assert(!answer->opt_rr);
+               return answer->opt_rr;
+       } else if (answer->opt_rr) {
+               return answer->opt_rr;
+       }
+
+       kr_assert(request->ctx->downstream_opt_rr);
+       answer->opt_rr = knot_rrset_copy(request->ctx->downstream_opt_rr, &answer->mm);
+       if (!answer->opt_rr)
+               return NULL;
+       if (knot_pkt_has_dnssec(request->qsource.packet))
+               knot_edns_set_do(answer->opt_rr);
+       return answer->opt_rr;
+}
+
 knot_pkt_t *kr_request_ensure_answer(struct kr_request *request)
 {
        if (request->answer)
@@ -749,14 +770,8 @@ knot_pkt_t *kr_request_ensure_answer(struct kr_request *request)
        }
 
        // Prepare EDNS if required.
-       if (knot_pkt_has_edns(qs_pkt)) {
-               answer->opt_rr = knot_rrset_copy(request->ctx->downstream_opt_rr,
-                                                &answer->mm);
-               if (!answer->opt_rr)
-                       goto enomem; // answer is on mempool, so "leak" is OK
-               if (knot_pkt_has_dnssec(qs_pkt))
-                       knot_edns_set_do(answer->opt_rr);
-       }
+       if (knot_pkt_has_edns(qs_pkt) && kr_fails_assert(kr_request_ensure_edns(request)))
+               goto enomem; // answer is on mempool, so "leak" is OK
 
        return request->answer;
 enomem:

diff --git a/lib/resolve.h b/lib/resolve.h
index 88605b1e3e8abd8163d2462aa73085c02c6cdbd0..efd665671350f6fb84b6ee589fa6061ac8d82369 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -272,6 +272,15 @@ struct kr_request {
 KR_EXPORT
 int kr_resolve_begin(struct kr_request *request, struct kr_context *ctx);
 
+/**
+ * Ensure that request->answer->opt_rr is present if query has EDNS.
+ *
+ * This function should be used after clearing a response packet to ensure its
+ * opt_rr is properly set. Returns the opt_rr (for convenience) or NULL.
+ */
+KR_EXPORT
+knot_rrset_t * kr_request_ensure_edns(struct kr_request *request);
+
 /**
  * Ensure that request->answer is usable, and return it (for convenience).
  *

diff --git a/modules/policy/policy.lua b/modules/policy/policy.lua
index 71d277b145db4f0d07558f3a7e343a7b7c5330ef..7cd8ff926a2cf289d28246004292400195a96b62 100644 (file)
--- a/modules/policy/policy.lua
+++ b/modules/policy/policy.lua
@@ -654,6 +654,7 @@ local function answer_clear(req)
        local pkt = req:ensure_answer()
        if pkt == nil then return nil end
        pkt:clear_payload()
+       req:ensure_edns()
        return pkt
 end