From ed7fabcbc24b376bbf4b168aebcdb6f1a32cf688 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 10 Mar 2018 17:19:45 +0900 Subject: [PATCH 1/4] Add SETTINGS_ENABLE_CONNECT_PROTOCOL --- lib/includes/nghttp2/nghttp2.h | 7 ++++++- lib/nghttp2_frame.c | 5 +++++ lib/nghttp2_session.c | 28 ++++++++++++++++++++++++++++ lib/nghttp2_session.h | 1 + tests/nghttp2_session_test.c | 23 +++++++++++++++++++++++ 5 files changed, 63 insertions(+), 1 deletion(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 8c54b9c8..3fe4e2d5 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -680,7 +680,12 @@ typedef enum { /** * SETTINGS_MAX_HEADER_LIST_SIZE */ - NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE = 0x06 + NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE = 0x06, + /** + * SETTINGS_ENABLE_CONNECT_PROTOCOL + * (https://tools.ietf.org/html/draft-ietf-httpbis-h2-websockets-00) + */ + NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL = 0x08 } nghttp2_settings_id; /* Note: If we add SETTINGS, update the capacity of NGHTTP2_INBOUND_NUM_IV as well */ diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 6e33f3c2..4821de40 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -1050,6 +1050,11 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv) { break; case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE: break; + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: + if (iv[i].value != 0 && iv[i].value != 1) { + return 0; + } + break; } } return 1; diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 418ad666..31e21023 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -4361,6 +4361,9 @@ int nghttp2_session_update_local_settings(nghttp2_session *session, case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE: session->local_settings.max_header_list_size = iv[i].value; break; + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: + session->local_settings.enable_connect_protocol = iv[i].value; + break; } } @@ -4499,6 +4502,26 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, session->remote_settings.max_header_list_size = entry->value; + break; + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: + + if (entry->value != 0 && entry->value != 1) { + return session_handle_invalid_connection( + session, frame, NGHTTP2_ERR_PROTO, + "SETTINGS: invalid SETTINGS_ENABLE_CONNECT_PROTOCOL"); + } + + if (!session->server && + session->remote_settings.enable_connect_protocol && + entry->value == 0) { + return session_handle_invalid_connection( + session, frame, NGHTTP2_ERR_PROTO, + "SETTINGS: server attempted to disable " + "SETTINGS_ENABLE_CONNECT_PROTOCOL"); + } + + session->remote_settings.enable_connect_protocol = entry->value; + break; } } @@ -5250,6 +5273,7 @@ static void inbound_frame_set_settings_entry(nghttp2_inbound_frame *iframe) { case NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE: case NGHTTP2_SETTINGS_MAX_FRAME_SIZE: case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE: + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: break; default: DEBUGF("recv: unknown settings id=0x%02x\n", iv.settings_id); @@ -7360,6 +7384,8 @@ uint32_t nghttp2_session_get_remote_settings(nghttp2_session *session, return session->remote_settings.max_frame_size; case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE: return session->remote_settings.max_header_list_size; + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: + return session->remote_settings.enable_connect_protocol; } assert(0); @@ -7381,6 +7407,8 @@ uint32_t nghttp2_session_get_local_settings(nghttp2_session *session, return session->local_settings.max_frame_size; case NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE: return session->local_settings.max_header_list_size; + case NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL: + return session->local_settings.enable_connect_protocol; } assert(0); diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 5add50bc..95c2afb7 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -164,6 +164,7 @@ typedef struct { uint32_t initial_window_size; uint32_t max_frame_size; uint32_t max_header_list_size; + uint32_t enable_connect_protocol; } nghttp2_settings_storage; typedef enum { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index a7a5605b..a119d255 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3480,6 +3480,29 @@ void test_nghttp2_session_on_settings_received(void) { CU_ASSERT(NGHTTP2_STREAM_CLOSING == stream1->state); nghttp2_session_del(session); + + /* It is invalid that peer disables ENABLE_CONNECT_PROTOCOL once it + has been enabled. */ + nghttp2_session_client_new(&session, &callbacks, NULL); + + session->remote_settings.enable_connect_protocol = 1; + + iv[0].settings_id = NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL; + iv[0].value = 0; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, dup_iv(iv, 1), + 1); + + CU_ASSERT(0 == nghttp2_session_on_settings_received(session, &frame, 0)); + + nghttp2_frame_settings_free(&frame.settings, mem); + + item = nghttp2_session_get_next_ob_item(session); + + CU_ASSERT(NULL != item); + CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); + + nghttp2_session_del(session); } void test_nghttp2_session_on_push_promise_received(void) { From 33f6e90a56da48d9af9e635bed39510f2e2705ff Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 10 Mar 2018 17:20:28 +0900 Subject: [PATCH 2/4] Add NGHTTP2_TOKEN__PROTOCOL --- genlibtokenlookup.py | 1 + lib/nghttp2_hd.c | 9 +++++++++ lib/nghttp2_hd.h | 1 + 3 files changed, 11 insertions(+) diff --git a/genlibtokenlookup.py b/genlibtokenlookup.py index 267a30b4..9477d07d 100755 --- a/genlibtokenlookup.py +++ b/genlibtokenlookup.py @@ -67,6 +67,7 @@ HEADERS = [ ('keep-alive',None), ('proxy-connection', None), ('upgrade', None), + (':protocol', None), ] def to_enum_hd(k): diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 82cc23de..a61f0d47 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -271,6 +271,15 @@ static int32_t lookup_token(const uint8_t *name, size_t namelen) { break; } break; + case 9: + switch (name[8]) { + case 'l': + if (memeq(":protoco", name, 8)) { + return NGHTTP2_TOKEN__PROTOCOL; + } + break; + } + break; case 10: switch (name[9]) { case 'e': diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index c64a1f2b..14ae9807 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -111,6 +111,7 @@ typedef enum { NGHTTP2_TOKEN_KEEP_ALIVE, NGHTTP2_TOKEN_PROXY_CONNECTION, NGHTTP2_TOKEN_UPGRADE, + NGHTTP2_TOKEN__PROTOCOL, } nghttp2_token; struct nghttp2_hd_entry; From a19d8f5d31047944a29a6b865fd7fca07a71eba6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 11 Mar 2018 10:11:07 +0900 Subject: [PATCH 3/4] Deal with :protocol pseudo header --- lib/nghttp2_http.c | 40 ++++++++++++++++++++------------ lib/nghttp2_http.h | 6 +++-- lib/nghttp2_session.c | 8 +++++-- lib/nghttp2_stream.h | 3 ++- tests/nghttp2_session_test.c | 44 ++++++++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 20 deletions(-) diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index b08f8863..5ecd2d67 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -113,7 +113,7 @@ static int check_path(nghttp2_stream *stream) { } static int http_request_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv, - int trailer) { + int trailer, int connect_protocol) { if (nv->name->base[0] == ':') { if (trailer || (stream->http_flags & NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED)) { @@ -146,10 +146,6 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv, 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': @@ -162,9 +158,6 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv, } break; case NGHTTP2_TOKEN__PATH: - if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) { - return NGHTTP2_ERR_HTTP_HEADER; - } if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__PATH)) { return NGHTTP2_ERR_HTTP_HEADER; } @@ -175,9 +168,6 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv, } break; case NGHTTP2_TOKEN__SCHEME: - if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) { - return NGHTTP2_ERR_HTTP_HEADER; - } if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__SCHEME)) { return NGHTTP2_ERR_HTTP_HEADER; } @@ -186,6 +176,15 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv, stream->http_flags |= NGHTTP2_HTTP_FLAG_SCHEME_HTTP; } break; + case NGHTTP2_TOKEN__PROTOCOL: + if (!connect_protocol) { + return NGHTTP2_ERR_HTTP_HEADER; + } + + if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__PROTOCOL)) { + return NGHTTP2_ERR_HTTP_HEADER; + } + break; case NGHTTP2_TOKEN_HOST: if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG_HOST)) { return NGHTTP2_ERR_HTTP_HEADER; @@ -458,16 +457,22 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, } if (session->server || frame->hd.type == NGHTTP2_PUSH_PROMISE) { - return http_request_on_header(stream, nv, trailer); + return http_request_on_header( + stream, nv, trailer, + session->server && session->local_settings.enable_connect_protocol); } return http_response_on_header(stream, nv, trailer); } int nghttp2_http_on_request_headers(nghttp2_stream *stream, - nghttp2_frame *frame) { - if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) { - if ((stream->http_flags & NGHTTP2_HTTP_FLAG__AUTHORITY) == 0) { + nghttp2_frame *frame, + int connect_protocol) { + if (!connect_protocol && + (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT)) { + if ((stream->http_flags & + (NGHTTP2_HTTP_FLAG__SCHEME | NGHTTP2_HTTP_FLAG__PATH)) || + (stream->http_flags & NGHTTP2_HTTP_FLAG__AUTHORITY) == 0) { return -1; } stream->content_length = -1; @@ -478,6 +483,11 @@ int nghttp2_http_on_request_headers(nghttp2_stream *stream, (NGHTTP2_HTTP_FLAG__AUTHORITY | NGHTTP2_HTTP_FLAG_HOST)) == 0) { return -1; } + if ((stream->http_flags & NGHTTP2_HTTP_FLAG__PROTOCOL) && + (!connect_protocol || + (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) == 0)) { + return -1; + } if (!check_path(stream)) { return -1; } diff --git a/lib/nghttp2_http.h b/lib/nghttp2_http.h index dd057cdb..c747675f 100644 --- a/lib/nghttp2_http.h +++ b/lib/nghttp2_http.h @@ -52,11 +52,13 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream, int trailer); /* - * This function is called when request header is received. This + * This function is called when request header is received. + * |connect_protocol| is nonzero if SETTINGS_ENABLE_CONNECT_PROTOCOL + * is enabled by the local endpoint (which must be server). This * function performs validation and returns 0 if it succeeds, or -1. */ int nghttp2_http_on_request_headers(nghttp2_stream *stream, - nghttp2_frame *frame); + nghttp2_frame *frame, int connect_protocol); /* * This function is called when response header is received. This diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 31e21023..36e372bc 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3746,13 +3746,17 @@ static int session_after_header_block_received(nghttp2_session *session) { subject_stream = nghttp2_session_get_stream( session, frame->push_promise.promised_stream_id); if (subject_stream) { - rv = nghttp2_http_on_request_headers(subject_stream, frame); + rv = nghttp2_http_on_request_headers( + subject_stream, frame, + session->server && session->local_settings.enable_connect_protocol); } } else { assert(frame->hd.type == NGHTTP2_HEADERS); switch (frame->headers.cat) { case NGHTTP2_HCAT_REQUEST: - rv = nghttp2_http_on_request_headers(stream, frame); + rv = nghttp2_http_on_request_headers( + stream, frame, + session->server && session->local_settings.enable_connect_protocol); break; case NGHTTP2_HCAT_RESPONSE: case NGHTTP2_HCAT_PUSH_RESPONSE: diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index d1d5856d..fb8dc14d 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -130,7 +130,8 @@ typedef enum { /* "http" or "https" scheme */ NGHTTP2_HTTP_FLAG_SCHEME_HTTP = 1 << 13, /* set if final response is expected */ - NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 14 + NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 14, + NGHTTP2_HTTP_FLAG__PROTOCOL = 1 << 15, } nghttp2_http_flag; struct nghttp2_stream { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index a119d255..5f284112 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -10907,6 +10907,17 @@ void test_nghttp2_http_mandatory_headers(void) { const nghttp2_nv asteriskoptions2_reqnv[] = { MAKE_NV(":scheme", "https"), MAKE_NV(":authority", "localhost"), MAKE_NV(":method", "OPTIONS"), MAKE_NV(":path", "*")}; + const nghttp2_nv connectproto_reqnv[] = { + MAKE_NV(":scheme", "https"), MAKE_NV(":path", "/"), + MAKE_NV(":method", "CONNECT"), MAKE_NV(":authority", "localhost"), + MAKE_NV(":protocol", "websocket")}; + const nghttp2_nv connectprotoget_reqnv[] = { + MAKE_NV(":scheme", "https"), MAKE_NV(":path", "/"), + MAKE_NV(":method", "GET"), MAKE_NV(":authority", "localhost"), + MAKE_NV(":protocol", "websocket")}; + const nghttp2_nv connectprotonopath_reqnv[] = { + MAKE_NV(":scheme", "https"), MAKE_NV(":method", "GET"), + MAKE_NV(":authority", "localhost"), MAKE_NV(":protocol", "websocket")}; mem = nghttp2_mem_default(); @@ -11054,6 +11065,39 @@ void test_nghttp2_http_mandatory_headers(void) { asteriskoptions2_reqnv, ARRLEN(asteriskoptions2_reqnv)); + /* :protocol is not allowed unless it is enabled by the local + endpoint. */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 27, -1, + connectproto_reqnv, + ARRLEN(connectproto_reqnv)); + + nghttp2_hd_deflate_free(&deflater); + + nghttp2_session_del(session); + + /* enable SETTINGS_CONNECT_PROTOCOL */ + nghttp2_session_server_new(&session, &callbacks, &ud); + + session->local_settings.enable_connect_protocol = 1; + + nghttp2_hd_deflate_init(&deflater, mem); + + /* :protocol is allowed if SETTINGS_CONNECT_PROTOCOL is enabled by + the local endpoint. */ + check_nghttp2_http_recv_headers_ok(session, &deflater, 1, -1, + connectproto_reqnv, + ARRLEN(connectproto_reqnv)); + + /* :protocol is only allowed with CONNECT method. */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 3, -1, + connectprotoget_reqnv, + ARRLEN(connectprotoget_reqnv)); + + /* CONNECT method with :protocol requires :path. */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 5, -1, + connectprotonopath_reqnv, + ARRLEN(connectprotonopath_reqnv)); + nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session); From b80dfaa8a0e61b375b5f0f878ee1a56b7a3fbd64 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 23 Sep 2018 11:22:30 +0900 Subject: [PATCH 4/4] Adjustment for RFC 8441 --- lib/includes/nghttp2/nghttp2.h | 2 +- lib/nghttp2_http.c | 4 ++-- tests/nghttp2_session_test.c | 11 ++++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 3fe4e2d5..e7198b3d 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -683,7 +683,7 @@ typedef enum { NGHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE = 0x06, /** * SETTINGS_ENABLE_CONNECT_PROTOCOL - * (https://tools.ietf.org/html/draft-ietf-httpbis-h2-websockets-00) + * (`RFC 8441 `_) */ NGHTTP2_SETTINGS_ENABLE_CONNECT_PROTOCOL = 0x08 } nghttp2_settings_id; diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index 5ecd2d67..0c3a3712 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -484,8 +484,8 @@ int nghttp2_http_on_request_headers(nghttp2_stream *stream, return -1; } if ((stream->http_flags & NGHTTP2_HTTP_FLAG__PROTOCOL) && - (!connect_protocol || - (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) == 0)) { + ((stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) == 0 || + (stream->http_flags & NGHTTP2_HTTP_FLAG__AUTHORITY) == 0)) { return -1; } if (!check_path(stream)) { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 5f284112..956ff6f7 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -10916,8 +10916,12 @@ void test_nghttp2_http_mandatory_headers(void) { MAKE_NV(":method", "GET"), MAKE_NV(":authority", "localhost"), MAKE_NV(":protocol", "websocket")}; const nghttp2_nv connectprotonopath_reqnv[] = { - MAKE_NV(":scheme", "https"), MAKE_NV(":method", "GET"), + MAKE_NV(":scheme", "https"), MAKE_NV(":method", "CONNECT"), MAKE_NV(":authority", "localhost"), MAKE_NV(":protocol", "websocket")}; + const nghttp2_nv connectprotonoauth_reqnv[] = { + MAKE_NV(":scheme", "http"), MAKE_NV(":path", "/"), + MAKE_NV(":method", "CONNECT"), MAKE_NV("host", "localhost"), + MAKE_NV(":protocol", "websocket")}; mem = nghttp2_mem_default(); @@ -11098,6 +11102,11 @@ void test_nghttp2_http_mandatory_headers(void) { connectprotonopath_reqnv, ARRLEN(connectprotonopath_reqnv)); + /* CONNECT method with :protocol requires :authority. */ + check_nghttp2_http_recv_headers_fail(session, &deflater, 7, -1, + connectprotonoauth_reqnv, + ARRLEN(connectprotonoauth_reqnv)); + nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session);