diff -up openssl-1.1.1-pre8/crypto/err/openssl.txt.bidi-shutdown openssl-1.1.1-pre8/crypto/err/openssl.txt --- openssl-1.1.1-pre8/crypto/err/openssl.txt.bidi-shutdown 2018-06-20 16:48:10.000000000 +0200 +++ openssl-1.1.1-pre8/crypto/err/openssl.txt 2018-08-13 15:43:07.248438031 +0200 @@ -2541,6 +2541,8 @@ SM2_R_INVALID_ENCODING:104:invalid encod SM2_R_INVALID_FIELD:105:invalid field SM2_R_NO_PARAMETERS_SET:109:no parameters set SM2_R_USER_ID_TOO_LARGE:106:user id too large +SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY:291:\ + application data after close notify SSL_R_APP_DATA_IN_HANDSHAKE:100:app data in handshake SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT:272:\ attempt to reuse session in different context diff -up openssl-1.1.1-pre8/include/openssl/sslerr.h.bidi-shutdown openssl-1.1.1-pre8/include/openssl/sslerr.h --- openssl-1.1.1-pre8/include/openssl/sslerr.h.bidi-shutdown 2018-06-20 16:48:13.000000000 +0200 +++ openssl-1.1.1-pre8/include/openssl/sslerr.h 2018-08-13 15:43:07.250438079 +0200 @@ -446,6 +446,7 @@ int ERR_load_SSL_strings(void); /* * SSL reason codes. */ +# define SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY 291 # define SSL_R_APP_DATA_IN_HANDSHAKE 100 # define SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT 272 # define SSL_R_AT_LEAST_TLS_1_0_NEEDED_IN_FIPS_MODE 143 diff -up openssl-1.1.1-pre8/ssl/record/rec_layer_s3.c.bidi-shutdown openssl-1.1.1-pre8/ssl/record/rec_layer_s3.c --- openssl-1.1.1-pre8/ssl/record/rec_layer_s3.c.bidi-shutdown 2018-06-20 16:48:13.000000000 +0200 +++ openssl-1.1.1-pre8/ssl/record/rec_layer_s3.c 2018-08-13 15:43:07.250438079 +0200 @@ -1457,40 +1457,6 @@ int ssl3_read_bytes(SSL *s, int type, in 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,12 +1549,70 @@ int ssl3_read_bytes(SSL *s, int type, in return -1; } - if (s->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a - * shutdown */ - s->rwstate = SSL_NOTHING; + /* + * 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. + */ SSL3_RECORD_set_length(rr, 0); SSL3_RECORD_set_read(rr); - return 0; + + 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); + } else { + /* + * The peer is continuing to send application data, but we have + * already sent close_notify. If this was expected we should have + * been called via SSL_read() and this would have been handled + * above. + * No alert sent because we already sent close_notify + */ + SSLfatal(s, SSL_AD_NO_ALERT, SSL_F_SSL3_READ_BYTES, + SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY); + } + return -1; + } + + /* + * 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) { diff -up openssl-1.1.1-pre8/ssl/ssl_err.c.bidi-shutdown openssl-1.1.1-pre8/ssl/ssl_err.c --- openssl-1.1.1-pre8/ssl/ssl_err.c.bidi-shutdown 2018-06-20 16:48:13.000000000 +0200 +++ openssl-1.1.1-pre8/ssl/ssl_err.c 2018-08-13 15:43:07.251438103 +0200 @@ -720,6 +720,8 @@ static const ERR_STRING_DATA SSL_str_fun }; static const ERR_STRING_DATA SSL_str_reasons[] = { + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY), + "application data after close notify"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_APP_DATA_IN_HANDSHAKE), "app data in handshake"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT), diff -up openssl-1.1.1-pre8/test/sslapitest.c.bidi-shutdown openssl-1.1.1-pre8/test/sslapitest.c --- openssl-1.1.1-pre8/test/sslapitest.c.bidi-shutdown 2018-06-20 16:48:15.000000000 +0200 +++ openssl-1.1.1-pre8/test/sslapitest.c 2018-08-13 15:43:07.252438127 +0200 @@ -4971,6 +4971,135 @@ static int test_ticket_callbacks(int tst return testresult; } +/* + * Test bi-directional shutdown. + * Test 0: TLSv1.2 + * Test 1: TLSv1.2, server continues to read/write after client shutdown + * Test 2: TLSv1.3, no pending NewSessionTicket messages + * Test 3: TLSv1.3, pending NewSessionTicket messages + * Test 4: TLSv1.3, server continues to read/write after client shutdown, client + * reads it + * Test 5: TLSv1.3, server continues to read/write after client shutdown, client + * doesn't read it + */ +static int test_shutdown(int tst) +{ + SSL_CTX *cctx = NULL, *sctx = NULL; + SSL *clientssl = NULL, *serverssl = NULL; + int testresult = 0; + char msg[] = "A test message"; + char buf[80]; + size_t written, readbytes; + +#ifdef OPENSSL_NO_TLS1_2 + if (tst == 0) + return 1; +#endif +#ifdef OPENSSL_NO_TLS1_3 + if (tst != 0) + return 1; +#endif + + if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(), + TLS_client_method(), + TLS1_VERSION, + (tst <= 1) ? TLS1_2_VERSION + : TLS1_3_VERSION, + &sctx, &cctx, cert, privkey)) + || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, + NULL, NULL))) + goto end; + + if (tst == 3) { + if (!TEST_true(create_bare_ssl_connection(serverssl, clientssl, + SSL_ERROR_NONE))) + goto end; + } else if (!TEST_true(create_ssl_connection(serverssl, clientssl, + SSL_ERROR_NONE))) { + goto end; + } + + if (!TEST_int_eq(SSL_shutdown(clientssl), 0)) + goto end; + + if (tst >= 4) { + /* + * Reading on the server after the client has sent close_notify should + * fail and provide SSL_ERROR_ZERO_RETURN + */ + if (!TEST_false(SSL_read_ex(serverssl, buf, sizeof(buf), &readbytes)) + || !TEST_int_eq(SSL_get_error(serverssl, 0), + SSL_ERROR_ZERO_RETURN) + || !TEST_int_eq(SSL_get_shutdown(serverssl), + SSL_RECEIVED_SHUTDOWN) + /* + * Even though we're shutdown on receive we should still be + * able to write. + */ + || !TEST_true(SSL_write(serverssl, msg, sizeof(msg))) + || !TEST_int_eq(SSL_shutdown(serverssl), 1)) + goto end; + if (tst == 4) { + /* Should still be able to read data from server */ + if (!TEST_true(SSL_read_ex(clientssl, buf, sizeof(buf), + &readbytes)) + || !TEST_size_t_eq(readbytes, sizeof(msg)) + || !TEST_int_eq(memcmp(msg, buf, readbytes), 0)) + goto end; + } + } + + /* Writing on the client after sending close_notify shouldn't be possible */ + if (!TEST_false(SSL_write_ex(clientssl, msg, sizeof(msg), &written))) + goto end; + + if (tst < 4) { + /* + * For these tests the client has sent close_notify but it has not yet + * been received by the server. The server has not sent close_notify + * 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; + } else if (tst == 4) { + /* + * In this test the client has sent close_notify and it has been + * received by the server which has responded with a close_notify. The + * client needs to read the close_notify sent by the server. + */ + if (!TEST_int_eq(SSL_shutdown(clientssl), 1)) + goto end; + } else { + /* + * tst == 5 + * + * The client has sent close_notify and is expecting a close_notify + * back, but instead there is application data first. The shutdown + * should fail with a fatal error. + */ + if (!TEST_int_eq(SSL_shutdown(clientssl), -1) + || !TEST_int_eq(SSL_get_error(clientssl, -1), SSL_ERROR_SSL)) + goto end; + } + + testresult = 1; + + end: + SSL_free(serverssl); + SSL_free(clientssl); + SSL_CTX_free(sctx); + SSL_CTX_free(cctx); + + return testresult; +} + int setup_tests(void) { if (!TEST_ptr(cert = test_get_argument(0)) @@ -5068,6 +5197,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_ssl_pending, 2); ADD_ALL_TESTS(test_ssl_get_shared_ciphers, OSSL_NELEM(shared_ciphers_data)); ADD_ALL_TESTS(test_ticket_callbacks, 12); + ADD_ALL_TESTS(test_shutdown, 6); return 1; } diff -up openssl-1.1.1-pre8/test/ssltestlib.c.bidi-shutdown openssl-1.1.1-pre8/test/ssltestlib.c --- openssl-1.1.1-pre8/test/ssltestlib.c.bidi-shutdown 2018-06-20 16:48:15.000000000 +0200 +++ openssl-1.1.1-pre8/test/ssltestlib.c 2018-08-13 15:42:22.250355791 +0200 @@ -680,12 +680,14 @@ int create_ssl_objects(SSL_CTX *serverct return 0; } -int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want) +/* + * Create an SSL connection, but does not ready any post-handshake + * NewSessionTicket messages. + */ +int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want) { - int retc = -1, rets = -1, err, abortctr = 0, i; + int retc = -1, rets = -1, err, abortctr = 0; int clienterr = 0, servererr = 0; - unsigned char buf; - size_t readbytes; int isdtls = SSL_is_dtls(serverssl); do { @@ -738,6 +740,22 @@ int create_ssl_connection(SSL *serverssl } } while (retc <=0 || rets <= 0); + return 1; +} + +/* + * Create an SSL connection including any post handshake NewSessionTicket + * messages. + */ +int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want) +{ + int i; + unsigned char buf; + size_t readbytes; + + if (!create_bare_ssl_connection(serverssl, clientssl, want)) + return 0; + /* * We attempt to read some data on the client side which we expect to fail. * This will ensure we have received the NewSessionTicket in TLSv1.3 where diff -up openssl-1.1.1-pre8/test/ssltestlib.h.bidi-shutdown openssl-1.1.1-pre8/test/ssltestlib.h --- openssl-1.1.1-pre8/test/ssltestlib.h.bidi-shutdown 2018-07-26 12:03:48.143770413 +0200 +++ openssl-1.1.1-pre8/test/ssltestlib.h 2018-08-13 15:42:22.250355791 +0200 @@ -18,6 +18,7 @@ int create_ssl_ctx_pair(const SSL_METHOD char *privkeyfile); int create_ssl_objects(SSL_CTX *serverctx, SSL_CTX *clientctx, SSL **sssl, SSL **cssl, BIO *s_to_c_fbio, BIO *c_to_s_fbio); +int create_bare_ssl_connection(SSL *serverssl, SSL *clientssl, int want); int create_ssl_connection(SSL *serverssl, SSL *clientssl, int want); void shutdown_ssl_connection(SSL *serverssl, SSL *clientssl);