Always set min_version / max_version.
Saves us some mess if they're never zero. This also fixes a bug in
ssl3_get_max_client_version where it didn't account for all versions being
disabled properly.
Change-Id: I4c95ff57cf8953cb4a528263b252379f252f3e01
Reviewed-on: https://boringssl-review.googlesource.com/8512
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index d9cae0c..0e3f3e2 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3619,12 +3619,10 @@
/* lock is used to protect various operations on this object. */
CRYPTO_MUTEX lock;
- /* max_version is the maximum acceptable protocol version. If zero, the
- * maximum supported version, currently (D)TLS 1.2, is used. */
+ /* max_version is the maximum acceptable wire protocol version. */
uint16_t max_version;
- /* min_version is the minimum acceptable protocl version. If zero, the
- * minimum supported version, currently SSL 3.0 and DTLS 1.0, is used */
+ /* min_version is the minimum acceptable wire protocol version. */
uint16_t min_version;
struct ssl_cipher_preference_list_st *cipher_list;
@@ -3868,12 +3866,10 @@
/* version is the protocol version. */
int version;
- /* max_version is the maximum acceptable protocol version. If zero, the
- * maximum supported version, currently (D)TLS 1.2, is used. */
+ /* max_version is the maximum acceptable wire protocol version. */
uint16_t max_version;
- /* min_version is the minimum acceptable protocl version. If zero, the
- * minimum supported version, currently SSL 3.0 and DTLS 1.0, is used */
+ /* min_version is the minimum acceptable wire protocol version. */
uint16_t min_version;
/* method is the method table corresponding to the current protocol (DTLS or
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 8f09222..5da339d 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -297,9 +297,14 @@
if (method->version != 0) {
SSL_CTX_set_max_version(ret, method->version);
SSL_CTX_set_min_version(ret, method->version);
- } else if (!method->method->is_dtls) {
+ } else if (method->method->is_dtls) {
+ /* TODO(svaldez): Enable DTLS 1.3 once implemented. */
+ SSL_CTX_set_max_version(ret, DTLS1_2_VERSION);
+ SSL_CTX_set_min_version(ret, DTLS1_VERSION);
+ } else {
/* TODO(svaldez): Enable TLS 1.3 once implemented. */
SSL_CTX_set_max_version(ret, TLS1_2_VERSION);
+ SSL_CTX_set_min_version(ret, SSL3_VERSION);
}
return ret;
@@ -2558,7 +2563,7 @@
if (SSL_IS_DTLS(ssl)) {
/* Clamp client_version to max_version. */
- if (ssl->max_version != 0 && client_version < ssl->max_version) {
+ if (client_version < ssl->max_version) {
client_version = ssl->max_version;
}
@@ -2571,13 +2576,13 @@
}
/* Check against min_version. */
- if (version != 0 && ssl->min_version != 0 && version > ssl->min_version) {
+ if (version != 0 && version > ssl->min_version) {
return 0;
}
return version;
} else {
/* Clamp client_version to max_version. */
- if (ssl->max_version != 0 && client_version > ssl->max_version) {
+ if (client_version > ssl->max_version) {
client_version = ssl->max_version;
}
@@ -2599,7 +2604,7 @@
}
/* Check against min_version. */
- if (version != 0 && ssl->min_version != 0 && version < ssl->min_version) {
+ if (version != 0 && version < ssl->min_version) {
return 0;
}
return version;
@@ -2630,7 +2635,7 @@
if (!(options & SSL_OP_NO_DTLSv1) && (options & SSL_OP_NO_DTLSv1_2)) {
version = DTLS1_VERSION;
}
- if (ssl->max_version != 0 && version < ssl->max_version) {
+ if (version != 0 && version < ssl->max_version) {
version = ssl->max_version;
}
} else {
@@ -2649,7 +2654,7 @@
if (!(options & SSL_OP_NO_SSLv3) && (options & SSL_OP_NO_TLSv1)) {
version = SSL3_VERSION;
}
- if (ssl->max_version != 0 && version > ssl->max_version) {
+ if (version != 0 && version > ssl->max_version) {
version = ssl->max_version;
}
}
@@ -2659,10 +2664,10 @@
int ssl3_is_version_enabled(SSL *ssl, uint16_t version) {
if (SSL_IS_DTLS(ssl)) {
- if (ssl->max_version != 0 && version < ssl->max_version) {
+ if (version < ssl->max_version) {
return 0;
}
- if (ssl->min_version != 0 && version > ssl->min_version) {
+ if (version > ssl->min_version) {
return 0;
}
@@ -2677,10 +2682,10 @@
return 0;
}
} else {
- if (ssl->max_version != 0 && version > ssl->max_version) {
+ if (version > ssl->max_version) {
return 0;
}
- if (ssl->min_version != 0 && version < ssl->min_version) {
+ if (version < ssl->min_version) {
return 0;
}
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 3e9cd1e..b6d4fd6 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -689,16 +689,13 @@
return true;
}
-static bool TestDefaultVersion(uint16_t version,
+static bool TestDefaultVersion(uint16_t min_version, uint16_t max_version,
const SSL_METHOD *(*method)(void)) {
ScopedSSL_CTX ctx(SSL_CTX_new(method()));
if (!ctx) {
return false;
}
- // TODO(svaldez): Remove TLS1_2_VERSION fallback upon implementing TLS 1.3.
- return ctx->min_version == version &&
- (ctx->max_version == version ||
- (version == 0 && ctx->max_version == TLS1_2_VERSION));
+ return ctx->min_version == min_version && ctx->max_version == max_version;
}
static bool CipherGetRFCName(std::string *out, uint16_t value) {
@@ -1361,14 +1358,15 @@
!TestBadSSL_SESSIONEncoding(kBadSessionExtraField) ||
!TestBadSSL_SESSIONEncoding(kBadSessionVersion) ||
!TestBadSSL_SESSIONEncoding(kBadSessionTrailingData) ||
- !TestDefaultVersion(0, &TLS_method) ||
- !TestDefaultVersion(SSL3_VERSION, &SSLv3_method) ||
- !TestDefaultVersion(TLS1_VERSION, &TLSv1_method) ||
- !TestDefaultVersion(TLS1_1_VERSION, &TLSv1_1_method) ||
- !TestDefaultVersion(TLS1_2_VERSION, &TLSv1_2_method) ||
- !TestDefaultVersion(0, &DTLS_method) ||
- !TestDefaultVersion(DTLS1_VERSION, &DTLSv1_method) ||
- !TestDefaultVersion(DTLS1_2_VERSION, &DTLSv1_2_method) ||
+ // 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_VERSION, &TLSv1_method) ||
+ !TestDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &TLSv1_1_method) ||
+ !TestDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &TLSv1_2_method) ||
+ !TestDefaultVersion(DTLS1_VERSION, DTLS1_2_VERSION, &DTLS_method) ||
+ !TestDefaultVersion(DTLS1_VERSION, DTLS1_VERSION, &DTLSv1_method) ||
+ !TestDefaultVersion(DTLS1_2_VERSION, DTLS1_2_VERSION, &DTLSv1_2_method) ||
!TestCipherGetRFCName() ||
!TestPaddingExtension() ||
!TestClientCAList() ||
