From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Wed, 2 Jul 2025 08:44:44 +0000 (+0200)
Subject: pull user_key from DoH URI and export it into dnstap
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2Fignore-doh-enpoint;p=thirdparty%2Fknot-resolver.git

pull user_key from DoH URI and export it into dnstap

TODO: this also changes some aspects of URI refusal,
though not based on user_key so far.
---

diff --git a/daemon/http.c b/daemon/http.c
index 2d2399062..6ac54a7a0 100644
--- a/daemon/http.c
+++ b/daemon/http.c
@@ -39,13 +39,14 @@
 #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);
 }
 
diff --git a/daemon/lua/kres-gen-33.lua b/daemon/lua/kres-gen-33.lua
index efe0d6856..dbf263201 100644
--- a/daemon/lua/kres-gen-33.lua
+++ b/daemon/lua/kres-gen-33.lua
@@ -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;
diff --git a/daemon/worker.c b/daemon/worker.c
index f7d253234..76c129a22 100644
--- a/daemon/worker.c
+++ b/daemon/worker.c
@@ -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) {
diff --git a/lib/resolve.h b/lib/resolve.h
index 8aad3bbae..c6f9de19b 100644
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -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 */
diff --git a/modules/dnstap/dnstap.c b/modules/dnstap/dnstap.c
index 6fcc192c6..049d1439d 100644
--- a/modules/dnstap/dnstap.c
+++ b/modules/dnstap/dnstap.c
@@ -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) {