From ef913bc929c9922764f885b5bf85828784a48554 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Mar 2015 21:52:57 +0900 Subject: [PATCH] Validate :path header field For "http" or "https" URIs, :path header field must start with "/". The only exception is OPTIONS method, which can contain "*" to represent system-wide OPTIONS request. --- lib/nghttp2_http.c | 72 ++++++++++++++++++++++++++++-------- lib/nghttp2_stream.h | 16 ++++++-- tests/nghttp2_session_test.c | 71 +++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 18 deletions(-) diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index 70ccbd3d..1cb0c1a8 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -225,6 +225,18 @@ static int expect_response_body(nghttp2_stream *stream) { stream->status_code != 204; } +/* For "http" or "https" URIs, OPTIONS request may have "*" in :path + header field to represent system-wide OPTIONS request. Otherwise, + :path header field value must start with "/". This function must + be called after ":method" header field was received. This function + returns nonzero if path is valid.*/ +static int check_path(nghttp2_stream *stream) { + return (stream->http_flags & NGHTTP2_HTTP_FLAG_SCHEME_HTTP) == 0 || + ((stream->http_flags & NGHTTP2_HTTP_FLAG_PATH_REGULAR) || + ((stream->http_flags & NGHTTP2_HTTP_FLAG_METH_OPTIONS) && + (stream->http_flags & NGHTTP2_HTTP_FLAG_PATH_ASTERISK))); +} + static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, int trailer) { int token; @@ -248,18 +260,34 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__METHOD)) { return NGHTTP2_ERR_HTTP_HEADER; } - if (streq("HEAD", nv->value, nv->valuelen)) { - stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD; - } else if (streq("CONNECT", nv->value, nv->valuelen)) { - if (stream->stream_id % 2 == 0) { - /* we won't allow CONNECT for push */ - return NGHTTP2_ERR_HTTP_HEADER; + switch (nv->valuelen) { + case 4: + if (streq("HEAD", nv->value, nv->valuelen)) { + stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD; } - stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_CONNECT; - if (stream->http_flags & - (NGHTTP2_HTTP_FLAG__PATH | NGHTTP2_HTTP_FLAG__SCHEME)) { - return NGHTTP2_ERR_HTTP_HEADER; + break; + case 7: + switch (nv->value[6]) { + case 'T': + if (streq("CONNECT", nv->value, nv->valuelen)) { + if (stream->stream_id % 2 == 0) { + /* we won't allow CONNECT for push */ + return NGHTTP2_ERR_HTTP_HEADER; + } + stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_CONNECT; + if (stream->http_flags & + (NGHTTP2_HTTP_FLAG__PATH | NGHTTP2_HTTP_FLAG__SCHEME)) { + return NGHTTP2_ERR_HTTP_HEADER; + } + } + break; + case 'S': + if (streq("OPTIONS", nv->value, nv->valuelen)) { + stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_OPTIONS; + } + break; } + break; } break; case NGHTTP2_TOKEN__PATH: @@ -269,6 +297,11 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__PATH)) { return NGHTTP2_ERR_HTTP_HEADER; } + if (nv->value[0] == '/') { + stream->http_flags |= NGHTTP2_HTTP_FLAG_PATH_REGULAR; + } else if (nv->valuelen == 1 && nv->value[0] == '*') { + stream->http_flags |= NGHTTP2_HTTP_FLAG_PATH_ASTERISK; + } break; case NGHTTP2_TOKEN__SCHEME: if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) { @@ -277,6 +310,10 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv, if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__SCHEME)) { return NGHTTP2_ERR_HTTP_HEADER; } + if ((nv->valuelen == 4 && memieq("http", nv->value, 4)) || + (nv->valuelen == 5 && memieq("https", nv->value, 5))) { + stream->http_flags |= NGHTTP2_HTTP_FLAG_SCHEME_HTTP; + } break; case NGHTTP2_TOKEN_HOST: if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG_HOST)) { @@ -434,11 +471,16 @@ int nghttp2_http_on_request_headers(nghttp2_stream *stream, return -1; } stream->content_length = -1; - } else if ((stream->http_flags & NGHTTP2_HTTP_FLAG_REQ_HEADERS) != - NGHTTP2_HTTP_FLAG_REQ_HEADERS || - (stream->http_flags & - (NGHTTP2_HTTP_FLAG__AUTHORITY | NGHTTP2_HTTP_FLAG_HOST)) == 0) { - return -1; + } else { + if ((stream->http_flags & NGHTTP2_HTTP_FLAG_REQ_HEADERS) != + NGHTTP2_HTTP_FLAG_REQ_HEADERS || + (stream->http_flags & + (NGHTTP2_HTTP_FLAG__AUTHORITY | NGHTTP2_HTTP_FLAG_HOST)) == 0) { + return -1; + } + if (!check_path(stream)) { + return -1; + } } if (frame->hd.type == NGHTTP2_PUSH_PROMISE) { diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index c47d8df4..8c4ec78f 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -120,10 +120,20 @@ typedef enum { /* HTTP method flags */ NGHTTP2_HTTP_FLAG_METH_CONNECT = 1 << 7, NGHTTP2_HTTP_FLAG_METH_HEAD = 1 << 8, - NGHTTP2_HTTP_FLAG_METH_ALL = - NGHTTP2_HTTP_FLAG_METH_CONNECT | NGHTTP2_HTTP_FLAG_METH_HEAD, + NGHTTP2_HTTP_FLAG_METH_OPTIONS = 1 << 9, + NGHTTP2_HTTP_FLAG_METH_ALL = NGHTTP2_HTTP_FLAG_METH_CONNECT | + NGHTTP2_HTTP_FLAG_METH_HEAD | + NGHTTP2_HTTP_FLAG_METH_OPTIONS, + /* :path category */ + /* path starts with "/" */ + NGHTTP2_HTTP_FLAG_PATH_REGULAR = 1 << 10, + /* path "*" */ + NGHTTP2_HTTP_FLAG_PATH_ASTERISK = 1 << 11, + /* scheme */ + /* "http" or "https" scheme */ + NGHTTP2_HTTP_FLAG_SCHEME_HTTP = 1 << 12, /* set if final response is expected */ - NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 9, + NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 13, } nghttp2_http_flag; typedef enum { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index f1debab9..040430da 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -6990,6 +6990,40 @@ static void check_nghttp2_http_recv_headers_fail( nghttp2_bufs_free(&bufs); } +static void check_nghttp2_http_recv_headers_ok( + nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id, + int stream_state, const nghttp2_nv *nva, size_t nvlen) { + nghttp2_mem *mem; + ssize_t rv; + nghttp2_bufs bufs; + my_user_data *ud; + + mem = nghttp2_mem_default(); + frame_pack_bufs_init(&bufs); + + ud = session->user_data; + + if (stream_state != -1) { + nghttp2_session_open_stream(session, stream_id, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, stream_state, NULL); + } + + rv = pack_headers(&bufs, deflater, stream_id, NGHTTP2_FLAG_END_HEADERS, nva, + nvlen, mem); + CU_ASSERT(0 == rv); + + ud->frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos, + nghttp2_buf_len(&bufs.head->buf)); + + CU_ASSERT(nghttp2_buf_len(&bufs.head->buf) == rv); + CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session)); + CU_ASSERT(1 == ud->frame_recv_cb_called); + + nghttp2_bufs_free(&bufs); +} + void test_nghttp2_http_mandatory_headers(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; @@ -7046,11 +7080,24 @@ void test_nghttp2_http_mandatory_headers(void) { MAKE_NV(":scheme", "https"), MAKE_NV(":method", "GET"), MAKE_NV("foo", "\x0d\x0a"), MAKE_NV(":authority", "localhost"), MAKE_NV(":path", "/")}; + const nghttp2_nv asteriskget1_reqnv[] = { + MAKE_NV(":path", "*"), MAKE_NV(":scheme", "https"), + MAKE_NV(":authority", "localhost"), MAKE_NV(":method", "GET")}; + const nghttp2_nv asteriskget2_reqnv[] = { + MAKE_NV(":scheme", "https"), MAKE_NV(":authority", "localhost"), + MAKE_NV(":method", "GET"), MAKE_NV(":path", "*")}; + const nghttp2_nv asteriskoptions1_reqnv[] = { + MAKE_NV(":path", "*"), MAKE_NV(":scheme", "https"), + MAKE_NV(":authority", "localhost"), MAKE_NV(":method", "OPTIONS")}; + const nghttp2_nv asteriskoptions2_reqnv[] = { + MAKE_NV(":scheme", "https"), MAKE_NV(":authority", "localhost"), + MAKE_NV(":method", "OPTIONS"), MAKE_NV(":path", "*")}; mem = nghttp2_mem_default(); memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.send_callback = null_send_callback; + callbacks.on_frame_recv_callback = on_frame_recv_callback; callbacks.on_invalid_frame_recv_callback = on_invalid_frame_recv_callback; nghttp2_session_client_new(&session, &callbacks, &ud); @@ -7146,6 +7193,30 @@ void test_nghttp2_http_mandatory_headers(void) { check_nghttp2_http_recv_headers_fail(session, &deflater, 17, -1, badhdbtw_reqnv, ARRLEN(badhdbtw_reqnv)); + /* request header has "*" in :path header field while method is GET. + :path is received before :method */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 19, -1, + asteriskget1_reqnv, + ARRLEN(asteriskget1_reqnv)); + + /* request header has "*" in :path header field while method is GET. + :method is received before :path */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 21, -1, + asteriskget2_reqnv, + ARRLEN(asteriskget2_reqnv)); + + /* OPTIONS method can include "*" in :path header field. :path is + received before :method. */ + check_nghttp2_http_recv_headers_ok(session, &deflater, 23, -1, + asteriskoptions1_reqnv, + ARRLEN(asteriskoptions1_reqnv)); + + /* OPTIONS method can include "*" in :path header field. :method is + received before :path. */ + check_nghttp2_http_recv_headers_ok(session, &deflater, 25, -1, + asteriskoptions2_reqnv, + ARRLEN(asteriskoptions2_reqnv)); + nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session);