1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
|
# HG changeset patch
# User Daiki Ueno <dueno@redhat.com>
# 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 <mbriand@witekio.com>
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<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 --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 */
|
