Implement the downgrade protection signal in DTLS 1.3
This was originally looking for the client to specifically support
the final TLS 1.3 version 0x0304. This has a side effect of not picking
up DTLS 1.3, which has a different codepoint.
We did it this way because, early in TLS 1.3's development, we had draft
versions of TLS 1.3 flying around, and only the final TLS 1.3 can safely
ship enforcing this check.
Those draft versions are now gone, and this check is now getting in the
way of DTLS 1.3. Switch it to checking hs->max_version.
Bug: 42290594
Change-Id: Ic2d143af965b4b8bafef524f3f0e85cc3efa42fe
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73728
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Nick Harper <nharper@chromium.org>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index b904e4f..afc99be 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -796,7 +796,7 @@
// Enforce the TLS 1.3 anti-downgrade feature.
if (!ssl->s3->initial_handshake_complete &&
- ssl_supports_version(hs, TLS1_3_VERSION)) {
+ hs->max_version >= TLS1_3_VERSION) {
static_assert(
sizeof(kTLS12DowngradeRandom) == sizeof(kTLS13DowngradeRandom),
"downgrade signals have different size");
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index e41b233..843e2ba 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -1057,7 +1057,7 @@
}
// Implement the TLS 1.3 anti-downgrade feature.
- if (ssl_supports_version(hs, TLS1_3_VERSION)) {
+ if (hs->max_version >= TLS1_3_VERSION) {
if (ssl_protocol_version(ssl) == TLS1_2_VERSION) {
if (hs->apply_jdk11_workaround) {
// JDK 11 implements the TLS 1.3 downgrade signal, so we cannot send it
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 19e3740..41fa454 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -6891,8 +6891,9 @@
name: "MinorVersionTolerance-DTLS",
config: Config{
Bugs: ProtocolBugs{
- SendClientVersion: 0xfe00,
- OmitSupportedVersions: true,
+ SendClientVersion: 0xfe00,
+ OmitSupportedVersions: true,
+ IgnoreTLS13DowngradeRandom: true,
},
},
expectations: connectionExpectations{
@@ -6905,8 +6906,9 @@
name: "MajorVersionTolerance-DTLS",
config: Config{
Bugs: ProtocolBugs{
- SendClientVersion: 0xfdff,
- OmitSupportedVersions: true,
+ SendClientVersion: 0xfdff,
+ OmitSupportedVersions: true,
+ IgnoreTLS13DowngradeRandom: true,
},
},
expectations: connectionExpectations{
@@ -6953,49 +6955,50 @@
})
// Test TLS 1.3's downgrade signal.
- var downgradeTests = []struct {
- name string
- version uint16
- clientShimError string
- }{
- {"TLS12", VersionTLS12, "tls: downgrade from TLS 1.3 detected"},
- {"TLS11", VersionTLS11, "tls: downgrade from TLS 1.2 detected"},
- // TLS 1.0 does not have a dedicated value.
- {"TLS10", VersionTLS10, "tls: downgrade from TLS 1.2 detected"},
- }
-
- for _, test := range downgradeTests {
- // The client should enforce the downgrade sentinel.
- testCases = append(testCases, testCase{
- name: "Downgrade-" + test.name + "-Client",
- config: Config{
- Bugs: ProtocolBugs{
- NegotiateVersion: test.version,
+ for _, protocol := range []protocol{tls, dtls} {
+ for _, vers := range allVersions(protocol) {
+ if vers.version >= VersionTLS13 {
+ continue
+ }
+ clientShimError := "tls: downgrade from TLS 1.3 detected"
+ if vers.version < VersionTLS12 {
+ clientShimError = "tls: downgrade from TLS 1.2 detected"
+ }
+ // for _, test := range downgradeTests {
+ // The client should enforce the downgrade sentinel.
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ name: "Downgrade-" + vers.name + "-Client-" + protocol.String(),
+ config: Config{
+ Bugs: ProtocolBugs{
+ NegotiateVersion: vers.wire(protocol),
+ },
},
- },
- expectations: connectionExpectations{
- version: test.version,
- },
- shouldFail: true,
- expectedError: ":TLS13_DOWNGRADE:",
- expectedLocalError: "remote error: illegal parameter",
- })
-
- // The server should emit the downgrade signal.
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Downgrade-" + test.name + "-Server",
- config: Config{
- Bugs: ProtocolBugs{
- SendSupportedVersions: []uint16{test.version},
+ expectations: connectionExpectations{
+ version: vers.version,
},
- },
- expectations: connectionExpectations{
- version: test.version,
- },
- shouldFail: true,
- expectedLocalError: test.clientShimError,
- })
+ shouldFail: true,
+ expectedError: ":TLS13_DOWNGRADE:",
+ expectedLocalError: "remote error: illegal parameter",
+ })
+
+ // The server should emit the downgrade signal.
+ testCases = append(testCases, testCase{
+ protocol: protocol,
+ testType: serverTest,
+ name: "Downgrade-" + vers.name + "-Server-" + protocol.String(),
+ config: Config{
+ Bugs: ProtocolBugs{
+ SendSupportedVersions: []uint16{vers.wire(protocol)},
+ },
+ },
+ expectations: connectionExpectations{
+ version: vers.version,
+ },
+ shouldFail: true,
+ expectedLocalError: clientShimError,
+ })
+ }
}
// SSL 3.0 support has been removed. Test that the shim does not
