diff options
author | Nikos Mavrogiannopoulos <nmav@gnutls.org> | 2018-08-06 15:20:34 +0000 |
---|---|---|
committer | Nikos Mavrogiannopoulos <nmav@gnutls.org> | 2018-08-06 15:20:34 +0000 |
commit | 695a2549763af25157db50f88d19ea097dd6ab8e (patch) | |
tree | f9750ab5f06d0dab88fb201821021c6e1a4c8434 | |
parent | 41ad338d2f77f57d7a75a5e9e4c94b6a7f2a022d (diff) | |
parent | dac058cc742e44a5ac29c55dbac4f1ba34206eec (diff) | |
download | gnutls-695a2549763af25157db50f88d19ea097dd6ab8e.tar.gz |
Merge branch 'tmp-handshake-interleave' into 'master'
Fix interleaved handshake handling in TLS 1.3
Closes #272
See merge request gnutls/gnutls!708
-rw-r--r-- | .gitlab-ci.yml | 1 | ||||
-rw-r--r-- | lib/buffers.c | 75 | ||||
-rw-r--r-- | lib/buffers.h | 4 | ||||
-rw-r--r-- | lib/gnutls_int.h | 1 | ||||
-rw-r--r-- | lib/mbuffers.c | 19 | ||||
-rw-r--r-- | lib/mbuffers.h | 2 | ||||
-rw-r--r-- | lib/record.c | 9 | ||||
-rw-r--r-- | tests/suite/tls-fuzzer/gnutls-nocert-tls13.json | 7 | ||||
-rw-r--r-- | tests/suite/tls-fuzzer/gnutls-nocert.json | 22 |
9 files changed, 93 insertions, 47 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index aa06a55cd5..116f600fc1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -211,6 +211,7 @@ asan.Fedora.x86_64: when: on_failure paths: - ./*.log + - fuzz/*.log - tests/*.log - tests/*/*.log - tests/suite/*/*.log diff --git a/lib/buffers.c b/lib/buffers.c index 100390b5dc..cee0d5fc59 100644 --- a/lib/buffers.c +++ b/lib/buffers.c @@ -882,7 +882,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel, { uint8_t *dataptr = NULL; /* for realloc */ size_t handshake_header_size = - HANDSHAKE_HEADER_SIZE(session), data_size; + HANDSHAKE_HEADER_SIZE(session), data_size, frag_size; /* Note: SSL2_HEADERS == 1 */ if (_mbuffer_get_udata_size(bufel) < handshake_header_size) @@ -898,7 +898,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel, && bufel->htype == GNUTLS_HANDSHAKE_CLIENT_HELLO_V2)) { handshake_header_size = SSL2_HEADERS; /* we've already read one byte */ - hsk->length = _mbuffer_get_udata_size(bufel) - handshake_header_size; /* we've read the first byte */ + frag_size = _mbuffer_get_udata_size(bufel) - handshake_header_size; /* we've read the first byte */ if (dataptr[0] != GNUTLS_HANDSHAKE_CLIENT_HELLO) return @@ -908,7 +908,7 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel, hsk->sequence = 0; hsk->start_offset = 0; - hsk->end_offset = hsk->length; + hsk->length = frag_size; } else #endif { /* TLS or DTLS handshake headers */ @@ -925,13 +925,12 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel, hsk->sequence = _gnutls_read_uint16(&dataptr[4]); hsk->start_offset = _gnutls_read_uint24(&dataptr[6]); - hsk->end_offset = - hsk->start_offset + + frag_size = _gnutls_read_uint24(&dataptr[9]); } else { hsk->sequence = 0; hsk->start_offset = 0; - hsk->end_offset = + frag_size = MIN((_mbuffer_get_udata_size(bufel) - handshake_header_size), hsk->length); } @@ -947,25 +946,25 @@ parse_handshake_header(gnutls_session_t session, mbuffer_st * bufel, } data_size = _mbuffer_get_udata_size(bufel) - handshake_header_size; - /* make the length offset */ - if (hsk->end_offset > 0) - hsk->end_offset--; + if (frag_size > 0) + hsk->end_offset = hsk->start_offset + frag_size - 1; + else + hsk->end_offset = 0; _gnutls_handshake_log ("HSK[%p]: %s (%u) was received. Length %d[%d], frag offset %d, frag length: %d, sequence: %d\n", session, _gnutls_handshake2str(hsk->htype), (unsigned) hsk->htype, (int) hsk->length, (int) data_size, - hsk->start_offset, hsk->end_offset - hsk->start_offset + 1, + hsk->start_offset, (int) frag_size, (int) hsk->sequence); hsk->header_size = handshake_header_size; memcpy(hsk->header, _mbuffer_get_udata_ptr(bufel), handshake_header_size); - if (hsk->length > 0 && (hsk->start_offset > hsk->end_offset || - hsk->end_offset - hsk->start_offset >= - data_size - || hsk->end_offset >= hsk->length)) { + if (hsk->length > 0 && (frag_size > data_size || + (frag_size > 0 && + hsk->end_offset >= hsk->length))) { return gnutls_assert_val(GNUTLS_E_UNEXPECTED_PACKET_LENGTH); } @@ -1197,7 +1196,7 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session) return GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE; if (!IS_DTLS(session)) { - ssize_t remain, append, header_size; + ssize_t append, header_size; do { if (bufel->type != GNUTLS_HANDSHAKE) @@ -1205,18 +1204,21 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session) gnutls_assert_val (GNUTLS_E_UNEXPECTED_PACKET); - /* if we have a half received message then complete it. - */ - remain = recv_buf[0].length - - recv_buf[0].data.length; - - /* this is the rest of a previous message */ - if (session->internals.handshake_recv_buffer_size > - 0 && recv_buf[0].length > 0 && remain > 0) { - if ((ssize_t) msg.size <= remain) - append = msg.size; - else - append = remain; + if (unlikely + (session->internals.handshake_recv_buffer_size == 0 && + msg.size < HANDSHAKE_HEADER_SIZE(session) && + session->internals.handshake_header_recv_buffer.byte_length < + HANDSHAKE_HEADER_SIZE(session) - msg.size)) { + bufel = _mbuffer_head_pop_first(&session->internals.record_buffer); + _mbuffer_enqueue(&session->internals.handshake_header_recv_buffer, + bufel); + break; + } else if (session->internals.handshake_recv_buffer_size > + 0 && recv_buf[0].length > recv_buf[0].data.length) { + /* this is the rest of a previous message */ + append = MIN(msg.size, + recv_buf[0].length - + recv_buf[0].data.length); ret = _gnutls_buffer_append_data(&recv_buf @@ -1231,6 +1233,25 @@ int _gnutls_parse_record_buffered_msgs(gnutls_session_t session) record_buffer, append); } else { /* received new message */ + if (unlikely + (session->internals. + handshake_header_recv_buffer.length > 0)) { + bufel = _mbuffer_head_pop_first(&session->internals. + record_buffer); + _mbuffer_enqueue(&session->internals. + handshake_header_recv_buffer, + bufel); + ret = _mbuffer_linearize_align16(&session->internals. + handshake_header_recv_buffer, + get_total_headers(session)); + if (ret < 0) + return gnutls_assert_val(ret); + bufel = _mbuffer_head_pop_first(&session->internals. + handshake_header_recv_buffer); + _mbuffer_head_push_first(&session->internals. + record_buffer, + bufel); + } ret = parse_handshake_header(session, bufel, diff --git a/lib/buffers.h b/lib/buffers.h index e03c45d40d..e76c452e43 100644 --- a/lib/buffers.h +++ b/lib/buffers.h @@ -22,6 +22,8 @@ #ifndef GNUTLS_BUFFERS_H #define GNUTLS_BUFFERS_H +#include "mbuffers.h" + #define MBUFFER_FLUSH 1 void @@ -98,6 +100,7 @@ inline static void _gnutls_handshake_recv_buffer_clear(gnutls_session_t _gnutls_handshake_buffer_clear(&session->internals. handshake_recv_buffer[i]); session->internals.handshake_recv_buffer_size = 0; + _mbuffer_head_clear(&session->internals.handshake_header_recv_buffer); } inline static void _gnutls_handshake_recv_buffer_init(gnutls_session_t @@ -109,6 +112,7 @@ inline static void _gnutls_handshake_recv_buffer_init(gnutls_session_t handshake_recv_buffer[i]); } session->internals.handshake_recv_buffer_size = 0; + _mbuffer_head_init(&session->internals.handshake_header_recv_buffer); } int _gnutls_parse_record_buffered_msgs(gnutls_session_t session); diff --git a/lib/gnutls_int.h b/lib/gnutls_int.h index fc60f7cc3e..412bb368a1 100644 --- a/lib/gnutls_int.h +++ b/lib/gnutls_int.h @@ -1097,6 +1097,7 @@ typedef struct { * protocol only. freed using _gnutls_handshake_io_buffer_clear(); */ mbuffer_head_st handshake_send_buffer; + mbuffer_head_st handshake_header_recv_buffer; handshake_buffer_st handshake_recv_buffer[MAX_HANDSHAKE_MSGS]; int handshake_recv_buffer_size; diff --git a/lib/mbuffers.c b/lib/mbuffers.c index 2bcd57e426..2e4c62a295 100644 --- a/lib/mbuffers.c +++ b/lib/mbuffers.c @@ -126,6 +126,25 @@ mbuffer_st *_mbuffer_dequeue(mbuffer_head_st * buf, mbuffer_st * bufel) return ret; } +/* Append a segment to the beginning of this buffer. + * + * Cost: O(1) + */ +void _mbuffer_head_push_first(mbuffer_head_st * buf, mbuffer_st * bufel) +{ + bufel->prev = NULL; + bufel->next = buf->head; + + buf->length++; + buf->byte_length += bufel->msg.size - bufel->mark; + + if (buf->head != NULL) + buf->head->prev = bufel; + else + buf->tail = bufel; + buf->head = bufel; +} + /* Get a reference to the first segment of the buffer and * remove it from the list. * diff --git a/lib/mbuffers.h b/lib/mbuffers.h index 034f8a3cb9..bbbe079daf 100644 --- a/lib/mbuffers.h +++ b/lib/mbuffers.h @@ -39,6 +39,8 @@ mbuffer_st *_mbuffer_head_get_first(mbuffer_head_st * buf, gnutls_datum_t * msg); mbuffer_st *_mbuffer_head_get_next(mbuffer_st * cur, gnutls_datum_t * msg); +void _mbuffer_head_push_first(mbuffer_head_st * buf, mbuffer_st * bufel); + mbuffer_st *_mbuffer_head_pop_first(mbuffer_head_st * buf); /* This is dangerous since it will replace bufel with a new diff --git a/lib/record.c b/lib/record.c index 4589765524..96bf5736a9 100644 --- a/lib/record.c +++ b/lib/record.c @@ -1190,8 +1190,15 @@ static int recv_headers(gnutls_session_t session, (session, "Received packet with illegal length: %u\n", (unsigned int) record->length); - if (record->length == 0) + if (record->length == 0) { + /* Empty, unencrypted records are always unexpected. */ + if (record_params->cipher->id == GNUTLS_CIPHER_NULL) + return + gnutls_assert_val + (GNUTLS_E_UNEXPECTED_PACKET); + return gnutls_assert_val(GNUTLS_E_DECRYPTION_FAILED); + } return gnutls_assert_val(GNUTLS_E_RECORD_OVERFLOW); } diff --git a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json index db351652bb..4b01fe4531 100644 --- a/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json +++ b/tests/suite/tls-fuzzer/gnutls-nocert-tls13.json @@ -43,12 +43,9 @@ {"name" : "test-tls13-version-negotiation.py", "arguments": ["-p", "@PORT@"]}, {"name" : "test-tls13-zero-length-data.py", - "comment" : "in these tests tlsfuzzer splits ClientHello into the first 2 bytes and the remainder, which gnutls doesn't support", + "comment" : "gnutls sends NST before receiving client Finished, that is not expected in the disabled test", "arguments": ["-p", "@PORT@", - "-e", "zero-length app data interleaved in handshake", - "-e", "zero-len app data with large padding during handshake", - "-e", "zero-len app data with large padding interleaved in handshake", - "-e", "zero-len app data with padding interleaved in handshake"]}, + "-e", "zero-len app data with large padding during handshake"]}, {"name" : "test-tls13-finished.py", "commoent" : "the disabled tests timeout very often due to slow tls-fuzzer implementation", "arguments": ["-p", "@PORT@", "-n", "5", diff --git a/tests/suite/tls-fuzzer/gnutls-nocert.json b/tests/suite/tls-fuzzer/gnutls-nocert.json index c69ecfaf7e..d99c17414b 100644 --- a/tests/suite/tls-fuzzer/gnutls-nocert.json +++ b/tests/suite/tls-fuzzer/gnutls-nocert.json @@ -17,20 +17,14 @@ "ciphers even 8199", "ciphers odd 8090", "ext padding, 16130 bytes", - "ext padding, 65367 bytes"]}, - {"name" : "test-large-hello.py", - "arguments" : - ["multiple extensions 9212", - "multiple extensions 1", - "multiple extensions 16353"]}, - {"name" : "test-large-hello.py", - "comment" : "These tests rely on fragmenting the first bytes of the handshake header. Gnutls is limited on that, and doesn't accept handshake header fragmentation.", - "arguments" : - ["sanity check - fragmented", - "fragmented, padding ext 0 bytes", - "fragmented, padding ext 65354 bytes", - "fragmented, padding ext 16213 bytes"], - "exp_pass" : false}, + "ext padding, 65367 bytes", + "multiple extensions 9212", + "multiple extensions 1", + "multiple extensions 16353", + "sanity check - fragmented", + "fragmented, padding ext 0 bytes", + "fragmented, padding ext 65354 bytes", + "fragmented, padding ext 16213 bytes"]}, {"name" : "test-ecdsa-sig-flexibility.py"}, {"name" : "test-ocsp-stapling.py", "arguments" : ["--no-status"] }, |