lib/resolve: correct EDNS padding logic for failures

The main problem was that SERVFAILs were not padded;
the recovery of answer->opt_rr has to happen before answer_padding()

diff --git a/NEWS b/NEWS
index 26e1dc652f4893ef8fa3ab1f8508635783b638fe..41c483977abf2f49a3e1c58e8f00df2cb2e20f44 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,10 @@ Improvements
 - net.listen(): allow binding to non-local address with freebind option (!898)
 - cache: pre-allocate the file to avoid SIGBUS later (not macOS; !917, #525)
 
+Bugfixes
+--------
+- correctly use EDNS(0) padding in failed answers (!921)
+
 
 Knot Resolver 4.3.0 (2019-12-04)
 ================================

diff --git a/lib/resolve.c b/lib/resolve.c
index 8e365d782c7cd282abbb753a3b731815aaad7c71..baa7002a2f473cf876aa38efaed6e086c9fd4025 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -517,6 +517,10 @@ static int answer_padding(struct kr_request *request)
                assert(false);
                return kr_error(EINVAL);
        }
+       if (!request->qsource.flags.tls) {
+               /* Not meaningful to pad without encryption. */
+               return kr_ok();
+       }
        int32_t padding = request->ctx->tls_padding;
        knot_pkt_t *answer = request->answer;
        knot_rrset_t *opt_rr = answer->opt_rr;
@@ -556,8 +560,8 @@ static void answer_fail(struct kr_request *request)
        knot_wire_set_rcode(answer->wire, KNOT_RCODE_SERVFAIL);
        if (ret == 0 && opt_rr) {
                knot_pkt_begin(answer, KNOT_ADDITIONAL);
-               answer_padding(request); /* Ignore failed padding in SERVFAIL answer. */
                answer->opt_rr = opt_rr;
+               answer_padding(request); /* Ignore failed padding in SERVFAIL answer. */
                edns_put(answer, false);
        }
 }
@@ -568,10 +572,7 @@ static int answer_append_edns(struct kr_request *request)
        knot_pkt_t *answer = request->answer;
        if (!answer->opt_rr)
                return kr_ok();
-       int ret = 0;
-       if (request->qsource.flags.tls) {
-               ret = answer_padding(request);
-       }
+       int ret = answer_padding(request);
        if (!ret) ret = knot_pkt_begin(answer, KNOT_ADDITIONAL);
        if (!ret) ret = knot_pkt_put(answer, KNOT_COMPR_HINT_NONE,
                                     answer->opt_rr, KNOT_PF_FREE);

diff --git a/lib/resolve.h b/lib/resolve.h
index b49aa8d2fd142b4a0a8dcbbaa2461e98564ca3e9..50eac7bf527e1fbeeaa71b824e3ae2f7c9b951b7 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -178,7 +178,7 @@ struct kr_context
  * (and lines here were too long anyway). */
 struct kr_request_qsource_flags {
        bool tcp:1; /**< true if the request is on TCP (or TLS); only meaningful if (dst_addr). */
-       bool tls:1; /**< true if the request is on TLS; only meaningful if (dst_addr). */
+       bool tls:1; /**< true if the request is on TLS (or HTTPS); only meaningful if (dst_addr). */
        bool http:1; /**< true if the request is on HTTP; only meaningful if (dst_addr). */
 };