Fix SSL_set_{min,max}_proto_version APIs in invalid versions.
SSL_set_max_proto_version(TLS1_3_DRAFT_VERSION) worked unintentionally.
Fix that. Also add an error when it fails.
Change-Id: I1048fede7b163e1c170e17bf4370b468221a7077
Reviewed-on: https://boringssl-review.googlesource.com/17525
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 84b7496..7737e75 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2567,6 +2567,10 @@
EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_VERSION));
EXPECT_EQ(TLS1_3_VERSION, ctx->conf_max_version);
+ // TLS1_3_DRAFT_VERSION is not an API-level version.
+ EXPECT_FALSE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_DRAFT_VERSION));
+ ERR_clear_error();
+
ctx.reset(SSL_CTX_new(DTLS_method()));
ASSERT_TRUE(ctx);
diff --git a/ssl/ssl_versions.c b/ssl/ssl_versions.c
index 6565688..80b62cc 100644
--- a/ssl/ssl_versions.c
+++ b/ssl/ssl_versions.c
@@ -95,12 +95,21 @@
/* The public API uses wire versions, except we use |TLS1_3_VERSION|
* everywhere to refer to any draft TLS 1.3 versions. In this direction, we
* map it to some representative TLS 1.3 draft version. */
+ if (version == TLS1_3_DRAFT_VERSION) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_SSL_VERSION);
+ return 0;
+ }
if (version == TLS1_3_VERSION) {
version = TLS1_3_DRAFT_VERSION;
}
- return method_supports_version(method, version) &&
- ssl_protocol_version_from_wire(out, version);
+ if (!method_supports_version(method, version) ||
+ !ssl_protocol_version_from_wire(out, version)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_SSL_VERSION);
+ return 0;
+ }
+
+ return 1;
}
static int set_min_version(const SSL_PROTOCOL_METHOD *method, uint16_t *out,
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 0a6648f..bd490d3 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -26,6 +26,11 @@
VersionTLS13 = 0x0304
)
+const (
+ VersionDTLS10 = 0xfeff
+ VersionDTLS12 = 0xfefd
+)
+
// A draft version of TLS 1.3 that is sent over the wire for the current draft.
const tls13DraftVersion = 0x7f12
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index d9218f2..ba6cc54 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1155,16 +1155,27 @@
type tlsVersion struct {
name string
version uint16
- flag string
- hasDTLS bool
+ // excludeFlag is the legacy shim flag to disable the version.
+ excludeFlag string
+ hasDTLS bool
+ // shimTLS and shimDTLS are values the shim uses to refer to these
+ // versions in TLS and DTLS, respectively.
+ shimTLS, shimDTLS int
+}
+
+func (vers tlsVersion) shimFlag(protocol protocol) string {
+ if protocol == dtls {
+ return strconv.Itoa(vers.shimDTLS)
+ }
+ return strconv.Itoa(vers.shimTLS)
}
var tlsVersions = []tlsVersion{
- {"SSL3", VersionSSL30, "-no-ssl3", false},
- {"TLS1", VersionTLS10, "-no-tls1", true},
- {"TLS11", VersionTLS11, "-no-tls11", false},
- {"TLS12", VersionTLS12, "-no-tls12", true},
- {"TLS13", VersionTLS13, "-no-tls13", false},
+ {"SSL3", VersionSSL30, "-no-ssl3", false, VersionSSL30, 0},
+ {"TLS1", VersionTLS10, "-no-tls1", true, VersionTLS10, VersionDTLS10},
+ {"TLS11", VersionTLS11, "-no-tls11", false, VersionTLS11, 0},
+ {"TLS12", VersionTLS12, "-no-tls12", true, VersionTLS12, VersionDTLS12},
+ {"TLS13", VersionTLS13, "-no-tls13", false, VersionTLS13, 0},
}
type testCipherSuite struct {
@@ -4541,7 +4552,7 @@
// Assemble flags to disable all newer versions on the shim.
var flags []string
for _, vers := range tlsVersions[i+1:] {
- flags = append(flags, vers.flag)
+ flags = append(flags, vers.excludeFlag)
}
// Test configuring the runner's maximum version.
@@ -4561,8 +4572,6 @@
suffix += "-DTLS"
}
- shimVersFlag := strconv.Itoa(int(versionToWire(shimVers.version, protocol == dtls)))
-
// Determine the expected initial record-layer versions.
clientVers := shimVers.version
if clientVers > VersionTLS10 {
@@ -4598,7 +4607,7 @@
ExpectInitialRecordVersion: clientVers,
},
},
- flags: []string{"-max-version", shimVersFlag},
+ flags: []string{"-max-version", shimVers.shimFlag(protocol)},
expectedVersion: expectedVersion,
})
@@ -4625,7 +4634,7 @@
ExpectInitialRecordVersion: serverVers,
},
},
- flags: []string{"-max-version", shimVersFlag},
+ flags: []string{"-max-version", shimVers.shimFlag(protocol)},
expectedVersion: expectedVersion,
})
}
@@ -4882,7 +4891,7 @@
// Assemble flags to disable all older versions on the shim.
var flags []string
for _, vers := range tlsVersions[:i] {
- flags = append(flags, vers.flag)
+ flags = append(flags, vers.excludeFlag)
}
for _, runnerVers := range tlsVersions {
@@ -4895,7 +4904,6 @@
if protocol == dtls {
suffix += "-DTLS"
}
- shimVersFlag := strconv.Itoa(int(versionToWire(shimVers.version, protocol == dtls)))
var expectedVersion uint16
var shouldFail bool
@@ -4942,7 +4950,7 @@
IgnorePeerCipherPreferences: shouldFail,
},
},
- flags: []string{"-min-version", shimVersFlag},
+ flags: []string{"-min-version", shimVers.shimFlag(protocol)},
expectedVersion: expectedVersion,
shouldFail: shouldFail,
expectedError: expectedError,
@@ -4969,7 +4977,7 @@
config: Config{
MaxVersion: runnerVers.version,
},
- flags: []string{"-min-version", shimVersFlag},
+ flags: []string{"-min-version", shimVers.shimFlag(protocol)},
expectedVersion: expectedVersion,
shouldFail: shouldFail,
expectedError: expectedError,
