From 4864787a1675d1b96af1d72d994305425fce417e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Je=C5=BEek?= <lukas.jezek@nic.cz>
Date: Mon, 16 Nov 2020 09:28:50 +0100
Subject: [PATCH] doh2: split POST and GET method processing
---
daemon/http.c | 97 +++++++++++++++++++++++++++++++++++++++------------
daemon/http.h | 8 +++++
2 files changed, 82 insertions(+), 23 deletions(-)
diff --git a/daemon/http.c b/daemon/http.c
index b8c150024..beccac202 100644
--- a/daemon/http.c
+++ b/daemon/http.c
@@ -166,7 +166,6 @@ static int process_uri_path(struct http_ctx *ctx, const char* path, int32_t stre
}
ctx->buf_pos += ret;
- ctx->incomplete_stream = stream_id;
queue_push(ctx->streams, stream_id);
return 0;
}
@@ -177,14 +176,39 @@ static void refuse_stream(nghttp2_session *h2, int32_t stream_id)
h2, NGHTTP2_FLAG_NONE, stream_id, NGHTTP2_REFUSED_STREAM);
}
+/*
+ * Save stream id from first header's frame.
+ *
+ * We don't support interweaving from different streams. To successfully parse
+ * multiple subsequent streams, each one must be fully received before processing
+ * a new stream.
+ */
+static int begin_headers_callback(nghttp2_session *h2, const nghttp2_frame *frame,
+ void *user_data)
+{
+ struct http_ctx *ctx = (struct http_ctx *)user_data;
+ int32_t stream_id = frame->hd.stream_id;
+
+ if (frame->hd.type != NGHTTP2_HEADERS ||
+ frame->headers.cat != NGHTTP2_HCAT_REQUEST) {
+ return 0;
+ }
+
+ if (ctx->incomplete_stream != -1) {
+ kr_log_verbose(
+ "[http] stream %d incomplete, refusing\n", ctx->incomplete_stream);
+ refuse_stream(h2, stream_id);
+ } else {
+ ctx->incomplete_stream = stream_id;
+ }
+ return 0;
+}
+
/*
* Process a received header name-value pair.
*
* In DoH, GET requests contain the base64url-encoded query in dns variable present in path.
* This variable is parsed from :path pseudoheader.
- *
- * Since we don't need any headers for POST request, avoid processing them entirely to
- * avoid potential issues if dns variable would be present in path.
*/
static int header_callback(nghttp2_session *h2, const nghttp2_frame *frame,
const uint8_t *name, size_t namelen, const uint8_t *value,
@@ -193,22 +217,35 @@ static int header_callback(nghttp2_session *h2, const nghttp2_frame *frame,
struct http_ctx *ctx = (struct http_ctx *)user_data;
int32_t stream_id = frame->hd.stream_id;
- /* If the HEADERS don't have END_STREAM set, there are some DATA frames,
- * which implies POST method. Skip header processing for POST. */
- if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0)
+
+ if (frame->hd.type != NGHTTP2_HEADERS)
+ return 0;
+
+ if (ctx->incomplete_stream != stream_id) {
+ kr_log_verbose(
+ "[http] stream %d incomplete, refusing\n", ctx->incomplete_stream);
+ refuse_stream(h2, stream_id);
return 0;
+ }
if (!strcasecmp(":path", (const char *)name)) {
- if (ctx->incomplete_stream != -1) {
- kr_log_verbose(
- "[http] stream %d incomplete, refusing\n", ctx->incomplete_stream);
- refuse_stream(h2, stream_id);
- return 0;
- }
+ ctx->uri_path = malloc(sizeof(*ctx->uri_path) * (valuelen + 1));
+ if (!ctx->uri_path)
+ return kr_error(ENOMEM);
+ memcpy(ctx->uri_path, value, valuelen);
+ ctx->uri_path[valuelen] = '\0';
+ }
- if (process_uri_path(ctx, (const char*)value, stream_id) < 0)
- refuse_stream(h2, stream_id);
+ if (!strcasecmp(":method", (const char *)name)) {
+ if (!strcasecmp("get", (const char *)value)) {
+ ctx->current_method = HTTP_METHOD_GET;
+ } else if (!strcasecmp("post", (const char *)value)) {
+ ctx->current_method = HTTP_METHOD_POST;
+ } else {
+ ctx->current_method = HTTP_METHOD_NONE;
+ }
}
+
return 0;
}
@@ -226,27 +263,30 @@ static int data_chunk_recv_callback(nghttp2_session *h2, uint8_t flags, int32_t
struct http_ctx *ctx = (struct http_ctx *)user_data;
ssize_t remaining;
ssize_t required;
+ bool is_first = queue_len(ctx->streams) == 0 || queue_tail(ctx->streams) != ctx->incomplete_stream;
- if (ctx->incomplete_stream != -1 && ctx->incomplete_stream != stream_id) {
+ if (ctx->incomplete_stream != stream_id) {
kr_log_verbose(
"[http] stream %d incomplete, refusing\n",
ctx->incomplete_stream);
refuse_stream(h2, stream_id);
+ ctx->incomplete_stream = -1;
return 0;
}
remaining = ctx->buf_size - ctx->submitted - ctx->buf_pos;
required = len;
- if (ctx->incomplete_stream == -1)
+ /* First data chunk of the new stream */
+ if (is_first)
required += sizeof(uint16_t);
if (required > remaining) {
kr_log_error("[http] insufficient space in buffer\n");
+ ctx->incomplete_stream = -1;
return NGHTTP2_ERR_CALLBACK_FAILURE;
}
- if (ctx->incomplete_stream == -1) {
- ctx->incomplete_stream = stream_id;
+ if (is_first) {
ctx->buf_pos = sizeof(uint16_t); /* Reserve 2B for dnsmsg len. */
queue_push(ctx->streams, stream_id);
}
@@ -272,7 +312,15 @@ static int on_frame_recv_callback(nghttp2_session *h2, const nghttp2_frame *fram
assert(stream_id != -1);
if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) && ctx->incomplete_stream == stream_id) {
+ if (ctx->current_method == HTTP_METHOD_GET) {
+ if (process_uri_path(ctx, ctx->uri_path, stream_id) < 0) {
+ refuse_stream(h2, stream_id);
+ }
+ free(ctx->uri_path);
+ ctx->uri_path = NULL;
+ }
ctx->incomplete_stream = -1;
+ ctx->current_method = HTTP_METHOD_NONE;
len = ctx->buf_pos - sizeof(uint16_t);
if (len <= 0 || len > KNOT_WIRE_MAX_PKTSIZE) {
@@ -336,6 +384,7 @@ struct http_ctx* http_new(struct session *session, http_send_callback send_cb)
nghttp2_session_callbacks_set_send_callback(callbacks, send_callback);
nghttp2_session_callbacks_set_send_data_callback(callbacks, send_data_callback);
nghttp2_session_callbacks_set_on_header_callback(callbacks, header_callback);
+ nghttp2_session_callbacks_set_on_begin_headers_callback(callbacks, begin_headers_callback);
nghttp2_session_callbacks_set_on_data_chunk_recv_callback(
callbacks, data_chunk_recv_callback);
nghttp2_session_callbacks_set_on_frame_recv_callback(
@@ -352,6 +401,8 @@ struct http_ctx* http_new(struct session *session, http_send_callback send_cb)
queue_init(ctx->streams);
ctx->incomplete_stream = -1;
ctx->submitted = 0;
+ ctx->current_method = HTTP_METHOD_NONE;
+ ctx->uri_path = NULL;
nghttp2_session_server_new(&ctx->h2, callbacks, ctx);
nghttp2_submit_settings(ctx->h2, NGHTTP2_FLAG_NONE,
@@ -437,13 +488,13 @@ static int http_send_response(nghttp2_session *h2, int32_t stream_id,
int ret;
const char *directive_max_age = "max-age=";
char size[MAX_DECIMAL_LENGTH(data->len)] = { 0 };
- char max_age[MAX_DECIMAL_LENGTH(data->ttl)+sizeof(directive_max_age)] = { 0 };
+ int max_age_len = MAX_DECIMAL_LENGTH(data->ttl) + strlen(directive_max_age);
+ char max_age[max_age_len];
int size_len;
- int max_age_len;
+ memset(max_age, 0, max_age_len * sizeof(*max_age));
size_len = snprintf(size, MAX_DECIMAL_LENGTH(data->len), "%zu", data->len);
- max_age_len = snprintf(max_age, MAX_DECIMAL_LENGTH(data->ttl)+sizeof(directive_max_age),
- "%s%u", directive_max_age, data->ttl);
+ max_age_len = snprintf(max_age, max_age_len, "%s%u", directive_max_age, data->ttl);
nghttp2_nv hdrs[] = {
MAKE_STATIC_NV(":status", "200"),
diff --git a/daemon/http.h b/daemon/http.h
index 367181efe..f0662779b 100644
--- a/daemon/http.h
+++ b/daemon/http.h
@@ -26,6 +26,12 @@ typedef ssize_t(*http_send_callback)(const uint8_t *buffer,
typedef queue_t(int32_t) queue_int32_t;
+typedef enum {
+ HTTP_METHOD_NONE = 0,
+ HTTP_METHOD_GET = 1,
+ HTTP_METHOD_POST = 2,
+} http_method_t;
+
struct http_ctx {
struct nghttp2_session *h2;
http_send_callback send_cb;
@@ -33,6 +39,8 @@ struct http_ctx {
queue_int32_t streams; /* IDs of streams present in the buffer. */
int32_t incomplete_stream;
ssize_t submitted;
+ http_method_t current_method;
+ char *uri_path;
uint8_t *buf; /* Part of the wire_buf that belongs to current HTTP/2 stream. */
ssize_t buf_pos;
ssize_t buf_size;
--
GitLab