From f3288092e8d74204f98c05a672b5dbbcfd4f52d7 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 22 Jul 2015 00:11:23 +0900 Subject: [PATCH] Strictly check occurrence of dynamic table size update RFC 7541 requires that dynamic table size update must occur at the beginning of the first header block, and is signaled as SETTINGS acknowledgement. This commit checks these conditions. If dynamic table size update appears other than the beginning of the first header block, it is treated as error. If SETTINGS ACK is received, and next HEADERS header block does not have dynamic table size update, it is treated as error. --- lib/includes/nghttp2/nghttp2.h | 11 ++++++++ lib/nghttp2_hd.c | 30 +++++++++++++++++++-- lib/nghttp2_hd.h | 2 ++ tests/main.c | 4 +++ tests/nghttp2_hd_test.c | 48 ++++++++++++++++++++++++++++++++++ tests/nghttp2_hd_test.h | 2 ++ 6 files changed, 95 insertions(+), 2 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index d8b1cc4e..856bfb1f 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -3777,11 +3777,22 @@ NGHTTP2_EXTERN void nghttp2_hd_inflate_del(nghttp2_hd_inflater *inflater); * The |settings_hd_table_bufsize_max| should be the value transmitted * in SETTINGS_HEADER_TABLE_SIZE. * + * This function must not be called while header block is being + * inflated. In other words, this function must be called after + * initialization of |inflater|, but before calling + * `nghttp2_hd_inflate_hd()`, or after + * `nghttp2_hd_inflate_end_headers()`. Otherwise, + * `NGHTTP2_ERR_INVALID_STATE` was returned. + * * This function returns 0 if it succeeds, or one of the following * negative error codes: * * :enum:`NGHTTP2_ERR_NOMEM` * Out of memory. + * :enum:`NGHTTP2_ERR_INVALID_STATE` + * The function is called while header block is being inflated. + * Probably, application missed to call + * `nghttp2_hd_inflate_end_headers()`. */ NGHTTP2_EXTERN int nghttp2_hd_inflate_change_table_size(nghttp2_hd_inflater *inflater, diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 83191211..07ce37e0 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -704,7 +704,7 @@ int nghttp2_hd_inflate_init(nghttp2_hd_inflater *inflater, nghttp2_mem *mem) { inflater->nv_keep = NULL; inflater->opcode = NGHTTP2_HD_OPCODE_NONE; - inflater->state = NGHTTP2_HD_STATE_OPCODE; + inflater->state = NGHTTP2_HD_STATE_INFLATE_START; rv = nghttp2_bufs_init3(&inflater->nvbufs, NGHTTP2_HD_MAX_NV / 8, 8, 1, 0, mem); @@ -1261,6 +1261,15 @@ int nghttp2_hd_deflate_change_table_size(nghttp2_hd_deflater *deflater, int nghttp2_hd_inflate_change_table_size(nghttp2_hd_inflater *inflater, size_t settings_hd_table_bufsize_max) { + switch (inflater->state) { + case NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE: + case NGHTTP2_HD_STATE_INFLATE_START: + break; + default: + return NGHTTP2_ERR_INVALID_STATE; + } + + inflater->state = NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE; inflater->settings_hd_table_bufsize_max = settings_hd_table_bufsize_max; inflater->ctx.hd_table_bufsize_max = settings_hd_table_bufsize_max; hd_context_shrink_table_size(&inflater->ctx); @@ -1951,9 +1960,25 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, for (; in != last || busy;) { busy = 0; switch (inflater->state) { + case NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE: + if ((*in & 0xe0u) != 0x20u) { + DEBUGF(fprintf(stderr, "inflatehd: header table size change was " + "expected, but saw 0x%02x as first byte", + *in)); + rv = NGHTTP2_ERR_HEADER_COMP; + goto fail; + } + /* fall through */ + case NGHTTP2_HD_STATE_INFLATE_START: case NGHTTP2_HD_STATE_OPCODE: if ((*in & 0xe0u) == 0x20u) { DEBUGF(fprintf(stderr, "inflatehd: header table size change\n")); + if (inflater->state == NGHTTP2_HD_STATE_OPCODE) { + DEBUGF(fprintf(stderr, "inflatehd: header table size change must " + "appear at the head of header block\n")); + rv = NGHTTP2_ERR_HEADER_COMP; + goto fail; + } inflater->opcode = NGHTTP2_HD_OPCODE_INDEXED; inflater->state = NGHTTP2_HD_STATE_READ_TABLE_SIZE; } else if (*in & 0x80u) { @@ -1997,7 +2022,7 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, DEBUGF(fprintf(stderr, "inflatehd: table_size=%zu\n", inflater->left)); inflater->ctx.hd_table_bufsize_max = inflater->left; hd_context_shrink_table_size(&inflater->ctx); - inflater->state = NGHTTP2_HD_STATE_OPCODE; + inflater->state = NGHTTP2_HD_STATE_INFLATE_START; break; case NGHTTP2_HD_STATE_READ_INDEX: { size_t prefixlen; @@ -2282,6 +2307,7 @@ fail: int nghttp2_hd_inflate_end_headers(nghttp2_hd_inflater *inflater) { hd_inflate_keep_free(inflater); + inflater->state = NGHTTP2_HD_STATE_INFLATE_START; return 0; } diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index bf40ab05..12177e8d 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -151,6 +151,8 @@ typedef enum { } nghttp2_hd_opcode; typedef enum { + NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE, + NGHTTP2_HD_STATE_INFLATE_START, NGHTTP2_HD_STATE_OPCODE, NGHTTP2_HD_STATE_READ_TABLE_SIZE, NGHTTP2_HD_STATE_READ_INDEX, diff --git a/tests/main.c b/tests/main.c index 7353ad74..39cb1327 100644 --- a/tests/main.c +++ b/tests/main.c @@ -326,6 +326,10 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_hd_inflate_clearall_inc) || !CU_add_test(pSuite, "hd_inflate_zero_length_huffman", test_nghttp2_hd_inflate_zero_length_huffman) || + !CU_add_test(pSuite, "hd_inflate_expect_table_size_update", + test_nghttp2_hd_inflate_expect_table_size_update) || + !CU_add_test(pSuite, "hd_inflate_unexpected_table_size_update", + test_nghttp2_hd_inflate_unexpected_table_size_update) || !CU_add_test(pSuite, "hd_ringbuf_reserve", test_nghttp2_hd_ringbuf_reserve) || !CU_add_test(pSuite, "hd_change_table_size", diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index ecd7bc11..db72e3fe 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -540,6 +540,54 @@ void test_nghttp2_hd_inflate_zero_length_huffman(void) { nghttp2_hd_inflate_free(&inflater); } +void test_nghttp2_hd_inflate_expect_table_size_update(void) { + nghttp2_hd_inflater inflater; + nghttp2_bufs bufs; + nghttp2_mem *mem; + /* Indexed Header: :method: GET */ + uint8_t data[] = {0x82}; + nva_out out; + + mem = nghttp2_mem_default(); + frame_pack_bufs_init(&bufs); + nva_out_init(&out); + + nghttp2_bufs_add(&bufs, data, sizeof(data)); + nghttp2_hd_inflate_init(&inflater, mem); + /* This will make inflater require table size update in the next + inflation. */ + nghttp2_hd_inflate_change_table_size(&inflater, 4096); + CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_bufs_free(&bufs); + nghttp2_hd_inflate_free(&inflater); +} + +void test_nghttp2_hd_inflate_unexpected_table_size_update(void) { + nghttp2_hd_inflater inflater; + nghttp2_bufs bufs; + nghttp2_mem *mem; + /* Indexed Header: :method: GET, followed by table size update. + This violates RFC 7541. */ + uint8_t data[] = {0x82, 0x20}; + nva_out out; + + mem = nghttp2_mem_default(); + frame_pack_bufs_init(&bufs); + nva_out_init(&out); + + nghttp2_bufs_add(&bufs, data, sizeof(data)); + nghttp2_hd_inflate_init(&inflater, mem); + CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_bufs_free(&bufs); + nghttp2_hd_inflate_free(&inflater); +} + void test_nghttp2_hd_ringbuf_reserve(void) { nghttp2_hd_deflater deflater; nghttp2_hd_inflater inflater; diff --git a/tests/nghttp2_hd_test.h b/tests/nghttp2_hd_test.h index e41fad73..158a98c5 100644 --- a/tests/nghttp2_hd_test.h +++ b/tests/nghttp2_hd_test.h @@ -35,6 +35,8 @@ void test_nghttp2_hd_inflate_newname_noinc(void); void test_nghttp2_hd_inflate_newname_inc(void); void test_nghttp2_hd_inflate_clearall_inc(void); void test_nghttp2_hd_inflate_zero_length_huffman(void); +void test_nghttp2_hd_inflate_expect_table_size_update(void); +void test_nghttp2_hd_inflate_unexpected_table_size_update(void); void test_nghttp2_hd_ringbuf_reserve(void); void test_nghttp2_hd_change_table_size(void); void test_nghttp2_hd_deflate_inflate(void);