From ba70904949d2f9eec160043bf9a97182b33a2b82 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 21 Jun 2018 13:30:38 +0100 Subject: [PATCH] Return SSL_ERROR_WANT_READ if SSL_shutdown() encounters handshake data In the case where we are shutdown for writing and awaiting a close_notify back from a subsequent SSL_shutdown() call we skip over handshake data that is received. This should not be treated as an error - instead it should be signalled with SSL_ERROR_WANT_READ. Reviewed-by: Bernd Edlinger Reviewed-by: Kurt Roeckx (Merged from https://github.com/openssl/openssl/pull/6340) --- ssl/record/rec_layer_s3.c | 20 +++++++++++++++----- test/sslapitest.c | 12 ++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c index 5e12c53806..e12a2178a4 100644 --- a/ssl/record/rec_layer_s3.c +++ b/ssl/record/rec_layer_s3.c @@ -1553,20 +1553,30 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, * 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 ) { + 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; + if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) { + BIO *rbio; + + if ((s->mode & SSL_MODE_AUTO_RETRY) != 0) + goto start; + + s->rwstate = SSL_READING; + rbio = SSL_get_rbio(s); + BIO_clear_retry_flags(rbio); + BIO_set_retry_read(rbio); + return -1; + } + + s->rwstate = SSL_NOTHING; return 0; } diff --git a/test/sslapitest.c b/test/sslapitest.c index d998e104d4..ec449560f4 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -5051,12 +5051,7 @@ static int test_shutdown(int tst) } /* Writing on the client after sending close_notify shouldn't be possible */ - if (!TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written)) - /* - * Writing on the server after sending close_notify shouldn't be - * possible. - */ - || !TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written))) + if (!TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written))) goto end; if (tst < 4) { @@ -5066,6 +5061,11 @@ static int test_shutdown(int tst) * yet. */ if (!TEST_int_eq(SSL_shutdown(serverssl), 0) + /* + * Writing on the server after sending close_notify shouldn't + * be possible. + */ + || !TEST_false(SSL_write_ex(serverssl, msg, sizeof(msg), &written)) || !TEST_int_eq(SSL_shutdown(clientssl), 1) || !TEST_int_eq(SSL_shutdown(serverssl), 1)) goto end; -- 2.25.1