From 21af775ce0c635176475c239ae9b7d64b1891faa Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 27 Apr 2017 00:03:20 +0900 Subject: [PATCH] Call nghttp2_on_invalid_frame_callback if altsvc validation fails --- lib/nghttp2_session.c | 34 ++++++++++++++++-- tests/nghttp2_session_test.c | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 789ba3af..d4408ab3 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3411,6 +3411,27 @@ static uint32_t get_error_code_from_lib_error_code(int lib_error_code) { } } +/* + * Calls on_invalid_frame_recv_callback if it is set to |session|. + * + * This function returns 0 if it succeeds, or one of the following + * negative error codes: + * + * NGHTTP2_ERR_CALLBACK_FAILURE + * User defined callback function fails. + */ +static int session_call_on_invalid_frame_recv_callback(nghttp2_session *session, + nghttp2_frame *frame, + int lib_error_code) { + if (session->callbacks.on_invalid_frame_recv_callback) { + if (session->callbacks.on_invalid_frame_recv_callback( + session, frame, lib_error_code, session->user_data) != 0) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } + } + return 0; +} + static int session_handle_invalid_stream2(nghttp2_session *session, int32_t stream_id, nghttp2_frame *frame, @@ -4761,11 +4782,13 @@ int nghttp2_session_on_altsvc_received(nghttp2_session *session, if (frame->hd.stream_id == 0) { if (altsvc->origin_len == 0) { - return 0; + return session_call_on_invalid_frame_recv_callback(session, frame, + NGHTTP2_ERR_PROTO); } } else { if (altsvc->origin_len > 0) { - return 0; + return session_call_on_invalid_frame_recv_callback(session, frame, + NGHTTP2_ERR_PROTO); } stream = nghttp2_session_get_stream(session, frame->hd.stream_id); @@ -4778,6 +4801,11 @@ int nghttp2_session_on_altsvc_received(nghttp2_session *session, } } + if (altsvc->field_value_len == 0) { + return session_call_on_invalid_frame_recv_callback(session, frame, + NGHTTP2_ERR_PROTO); + } + return session_call_on_frame_received(session, frame); } @@ -5940,7 +5968,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, DEBUGF("recv: origin_len=%zu\n", origin_len); - if (2 + origin_len > iframe->payloadleft) { + if (origin_len > iframe->payloadleft) { busy = 1; iframe->state = NGHTTP2_IB_FRAME_SIZE_ERROR; break; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index d828028a..28ca0ac6 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -2156,6 +2156,7 @@ void test_nghttp2_session_recv_altsvc(void) { memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.on_frame_recv_callback = on_frame_recv_callback; + callbacks.on_invalid_frame_recv_callback = on_invalid_frame_recv_callback; nghttp2_option_new(&option); nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ALTSVC); @@ -2203,6 +2204,72 @@ void test_nghttp2_session_recv_altsvc(void) { nghttp2_session_del(session); + /* zero-length value */ + nghttp2_buf_reset(&buf); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(origin) - 1, NGHTTP2_ALTSVC, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + + ud.invalid_frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(1 == ud.invalid_frame_recv_cb_called); + + nghttp2_session_del(session); + + /* non-empty origin to a stream other than 0 */ + nghttp2_buf_reset(&buf); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + open_sent_stream(session, 1); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(origin) - 1 + sizeof(field_value) - 1, + NGHTTP2_ALTSVC, NGHTTP2_FLAG_NONE, 1); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, sizeof(origin) - 1); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, origin, sizeof(origin) - 1); + buf.last = nghttp2_cpymem(buf.last, field_value, sizeof(field_value) - 1); + + ud.invalid_frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(1 == ud.invalid_frame_recv_cb_called); + + nghttp2_session_del(session); + + /* empty origin to stream 0 */ + nghttp2_buf_reset(&buf); + + nghttp2_session_client_new2(&session, &callbacks, &ud, option); + + nghttp2_frame_hd_init(&hd, 2 + sizeof(field_value) - 1, NGHTTP2_ALTSVC, + NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_pack_frame_hd(buf.last, &hd); + buf.last += NGHTTP2_FRAME_HDLEN; + nghttp2_put_uint16be(buf.last, 0); + buf.last += 2; + buf.last = nghttp2_cpymem(buf.last, field_value, sizeof(field_value) - 1); + + ud.invalid_frame_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, buf.pos, nghttp2_buf_len(&buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&buf) == rv); + CU_ASSERT(1 == ud.invalid_frame_recv_cb_called); + + nghttp2_session_del(session); + /* send large frame (16KiB) */ nghttp2_buf_reset(&buf);