diff options
-rw-r--r-- | gtests/ssl_gtest/ssl_tls13compat_unittest.cc | 79 | ||||
-rw-r--r-- | lib/ssl/ssl3con.c | 18 | ||||
-rw-r--r-- | lib/ssl/sslimpl.h | 4 |
3 files changed, 98 insertions, 3 deletions
diff --git a/gtests/ssl_gtest/ssl_tls13compat_unittest.cc b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc index 6905ed0c0..dcede798c 100644 --- a/gtests/ssl_gtest/ssl_tls13compat_unittest.cc +++ b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc @@ -348,6 +348,85 @@ TEST_F(TlsConnectStreamTls13, ChangeCipherSpecBeforeClientHelloTwice) { 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<TlsHandshakeDropper>(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<TlsHandshakeDropper>(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 --git a/lib/ssl/ssl3con.c b/lib/ssl/ssl3con.c index 8be53d357..5f22872f8 100644 --- a/lib/ssl/ssl3con.c +++ b/lib/ssl/ssl3con.c @@ -6645,7 +6645,11 @@ ssl_CheckServerSessionIdCorrectness(sslSocket *ss, SECItem *sidBytes) /* 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. */ @@ -8692,6 +8696,7 @@ ssl3_HandleClientHello(sslSocket *ss, PRUint8 *b, PRUint32 length) errCode = PORT_GetError(); goto alert_loser; } + ss->ssl3.hs.allowCcs = PR_TRUE; } /* TLS 1.3 requires that compression include only null. */ @@ -13061,8 +13066,15 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Ciphertext *cText) 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) && !dtls13_AeadLimitReached(spec)) || diff --git a/lib/ssl/sslimpl.h b/lib/ssl/sslimpl.h index b0e44a088..44c43a0e6 100644 --- a/lib/ssl/sslimpl.h +++ b/lib/ssl/sslimpl.h @@ -710,6 +710,10 @@ typedef struct SSL3HandshakeStateStr { * 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. */ PRBool endOfFlight; /* Processed a full flight (DTLS 1.3). */ ssl3KEADef kea_def_mutable; /* Used to hold the writable kea_def |