diff options
| author | David Benjamin <davidben@chromium.org> | 2015-06-16 18:40:24 +0300 |
|---|---|---|
| committer | Adam Langley <agl@google.com> | 2015-06-16 21:20:56 +0300 |
| commit | 11fc66a04c100af403efb075fe3f7f6aafa87602 (patch) | |
| tree | de8d23b6a5ec131cddf4a68c032f5ba54d3644e2 /ssl/d1_both.c | |
| parent | 95d31825760c6d35e597234900e33ef48d9ab809 (diff) | |
DTLS fragments may not be split across two records.
See also upstream's 9dcab127e14467733523ff7626da8906e67eedd6. The root problem
is dtls1_read_bytes is wrong, but we can get the right behavior now and add a
regression test for it before cleaning it up.
Change-Id: I4e5c39ab254a872d9f64242c9b77b020bdded6e6
Reviewed-on: https://boringssl-review.googlesource.com/5123
Reviewed-by: Adam Langley <agl@google.com>
Diffstat (limited to 'ssl/d1_both.c')
| -rw-r--r-- | ssl/d1_both.c | 15 |
1 files changed, 7 insertions, 8 deletions
diff --git a/ssl/d1_both.c b/ssl/d1_both.c index ac35a660..f8724bac 100644 --- a/ssl/d1_both.c +++ b/ssl/d1_both.c @@ -473,11 +473,7 @@ static size_t dtls1_max_handshake_message_len(const SSL *s) { /* dtls1_process_fragment reads a handshake fragment and processes it. It * returns one if a fragment was successfully processed and 0 or -1 on error. */ static int dtls1_process_fragment(SSL *s) { - /* Read handshake message header. - * - * TODO(davidben): ssl_read_bytes allows splitting the fragment header and - * body across two records. Change this interface to consume the fragment in - * one pass. */ + /* Read handshake message header. */ uint8_t header[DTLS1_HM_HEADER_LENGTH]; int ret = dtls1_read_bytes(s, SSL3_RT_HANDSHAKE, header, DTLS1_HM_HEADER_LENGTH, 0); @@ -494,12 +490,15 @@ static int dtls1_process_fragment(SSL *s) { struct hm_header_st msg_hdr; dtls1_get_message_header(header, &msg_hdr); + /* TODO(davidben): dtls1_read_bytes is the wrong abstraction for DTLS. There + * should be no need to reach into |s->s3->rrec.length|. */ const size_t frag_off = msg_hdr.frag_off; const size_t frag_len = msg_hdr.frag_len; const size_t msg_len = msg_hdr.msg_len; if (frag_off > msg_len || frag_off + frag_len < frag_off || frag_off + frag_len > msg_len || - msg_len > dtls1_max_handshake_message_len(s)) { + msg_len > dtls1_max_handshake_message_len(s) || + frag_len > s->s3->rrec.length) { OPENSSL_PUT_ERROR(SSL, dtls1_process_fragment, SSL_R_EXCESSIVE_MESSAGE_SIZE); ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); @@ -535,8 +534,8 @@ static int dtls1_process_fragment(SSL *s) { ret = dtls1_read_bytes(s, SSL3_RT_HANDSHAKE, frag->fragment + frag_off, frag_len, 0); if (ret != frag_len) { - OPENSSL_PUT_ERROR(SSL, dtls1_process_fragment, SSL_R_UNEXPECTED_MESSAGE); - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE); + OPENSSL_PUT_ERROR(SSL, dtls1_process_fragment, ERR_R_INTERNAL_ERROR); + ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return -1; } dtls1_hm_fragment_mark(frag, frag_off, frag_off + frag_len); |