diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index d3f81a30..9c580a3f 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -341,7 +341,12 @@ typedef enum { /** * The user callback function failed. This is a fatal error. */ - NGHTTP2_ERR_CALLBACK_FAILURE = -902 + NGHTTP2_ERR_CALLBACK_FAILURE = -902, + /** + * Invalid connection preface was received and further processing is + * not possible. + */ + NGHTTP2_ERR_BAD_PREFACE = -903 } nghttp2_error; /** @@ -1736,6 +1741,24 @@ void nghttp2_option_set_no_auto_window_update(nghttp2_option *option, int val); void nghttp2_option_set_peer_max_concurrent_streams(nghttp2_option *option, uint32_t val); +/** + * @function + * + * By default, nghttp2 library only handles HTTP/2 frames and does not + * recognize first 24 bytes of client connection preface. This design + * choice is done due to the fact that server may want to detect the + * application protocol based on first few bytes on clear text + * communication. But for simple servers which only speak HTTP/2, it + * is easier for developers if nghttp2 library takes care of client + * connection preface. + * + * If this option is used with nonzero |val|, nghttp2 library checks + * first 24 bytes client connection preface. If it is not a valid + * one, `nghttp2_session_recv()` and `nghttp2_session_mem_recv()` will + * return error :enum:`NGHTTP2_ERR_BAD_PREFACE`, which is fatal error. + */ +void nghttp2_option_set_recv_client_preface(nghttp2_option *option, int val); + /** * @function * @@ -1986,6 +2009,10 @@ ssize_t nghttp2_session_mem_send(nghttp2_session *session, * Out of memory. * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` * The callback function failed. + * :enum:`NGHTTP2_ERR_BAD_PREFACE` + * Invalid client preface was detected. This error only returns + * when |session| was configured as server and + * `nghttp2_option_set_recv_client_preface()` is used. */ int nghttp2_session_recv(nghttp2_session *session); @@ -2017,6 +2044,10 @@ int nghttp2_session_recv(nghttp2_session *session); * Out of memory. * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` * The callback function failed. + * :enum:`NGHTTP2_ERR_BAD_PREFACE` + * Invalid client preface was detected. This error only returns + * when |session| was configured as server and + * `nghttp2_option_set_recv_client_preface()` is used. */ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, size_t inlen); diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c index adb59bc0..914714a1 100644 --- a/lib/nghttp2_option.c +++ b/lib/nghttp2_option.c @@ -52,3 +52,10 @@ void nghttp2_option_set_peer_max_concurrent_streams(nghttp2_option *option, option->opt_set_mask |= NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS; option->peer_max_concurrent_streams = val; } + +void nghttp2_option_set_recv_client_preface +(nghttp2_option *option, int val) +{ + option->opt_set_mask |= NGHTTP2_OPT_RECV_CLIENT_PREFACE; + option->recv_client_preface = val; +} diff --git a/lib/nghttp2_option.h b/lib/nghttp2_option.h index d5bad011..aefd87be 100644 --- a/lib/nghttp2_option.h +++ b/lib/nghttp2_option.h @@ -56,7 +56,8 @@ typedef enum { * will be overwritten if the local endpoint receives * SETTINGS_MAX_CONCURRENT_STREAMS from the remote endpoint. */ - NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 1 + NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 1, + NGHTTP2_OPT_RECV_CLIENT_PREFACE = 1 << 2, } nghttp2_option_flag; /** @@ -76,6 +77,10 @@ struct nghttp2_option { * NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE */ uint8_t no_auto_window_update; + /** + * NGHTTP2_OPT_RECV_CLIENT_PREFACE + */ + uint8_t recv_client_preface; }; #endif /* NGHTTP2_OPTION_H */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index e0711281..d156800a 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -398,6 +398,12 @@ static int session_new(nghttp2_session **session_ptr, option->peer_max_concurrent_streams; } + + if(option->opt_set_mask & NGHTTP2_OPT_RECV_CLIENT_PREFACE) { + + (*session_ptr)->opt_flags |= NGHTTP2_OPTMASK_RECV_CLIENT_PREFACE; + + } } (*session_ptr)->callbacks = *callbacks; @@ -405,6 +411,15 @@ static int session_new(nghttp2_session **session_ptr, session_inbound_frame_reset(*session_ptr); + if(server && + ((*session_ptr)->opt_flags & NGHTTP2_OPTMASK_RECV_CLIENT_PREFACE)) { + + nghttp2_inbound_frame *iframe = &(*session_ptr)->iframe; + + iframe->state = NGHTTP2_IB_READ_CLIENT_PREFACE; + iframe->payloadleft = NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN; + } + return 0; fail_aob_framebuf: @@ -4338,6 +4353,23 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, for(;;) { switch(iframe->state) { + case NGHTTP2_IB_READ_CLIENT_PREFACE: + readlen = nghttp2_min(inlen, iframe->payloadleft); + + if(memcmp(NGHTTP2_CLIENT_CONNECTION_PREFACE + + NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN - iframe->payloadleft, + in, readlen) != 0) { + return NGHTTP2_ERR_BAD_PREFACE; + } + + iframe->payloadleft -= readlen; + in += readlen; + + if(iframe->payloadleft == 0) { + session_inbound_frame_reset(session); + } + + break; case NGHTTP2_IB_READ_HEAD: { int on_begin_frame_called = 0; diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index ac60dd96..4c6943ee 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -45,6 +45,7 @@ */ typedef enum { NGHTTP2_OPTMASK_NO_AUTO_WINDOW_UPDATE = 1 << 0, + NGHTTP2_OPTMASK_RECV_CLIENT_PREFACE = 1 << 1, } nghttp2_optmask; typedef enum { @@ -65,6 +66,7 @@ typedef struct { /* Internal state when receiving incoming frame */ typedef enum { /* Receiving frame header */ + NGHTTP2_IB_READ_CLIENT_PREFACE, NGHTTP2_IB_READ_HEAD, NGHTTP2_IB_READ_NBYTE, NGHTTP2_IB_READ_HEADER_BLOCK, diff --git a/src/HttpServer.cc b/src/HttpServer.cc index e992b44a..b93e6744 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -104,6 +104,7 @@ void append_nv(Stream *stream, const std::vector& nva) Config::Config() : stream_read_timeout{60, 0}, stream_write_timeout{60, 0}, + session_option(nullptr), data_ptr(nullptr), padding(0), num_worker(1), @@ -115,7 +116,10 @@ Config::Config() no_tls(false), error_gzip(false), early_response(false) -{} +{ + nghttp2_option_new(&session_option); + nghttp2_option_set_recv_client_preface(session_option, 1); +} Stream::Stream(Http2Handler *handler, int32_t stream_id) : handler(handler), @@ -337,7 +341,6 @@ Http2Handler::Http2Handler(Sessions *sessions, settings_timerev_(nullptr), pending_data_(nullptr), pending_datalen_(0), - left_connhd_len_(NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN), fd_(fd) { nghttp2_buf_wrap_init(&sendbuf_, sendbufarray_, sizeof(sendbufarray_)); @@ -651,27 +654,6 @@ int Http2Handler::on_read() nread = rv; bufp = buf; - if(left_connhd_len_ > 0) { - auto len = std::min(left_connhd_len_, nread); - const char *conhead = NGHTTP2_CLIENT_CONNECTION_PREFACE; - - if(memcmp(conhead + NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN - - left_connhd_len_, bufp, len) != 0) { - return -1; - } - - left_connhd_len_ -= len; - nread -= len; - - if(nread == 0) { - wait_events(); - - return 0; - } - - bufp += len; - } - rv = nghttp2_session_mem_recv(session_, bufp, nread); if(rv < 0) { std::cerr << "nghttp2_session_mem_recv() returned error: " @@ -736,7 +718,8 @@ int Http2Handler::on_connect() { int r; - r = nghttp2_session_server_new(&session_, sessions_->get_callbacks(), this); + r = nghttp2_session_server_new2(&session_, sessions_->get_callbacks(), this, + sessions_->get_config()->session_option); if(r != 0) { return r; } @@ -947,16 +930,6 @@ const Config* Http2Handler::get_config() const return sessions_->get_config(); } -size_t Http2Handler::get_left_connhd_len() const -{ - return left_connhd_len_; -} - -void Http2Handler::set_left_connhd_len(size_t left) -{ - left_connhd_len_ = left; -} - void Http2Handler::remove_settings_timer() { if(settings_timerev_) { diff --git a/src/HttpServer.h b/src/HttpServer.h index d7e3fa70..09b63975 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -66,6 +66,7 @@ struct Config { std::string dh_param_file; timeval stream_read_timeout; timeval stream_write_timeout; + nghttp2_option *session_option; void *data_ptr; size_t padding; size_t num_worker; @@ -138,8 +139,6 @@ public: int64_t session_id() const; Sessions* get_sessions() const; const Config* get_config() const; - size_t get_left_connhd_len() const; - void set_left_connhd_len(size_t left); void remove_settings_timer(); void terminate_session(uint32_t error_code); int tls_handshake(); @@ -159,7 +158,6 @@ private: event *settings_timerev_; const uint8_t *pending_data_; size_t pending_datalen_; - size_t left_connhd_len_; int fd_; uint8_t sendbufarray_[65536]; }; diff --git a/tests/main.c b/tests/main.c index 4338ec04..1c8f3e6f 100644 --- a/tests/main.c +++ b/tests/main.c @@ -243,6 +243,8 @@ int main(int argc, char* argv[]) test_nghttp2_session_graceful_shutdown) || !CU_add_test(pSuite, "session_on_header_temporal_failure", test_nghttp2_session_on_header_temporal_failure) || + !CU_add_test(pSuite, "session_recv_client_preface", + test_nghttp2_session_recv_client_preface) || !CU_add_test(pSuite, "frame_pack_headers", test_nghttp2_frame_pack_headers) || !CU_add_test(pSuite, "frame_pack_headers_frame_too_large", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index b11930fc..9bca793c 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -6400,3 +6400,52 @@ void test_nghttp2_session_on_header_temporal_failure(void) nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session); } + +void test_nghttp2_session_recv_client_preface(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_option *option; + ssize_t rv; + + memset(&callbacks, 0, sizeof(callbacks)); + + nghttp2_option_new(&option); + nghttp2_option_set_recv_client_preface(option, 1); + + /* Check success case */ + nghttp2_session_server_new2(&session, &callbacks, NULL, option); + + CU_ASSERT(session->opt_flags & NGHTTP2_OPTMASK_RECV_CLIENT_PREFACE); + + rv = nghttp2_session_mem_recv + (session, + (const uint8_t*)NGHTTP2_CLIENT_CONNECTION_PREFACE, + NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN); + + CU_ASSERT(rv == NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN); + CU_ASSERT(NGHTTP2_IB_READ_HEAD == session->iframe.state); + + nghttp2_session_del(session); + + /* Check bad case */ + nghttp2_session_server_new2(&session, &callbacks, NULL, option); + + /* Feed preface with one byte less */ + rv = nghttp2_session_mem_recv + (session, + (const uint8_t*)NGHTTP2_CLIENT_CONNECTION_PREFACE, + NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN - 1); + + CU_ASSERT(rv == NGHTTP2_CLIENT_CONNECTION_PREFACE_LEN - 1); + CU_ASSERT(NGHTTP2_IB_READ_CLIENT_PREFACE == session->iframe.state); + CU_ASSERT(1 == session->iframe.payloadleft); + + rv = nghttp2_session_mem_recv(session, (const uint8_t*)"\0", 1); + + CU_ASSERT(NGHTTP2_ERR_BAD_PREFACE == rv); + + nghttp2_session_del(session); + + nghttp2_option_del(option); +} diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 4b1be44b..af54d3ac 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -113,5 +113,6 @@ void test_nghttp2_session_stream_attach_data_subtree(void); void test_nghttp2_session_keep_closed_stream(void); void test_nghttp2_session_graceful_shutdown(void); void test_nghttp2_session_on_header_temporal_failure(void); +void test_nghttp2_session_recv_client_preface(void); #endif /* NGHTTP2_SESSION_TEST_H */