Disable SSLv3 by default.
As a precursor to removing the code entirely later, disable the protocol
by default. Callers must use SSL_CTX_set_min_version to enable it.
This change also makes SSLv3_method *not* enable SSL 3.0. Normally
version-specific methods set the minimum and maximum version to their
version. SSLv3_method leaves the minimum at the default, so we will
treat it as all versions disabled. To help debugging, the error code is
switched from WRONG_SSL_VERSION to a new NO_SUPPORTED_VERSIONS_ENABLED.
This also defines OPENSSL_NO_SSL3 and OPENSSL_NO_SSL3_METHOD to kick in
any no-ssl3 build paths in consumers which should provide a convenient
hook for any upstreaming changes that may be needed. (OPENSSL_NO_SSL3
existed in older versions of OpenSSL, so in principle one may encounter
an OpenSSL with the same settings.)
Change-Id: I96a8f2f568eb77b2537b3a774b2f7108bd67dd0c
Reviewed-on: https://boringssl-review.googlesource.com/14031
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/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 5e38b30..9ac015c 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -101,6 +101,7 @@
SSL,183,NO_REQUIRED_DIGEST
SSL,184,NO_SHARED_CIPHER
SSL,266,NO_SHARED_GROUP
+SSL,280,NO_SUPPORTED_VERSIONS_ENABLED
SSL,185,NULL_SSL_CTX
SSL,186,NULL_SSL_METHOD_PASSED
SSL,187,OLD_SESSION_CIPHER_NOT_RETURNED
diff --git a/include/openssl/opensslconf.h b/include/openssl/opensslconf.h
index bf65fc3..deff101 100644
--- a/include/openssl/opensslconf.h
+++ b/include/openssl/opensslconf.h
@@ -52,6 +52,8 @@
#define OPENSSL_NO_SEED
#define OPENSSL_NO_SRP
#define OPENSSL_NO_SSL2
+#define OPENSSL_NO_SSL3
+#define OPENSSL_NO_SSL3_METHOD
#define OPENSSL_NO_STATIC_ENGINE
#define OPENSSL_NO_STORE
#define OPENSSL_NO_WHIRLPOOL
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 327f785..13ebae5 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3402,13 +3402,15 @@
* |DTLS_method| except they also call |SSL_CTX_set_min_proto_version| and
* |SSL_CTX_set_max_proto_version| to lock connections to that protocol
* version. */
-OPENSSL_EXPORT const SSL_METHOD *SSLv3_method(void);
OPENSSL_EXPORT const SSL_METHOD *TLSv1_method(void);
OPENSSL_EXPORT const SSL_METHOD *TLSv1_1_method(void);
OPENSSL_EXPORT const SSL_METHOD *TLSv1_2_method(void);
OPENSSL_EXPORT const SSL_METHOD *DTLSv1_method(void);
OPENSSL_EXPORT const SSL_METHOD *DTLSv1_2_method(void);
+/* SSLv3_method returns an |SSL_METHOD| with no versions enabled. */
+OPENSSL_EXPORT const SSL_METHOD *SSLv3_method(void);
+
/* These client- and server-specific methods call their corresponding generic
* methods. */
OPENSSL_EXPORT const SSL_METHOD *TLS_server_method(void);
@@ -4641,6 +4643,7 @@
#define SSL_R_ALPN_MISMATCH_ON_EARLY_DATA 277
#define SSL_R_WRONG_VERSION_ON_EARLY_DATA 278
#define SSL_R_CHANNEL_ID_ON_EARLY_DATA 279
+#define SSL_R_NO_SUPPORTED_VERSIONS_ENABLED 280
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 7adf103..9f77c5a 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -296,9 +296,12 @@
ret->mode = SSL_MODE_NO_AUTO_CHAIN;
/* Lock the SSL_CTX to the specified version, for compatibility with legacy
- * uses of SSL_METHOD. */
+ * uses of SSL_METHOD, but we do not set the minimum version for
+ * |SSLv3_method|. */
if (!SSL_CTX_set_max_proto_version(ret, method->version) ||
- !SSL_CTX_set_min_proto_version(ret, method->version)) {
+ !SSL_CTX_set_min_proto_version(ret, method->version == SSL3_VERSION
+ ? 0 /* default */
+ : method->version)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
goto err2;
}
@@ -945,6 +948,10 @@
/* Zero is interpreted as the default minimum version. */
if (version == 0) {
*out = method->min_version;
+ /* SSL 3.0 is disabled unless explicitly enabled. */
+ if (*out < TLS1_VERSION) {
+ *out = TLS1_VERSION;
+ }
return 1;
}
@@ -2398,7 +2405,7 @@
}
if (!any_enabled) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SUPPORTED_VERSIONS_ENABLED);
return 0;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 6678b57..17ad4e4 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1857,6 +1857,8 @@
"CHACHA20:ALL";
#endif
if (!ctx ||
+ // SSLv3 is off by default.
+ !SSL_CTX_set_min_proto_version(ctx.get(), SSL3_VERSION) ||
!SSL_CTX_set_max_proto_version(ctx.get(), version) ||
!SSL_CTX_set_strict_cipher_list(ctx.get(), cipher_list)) {
return false;
@@ -2650,7 +2652,13 @@
EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), 0));
EXPECT_EQ(TLS1_2_VERSION, ctx->max_version);
EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), 0));
+ EXPECT_EQ(TLS1_VERSION, ctx->min_version);
+
+ // SSL 3.0 and TLS 1.3 are available, but not by default.
+ EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), SSL3_VERSION));
EXPECT_EQ(SSL3_VERSION, ctx->min_version);
+ EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), TLS1_3_VERSION));
+ EXPECT_EQ(TLS1_3_VERSION, ctx->max_version);
ctx.reset(SSL_CTX_new(DTLS_method()));
ASSERT_TRUE(ctx);
@@ -3517,6 +3525,42 @@
ssl_test_ticket_aead_open_soft_fail,
ssl_test_ticket_aead_open_hard_fail)));
+TEST(SSLTest, SSL3Method) {
+ bssl::UniquePtr<X509> cert = GetTestCertificate();
+ ASSERT_TRUE(cert);
+
+ // For compatibility, SSLv3_method should work up to SSL_CTX_new and SSL_new.
+ bssl::UniquePtr<SSL_CTX> ssl3_ctx(SSL_CTX_new(SSLv3_method()));
+ ASSERT_TRUE(ssl3_ctx);
+ ASSERT_TRUE(SSL_CTX_use_certificate(ssl3_ctx.get(), cert.get()));
+ bssl::UniquePtr<SSL> ssl(SSL_new(ssl3_ctx.get()));
+ EXPECT_TRUE(ssl);
+
+ // Create a normal TLS context to test against.
+ bssl::UniquePtr<SSL_CTX> tls_ctx(SSL_CTX_new(TLS_method()));
+ ASSERT_TRUE(tls_ctx);
+ ASSERT_TRUE(SSL_CTX_use_certificate(tls_ctx.get(), cert.get()));
+
+ // However, handshaking an SSLv3_method server should fail to resolve the
+ // version range. Explicit calls to SSL_CTX_set_min_proto_version are the only
+ // way to enable SSL 3.0.
+ bssl::UniquePtr<SSL> client, server;
+ EXPECT_FALSE(ConnectClientAndServer(&client, &server, tls_ctx.get(),
+ ssl3_ctx.get(),
+ nullptr /* no session */));
+ uint32_t err = ERR_get_error();
+ EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err));
+ EXPECT_EQ(SSL_R_NO_SUPPORTED_VERSIONS_ENABLED, ERR_GET_REASON(err));
+
+ // Likewise for SSLv3_method clients.
+ EXPECT_FALSE(ConnectClientAndServer(&client, &server, ssl3_ctx.get(),
+ tls_ctx.get(),
+ nullptr /* no session */));
+ err = ERR_get_error();
+ EXPECT_EQ(ERR_LIB_SSL, ERR_GET_LIB(err));
+ EXPECT_EQ(SSL_R_NO_SUPPORTED_VERSIONS_ENABLED, ERR_GET_REASON(err));
+}
+
// TODO(davidben): Convert this file to GTest properly.
TEST(SSLTest, AllTests) {
if (!TestCipherRules() ||
@@ -3528,8 +3572,7 @@
!TestBadSSL_SESSIONEncoding(kBadSessionVersion) ||
!TestBadSSL_SESSIONEncoding(kBadSessionTrailingData) ||
// TODO(svaldez): Update this when TLS 1.3 is enabled by default.
- !TestDefaultVersion(SSL3_VERSION, TLS1_2_VERSION, &TLS_method) ||
- !TestDefaultVersion(SSL3_VERSION, SSL3_VERSION, &SSLv3_method) ||
+ !TestDefaultVersion(TLS1_VERSION, TLS1_2_VERSION, &TLS_method) ||
!TestDefaultVersion(TLS1_VERSION, TLS1_VERSION, &TLSv1_method) ||
!TestDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &TLSv1_1_method) ||
!TestDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &TLSv1_2_method) ||
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 39b28e0..6421af5 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1038,9 +1038,10 @@
SSL_CTX_set0_buffer_pool(ssl_ctx.get(), g_pool);
- // Enable TLS 1.3 for tests.
+ // Enable SSL 3.0 and TLS 1.3 for tests.
if (!config->is_dtls &&
- !SSL_CTX_set_max_proto_version(ssl_ctx.get(), TLS1_3_VERSION)) {
+ (!SSL_CTX_set_min_proto_version(ssl_ctx.get(), SSL3_VERSION) ||
+ !SSL_CTX_set_max_proto_version(ssl_ctx.get(), TLS1_3_VERSION))) {
return nullptr;
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 2da0bc5..f797c36 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1462,14 +1462,14 @@
name: "DisableEverything",
flags: []string{"-no-tls13", "-no-tls12", "-no-tls11", "-no-tls1", "-no-ssl3"},
shouldFail: true,
- expectedError: ":WRONG_SSL_VERSION:",
+ expectedError: ":NO_SUPPORTED_VERSIONS_ENABLED:",
},
{
protocol: dtls,
name: "DisableEverything-DTLS",
flags: []string{"-no-tls12", "-no-tls1"},
shouldFail: true,
- expectedError: ":WRONG_SSL_VERSION:",
+ expectedError: ":NO_SUPPORTED_VERSIONS_ENABLED:",
},
{
protocol: dtls,