From e10349ab8dda8a3f11be6aa19f2b6e29fe814044 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Fri, 23 Feb 2018 11:04:39 +0100 Subject: Update NSS to 3.35-RTM --- security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc | 678 +++++++++++++++++++++- 1 file changed, 651 insertions(+), 27 deletions(-) (limited to 'security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc') diff --git a/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc b/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc index 39055f641..93e19a720 100644 --- a/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc +++ b/security/nss/gtests/ssl_gtest/ssl_hrr_unittest.cc @@ -187,6 +187,590 @@ TEST_P(TlsConnectTls13, RetryWithSameKeyShare) { EXPECT_EQ(SSL_ERROR_ILLEGAL_PARAMETER_ALERT, client_->error_code()); } +// Here we modify the second ClientHello so that the client retries with the +// same shares, even though the server wanted something else. +TEST_P(TlsConnectTls13, RetryWithTwoShares) { + EnsureTlsSetup(); + EXPECT_EQ(SECSuccess, SSL_SendAdditionalKeyShares(client_->ssl_fd(), 1)); + client_->SetPacketFilter(std::make_shared()); + + static const std::vector groups = {ssl_grp_ec_secp384r1, + ssl_grp_ec_secp521r1}; + server_->ConfigNamedGroups(groups); + ConnectExpectAlert(server_, kTlsAlertIllegalParameter); + EXPECT_EQ(SSL_ERROR_BAD_2ND_CLIENT_HELLO, server_->error_code()); + EXPECT_EQ(SSL_ERROR_ILLEGAL_PARAMETER_ALERT, client_->error_code()); +} + +TEST_P(TlsConnectTls13, RetryCallbackAccept) { + EnsureTlsSetup(); + + auto accept_hello = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) { + auto* called = reinterpret_cast(arg); + *called = true; + + EXPECT_TRUE(firstHello); + EXPECT_EQ(0U, clientTokenLen); + return ssl_hello_retry_accept; + }; + + bool cb_run = false; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + accept_hello, &cb_run)); + Connect(); + EXPECT_TRUE(cb_run); +} + +TEST_P(TlsConnectTls13, RetryCallbackAcceptGroupMismatch) { + EnsureTlsSetup(); + + auto accept_hello_twice = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, + unsigned int appTokenMax, void* arg) { + auto* called = reinterpret_cast(arg); + ++*called; + + EXPECT_EQ(0U, clientTokenLen); + return ssl_hello_retry_accept; + }; + + auto capture = std::make_shared(ssl_tls13_cookie_xtn); + capture->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(capture); + + static const std::vector groups = {ssl_grp_ec_secp384r1}; + server_->ConfigNamedGroups(groups); + + size_t cb_run = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback( + server_->ssl_fd(), accept_hello_twice, &cb_run)); + Connect(); + EXPECT_EQ(2U, cb_run); + EXPECT_TRUE(capture->captured()) << "expected a cookie in HelloRetryRequest"; +} + +TEST_P(TlsConnectTls13, RetryCallbackFail) { + EnsureTlsSetup(); + + auto fail_hello = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) { + auto* called = reinterpret_cast(arg); + *called = true; + + EXPECT_TRUE(firstHello); + EXPECT_EQ(0U, clientTokenLen); + return ssl_hello_retry_fail; + }; + + bool cb_run = false; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + fail_hello, &cb_run)); + ConnectExpectAlert(server_, kTlsAlertHandshakeFailure); + server_->CheckErrorCode(SSL_ERROR_APPLICATION_ABORT); + EXPECT_TRUE(cb_run); +} + +// Asking for retry twice isn't allowed. +TEST_P(TlsConnectTls13, RetryCallbackRequestHrrTwice) { + EnsureTlsSetup(); + + auto bad_callback = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) -> SSLHelloRetryRequestAction { + return ssl_hello_retry_request; + }; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + bad_callback, NULL)); + ConnectExpectAlert(server_, kTlsAlertInternalError); + server_->CheckErrorCode(SSL_ERROR_APP_CALLBACK_ERROR); +} + +// Accepting the CH and modifying the token isn't allowed. +TEST_P(TlsConnectTls13, RetryCallbackAcceptAndSetToken) { + EnsureTlsSetup(); + + auto bad_callback = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) -> SSLHelloRetryRequestAction { + *appTokenLen = 1; + return ssl_hello_retry_accept; + }; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + bad_callback, NULL)); + ConnectExpectAlert(server_, kTlsAlertInternalError); + server_->CheckErrorCode(SSL_ERROR_APP_CALLBACK_ERROR); +} + +// As above, but with reject. +TEST_P(TlsConnectTls13, RetryCallbackRejectAndSetToken) { + EnsureTlsSetup(); + + auto bad_callback = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) -> SSLHelloRetryRequestAction { + *appTokenLen = 1; + return ssl_hello_retry_fail; + }; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + bad_callback, NULL)); + ConnectExpectAlert(server_, kTlsAlertInternalError); + server_->CheckErrorCode(SSL_ERROR_APP_CALLBACK_ERROR); +} + +// This is a (pretend) buffer overflow. +TEST_P(TlsConnectTls13, RetryCallbackSetTooLargeToken) { + EnsureTlsSetup(); + + auto bad_callback = [](PRBool firstHello, const PRUint8* clientToken, + unsigned int clientTokenLen, PRUint8* appToken, + unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) -> SSLHelloRetryRequestAction { + *appTokenLen = appTokenMax + 1; + return ssl_hello_retry_accept; + }; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + bad_callback, NULL)); + ConnectExpectAlert(server_, kTlsAlertInternalError); + server_->CheckErrorCode(SSL_ERROR_APP_CALLBACK_ERROR); +} + +SSLHelloRetryRequestAction RetryHello(PRBool firstHello, + const PRUint8* clientToken, + unsigned int clientTokenLen, + PRUint8* appToken, + unsigned int* appTokenLen, + unsigned int appTokenMax, void* arg) { + auto* called = reinterpret_cast(arg); + ++*called; + + EXPECT_EQ(0U, clientTokenLen); + return firstHello ? ssl_hello_retry_request : ssl_hello_retry_accept; +} + +TEST_P(TlsConnectTls13, RetryCallbackRetry) { + EnsureTlsSetup(); + + auto capture_hrr = std::make_shared( + ssl_hs_hello_retry_request); + auto capture_key_share = + std::make_shared(ssl_tls13_key_share_xtn); + capture_key_share->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + std::vector> chain = {capture_hrr, + capture_key_share}; + server_->SetPacketFilter(std::make_shared(chain)); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHello, &cb_called)); + + // Do the first message exchange. + StartConnect(); + client_->Handshake(); + server_->Handshake(); + + EXPECT_EQ(1U, cb_called) << "callback should be called once here"; + EXPECT_LT(0U, capture_hrr->buffer().len()) << "HelloRetryRequest expected"; + EXPECT_FALSE(capture_key_share->captured()) + << "no key_share extension expected"; + + auto capture_cookie = + std::make_shared(ssl_tls13_cookie_xtn); + client_->SetPacketFilter(capture_cookie); + + Handshake(); + CheckConnected(); + EXPECT_EQ(2U, cb_called); + EXPECT_TRUE(capture_cookie->captured()) << "should have a cookie"; +} + +static size_t CountShares(const DataBuffer& key_share) { + size_t count = 0; + uint32_t len = 0; + size_t offset = 2; + + EXPECT_TRUE(key_share.Read(0, 2, &len)); + EXPECT_EQ(key_share.len() - 2, len); + while (offset < key_share.len()) { + offset += 2; // Skip KeyShareEntry.group + EXPECT_TRUE(key_share.Read(offset, 2, &len)); + offset += 2 + len; // Skip KeyShareEntry.key_exchange + ++count; + } + return count; +} + +TEST_P(TlsConnectTls13, RetryCallbackRetryWithAdditionalShares) { + EnsureTlsSetup(); + EXPECT_EQ(SECSuccess, SSL_SendAdditionalKeyShares(client_->ssl_fd(), 1)); + + auto capture_server = + std::make_shared(ssl_tls13_key_share_xtn); + capture_server->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(capture_server); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHello, &cb_called)); + + // Do the first message exchange. + StartConnect(); + client_->Handshake(); + server_->Handshake(); + + EXPECT_EQ(1U, cb_called) << "callback should be called once here"; + EXPECT_FALSE(capture_server->captured()) + << "no key_share extension expected from server"; + + auto capture_client_2nd = + std::make_shared(ssl_tls13_key_share_xtn); + client_->SetPacketFilter(capture_client_2nd); + + Handshake(); + CheckConnected(); + EXPECT_EQ(2U, cb_called); + EXPECT_TRUE(capture_client_2nd->captured()) << "client should send key_share"; + EXPECT_EQ(2U, CountShares(capture_client_2nd->extension())) + << "client should still send two shares"; +} + +// The callback should be run even if we have another reason to send +// HelloRetryRequest. In this case, the server sends HRR because the server +// wants a P-384 key share and the client didn't offer one. +TEST_P(TlsConnectTls13, RetryCallbackRetryWithGroupMismatch) { + EnsureTlsSetup(); + + auto capture_cookie = + std::make_shared(ssl_tls13_cookie_xtn); + capture_cookie->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + auto capture_key_share = + std::make_shared(ssl_tls13_key_share_xtn); + capture_key_share->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(std::make_shared( + ChainedPacketFilterInit{capture_cookie, capture_key_share})); + + static const std::vector groups = {ssl_grp_ec_secp384r1}; + server_->ConfigNamedGroups(groups); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHello, &cb_called)); + Connect(); + EXPECT_EQ(2U, cb_called); + EXPECT_TRUE(capture_cookie->captured()) << "cookie expected"; + EXPECT_TRUE(capture_key_share->captured()) << "key_share expected"; +} + +static const uint8_t kApplicationToken[] = {0x92, 0x44, 0x00}; + +SSLHelloRetryRequestAction RetryHelloWithToken( + PRBool firstHello, const PRUint8* clientToken, unsigned int clientTokenLen, + PRUint8* appToken, unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) { + auto* called = reinterpret_cast(arg); + ++*called; + + if (firstHello) { + memcpy(appToken, kApplicationToken, sizeof(kApplicationToken)); + *appTokenLen = sizeof(kApplicationToken); + return ssl_hello_retry_request; + } + + EXPECT_EQ(DataBuffer(kApplicationToken, sizeof(kApplicationToken)), + DataBuffer(clientToken, static_cast(clientTokenLen))); + return ssl_hello_retry_accept; +} + +TEST_P(TlsConnectTls13, RetryCallbackRetryWithToken) { + EnsureTlsSetup(); + + auto capture_key_share = + std::make_shared(ssl_tls13_key_share_xtn); + capture_key_share->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(capture_key_share); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, + SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHelloWithToken, &cb_called)); + Connect(); + EXPECT_EQ(2U, cb_called); + EXPECT_FALSE(capture_key_share->captured()) << "no key share expected"; +} + +TEST_P(TlsConnectTls13, RetryCallbackRetryWithTokenAndGroupMismatch) { + EnsureTlsSetup(); + + static const std::vector groups = {ssl_grp_ec_secp384r1}; + server_->ConfigNamedGroups(groups); + + auto capture_key_share = + std::make_shared(ssl_tls13_key_share_xtn); + capture_key_share->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(capture_key_share); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, + SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHelloWithToken, &cb_called)); + Connect(); + EXPECT_EQ(2U, cb_called); + EXPECT_TRUE(capture_key_share->captured()) << "key share expected"; +} + +SSLHelloRetryRequestAction CheckTicketToken( + PRBool firstHello, const PRUint8* clientToken, unsigned int clientTokenLen, + PRUint8* appToken, unsigned int* appTokenLen, unsigned int appTokenMax, + void* arg) { + auto* called = reinterpret_cast(arg); + *called = true; + + EXPECT_TRUE(firstHello); + EXPECT_EQ(DataBuffer(kApplicationToken, sizeof(kApplicationToken)), + DataBuffer(clientToken, static_cast(clientTokenLen))); + return ssl_hello_retry_accept; +} + +// Stream because SSL_SendSessionTicket only supports that. +TEST_F(TlsConnectStreamTls13, RetryCallbackWithSessionTicketToken) { + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + Connect(); + EXPECT_EQ(SECSuccess, + SSL_SendSessionTicket(server_->ssl_fd(), kApplicationToken, + sizeof(kApplicationToken))); + SendReceive(); + + Reset(); + ConfigureSessionCache(RESUME_BOTH, RESUME_TICKET); + ExpectResumption(RESUME_TICKET); + + bool cb_run = false; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback( + server_->ssl_fd(), CheckTicketToken, &cb_run)); + Connect(); + EXPECT_TRUE(cb_run); +} + +void TriggerHelloRetryRequest(std::shared_ptr& client, + std::shared_ptr& server) { + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server->ssl_fd(), + RetryHello, &cb_called)); + + // Start the handshake. + client->StartConnect(); + server->StartConnect(); + client->Handshake(); + server->Handshake(); + EXPECT_EQ(1U, cb_called); +} + +TEST_P(TlsConnectTls13, RetryStateless) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + Handshake(); + SendReceive(); +} + +TEST_P(TlsConnectTls13, RetryStatefulDropCookie) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + client_->SetPacketFilter( + std::make_shared(ssl_tls13_cookie_xtn)); + + ExpectAlert(server_, kTlsAlertMissingExtension); + Handshake(); + client_->CheckErrorCode(SSL_ERROR_MISSING_EXTENSION_ALERT); + server_->CheckErrorCode(SSL_ERROR_MISSING_COOKIE_EXTENSION); +} + +// Stream only because DTLS drops bad packets. +TEST_F(TlsConnectStreamTls13, RetryStatelessDamageFirstClientHello) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + auto damage_ch = std::make_shared(0xfff3, DataBuffer()); + client_->SetPacketFilter(damage_ch); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + // Key exchange fails when the handshake continues because client and server + // disagree about the transcript. + client_->ExpectSendAlert(kTlsAlertBadRecordMac); + server_->ExpectSendAlert(kTlsAlertBadRecordMac); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); + client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); +} + +TEST_F(TlsConnectStreamTls13, RetryStatelessDamageSecondClientHello) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + auto damage_ch = std::make_shared(0xfff3, DataBuffer()); + client_->SetPacketFilter(damage_ch); + + // Key exchange fails when the handshake continues because client and server + // disagree about the transcript. + client_->ExpectSendAlert(kTlsAlertBadRecordMac); + server_->ExpectSendAlert(kTlsAlertBadRecordMac); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); + client_->CheckErrorCode(SSL_ERROR_BAD_MAC_READ); +} + +// Read the cipher suite from the HRR and disable it on the identified agent. +static void DisableSuiteFromHrr( + std::shared_ptr& agent, + std::shared_ptr& capture_hrr) { + uint32_t tmp; + size_t offset = 2 + 32; // skip version + server_random + ASSERT_TRUE( + capture_hrr->buffer().Read(offset, 1, &tmp)); // session_id length + EXPECT_EQ(0U, tmp); + offset += 1 + tmp; + ASSERT_TRUE(capture_hrr->buffer().Read(offset, 2, &tmp)); // suite + EXPECT_EQ( + SECSuccess, + SSL_CipherPrefSet(agent->ssl_fd(), static_cast(tmp), PR_FALSE)); +} + +TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteClient) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + auto capture_hrr = std::make_shared( + ssl_hs_hello_retry_request); + server_->SetPacketFilter(capture_hrr); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + DisableSuiteFromHrr(client_, capture_hrr); + + // The client thinks that the HelloRetryRequest is bad, even though its + // because it changed its mind about the cipher suite. + ExpectAlert(client_, kTlsAlertIllegalParameter); + Handshake(); + client_->CheckErrorCode(SSL_ERROR_NO_CYPHER_OVERLAP); + server_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); +} + +TEST_P(TlsConnectTls13, RetryStatelessDisableSuiteServer) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + auto capture_hrr = std::make_shared( + ssl_hs_hello_retry_request); + server_->SetPacketFilter(capture_hrr); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + DisableSuiteFromHrr(server_, capture_hrr); + + ExpectAlert(server_, kTlsAlertIllegalParameter); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO); + client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); +} + +TEST_P(TlsConnectTls13, RetryStatelessDisableGroupClient) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + static const std::vector groups = {ssl_grp_ec_secp384r1}; + client_->ConfigNamedGroups(groups); + + // We're into undefined behavior on the client side, but - at the point this + // test was written - the client here doesn't amend its key shares because the + // server doesn't ask it to. The server notices that the key share (x25519) + // doesn't match the negotiated group (P-384) and objects. + ExpectAlert(server_, kTlsAlertIllegalParameter); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO); + client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); +} + +TEST_P(TlsConnectTls13, RetryStatelessDisableGroupServer) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + MakeNewServer(); + + static const std::vector groups = {ssl_grp_ec_secp384r1}; + server_->ConfigNamedGroups(groups); + + ExpectAlert(server_, kTlsAlertIllegalParameter); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO); + client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); +} + +TEST_P(TlsConnectTls13, RetryStatelessBadCookie) { + ConfigureSelfEncrypt(); + EnsureTlsSetup(); + + TriggerHelloRetryRequest(client_, server_); + + // Now replace the self-encrypt MAC key with a garbage key. + static const uint8_t bad_hmac_key[32] = {0}; + SECItem key_item = {siBuffer, const_cast(bad_hmac_key), + sizeof(bad_hmac_key)}; + ScopedPK11SlotInfo slot(PK11_GetInternalSlot()); + PK11SymKey* hmac_key = + PK11_ImportSymKey(slot.get(), CKM_SHA256_HMAC, PK11_OriginUnwrap, + CKA_SIGN, &key_item, nullptr); + ASSERT_NE(nullptr, hmac_key); + SSLInt_SetSelfEncryptMacKey(hmac_key); // Passes ownership. + + MakeNewServer(); + + ExpectAlert(server_, kTlsAlertIllegalParameter); + Handshake(); + server_->CheckErrorCode(SSL_ERROR_BAD_2ND_CLIENT_HELLO); + client_->CheckErrorCode(SSL_ERROR_ILLEGAL_PARAMETER_ALERT); +} + +// Stream because the server doesn't consume the alert and terminate. +TEST_F(TlsConnectStreamTls13, RetryWithDifferentCipherSuite) { + EnsureTlsSetup(); + // Force a HelloRetryRequest. + static const std::vector groups = {ssl_grp_ec_secp384r1}; + server_->ConfigNamedGroups(groups); + // Then switch out the default suite (TLS_AES_128_GCM_SHA256). + server_->SetPacketFilter(std::make_shared( + TLS_CHACHA20_POLY1305_SHA256)); + + client_->ExpectSendAlert(kTlsAlertIllegalParameter); + server_->ExpectSendAlert(kTlsAlertBadRecordMac); + ConnectExpectFail(); + EXPECT_EQ(SSL_ERROR_RX_MALFORMED_SERVER_HELLO, client_->error_code()); + EXPECT_EQ(SSL_ERROR_BAD_MAC_READ, server_->error_code()); +} + // This tests that the second attempt at sending a ClientHello (after receiving // a HelloRetryRequest) is correctly retransmitted. TEST_F(TlsConnectDatagram13, DropClientSecondFlightWithHelloRetry) { @@ -233,6 +817,54 @@ TEST_P(TlsKeyExchange13, ConnectEcdhePreferenceMismatchHrrExtraShares) { CheckKEXDetails(client_groups, client_groups); } +// The callback should be run even if we have another reason to send +// HelloRetryRequest. In this case, the server sends HRR because the server +// wants an X25519 key share and the client didn't offer one. +TEST_P(TlsKeyExchange13, + RetryCallbackRetryWithGroupMismatchAndAdditionalShares) { + EnsureKeyShareSetup(); + + static const std::vector client_groups = { + ssl_grp_ec_secp256r1, ssl_grp_ec_secp384r1, ssl_grp_ec_curve25519}; + client_->ConfigNamedGroups(client_groups); + static const std::vector server_groups = { + ssl_grp_ec_curve25519}; + server_->ConfigNamedGroups(server_groups); + EXPECT_EQ(SECSuccess, SSL_SendAdditionalKeyShares(client_->ssl_fd(), 1)); + + auto capture_server = + std::make_shared(ssl_tls13_key_share_xtn); + capture_server->SetHandshakeTypes({kTlsHandshakeHelloRetryRequest}); + server_->SetPacketFilter(std::make_shared( + ChainedPacketFilterInit{capture_hrr_, capture_server})); + + size_t cb_called = 0; + EXPECT_EQ(SECSuccess, SSL_HelloRetryRequestCallback(server_->ssl_fd(), + RetryHello, &cb_called)); + + // Do the first message exchange. + StartConnect(); + client_->Handshake(); + server_->Handshake(); + + EXPECT_EQ(1U, cb_called) << "callback should be called once here"; + EXPECT_TRUE(capture_server->captured()) << "key_share extension expected"; + + uint32_t server_group = 0; + EXPECT_TRUE(capture_server->extension().Read(0, 2, &server_group)); + EXPECT_EQ(ssl_grp_ec_curve25519, static_cast(server_group)); + + Handshake(); + CheckConnected(); + EXPECT_EQ(2U, cb_called); + EXPECT_TRUE(shares_capture2_->captured()) << "client should send shares"; + + CheckKeys(); + static const std::vector client_shares( + client_groups.begin(), client_groups.begin() + 2); + CheckKEXDetails(client_groups, client_shares, server_groups[0]); +} + TEST_F(TlsConnectTest, Select12AfterHelloRetryRequest) { EnsureTlsSetup(); client_->SetVersionRange(SSL_LIBRARY_VERSION_TLS_1_2, @@ -245,8 +877,7 @@ TEST_F(TlsConnectTest, Select12AfterHelloRetryRequest) { static const std::vector server_groups = { ssl_grp_ec_secp384r1, ssl_grp_ec_secp521r1}; server_->ConfigNamedGroups(server_groups); - client_->StartConnect(); - server_->StartConnect(); + StartConnect(); client_->Handshake(); server_->Handshake(); @@ -276,15 +907,30 @@ class HelloRetryRequestAgentTest : public TlsAgentTestClient { void MakeCannedHrr(const uint8_t* body, size_t len, DataBuffer* hrr_record, uint32_t seq_num = 0) const { DataBuffer hrr_data; - hrr_data.Allocate(len + 4); + const uint8_t ssl_hello_retry_random[] = { + 0xCF, 0x21, 0xAD, 0x74, 0xE5, 0x9A, 0x61, 0x11, 0xBE, 0x1D, 0x8C, + 0x02, 0x1E, 0x65, 0xB8, 0x91, 0xC2, 0xA2, 0x11, 0x16, 0x7A, 0xBB, + 0x8C, 0x5E, 0x07, 0x9E, 0x09, 0xE2, 0xC8, 0xA8, 0x33, 0x9C}; + + hrr_data.Allocate(len + 6); size_t i = 0; + i = hrr_data.Write(i, 0x0303, 2); + i = hrr_data.Write(i, ssl_hello_retry_random, + sizeof(ssl_hello_retry_random)); + i = hrr_data.Write(i, static_cast(0), 1); // session_id + i = hrr_data.Write(i, TLS_AES_128_GCM_SHA256, 2); + i = hrr_data.Write(i, ssl_compression_null, 1); + // Add extensions. First a length, which includes the supported version. + i = hrr_data.Write(i, static_cast(len) + 6, 2); + // Now the supported version. + i = hrr_data.Write(i, ssl_tls13_supported_versions_xtn, 2); + i = hrr_data.Write(i, 2, 2); i = hrr_data.Write(i, 0x7f00 | TLS_1_3_DRAFT_VERSION, 2); - i = hrr_data.Write(i, static_cast(len), 2); if (len) { hrr_data.Write(i, body, len); } DataBuffer hrr; - MakeHandshakeMessage(kTlsHandshakeHelloRetryRequest, hrr_data.data(), + MakeHandshakeMessage(kTlsHandshakeServerHello, hrr_data.data(), hrr_data.len(), &hrr, seq_num); MakeRecord(kTlsHandshakeType, SSL_LIBRARY_VERSION_TLS_1_3, hrr.data(), hrr.len(), hrr_record, seq_num); @@ -334,28 +980,6 @@ TEST_P(HelloRetryRequestAgentTest, HandleNoopHelloRetryRequest) { SSL_ERROR_RX_MALFORMED_HELLO_RETRY_REQUEST); } -TEST_P(HelloRetryRequestAgentTest, HandleHelloRetryRequestCookie) { - const uint8_t canned_cookie_hrr[] = { - static_cast(ssl_tls13_cookie_xtn >> 8), - static_cast(ssl_tls13_cookie_xtn), - 0, - 5, // length of cookie extension - 0, - 3, // cookie value length - 0xc0, - 0x0c, - 0x13}; - DataBuffer hrr; - MakeCannedHrr(canned_cookie_hrr, sizeof(canned_cookie_hrr), &hrr); - auto capture = std::make_shared(ssl_tls13_cookie_xtn); - agent_->SetPacketFilter(capture); - ProcessMessage(hrr, TlsAgent::STATE_CONNECTING); - const size_t cookie_pos = 2 + 2; // cookie_xtn, extension len - DataBuffer cookie(canned_cookie_hrr + cookie_pos, - sizeof(canned_cookie_hrr) - cookie_pos); - EXPECT_EQ(cookie, capture->extension()); -} - INSTANTIATE_TEST_CASE_P(HelloRetryRequestAgentTests, HelloRetryRequestAgentTest, ::testing::Combine(TlsConnectTestBase::kTlsVariantsAll, TlsConnectTestBase::kTlsV13)); -- cgit v1.2.3