From 93f528f36eb9423c31b2d75669cea85da97f9633 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 23 May 2018 12:00:10 +0100 Subject: [PATCH] Auto retry if we ditch records during shutdown If we've sent a close_notify and we're waiting for one back we drop incoming records until we see the close_notify we're looking for. If SSL_MODE_AUTO_RETRY is on, then we should immediately try and read the next record. Fixes #6262 Reviewed-by: Bernd Edlinger Reviewed-by: Kurt Roeckx (Merged from https://github.com/openssl/openssl/pull/6340) --- ssl/record/rec_layer_s3.c | 79 +++++++++++++++++++++------------------ 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 8d5b53fb39..5e12c53806 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1457,40 +1457,6 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, return -1; } - /* - * In case of record types for which we have 'fragment' storage, fill - * that so that we can process the data at a fixed place. - */ - { - size_t dest_maxlen = 0; - unsigned char *dest = NULL; - size_t *dest_len = NULL; - - if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) { - dest_maxlen = sizeof(s->rlayer.handshake_fragment); - dest = s->rlayer.handshake_fragment; - dest_len = &s->rlayer.handshake_fragment_len; - } - - if (dest_maxlen > 0) { - n = dest_maxlen - *dest_len; /* available space in 'dest' */ - if (SSL3_RECORD_get_length(rr) < n) - n = SSL3_RECORD_get_length(rr); /* available bytes */ - - /* now move 'n' bytes: */ - memcpy(dest + *dest_len, - SSL3_RECORD_get_data(rr) + SSL3_RECORD_get_off(rr), n); - SSL3_RECORD_add_off(rr, n); - SSL3_RECORD_sub_length(rr, n); - *dest_len += n; - if (SSL3_RECORD_get_length(rr) == 0) - SSL3_RECORD_set_read(rr); - - if (*dest_len < dest_maxlen) - goto start; /* fragment was too small */ - } - } - /*- * s->rlayer.handshake_fragment_len == 4 iff rr->type == SSL3_RT_HANDSHAKE; * (Possibly rr is 'empty' now, i.e. rr->length may be 0.) @@ -1583,14 +1549,55 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, return -1; } - if (s->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a - * shutdown */ + /* + * If we've sent a close_notify but not yet received one back then ditch + * anything we read. + */ + if ((s->shutdown & SSL_SENT_SHUTDOWN) != 0 ) { + /* + * In TLSv1.3 this could get problematic if we receive a KeyUpdate + * message after we sent a close_notify because we're about to ditch it, + * so we won't be able to read a close_notify sent afterwards! We don't + * support that. + */ s->rwstate = SSL_NOTHING; SSL3_RECORD_set_length(rr, 0); SSL3_RECORD_set_read(rr); + + if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE + && (s->mode & SSL_MODE_AUTO_RETRY) != 0) + goto start; return 0; } + /* + * For handshake data we have 'fragment' storage, so fill that so that we + * can process the header at a fixed place. This is done after the + * "SHUTDOWN" code above to avoid filling the fragment storage with data + * that we're just going to discard. + */ + if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) { + size_t dest_maxlen = sizeof(s->rlayer.handshake_fragment); + unsigned char *dest = s->rlayer.handshake_fragment; + size_t *dest_len = &s->rlayer.handshake_fragment_len; + + n = dest_maxlen - *dest_len; /* available space in 'dest' */ + if (SSL3_RECORD_get_length(rr) < n) + n = SSL3_RECORD_get_length(rr); /* available bytes */ + + /* now move 'n' bytes: */ + memcpy(dest + *dest_len, + SSL3_RECORD_get_data(rr) + SSL3_RECORD_get_off(rr), n); + SSL3_RECORD_add_off(rr, n); + SSL3_RECORD_sub_length(rr, n); + *dest_len += n; + if (SSL3_RECORD_get_length(rr) == 0) + SSL3_RECORD_set_read(rr); + + if (*dest_len < dest_maxlen) + goto start; /* fragment was too small */ + } + if (SSL3_RECORD_get_type(rr) == SSL3_RT_CHANGE_CIPHER_SPEC) { SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_READ_BYTES, SSL_R_CCS_RECEIVED_EARLY); -- 2.25.1