From 8f73135485f376f0a08c8d54c0c0e12a5fb9a7d7 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 10 Mar 2016 01:15:15 -0500 Subject: [PATCH] Consolidate SSL_RECEIVED_SHUTDOWN checks. SSL_RECEIVED_SHUTDOWN checks in the record layer happen in two different places. Some operations (but not all) check it, and so does read_bytes. Move it to get_record. This check should be at a low-level since it is otherwise duplicated in every operation. It is also a signal which originates from around the peer's record layer, so it makes sense to check it near the same code. (This one's in get_record which is technically lower-level than read_bytes, but we're trying to get rid of read_bytes. They're very coupled functions.) Also, if we've seen a fatal alert, replay an error, not an EOF. Change-Id: Idec35c5068ddabe5b1a9145016d8f945da2421cf Reviewed-on: https://boringssl-review.googlesource.com/7436 Reviewed-by: David Benjamin --- ssl/d1_pkt.c | 16 ++++++++-------- ssl/s3_pkt.c | 15 ++++++++------- ssl/ssl_lib.c | 4 ---- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/ssl/d1_pkt.c b/ssl/d1_pkt.c index 4690486c..96418dfb 100644 --- a/ssl/d1_pkt.c +++ b/ssl/d1_pkt.c @@ -131,6 +131,14 @@ static int do_dtls1_write(SSL *ssl, int type, const uint8_t *buf, * more data is needed. */ static int dtls1_get_record(SSL *ssl) { again: + if (ssl->shutdown & SSL_RECEIVED_SHUTDOWN) { + if (ssl->s3->clean_shutdown) { + return 0; + } + OPENSSL_PUT_ERROR(SSL, SSL_R_PROTOCOL_IS_SHUTDOWN); + return -1; + } + /* Read a new packet if there is no unconsumed one. */ if (ssl_read_buffer_len(ssl) == 0) { int ret = ssl_read_buffer_extend_to(ssl, 0 /* unused */); @@ -273,14 +281,6 @@ start: /* we now have a packet which can be read and processed */ - /* If the other end has shut down, throw anything we read away (even in - * 'peek' mode) */ - if (ssl->shutdown & SSL_RECEIVED_SHUTDOWN) { - rr->length = 0; - return 0; - } - - if (type == rr->type) { /* Make sure that we are not getting application data when we * are doing a handshake for the first time. */ diff --git a/ssl/s3_pkt.c b/ssl/s3_pkt.c index d9c21d42..a034c298 100644 --- a/ssl/s3_pkt.c +++ b/ssl/s3_pkt.c @@ -133,6 +133,14 @@ static const uint8_t kMaxWarningAlerts = 4; static int ssl3_get_record(SSL *ssl) { int ret; again: + if (ssl->shutdown & SSL_RECEIVED_SHUTDOWN) { + if (ssl->s3->clean_shutdown) { + return 0; + } + OPENSSL_PUT_ERROR(SSL, SSL_R_PROTOCOL_IS_SHUTDOWN); + return -1; + } + /* Ensure the buffer is large enough to decrypt in-place. */ ret = ssl_read_buffer_extend_to(ssl, ssl_record_prefix_len(ssl)); if (ret <= 0) { @@ -393,13 +401,6 @@ start: /* we now have a packet which can be read and processed */ - /* If the other end has shut down, throw anything we read away (even in - * 'peek' mode) */ - if (ssl->shutdown & SSL_RECEIVED_SHUTDOWN) { - rr->length = 0; - return 0; - } - if (type != 0 && type == rr->type) { ssl->s3->warning_alert_count = 0; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 1713c08e..ade7120f 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -601,10 +601,6 @@ static int ssl_read_impl(SSL *ssl, void *buf, int num, int peek) { return -1; } - if (ssl->shutdown & SSL_RECEIVED_SHUTDOWN) { - return 0; - } - /* This may require multiple iterations. False Start will cause * |ssl->handshake_func| to signal success one step early, but the handshake * must be completely finished before other modes are accepted. */