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
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
|
# HG changeset patch
# User Daiki Ueno <dueno@redhat.com>
# Date 1603691171 -3600
# Node ID b03a4fc5b902498414b02640dcb2717dfef9682f
# Parent 6f79a76958129dc09c353c288f115fd9a51ab7d4
Bug 1672703, always tolerate the first CCS in TLS 1.3, r=mt
Summary:
This flips the meaning of the flag for checking excessive CCS
messages, so it only rejects multiple CCS messages while the first CCS
message is always accepted.
Reviewers: mt
Reviewed By: mt
Bug #: 1672703
Differential Revision: https://phabricator.services.mozilla.com/D94603
--- a/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
+++ b/gtests/ssl_gtest/ssl_tls13compat_unittest.cc
@@ -343,29 +343,28 @@ TEST_F(TlsConnectStreamTls13, ChangeCiph
// Client sends CCS before starting the handshake.
client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs)));
client_->SendDirect(DataBuffer(kCannedCcs, sizeof(kCannedCcs)));
ConnectExpectAlert(server_, kTlsAlertUnexpectedMessage);
server_->CheckErrorCode(SSL_ERROR_RX_UNEXPECTED_CHANGE_CIPHER);
client_->CheckErrorCode(SSL_ERROR_HANDSHAKE_UNEXPECTED_ALERT);
}
-// The server rejects a ChangeCipherSpec if the client advertises an
-// empty session ID.
+// The server accepts a ChangeCipherSpec even 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);
+ Handshake();
+ CheckConnected();
}
// 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();
@@ -376,36 +375,37 @@ TEST_F(Tls13CompatTest, ChangeCipherSpec
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
+// The client accepts a ChangeCipherSpec even 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);
+
+ // No alert is sent from the client. As Finished is dropped, we
+ // can't use Handshake() and CheckConnected().
+ client_->Handshake();
}
// 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();
--- a/lib/ssl/ssl3con.c
+++ b/lib/ssl/ssl3con.c
@@ -6640,21 +6640,17 @@ ssl_CheckServerSessionIdCorrectness(sslS
if (sentFakeSid) {
return !sidMatch;
}
return PR_TRUE;
}
/* TLS 1.3: We sent a session ID. The server's should match. */
if (!IS_DTLS(ss) && (sentRealSid || sentFakeSid)) {
- if (sidMatch) {
- ss->ssl3.hs.allowCcs = PR_TRUE;
- return PR_TRUE;
- }
- return PR_FALSE;
+ return sidMatch;
}
/* TLS 1.3 (no SID)/DTLS 1.3: The server shouldn't send a session ID. */
return sidBytes->len == 0;
}
static SECStatus
ssl_CheckServerRandom(sslSocket *ss)
@@ -8691,17 +8687,16 @@ ssl3_HandleClientHello(sslSocket *ss, PR
if (sidBytes.len > 0 && !IS_DTLS(ss)) {
SECITEM_FreeItem(&ss->ssl3.hs.fakeSid, PR_FALSE);
rv = SECITEM_CopyItem(NULL, &ss->ssl3.hs.fakeSid, &sidBytes);
if (rv != SECSuccess) {
desc = internal_error;
errCode = PORT_GetError();
goto alert_loser;
}
- ss->ssl3.hs.allowCcs = PR_TRUE;
}
/* TLS 1.3 requires that compression include only null. */
if (comps.len != 1 || comps.data[0] != ssl_compression_null) {
goto alert_loser;
}
/* If there is a cookie, then this is a second ClientHello (TLS 1.3). */
@@ -13061,25 +13056,24 @@ ssl3_HandleRecord(sslSocket *ss, SSL3Cip
* will fail if the server fails to negotiate compatibility mode in a
* 0-RTT session that is resumed from a session that did negotiate it.
* We don't care about that corner case right now. */
if (ss->version >= SSL_LIBRARY_VERSION_TLS_1_3 &&
cText->hdr[0] == ssl_ct_change_cipher_spec &&
ss->ssl3.hs.ws != idle_handshake &&
cText->buf->len == 1 &&
cText->buf->buf[0] == change_cipher_spec_choice) {
- if (ss->ssl3.hs.allowCcs) {
- /* Ignore the first CCS. */
- ss->ssl3.hs.allowCcs = PR_FALSE;
+ if (!ss->ssl3.hs.rejectCcs) {
+ /* Allow only the first CCS. */
+ ss->ssl3.hs.rejectCcs = PR_TRUE;
return SECSuccess;
- }
-
- /* Compatibility mode is not negotiated. */
- alert = unexpected_message;
- PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER);
+ } else {
+ alert = unexpected_message;
+ PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER);
+ }
}
if ((IS_DTLS(ss) && !dtls13_AeadLimitReached(spec)) ||
(!IS_DTLS(ss) && ss->sec.isServer &&
ss->ssl3.hs.zeroRttIgnore == ssl_0rtt_ignore_trial)) {
/* Silently drop the packet unless we sent a fatal alert. */
if (ss->ssl3.fatalAlertSent) {
return SECFailure;
--- a/lib/ssl/sslimpl.h
+++ b/lib/ssl/sslimpl.h
@@ -705,20 +705,17 @@ typedef struct SSL3HandshakeStateStr {
sslZeroRttIgnore zeroRttIgnore; /* Are we ignoring 0-RTT? */
ssl3CipherSuite zeroRttSuite; /* The cipher suite we used for 0-RTT. */
PRCList bufferedEarlyData; /* Buffered TLS 1.3 early data
* on server.*/
PRBool helloRetry; /* True if HelloRetryRequest has been sent
* 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 rejectCcs; /* Excessive ChangeCipherSpecs are rejected. */
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
* we use for TLS 1.3 */
PRUint16 ticketNonce; /* A counter we use for tickets. */
SECItem fakeSid; /* ... (server) the SID the client used. */
/* rttEstimate is used to guess the round trip time between server and client.
|
