Skip to content

Commit

Permalink
Implement final TLS 1.3 RFC!!!
Browse files Browse the repository at this point in the history
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 <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
  • Loading branch information
dvorak42 authored and CQ bot account: commit-bot@chromium.org committed Aug 13, 2018
1 parent 1c337e5 commit d451453
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 27 deletions.
6 changes: 6 additions & 0 deletions include/openssl/ssl.h
Expand Up @@ -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
Expand Down
11 changes: 7 additions & 4 deletions ssl/ssl_versions.cc
Expand Up @@ -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;

Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down
31 changes: 24 additions & 7 deletions ssl/test/runner/common.go
Expand Up @@ -42,9 +42,11 @@ const (
TLS13Default = 0
TLS13Draft23 = 1
TLS13Draft28 = 2
TLS13RFC = 3
)

var allTLSWireVersions = []uint16{
VersionTLS13,
tls13Draft28Version,
tls13Draft23Version,
VersionTLS12,
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down
27 changes: 11 additions & 16 deletions ssl/test/runner/runner.go
Expand Up @@ -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" {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -2831,7 +2839,7 @@ read alert 1 0
messageCount: 5,
keyUpdateRequest: keyUpdateRequested,
readWithUnfinishedWrite: true,
flags: []string{"-async"},
flags: []string{"-async"},
},
{
name: "SendSNIWarningAlert",
Expand Down Expand Up @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions tool/client.cc
Expand Up @@ -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;
}

Expand Down
4 changes: 4 additions & 0 deletions tool/server.cc
Expand Up @@ -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;
}

Expand Down

0 comments on commit d451453

Please sign in to comment.