Use default padding policy for responses.

net.tls_padding() can now take a boolean in addition to a numeric
value.  true means "use sensible default padding policy", false means
"never pad".

In the struct kr_context, we change tls_padding from a uint32_t to an
int32_t so that we can explicitly represent the default value (-1).
This should be a safe ABI/API change, since no one had ever set a
padding > 4096 anyway.

This depends on libknot having adopted the changes from
2dd9f406e333a1cedfa2380ffad57913cecf8efb which is first included in
2.4.3.  In the event that knot-resolver is compiled against an older
version of libknot, it just uses a baked-in default of 468 as before.

See https://gitlab.labs.nic.cz/knot/resolver/merge_requests/249 for
further discussion.

diff --git a/daemon/README.rst b/daemon/README.rst
index 0c18f5c7099f5e47db905c5070e739a4da2a5034..26301b9fc37b44948063c4ff03ed252317eab861 100644 (file)
--- a/daemon/README.rst
+++ b/daemon/README.rst
@@ -604,10 +604,13 @@ For when listening on ``localhost`` just doesn't cut it.
 
 .. function:: net.tls_padding([padding])
 
-   Get/set EDNS(0) padding.  If set to value >= 2 it will pad the answers
-   to nearest *padding* boundary, e.g. if set to `64`, the answer will
-   have size of multiplies of 64 (64, 128, 192, ...).  Setting padding to
-   value < 2 will disable it.
+   Get/set EDNS(0) padding of answers to queries that arrive over TLS
+   transport.  If set to `true` (the default), it will use a sensible
+   default padding scheme, as implemented by libknot if available at
+   compile time.  If set to a numeric value >= 2 it will pad the
+   answers to nearest *padding* boundary, e.g. if set to `64`, the
+   answer will have size of a multiple of 64 (64, 128, 192, ...).  If
+   set to `false` (or a number < 2), it will disable padding entirely.
 
 .. function:: net.outgoing_v4([string address])
 

diff --git a/daemon/bindings.c b/daemon/bindings.c
index aa2d41ca1c62aeea1326a7fe07a561eb0945856c..4f3dfe639525e0e030a40203b58207f6f230ac10 100644 (file)
--- a/daemon/bindings.c
+++ b/daemon/bindings.c
@@ -404,23 +404,39 @@ static int net_tls_padding(lua_State *L)
 
        /* Only return current padding. */
        if (lua_gettop(L) == 0) {
-               if (engine->resolver.tls_padding == 0) {
-                       return -1;
+               if (engine->resolver.tls_padding < 0) {
+                       lua_pushboolean(L, true);
+                       return 1;
+               } else if (engine->resolver.tls_padding == 0) {
+                       lua_pushboolean(L, false);
+                       return 1;
                }
                lua_pushinteger(L, engine->resolver.tls_padding);
                return 1;
        }
 
-       if ((lua_gettop(L) != 1) || !lua_isnumber(L, 1)) {
-               lua_pushstring(L, "net.tls_padding takes one numeric parameter: (\"padding\")");
+       if ((lua_gettop(L) != 1)) {
+               lua_pushstring(L, "net.tls_padding takes one parameter: (\"padding\")");
                lua_error(L);
        }
-       int padding = lua_tointeger(L, 1);
-       if ((padding < 0) || (padding > MAX_TLS_PADDING)) {
-               lua_pushstring(L, "net.tls_padding parameter has to be a number between <0, " xstr(MAX_TLS_PADDING) ">");
+       if (lua_isboolean(L, 1)) {
+               bool x = lua_toboolean(L, 1);
+               if (x) {
+                       engine->resolver.tls_padding = -1;
+               } else {
+                       engine->resolver.tls_padding = 0;
+               }
+       } else if (lua_isnumber(L, 1)) {
+               int padding = lua_tointeger(L, 1);
+               if ((padding < 0) || (padding > MAX_TLS_PADDING)) {
+                       lua_pushstring(L, "net.tls_padding parameter has to be true, false, or a number between <0, " xstr(MAX_TLS_PADDING) ">");
+                       lua_error(L);
+               }
+               engine->resolver.tls_padding = padding;
+       } else {
+               lua_pushstring(L, "net.tls_padding parameter has to be true, false, or a number between <0, " xstr(MAX_TLS_PADDING) ">");
                lua_error(L);
        }
-       engine->resolver.tls_padding = padding;
        lua_pushboolean(L, true);
        return 1;
 }

diff --git a/daemon/engine.c b/daemon/engine.c
index c46898079ce76ec078f756dccfa1c0b5e439b229..520cd3cd32a9ff038b75e9f8b8ebefa1b2e7904f 100644 (file)
--- a/daemon/engine.c
+++ b/daemon/engine.c
@@ -496,8 +496,8 @@ static int init_resolver(struct engine *engine)
                return kr_error(ENOMEM);
        }
        knot_edns_init(engine->resolver.opt_rr, KR_EDNS_PAYLOAD, 0, KR_EDNS_VERSION, engine->pool);
-       /* Set default TLS padding */
-       engine->resolver.tls_padding = KR_DEFAULT_TLS_PADDING;
+       /* Use default TLS padding */
+       engine->resolver.tls_padding = -1;
        /* Set default root hints */
        kr_zonecut_init(&engine->resolver.root_hints, (const uint8_t *)"", engine->pool);
        kr_zonecut_set_sbelt(&engine->resolver, &engine->resolver.root_hints);

diff --git a/lib/resolve.c b/lib/resolve.c
index 68b66922bd1f05f9d205a69e663eec7ddf385a2a..1012fbb74ed7d52f1d81246e06d2c5e9b24d82e1 100644 (file)
--- a/lib/resolve.c
+++ b/lib/resolve.c
@@ -423,8 +423,14 @@ static int edns_create(knot_pkt_t *pkt, knot_pkt_t *template, struct kr_request
                wire_size += KR_COOKIE_OPT_MAX_LEN;
        }
 #endif /* defined(ENABLE_COOKIES) */
-       if (req->has_tls && req->ctx->tls_padding >= 2) {
-               wire_size += KNOT_EDNS_OPTION_HDRLEN + req->ctx->tls_padding;
+       if (req->has_tls) {
+               if (req->ctx->tls_padding == -1)
+                       /* FIXME: we do not know how to reserve space for the
+                        * default padding policy, since we can't predict what
+                        * it will select. So i'm just guessing :/ */
+                       wire_size += KNOT_EDNS_OPTION_HDRLEN + 512;
+               if (req->ctx->tls_padding >= 2)
+                       wire_size += KNOT_EDNS_OPTION_HDRLEN + req->ctx->tls_padding;
        }
        return knot_pkt_reserve(pkt, wire_size);
 }
@@ -509,17 +515,24 @@ static int answer_padding(struct kr_request *request)
                assert(false);
                return kr_error(EINVAL);
        }
-       uint16_t padding = request->ctx->tls_padding;
+       int32_t padding = request->ctx->tls_padding;
        knot_pkt_t *answer = request->answer;
        knot_rrset_t *opt_rr = answer->opt_rr;
-
-       if (padding < 2) {
-               return kr_ok();
+       int32_t pad_bytes = -1;
+
+       if (padding == -1) { /* use the default padding policy from libknot */
+#if KNOT_VERSION_HEX < ((2 << 16) | (4 << 8) | 3)
+               /* no knot_edns_default_padding_size available in libknot */
+               padding = KR_DEFAULT_TLS_PADDING;
+#else
+               pad_bytes =  knot_edns_default_padding_size(answer, opt_rr);
+#endif
        }
-       int32_t max_pad_bytes = knot_edns_get_payload(opt_rr) - (answer->size + knot_rrset_size(opt_rr));
-
-       int32_t pad_bytes = MIN(knot_edns_alignment_size(answer->size, knot_rrset_size(opt_rr), padding),
+       if (padding >= 2) {
+               int32_t max_pad_bytes = knot_edns_get_payload(opt_rr) - (answer->size + knot_rrset_size(opt_rr));
+               pad_bytes = MIN(knot_edns_alignment_size(answer->size, knot_rrset_size(opt_rr), padding),
                                max_pad_bytes);
+       }
 
        if (pad_bytes >= 0) {
                uint8_t zeros[MAX(1, pad_bytes)];

diff --git a/lib/resolve.h b/lib/resolve.h
index 8b6b9dfc44f19cf31f1b2b2dac320b1cc1dfe628..dc829e6d07474340bdf4f4f1aa39df8b73390ab8 100644 (file)
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -160,7 +160,7 @@ struct kr_context
         * module because of better access. */
        struct kr_cookie_ctx cookie_ctx;
        kr_cookie_lru_t *cache_cookie;
-       uint32_t tls_padding; /**< See net.tls_padding in ../daemon/README.rst */
+       int32_t tls_padding; /**< See net.tls_padding in ../daemon/README.rst -- -1 is "true" (default policy), 0 is "false" (no padding) */
        knot_mm_t *pool;
 };