diff --git a/ssl/handshake.cc b/ssl/handshake.cc index a03b1401..b54a0bb2 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc @@ -427,7 +427,7 @@ int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) { // Resolve the operation the handshake was waiting on. switch (hs->wait) { case ssl_hs_error: - OPENSSL_PUT_ERROR(SSL, SSL_R_SSL_HANDSHAKE_FAILURE); + ERR_restore_state(hs->error.get()); return -1; case ssl_hs_flush: { @@ -531,8 +531,7 @@ int ssl_run_handshake(SSL_HANDSHAKE *hs, bool *out_early_return) { // Run the state machine again. hs->wait = ssl->do_handshake(hs); if (hs->wait == ssl_hs_error) { - // Don't loop around to avoid a stray |SSL_R_SSL_HANDSHAKE_FAILURE| the - // first time around. + hs->error.reset(ERR_save_state()); return -1; } if (hs->wait == ssl_hs_ok) { diff --git a/ssl/internal.h b/ssl/internal.h index 4247425e..32199aa5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -158,6 +158,7 @@ #include #include +#include "../crypto/err/internal.h" #include "../crypto/internal.h" @@ -1292,6 +1293,9 @@ struct SSL_HANDSHAKE { // TLS 1.3. uint16_t retry_group = 0; + // error, if |wait| is |ssl_hs_error|, is the error the handshake failed on. + UniquePtr error; + // key_share is the current key exchange instance. UniquePtr key_share; diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index f9cf83c4..9901ce5d 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -3760,6 +3760,47 @@ TEST_P(SSLVersionTest, SessionVersion) { EXPECT_EQ(version(), SSL_SESSION_get_protocol_version(session.get())); } +// Test that a handshake-level errors are sticky. +TEST_P(SSLVersionTest, StickyErrorHandshake_ParseClientHello) { + UniquePtr ctx = CreateContext(); + ASSERT_TRUE(ctx); + UniquePtr ssl(SSL_new(ctx.get())); + ASSERT_TRUE(ssl); + SSL_set_accept_state(ssl.get()); + + // Pass in an empty ClientHello. + if (is_dtls()) { + static const uint8_t kBadClientHello[] = { + 0x16, 0xfe, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x0c, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; + SSL_set0_rbio(ssl.get(), + BIO_new_mem_buf(kBadClientHello, sizeof(kBadClientHello))); + } else { + static const uint8_t kBadClientHello[] = {0x16, 0x03, 0x01, 0x00, 0x04, + 0x01, 0x00, 0x00, 0x00}; + SSL_set0_rbio(ssl.get(), + BIO_new_mem_buf(kBadClientHello, sizeof(kBadClientHello))); + } + + SSL_set0_wbio(ssl.get(), BIO_new(BIO_s_mem())); + + // The handshake logic should reject the ClientHello. + int ret = SSL_do_handshake(ssl.get()); + EXPECT_NE(1, ret); + EXPECT_EQ(SSL_ERROR_SSL, SSL_get_error(ssl.get(), ret)); + EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(ERR_peek_error())); + EXPECT_EQ(SSL_R_DECODE_ERROR, ERR_GET_REASON(ERR_peek_error())); + ERR_clear_error(); + + // If we drive the handshake again, the error is replayed. + ret = SSL_do_handshake(ssl.get()); + EXPECT_NE(1, ret); + EXPECT_EQ(SSL_ERROR_SSL, SSL_get_error(ssl.get(), ret)); + EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(ERR_peek_error())); + EXPECT_EQ(SSL_R_DECODE_ERROR, ERR_GET_REASON(ERR_peek_error())); +} + // TODO(davidben): Convert this file to GTest properly. TEST(SSLTest, AllTests) { if (!TestSSL_SESSIONEncoding(kOpenSSLSession) ||