From d451453067cd665a5c38830fbbaac9e599234a5e Mon Sep 17 00:00:00 2001 From: Steven Valdez Date: Mon, 13 Aug 2018 10:07:45 -0400 Subject: [PATCH] Implement final TLS 1.3 RFC!!! The anti-downgrade signal is being implemented in a follow-up change. Change-Id: I5ea3ff429ed1389a3577026588fef3660d2d0615 Reviewed-on: https://boringssl-review.googlesource.com/30904 Commit-Queue: Steven Valdez CQ-Verified: CQ bot account: commit-bot@chromium.org Reviewed-by: Steven Valdez Reviewed-by: David Benjamin --- include/openssl/ssl.h | 6 ++++++ ssl/ssl_versions.cc | 11 +++++++---- ssl/test/runner/common.go | 31 ++++++++++++++++++++++++------- ssl/test/runner/runner.go | 27 +++++++++++---------------- tool/client.cc | 4 ++++ tool/server.cc | 4 ++++ 6 files changed, 56 insertions(+), 27 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index d6169816..85b244b6 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -3400,10 +3400,16 @@ OPENSSL_EXPORT int SSL_renegotiate_pending(SSL *ssl); // performed by |ssl|. This includes the pending renegotiation, if any. OPENSSL_EXPORT int SSL_total_renegotiations(const SSL *ssl); +// tls13_variant_t determines what TLS 1.3 variant to negotiate. +// +// TODO(svaldez): Make |tls13_rfc| the default after callers are switched to +// explicitly enable |tls13_all|. enum tls13_variant_t { tls13_default = 0, tls13_draft23, tls13_draft28, + tls13_rfc, + tls13_all = tls13_default, }; // SSL_CTX_set_tls13_variant sets which variant of TLS 1.3 we negotiate. On the diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc index 6a8143d9..6f07b937 100644 --- a/ssl/ssl_versions.cc +++ b/ssl/ssl_versions.cc @@ -30,6 +30,7 @@ bool ssl_protocol_version_from_wire(uint16_t *out, uint16_t version) { case TLS1_VERSION: case TLS1_1_VERSION: case TLS1_2_VERSION: + case TLS1_3_VERSION: *out = version; return true; @@ -56,6 +57,7 @@ bool ssl_protocol_version_from_wire(uint16_t *out, uint16_t version) { // decreasing preference. static const uint16_t kTLSVersions[] = { + TLS1_3_VERSION, TLS1_3_DRAFT28_VERSION, TLS1_3_DRAFT23_VERSION, TLS1_2_VERSION, @@ -101,6 +103,7 @@ static const char *ssl_version_to_string(uint16_t version) { switch (version) { case TLS1_3_DRAFT23_VERSION: case TLS1_3_DRAFT28_VERSION: + case TLS1_3_VERSION: return "TLSv1.3"; case TLS1_2_VERSION: @@ -128,6 +131,7 @@ static uint16_t wire_version_to_api(uint16_t version) { // Report TLS 1.3 draft versions as TLS 1.3 in the public API. case TLS1_3_DRAFT23_VERSION: case TLS1_3_DRAFT28_VERSION: + case TLS1_3_VERSION: return TLS1_3_VERSION; default: return version; @@ -142,9 +146,6 @@ static bool api_version_to_wire(uint16_t *out, uint16_t version) { version == TLS1_3_DRAFT28_VERSION) { return false; } - if (version == TLS1_3_VERSION) { - version = TLS1_3_DRAFT23_VERSION; - } // Check it is a real protocol version. uint16_t unused; @@ -301,6 +302,8 @@ bool ssl_supports_version(SSL_HANDSHAKE *hs, uint16_t version) { return version == TLS1_3_DRAFT23_VERSION; case tls13_draft28: return version == TLS1_3_DRAFT28_VERSION; + case tls13_rfc: + return version == TLS1_3_VERSION; case tls13_default: return true; } @@ -354,7 +357,7 @@ bool ssl_negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert, } bool ssl_is_draft28(uint16_t version) { - return version == TLS1_3_DRAFT28_VERSION; + return version == TLS1_3_DRAFT28_VERSION || version == TLS1_3_VERSION; } } // namespace bssl diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index aa173501..aeb7ad01 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -42,9 +42,11 @@ const ( TLS13Default = 0 TLS13Draft23 = 1 TLS13Draft28 = 2 + TLS13RFC = 3 ) var allTLSWireVersions = []uint16{ + VersionTLS13, tls13Draft28Version, tls13Draft23Version, VersionTLS12, @@ -1740,7 +1742,7 @@ func wireToVersion(vers uint16, isDTLS bool) (uint16, bool) { } } else { switch vers { - case VersionSSL30, VersionTLS10, VersionTLS11, VersionTLS12: + case VersionSSL30, VersionTLS10, VersionTLS11, VersionTLS12, VersionTLS13: return vers, true case tls13Draft23Version, tls13Draft28Version: return VersionTLS13, true @@ -1751,22 +1753,37 @@ func wireToVersion(vers uint16, isDTLS bool) (uint16, bool) { } func isDraft28(vers uint16) bool { - return vers == tls13Draft28Version + return vers == tls13Draft28Version || vers == VersionTLS13 } // isSupportedVersion checks if the specified wire version is acceptable. If so, // it returns true and the corresponding protocol version. Otherwise, it returns // false. func (c *Config) isSupportedVersion(wireVers uint16, isDTLS bool) (uint16, bool) { - if (c.TLS13Variant == TLS13Draft23 && wireVers == tls13Draft28Version) || - (c.TLS13Variant == TLS13Draft28 && wireVers == tls13Draft23Version) { - return 0, false - } - vers, ok := wireToVersion(wireVers, isDTLS) if !ok || c.minVersion(isDTLS) > vers || vers > c.maxVersion(isDTLS) { return 0, false } + if vers == VersionTLS13 { + switch c.TLS13Variant { + case TLS13Draft23: + if wireVers != tls13Draft23Version { + return 0, false + } + case TLS13Draft28: + if wireVers != tls13Draft28Version { + return 0, false + } + case TLS13RFC: + if wireVers != VersionTLS13 { + return 0, false + } + case TLS13Default: + // Allow all of them. + default: + panic(c.TLS13Variant) + } + } return vers, true } diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 47c45133..d95cc28c 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1024,7 +1024,8 @@ func runTest(test *testCase, shimPath string, mallocNumToFail int64) error { panic(fmt.Sprintf("The name of test %q suggests that it's version specific, but min/max version in the Config is %x/%x. One of them should probably be %x", test.name, test.config.MinVersion, test.config.MaxVersion, ver.version)) } - if ver.tls13Variant != 0 { + // Ignore this check against "TLS13", since TLS13 is used in many test names. + if ver.tls13Variant != 0 && ver.tls13Variant != TLS13RFC { var foundFlag bool for _, flag := range test.flags { if flag == "-tls13-variant" { @@ -1375,6 +1376,13 @@ var tlsVersions = []tlsVersion{ hasDTLS: true, versionDTLS: VersionDTLS12, }, + { + name: "TLS13", + version: VersionTLS13, + excludeFlag: "-no-tls13", + versionWire: VersionTLS13, + tls13Variant: TLS13RFC, + }, { name: "TLS13Draft23", version: VersionTLS13, @@ -1480,7 +1488,7 @@ func bigFromHex(hex string) *big.Int { func convertToSplitHandshakeTests(tests []testCase) (splitHandshakeTests []testCase) { var stdout bytes.Buffer shim := exec.Command(*shimPath, "-is-handshaker-supported") - shim.Stdout = &stdout; + shim.Stdout = &stdout if err := shim.Run(); err != nil { panic(err) } @@ -2831,7 +2839,7 @@ read alert 1 0 messageCount: 5, keyUpdateRequest: keyUpdateRequested, readWithUnfinishedWrite: true, - flags: []string{"-async"}, + flags: []string{"-async"}, }, { name: "SendSNIWarningAlert", @@ -5748,19 +5756,6 @@ func addVersionNegotiationTests() { expectedVersion: VersionTLS12, }) - testCases = append(testCases, testCase{ - testType: serverTest, - name: "RejectFinalTLS13", - config: Config{ - Bugs: ProtocolBugs{ - SendSupportedVersions: []uint16{VersionTLS13, VersionTLS12}, - }, - }, - // We currently implement a draft TLS 1.3 version. Ensure that - // the true TLS 1.3 value is ignored for now. - expectedVersion: VersionTLS12, - }) - // Test that TLS 1.2 isn't negotiated by the supported_versions extension in // the ServerHello. testCases = append(testCases, testCase{ diff --git a/tool/client.cc b/tool/client.cc index 4ab07421..90129936 100644 --- a/tool/client.cc +++ b/tool/client.cc @@ -337,6 +337,10 @@ static bool GetTLS13Variant(tls13_variant_t *out, const std::string &in) { *out = tls13_draft28; return true; } + if (in == "rfc") { + *out = tls13_rfc; + return true; + } return false; } diff --git a/tool/server.cc b/tool/server.cc index bc906303..824538a4 100644 --- a/tool/server.cc +++ b/tool/server.cc @@ -157,6 +157,10 @@ static bool GetTLS13Variant(tls13_variant_t *out, const std::string &in) { *out = tls13_draft28; return true; } + if (in == "rfc") { + *out = tls13_rfc; + return true; + } return false; }