]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
pull user_key from DoH URI and export it into dnstap docs-ignore-doh-e-gegc2h/deployments/7169 ignore-doh-enpoint 1708/head
authorVladimír Čunát <vladimir.cunat@nic.cz>
Wed, 2 Jul 2025 08:44:44 +0000 (10:44 +0200)
committerVladimír Čunát <vladimir.cunat@nic.cz>
Thu, 10 Jul 2025 14:21:51 +0000 (16:21 +0200)
TODO: this also changes some aspects of URI refusal,
though not based on user_key so far.

daemon/http.c
daemon/lua/kres-gen-33.lua
daemon/worker.c
lib/resolve.h
modules/dnstap/dnstap.c

index 2d23990622e8223f3c01c45c97bc214afaf8a148..6ac54a7a0d75c844e173ed0456783c0add4fe487 100644 (file)
 #define HTTP_FRAME_HDLEN 9
 #define HTTP_FRAME_PADLEN 1
 
-/* accept any url path,
+/** accept only non-normal URIs,
  * otherwise only /doh and /dns-query are accepted */
-#define IGNORE_ENDPOINT
+#define DOH_IS_PRIVATE 1
 
 struct http_stream {
        int32_t id;
        kr_http_header_array_t *headers;
+       char *user_key;
 };
 
 typedef queue_t(struct http_stream) queue_http_stream;
@@ -65,6 +66,7 @@ enum http_method {
 enum http_status {
        HTTP_STATUS_OK                              = 200,
        HTTP_STATUS_BAD_REQUEST                     = 400,
+       HTTP_STATUS_FORBIDDEN                       = 403,
        HTTP_STATUS_NOT_FOUND                       = 404,
        HTTP_STATUS_PAYLOAD_TOO_LARGE               = 413,
        HTTP_STATUS_UNSUPPORTED_MEDIA_TYPE          = 415,
@@ -116,17 +118,15 @@ static inline void set_status(struct pl_http_sess_data *ctx, enum http_status st
 /*
  * Check endpoint and uri path
  */
-static int check_uri(const char* path)
+static bool check_uri(struct pl_http_sess_data *ctx, const char *path)
 {
-#ifdef IGNORE_ENDPOINT
-       return kr_ok();
-#else
        static const char *endpoints[] = {"dns-query", "doh"};
        ssize_t endpoint_len;
-       ssize_t ret;
 
-       if (!path)
-               return kr_error(EINVAL);
+       if (!path) {
+               set_status(ctx, HTTP_STATUS_BAD_REQUEST);
+               return false;
+       }
 
        char *query_mark = strstr(path, "?");
 
@@ -134,18 +134,43 @@ static int check_uri(const char* path)
        endpoint_len = (query_mark) ? query_mark - path - 1 : strlen(path) - 1;
 
        /* check endpoint */
-       ret = -1;
-       for(int i = 0; i < sizeof(endpoints)/sizeof(*endpoints); i++)
-       {
-               if (strlen(endpoints[i]) != endpoint_len)
-                       continue;
-               ret = strncmp(path + 1, endpoints[i], strlen(endpoints[i]));
-               if (!ret)
-                       break;
+       bool match_found = false;
+       for (int i = 0; !match_found && i < sizeof(endpoints) / sizeof(*endpoints); ++i) {
+               match_found = strlen(endpoints[i]) == endpoint_len
+                       && !strncmp(path + 1, endpoints[i], strlen(endpoints[i]));
        }
 
-       return (ret) ? kr_error(ENOENT) : kr_ok();
-#endif /* IGNORE_ENDPOINT */
+       if (DOH_IS_PRIVATE && match_found) { // specifically forbid normal DoH URIs
+               set_status(ctx, HTTP_STATUS_FORBIDDEN);
+               return false;
+       }
+       if (!DOH_IS_PRIVATE && !match_found) {
+               set_status(ctx, HTTP_STATUS_NOT_FOUND);
+               return false;
+       }
+       return true;
+}
+
+/** Return whether the query/URI is allowed.  For now, any query is allowed. */
+static bool check_user_key(const char *path, char **user_key)
+{
+       if (!DOH_IS_PRIVATE) {
+               *user_key = NULL;
+               return true;
+       }
+       if (path[0] != '/')
+               return false;
+       ++path;
+       const char *query_mark = strstr(path, "?");
+       if (query_mark) {
+               size_t len = query_mark - path;
+               *user_key = malloc(len + 1);
+               memcpy(*user_key, path, len);
+               (*user_key)[len] = '\0';
+       } else {
+               *user_key = strdup(path);
+       }
+       return true;
 }
 
 static kr_http_header_array_t *headers_dup(kr_http_header_array_t *src)
@@ -213,8 +238,13 @@ static int process_uri_path(struct pl_http_sess_data *ctx, const char* path, int
 
        struct http_stream stream = {
                .id = stream_id,
-               .headers = headers_dup(ctx->headers)
        };
+       if (!check_user_key(path, &stream.user_key)) {
+               wire_buf_reset(wb);
+               kr_log_info(DOH, "[%p] URI failed to pass: %s\n", (void *)ctx->h2, path);
+               return kr_error(EINVAL);
+       }
+       stream.headers = headers_dup(ctx->headers);
        queue_push(ctx->streams, stream);
 
        return kr_ok();
@@ -606,15 +636,8 @@ static int header_callback(nghttp2_session *h2, const nghttp2_frame *frame,
        }
 
        if (!strcasecmp(":path", (const char *)name)) {
-               int uri_result = check_uri((const char *)value);
-               if (uri_result == kr_error(ENOENT)) {
-                       set_status(ctx, HTTP_STATUS_NOT_FOUND);
-                       return 0;
-               } else if (uri_result < 0) {
-                       set_status(ctx, HTTP_STATUS_BAD_REQUEST);
+               if (!check_uri(ctx, (const char *)value))
                        return 0;
-               }
-
                kr_assert(ctx->uri_path == NULL);
                ctx->uri_path = malloc(sizeof(*ctx->uri_path) * (valuelen + 1));
                if (!ctx->uri_path)
@@ -686,12 +709,23 @@ static int data_chunk_recv_callback(nghttp2_session *h2, uint8_t flags, int32_t
        }
 
        if (is_first) {
+               if (!check_uri(ctx, ctx->uri_path)) {
+                       ctx->incomplete_stream = -1;
+                       return NGHTTP2_ERR_CALLBACK_FAILURE;
+                       // TODO: this gets interpreted as a non-reply, apparently (for GET it works)
+               }
                /* queue_push() should be moved: see FIXME in
                 * submit_to_wirebuffer() */
                struct http_stream stream = {
                        .id = stream_id,
-                       .headers = headers_dup(ctx->headers)
                };
+               if (!check_user_key(ctx->uri_path, &stream.user_key)) {
+                       kr_log_info(DOH, "[%p] URI failed to pass: %s\n",
+                                       (void *)ctx->h2, ctx->uri_path);
+                       ctx->incomplete_stream = -1;
+                       return NGHTTP2_ERR_CALLBACK_FAILURE;
+               }
+               stream.headers = headers_dup(ctx->headers);
                queue_push(ctx->streams, stream);
        }
 
@@ -920,6 +954,7 @@ static int pl_http_sess_deinit(struct session2 *session, void *data)
        while (queue_len(http->streams) > 0) {
                struct http_stream *stream = &queue_head(http->streams);
                http_free_headers(stream->headers);
+               free(stream->user_key);
                queue_pop(http->streams);
        }
        queue_deinit(http->streams);
@@ -1032,10 +1067,12 @@ static void pl_http_request_init(struct session2 *session,
        struct http_stream *stream = &queue_head(http->streams);
        req->qsource.stream_id = stream->id;
        if (stream->headers) {
-               // the request takes ownership of the referred-to memory
+               // the request takes ownership of the referred-to memory and user_key
                req->qsource.headers = *stream->headers;
                free(stream->headers);
        }
+       req->qsource.user_key = stream->user_key;
+       //if (stream->user_key) kr_log_notice(DEVEL, "XXX user_key=%s\n", stream->user_key);
        queue_pop(http->streams);
 }
 
index efe0d68564fb0ecece2cc7231b7deced696f55b6..dbf263201fea9e10b644290a6913a5fb01441dd8 100644 (file)
@@ -248,6 +248,7 @@ struct kr_request {
                size_t size;
                int32_t stream_id;
                kr_http_header_array_t headers;
+               const char *user_key;
        } qsource;
        struct {
                unsigned int rtt;
index f7d2532342805ba28e4b7515c3b59326d6d5b3c5..76c129a22c3c7f51f0847d5cf65f6d9f63d5a23d 100644 (file)
@@ -444,6 +444,7 @@ static void request_free(struct request_ctx *ctx)
                free(ctx->req.qsource.headers.at[i].value);
        }
        array_clear(ctx->req.qsource.headers);
+       free_const(ctx->req.qsource.user_key);
 
        /* Make sure to free XDP buffer in case it wasn't sent. */
        if (ctx->req.alloc_wire_cb) {
index 8aad3bbae349b94c2196eaa691585b25d0c232de..c6f9de19b39477cde16ae8c192be15d88180c96a 100644 (file)
@@ -256,6 +256,8 @@ struct kr_request {
                 * Note that this owns malloc-ed memory inside (outside ->pool).
                 */
                kr_http_header_array_t headers;
+               /** Key identifying the user or NULL (DoH only).  Same ownership as `headers`. */
+               const char *user_key;
        } qsource;
        struct {
                unsigned rtt;                  /**< Current upstream RTT */
index 6fcc192c62425253c3b4d7605040261da1d4233b..049d1439d12995a5f3b4fad3f496764557f72e17 100644 (file)
@@ -197,6 +197,7 @@ static int dnstap_log(kr_layer_t *ctx, enum dnstap_log_phase phase) {
                }
        }
 
+       auto_free char *user_key_buf = NULL;
        char dnstap_extra_buf[24];
        if (phase == CLIENT_QUERY_PHASE) {
                m.type = DNSTAP__MESSAGE__TYPE__CLIENT_QUERY;
@@ -233,6 +234,19 @@ static int dnstap_log(kr_layer_t *ctx, enum dnstap_log_phase phase) {
 #else
                (void)dnstap_extra_buf;
 #endif
+
+               if (req->qsource.user_key) {
+                       int len = dnstap.has_extra
+                               ? asprintf(&user_key_buf, "user_key=%s\n%s", req->qsource.user_key,
+                                               (const char *)dnstap.extra.data)
+                               : asprintf(&user_key_buf, "user_key=%s\n", req->qsource.user_key);
+                       if (len > 0) {
+                               dnstap.extra.data = (uint8_t *)user_key_buf;
+                               dnstap.extra.len = len;
+                               dnstap.has_extra = true;
+                       }
+               }
+
        } else if (phase == CLIENT_RESPONSE_PHASE) {
                m.type = DNSTAP__MESSAGE__TYPE__CLIENT_RESPONSE;
 
@@ -254,6 +268,15 @@ static int dnstap_log(kr_layer_t *ctx, enum dnstap_log_phase phase) {
                m.has_response_time_sec = true;
                m.response_time_nsec = now.tv_usec * 1000;
                m.has_response_time_nsec = true;
+
+               if (req->qsource.user_key) {
+                       int len = asprintf(&user_key_buf, "user_key=%s\n", req->qsource.user_key);
+                       if (len > 0) {
+                               dnstap.extra.data = (uint8_t *)user_key_buf;
+                               dnstap.extra.len = len;
+                               dnstap.has_extra = true;
+                       }
+               }
        }
 
        if (dnstap_dt->identity) {