# HG changeset patch # User Daiki Ueno # Date 1602524521 0 # Node ID 57bbefa793232586d27cee83e74411171e128361 # Parent 6e3bc17f05086854ffd2b06f7fae9371f7a0c174 Bug 1641480, TLS 1.3: tighten CCS handling in compatibility mode, r=mt This makes the server reject CCS when the client doesn't indicate the use of the middlebox compatibility mode with a non-empty ClientHello.legacy_session_id, or it sends multiple CCS in a row. Differential Revision: https://phabricator.services.mozilla.com/D79994 Upstream-Status: Backport CVE: CVE-2020-25648 Reference to upstream patch: https://hg.mozilla.org/projects/nss/rev/57bbefa793232586d27cee83e74411171e128361 Signed-off-by: Mathieu Dubois-Briand diff --color -Naur nss-3.51.1_old/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc nss-3.51.1/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc --- nss-3.51.1_old/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc 2022-12-08 16:05:47.447142660 +0100 +++ nss-3.51.1/nss/gtests/ssl_gtest/ssl_tls13compat_unittest.cc 2022-12-08 16:12:32.645932052 +0100 @@ -348,6 +348,85 @@ client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT); } +// The server rejects a ChangeCipherSpec if the client advertises an +// empty session ID. +TEST_F(TlsConnectStreamTls13, ChangeCipherSpecAfterClientHelloEmptySid) { + EnsureTlsSetup(); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); + + StartConnect(); + client_->Handshake(); // Send ClientHello + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); // Send CCS + + server_->ExpectSendAlert(kTlsAlertUnexpectedMessage); + server_->Handshake(); // Consume ClientHello and CCS + server_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); +} + +// The server rejects multiple ChangeCipherSpec even if the client +// indicates compatibility mode with non-empty session ID. +TEST_F(Tls13CompatTest, ChangeCipherSpecAfterClientHelloTwice) { + EnsureTlsSetup(); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); + EnableCompatMode(); + + StartConnect(); + client_->Handshake(); // Send ClientHello + // Send CCS twice in a row + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + + server_->ExpectSendAlert(kTlsAlertUnexpectedMessage); + server_->Handshake(); // Consume ClientHello and CCS. + server_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); +} + +// The client rejects a ChangeCipherSpec if it advertises an empty +// session ID. +TEST_F(TlsConnectStreamTls13, ChangeCipherSpecAfterServerHelloEmptySid) { + EnsureTlsSetup(); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); + + // To replace Finished with a CCS below + auto filter = MakeTlsFilter(server_); + filter->SetHandshakeTypes({kTlsHandshakeFinished}); + filter->EnableDecryption(); + + StartConnect(); + client_->Handshake(); // Send ClientHello + server_->Handshake(); // Consume ClientHello, and + // send ServerHello..CertificateVerify + // Send CCS + server_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + client_->ExpectSendAlert(kTlsAlertUnexpectedMessage); + client_->Handshake(); // Consume ClientHello and CCS + client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); +} + +// The client rejects multiple ChangeCipherSpec in a row even if the +// client indicates compatibility mode with non-empty session ID. +TEST_F(Tls13CompatTest, ChangeCipherSpecAfterServerHelloTwice) { + EnsureTlsSetup(); + ConfigureVersion(SSL_LIBRARY_VERSION_TLS_1_3); + EnableCompatMode(); + + // To replace Finished with a CCS below + auto filter = MakeTlsFilter(server_); + filter->SetHandshakeTypes({kTlsHandshakeFinished}); + filter->EnableDecryption(); + + StartConnect(); + client_->Handshake(); // Send ClientHello + server_->Handshake(); // Consume ClientHello, and + // send ServerHello..CertificateVerify + // the ServerHello is followed by CCS + // Send another CCS + server_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs))); + client_->ExpectSendAlert(kTlsAlertUnexpectedMessage); + client_->Handshake(); // Consume ClientHello and CCS + client_->CheckErrorCode(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); +} + // If we negotiate 1.2, we abort. TEST_F(TlsConnectStreamTls13, ChangeCipherSpecBeforeClientHello12) { EnsureTlsSetup(); diff --color -Naur nss-3.51.1_old/nss/lib/ssl/ssl3con.c nss-3.51.1/nss/lib/ssl/ssl3con.c --- nss-3.51.1_old/nss/lib/ssl/ssl3con.c 2022-12-08 16:05:47.471142833 +0100 +++ nss-3.51.1/nss/lib/ssl/ssl3con.c 2022-12-08 16:12:42.037994262 +0100 @@ -6711,7 +6711,11 @@ /* TLS 1.3: We sent a session ID. The server's should match. */ if (!IS_DTLS(ss) && (sentRealSid || sentFakeSid)) { - return sidMatch; + if (sidMatch) { + ss->ssl3.hs.allowCcs = PR_TRUE; + return PR_TRUE; + } + return PR_FALSE; } /* TLS 1.3 (no SID)/DTLS 1.3: The server shouldn't send a session ID. */ @@ -8730,6 +8734,7 @@ errCode = PORT_GetError(); goto alert_loser; } + ss->ssl3.hs.allowCcs = PR_TRUE; } /* TLS 1.3 requires that compression include only null. */ @@ -13058,8 +13063,15 @@ ss->ssl3.hs.ws != idle_handshake && cText->buf->len == 1 && cText->buf->buf[0] == change_cipher_spec_choice) { - /* Ignore the CCS. */ - return SECSuccess; + if (ss->ssl3.hs.allowCcs) { + /* Ignore the first CCS. */ + ss->ssl3.hs.allowCcs = PR_FALSE; + return SECSuccess; + } + + /* Compatibility mode is not negotiated. */ + alert = unexpected_message; + PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER); } if (IS_DTLS(ss) || diff --color -Naur nss-3.51.1_old/nss/lib/ssl/sslimpl.h nss-3.51.1/nss/lib/ssl/sslimpl.h --- nss-3.51.1_old/nss/lib/ssl/sslimpl.h 2022-12-08 16:05:47.471142833 +0100 +++ nss-3.51.1/nss/lib/ssl/sslimpl.h 2022-12-08 16:12:45.106014567 +0100 @@ -711,6 +711,10 @@ * or received. */ PRBool receivedCcs; /* A server received ChangeCipherSpec * before the handshake started. */ + PRBool allowCcs; /* A server allows ChangeCipherSpec + * as the middlebox compatibility mode + * is explicitly indicarted by + * legacy_session_id in TLS 1.3 ClientHello. */ PRBool clientCertRequested; /* True if CertificateRequest received. */ ssl3KEADef kea_def_mutable; /* Used to hold the writable kea_def * we use for TLS 1.3 */