Don't return invalid versions in version_from_wire.
This is in preparation for using the supported_versions extension to
experiment with draft TLS 1.3 versions, since we don't wish to restore
the fallback. With versions begin opaque values, we will want
version_from_wire to reject unknown values, not attempt to preserve
order in some way.
This means ClientHello.version processing needs to be separate code.
That's just written out fully in negotiate_version now. It also means
SSL_set_{min,max}_version will notice invalid inputs which aligns us
better with upstream's versions of those APIs.
This CL doesn't replace ssl->version with an internal-representation
version, though follow work should do it once a couple of changes land
in consumers.
BUG=90
Change-Id: Id2f5e1fa72847c823ee7f082e9e69f55e51ce9da
Reviewed-on: https://boringssl-review.googlesource.com/11122
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/include/openssl/ssl.h b/include/openssl/ssl.h
index c68dc12..aee297c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -565,20 +565,20 @@
#define TLS1_3_DRAFT_VERSION 14
/* SSL_CTX_set_min_version sets the minimum protocol version for |ctx| to
- * |version|. */
-OPENSSL_EXPORT void SSL_CTX_set_min_version(SSL_CTX *ctx, uint16_t version);
+ * |version|. It returns one on success and zero if |version| is invalid. */
+OPENSSL_EXPORT int SSL_CTX_set_min_version(SSL_CTX *ctx, uint16_t version);
/* SSL_CTX_set_max_version sets the maximum protocol version for |ctx| to
- * |version|. */
-OPENSSL_EXPORT void SSL_CTX_set_max_version(SSL_CTX *ctx, uint16_t version);
+ * |version|. It returns one on success and zero if |version| is invalid. */
+OPENSSL_EXPORT int SSL_CTX_set_max_version(SSL_CTX *ctx, uint16_t version);
/* SSL_set_min_version sets the minimum protocol version for |ssl| to
- * |version|. */
-OPENSSL_EXPORT void SSL_set_min_version(SSL *ssl, uint16_t version);
+ * |version|. It returns one on success and zero if |version| is invalid. */
+OPENSSL_EXPORT int SSL_set_min_version(SSL *ssl, uint16_t version);
/* SSL_set_max_version sets the maximum protocol version for |ssl| to
- * |version|. */
-OPENSSL_EXPORT void SSL_set_max_version(SSL *ssl, uint16_t version);
+ * |version|. It returns one on success and zero if |version| is invalid. */
+OPENSSL_EXPORT int SSL_set_max_version(SSL *ssl, uint16_t version);
/* SSL_version returns the TLS or DTLS protocol version used by |ssl|, which is
* one of the |*_VERSION| values. (E.g. |TLS1_2_VERSION|.) Before the version
diff --git a/ssl/dtls_method.c b/ssl/dtls_method.c
index e2e1726..9d791b5 100644
--- a/ssl/dtls_method.c
+++ b/ssl/dtls_method.c
@@ -65,31 +65,33 @@
#include "internal.h"
-static uint16_t dtls1_version_from_wire(uint16_t wire_version) {
- uint16_t tls_version = ~wire_version;
- uint16_t version = tls_version + 0x0201;
- /* If either component overflowed, clamp it so comparisons still work. */
- if ((version >> 8) < (tls_version >> 8)) {
- version = 0xff00 | (version & 0xff);
+static int dtls1_version_from_wire(uint16_t *out_version,
+ uint16_t wire_version) {
+ switch (wire_version) {
+ case DTLS1_VERSION:
+ /* DTLS 1.0 maps to TLS 1.1, not TLS 1.0. */
+ *out_version = TLS1_1_VERSION;
+ return 1;
+ case DTLS1_2_VERSION:
+ *out_version = TLS1_2_VERSION;
+ return 1;
}
- if ((version & 0xff) < (tls_version & 0xff)) {
- version = (version & 0xff00) | 0xff;
- }
- /* DTLS 1.0 maps to TLS 1.1, not TLS 1.0. */
- if (version == TLS1_VERSION) {
- version = TLS1_1_VERSION;
- }
- return version;
+
+ return 0;
}
static uint16_t dtls1_version_to_wire(uint16_t version) {
- assert(version >= TLS1_1_VERSION);
-
- /* DTLS 1.0 maps to TLS 1.1, not TLS 1.0. */
- if (version == TLS1_1_VERSION) {
- return DTLS1_VERSION;
+ switch (version) {
+ case TLS1_1_VERSION:
+ /* DTLS 1.0 maps to TLS 1.1, not TLS 1.0. */
+ return DTLS1_VERSION;
+ case TLS1_2_VERSION:
+ return DTLS1_2_VERSION;
}
- return ~(version - 0x0201);
+
+ /* It is an error to use this function with an invalid version. */
+ assert(0);
+ return 0;
}
static int dtls1_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 4e4cf5c..d78d0a4 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -609,9 +609,11 @@
return 0;
}
/* Add PSK ciphers for TLS 1.3 resumption. */
+ uint16_t session_version;
if (ssl->session != NULL &&
- ssl->method->version_from_wire(ssl->session->ssl_version) >=
- TLS1_3_VERSION) {
+ ssl->method->version_from_wire(&session_version,
+ ssl->session->ssl_version) &&
+ session_version >= TLS1_3_VERSION) {
uint16_t resumption_cipher;
if (ssl_cipher_get_ecdhe_psk_cipher(cipher, &resumption_cipher) &&
!CBB_add_u16(&child, resumption_cipher)) {
@@ -716,9 +718,10 @@
/* If the configured session has expired or was created at a disabled
* version, drop it. */
if (ssl->session != NULL) {
- uint16_t session_version =
- ssl->method->version_from_wire(ssl->session->ssl_version);
- if ((session_version < TLS1_3_VERSION &&
+ uint16_t session_version;
+ if (!ssl->method->version_from_wire(&session_version,
+ ssl->session->ssl_version) ||
+ (session_version < TLS1_3_VERSION &&
ssl->session->session_id_length == 0) ||
ssl->session->not_resumable ||
!ssl_session_is_time_valid(ssl, ssl->session) ||
@@ -797,7 +800,7 @@
CERT *ct = ssl->cert;
int al = SSL_AD_INTERNAL_ERROR;
CBS server_hello, server_random, session_id;
- uint16_t server_wire_version, server_version, cipher_suite;
+ uint16_t server_wire_version, cipher_suite;
uint8_t compression_method;
int ret = ssl->method->ssl_get_message(ssl, -1, ssl_hash_message);
@@ -831,10 +834,9 @@
goto f_err;
}
- server_version = ssl->method->version_from_wire(server_wire_version);
-
- uint16_t min_version, max_version;
+ uint16_t min_version, max_version, server_version;
if (!ssl_get_version_range(ssl, &min_version, &max_version) ||
+ !ssl->method->version_from_wire(&server_version, server_wire_version) ||
server_version < min_version || server_version > max_version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
al = SSL_AD_PROTOCOL_VERSION;
diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index 3790762..d57735a 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -564,12 +564,30 @@
return 0;
}
- uint16_t client_version =
- ssl->method->version_from_wire(client_hello->version);
- ssl->client_version = client_hello->version;
+ /* For TLS versions which use ClientHello.version, convert it to a version we
+ * are aware of. */
+ uint16_t version = 0;
+ if (SSL_is_dtls(ssl)) {
+ if (client_hello->version <= DTLS1_2_VERSION) {
+ version = TLS1_2_VERSION;
+ } else if (client_hello->version <= DTLS1_VERSION) {
+ version = TLS1_1_VERSION;
+ }
+ } else {
+ if (client_hello->version >= TLS1_3_VERSION) {
+ version = TLS1_3_VERSION;
+ } else if (client_hello->version >= TLS1_2_VERSION) {
+ version = TLS1_2_VERSION;
+ } else if (client_hello->version >= TLS1_1_VERSION) {
+ version = TLS1_1_VERSION;
+ } else if (client_hello->version >= TLS1_VERSION) {
+ version = TLS1_VERSION;
+ } else if (client_hello->version >= SSL3_VERSION) {
+ version = SSL3_VERSION;
+ }
+ }
- /* Select the version to use. */
- uint16_t version = client_version;
+ /* Apply our minimum and maximum version. */
if (version > max_version) {
version = max_version;
}
@@ -589,6 +607,7 @@
return 0;
}
+ ssl->client_version = client_hello->version;
ssl->version = ssl->method->version_to_wire(version);
ssl->s3->enc_method = ssl3_get_enc_method(version);
assert(ssl->s3->enc_method != NULL);
diff --git a/ssl/internal.h b/ssl/internal.h
index bdb392c..eff5672 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1090,14 +1090,10 @@
uint16_t min_version;
/* max_version is the maximum implemented version. */
uint16_t max_version;
- /* version_from_wire maps |wire_version| to a protocol version. For
- * SSLv3/TLS, the version is returned as-is. For DTLS, the corresponding TLS
- * version is used. Note that this mapping is not injective but preserves
- * comparisons.
- *
- * TODO(davidben): To normalize some DTLS-specific code, move away from using
- * the wire version except at API boundaries. */
- uint16_t (*version_from_wire)(uint16_t wire_version);
+ /* version_from_wire maps |wire_version| to a protocol version. On success, it
+ * sets |*out_version| to the result and returns one. If the version is
+ * unknown, it returns zero. */
+ int (*version_from_wire)(uint16_t *out_version, uint16_t wire_version);
/* version_to_wire maps |version| to the wire representation. It is an error
* to call it with an invalid version. */
uint16_t (*version_to_wire)(uint16_t version);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 3e27f37..0e8b344 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -949,20 +949,20 @@
return SSL_ERROR_SYSCALL;
}
-void SSL_CTX_set_min_version(SSL_CTX *ctx, uint16_t version) {
- ctx->min_version = ctx->method->version_from_wire(version);
+int SSL_CTX_set_min_version(SSL_CTX *ctx, uint16_t version) {
+ return ctx->method->version_from_wire(&ctx->min_version, version);
}
-void SSL_CTX_set_max_version(SSL_CTX *ctx, uint16_t version) {
- ctx->max_version = ctx->method->version_from_wire(version);
+int SSL_CTX_set_max_version(SSL_CTX *ctx, uint16_t version) {
+ return ctx->method->version_from_wire(&ctx->max_version, version);
}
-void SSL_set_min_version(SSL *ssl, uint16_t version) {
- ssl->min_version = ssl->method->version_from_wire(version);
+int SSL_set_min_version(SSL *ssl, uint16_t version) {
+ return ssl->method->version_from_wire(&ssl->min_version, version);
}
-void SSL_set_max_version(SSL *ssl, uint16_t version) {
- ssl->max_version = ssl->method->version_from_wire(version);
+int SSL_set_max_version(SSL *ssl, uint16_t version) {
+ return ssl->method->version_from_wire(&ssl->max_version, version);
}
uint32_t SSL_CTX_set_options(SSL_CTX *ctx, uint32_t options) {
@@ -2750,7 +2750,15 @@
uint16_t ssl3_protocol_version(const SSL *ssl) {
assert(ssl->s3->have_version);
- return ssl->method->version_from_wire(ssl->version);
+ uint16_t version;
+ if (!ssl->method->version_from_wire(&version, ssl->version)) {
+ /* TODO(davidben): Use the internal version representation for ssl->version
+ * and map to the public API representation at API boundaries. */
+ assert(0);
+ return 0;
+ }
+
+ return version;
}
int SSL_is_server(const SSL *ssl) { return ssl->server; }
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 8d45ace..4c4c0f4 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1523,11 +1523,11 @@
bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
if (!ctx ||
!SSL_CTX_use_certificate(ctx.get(), cert.get()) ||
- !SSL_CTX_use_PrivateKey(ctx.get(), key.get())) {
+ !SSL_CTX_use_PrivateKey(ctx.get(), key.get()) ||
+ !SSL_CTX_set_min_version(ctx.get(), version) ||
+ !SSL_CTX_set_max_version(ctx.get(), version)) {
return false;
}
- SSL_CTX_set_min_version(ctx.get(), version);
- SSL_CTX_set_max_version(ctx.get(), version);
SSL_CTX_set_verify(
ctx.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr);
SSL_CTX_set_cert_verify_callback(ctx.get(), VerifySucceed, NULL);
@@ -1590,11 +1590,11 @@
bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
if (!ctx ||
!SSL_CTX_use_certificate(ctx.get(), cert.get()) ||
- !SSL_CTX_use_PrivateKey(ctx.get(), key.get())) {
+ !SSL_CTX_use_PrivateKey(ctx.get(), key.get()) ||
+ !SSL_CTX_set_min_version(ctx.get(), version) ||
+ !SSL_CTX_set_max_version(ctx.get(), version)) {
return false;
}
- SSL_CTX_set_min_version(ctx.get(), version);
- SSL_CTX_set_max_version(ctx.get(), version);
SSL_CTX_set_verify(
ctx.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, nullptr);
SSL_CTX_set_cert_verify_callback(ctx.get(), VerifySucceed, NULL);
@@ -1631,15 +1631,14 @@
static bool ClientHelloMatches(uint16_t version, const uint8_t *expected,
size_t expected_len) {
bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
- if (!ctx) {
+ if (!ctx ||
+ !SSL_CTX_set_max_version(ctx.get(), version) ||
+ // Our default cipher list varies by CPU capabilities, so manually place
+ // the ChaCha20 ciphers in front.
+ !SSL_CTX_set_cipher_list(ctx.get(), "CHACHA20:ALL")) {
return false;
}
- SSL_CTX_set_max_version(ctx.get(), version);
- // Our default cipher list varies by CPU capabilities, so manually place the
- // ChaCha20 ciphers in front.
- if (!SSL_CTX_set_cipher_list(ctx.get(), "CHACHA20:ALL")) {
- return false;
- }
+
bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get()));
if (!ssl) {
return false;
@@ -1872,16 +1871,15 @@
!SSL_CTX_use_certificate(server_ctx.get(), cert.get()) ||
!SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()) ||
!SSL_CTX_set_session_id_context(server_ctx.get(), kContext1,
- sizeof(kContext1))) {
+ sizeof(kContext1)) ||
+ !SSL_CTX_set_min_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_min_version(server_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(server_ctx.get(), version)) {
return false;
}
- SSL_CTX_set_min_version(client_ctx.get(), version);
- SSL_CTX_set_max_version(client_ctx.get(), version);
SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH);
-
- SSL_CTX_set_min_version(server_ctx.get(), version);
- SSL_CTX_set_max_version(server_ctx.get(), version);
SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH);
bssl::UniquePtr<SSL_SESSION> session =
@@ -1933,16 +1931,16 @@
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
if (!server_ctx || !client_ctx ||
!SSL_CTX_use_certificate(server_ctx.get(), cert.get()) ||
- !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get())) {
+ !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()) ||
+ !SSL_CTX_set_min_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_min_version(server_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(server_ctx.get(), version)) {
return false;
}
- SSL_CTX_set_min_version(client_ctx.get(), version);
- SSL_CTX_set_max_version(client_ctx.get(), version);
SSL_CTX_set_session_cache_mode(client_ctx.get(), SSL_SESS_CACHE_BOTH);
- SSL_CTX_set_min_version(server_ctx.get(), version);
- SSL_CTX_set_max_version(server_ctx.get(), version);
SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH);
SSL_CTX_set_current_time_cb(server_ctx.get(), CurrentTimeCallback);
@@ -2012,17 +2010,16 @@
// this doesn't happen when |version| is TLS 1.2, configure the private
// key to only sign SHA-256.
!SSL_CTX_set_signing_algorithm_prefs(server_ctx2.get(),
- &kECDSAWithSHA256, 1)) {
+ &kECDSAWithSHA256, 1) ||
+ !SSL_CTX_set_min_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(client_ctx.get(), version) ||
+ !SSL_CTX_set_min_version(server_ctx.get(), version) ||
+ !SSL_CTX_set_max_version(server_ctx.get(), version) ||
+ !SSL_CTX_set_min_version(server_ctx2.get(), version) ||
+ !SSL_CTX_set_max_version(server_ctx2.get(), version)) {
return false;
}
- SSL_CTX_set_min_version(client_ctx.get(), version);
- SSL_CTX_set_max_version(client_ctx.get(), version);
- SSL_CTX_set_min_version(server_ctx.get(), version);
- SSL_CTX_set_max_version(server_ctx.get(), version);
- SSL_CTX_set_min_version(server_ctx2.get(), version);
- SSL_CTX_set_max_version(server_ctx2.get(), version);
-
SSL_CTX_set_tlsext_servername_callback(server_ctx.get(), SwitchContext);
SSL_CTX_set_tlsext_servername_arg(server_ctx.get(), server_ctx2.get());
@@ -2047,7 +2044,10 @@
}
static int SetMaxVersion(const struct ssl_early_callback_ctx *ctx) {
- SSL_set_max_version(ctx->ssl, TLS1_2_VERSION);
+ if (!SSL_set_max_version(ctx->ssl, TLS1_2_VERSION)) {
+ return -1;
+ }
+
return 1;
}
@@ -2060,13 +2060,12 @@
bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
if (!cert || !key || !server_ctx || !client_ctx ||
!SSL_CTX_use_certificate(server_ctx.get(), cert.get()) ||
- !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get())) {
+ !SSL_CTX_use_PrivateKey(server_ctx.get(), key.get()) ||
+ !SSL_CTX_set_max_version(client_ctx.get(), TLS1_3_VERSION) ||
+ !SSL_CTX_set_max_version(server_ctx.get(), TLS1_3_VERSION)) {
return false;
}
- SSL_CTX_set_max_version(client_ctx.get(), TLS1_3_VERSION);
- SSL_CTX_set_max_version(server_ctx.get(), TLS1_3_VERSION);
-
SSL_CTX_set_select_certificate_cb(server_ctx.get(), SetMaxVersion);
bssl::UniquePtr<SSL> client, server;
@@ -2083,6 +2082,58 @@
return true;
}
+static bool TestSetVersion() {
+ bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
+ if (!ctx) {
+ return false;
+ }
+
+ if (!SSL_CTX_set_max_version(ctx.get(), TLS1_VERSION) ||
+ !SSL_CTX_set_max_version(ctx.get(), TLS1_1_VERSION) ||
+ !SSL_CTX_set_min_version(ctx.get(), TLS1_VERSION) ||
+ !SSL_CTX_set_min_version(ctx.get(), TLS1_1_VERSION)) {
+ fprintf(stderr, "Could not set valid TLS version.\n");
+ return false;
+ }
+
+ if (SSL_CTX_set_max_version(ctx.get(), DTLS1_VERSION) ||
+ SSL_CTX_set_max_version(ctx.get(), 0x0200) ||
+ SSL_CTX_set_max_version(ctx.get(), 0x1234) ||
+ SSL_CTX_set_min_version(ctx.get(), DTLS1_VERSION) ||
+ SSL_CTX_set_min_version(ctx.get(), 0x0200) ||
+ SSL_CTX_set_min_version(ctx.get(), 0x1234)) {
+ fprintf(stderr, "Unexpectedly set invalid TLS version.\n");
+ return false;
+ }
+
+ ctx.reset(SSL_CTX_new(DTLS_method()));
+ if (!ctx) {
+ return false;
+ }
+
+ if (!SSL_CTX_set_max_version(ctx.get(), DTLS1_VERSION) ||
+ !SSL_CTX_set_max_version(ctx.get(), DTLS1_2_VERSION) ||
+ !SSL_CTX_set_min_version(ctx.get(), DTLS1_VERSION) ||
+ !SSL_CTX_set_min_version(ctx.get(), DTLS1_2_VERSION)) {
+ fprintf(stderr, "Could not set valid DTLS version.\n");
+ return false;
+ }
+
+ if (SSL_CTX_set_max_version(ctx.get(), TLS1_VERSION) ||
+ SSL_CTX_set_max_version(ctx.get(), 0xfefe /* DTLS 1.1 */) ||
+ SSL_CTX_set_max_version(ctx.get(), 0xfffe /* DTLS 0.1 */) ||
+ SSL_CTX_set_max_version(ctx.get(), 0x1234) ||
+ SSL_CTX_set_min_version(ctx.get(), TLS1_VERSION) ||
+ SSL_CTX_set_min_version(ctx.get(), 0xfefe /* DTLS 1.1 */) ||
+ SSL_CTX_set_min_version(ctx.get(), 0xfffe /* DTLS 0.1 */) ||
+ SSL_CTX_set_min_version(ctx.get(), 0x1234)) {
+ fprintf(stderr, "Unexpectedly set invalid DTLS version.\n");
+ return false;
+ }
+
+ return true;
+}
+
int main() {
CRYPTO_library_init();
@@ -2118,7 +2169,8 @@
!TestSessionIDContext() ||
!TestSessionTimeout() ||
!TestSNICallback() ||
- !TestEarlyCallbackVersionSwitch()) {
+ !TestEarlyCallbackVersionSwitch() ||
+ !TestSetVersion()) {
ERR_print_errors_fp(stderr);
return 1;
}
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 5871be2..a89f5cf 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1038,12 +1038,14 @@
* advertise the extension to avoid potentially breaking servers which carry
* over the state from the previous handshake, such as OpenSSL servers
* without upstream's 3c3f0259238594d77264a78944d409f2127642c4. */
+ uint16_t session_version;
if (!ssl->s3->initial_handshake_complete &&
ssl->session != NULL &&
ssl->session->tlsext_tick != NULL &&
/* Don't send TLS 1.3 session tickets in the ticket extension. */
- ssl->method->version_from_wire(ssl->session->ssl_version) <
- TLS1_3_VERSION) {
+ ssl->method->version_from_wire(&session_version,
+ ssl->session->ssl_version) &&
+ session_version < TLS1_3_VERSION) {
ticket_data = ssl->session->tlsext_tick;
ticket_len = ssl->session->tlsext_ticklen;
}
@@ -1107,7 +1109,12 @@
* https://tools.ietf.org/html/rfc5246#section-7.4.1.4.1 */
static int ext_sigalgs_add_clienthello(SSL *ssl, CBB *out) {
- if (ssl->method->version_from_wire(ssl->client_version) < TLS1_2_VERSION) {
+ uint16_t min_version, max_version;
+ if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
+ return 0;
+ }
+
+ if (max_version < TLS1_2_VERSION) {
return 1;
}
@@ -1990,9 +1997,11 @@
return 0;
}
+ uint16_t session_version;
if (max_version < TLS1_3_VERSION || ssl->session == NULL ||
- ssl->method->version_from_wire(ssl->session->ssl_version) <
- TLS1_3_VERSION) {
+ !ssl->method->version_from_wire(&session_version,
+ ssl->session->ssl_version) ||
+ session_version < TLS1_3_VERSION) {
return 1;
}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 7196e49..55b6599 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -814,9 +814,10 @@
return nullptr;
}
- if (!config->is_dtls) {
- // Enable TLS 1.3 for tests.
- SSL_CTX_set_max_version(ssl_ctx.get(), TLS1_3_VERSION);
+ // Enable TLS 1.3 for tests.
+ if (!config->is_dtls &&
+ !SSL_CTX_set_max_version(ssl_ctx.get(), TLS1_3_VERSION)) {
+ return nullptr;
}
std::string cipher_list = "ALL";
@@ -1364,11 +1365,13 @@
!SSL_enable_signed_cert_timestamps(ssl.get())) {
return false;
}
- if (config->min_version != 0) {
- SSL_set_min_version(ssl.get(), (uint16_t)config->min_version);
+ if (config->min_version != 0 &&
+ !SSL_set_min_version(ssl.get(), (uint16_t)config->min_version)) {
+ return false;
}
- if (config->max_version != 0) {
- SSL_set_max_version(ssl.get(), (uint16_t)config->max_version);
+ if (config->max_version != 0 &&
+ !SSL_set_max_version(ssl.get(), (uint16_t)config->max_version)) {
+ return false;
}
if (config->mtu != 0) {
SSL_set_options(ssl.get(), SSL_OP_NO_QUERY_MTU);
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 039cd5c..99d7fac 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4221,6 +4221,17 @@
expectedError: ":UNSUPPORTED_PROTOCOL:",
})
+ testCases = append(testCases, testCase{
+ name: "ServerBogusVersion",
+ config: Config{
+ Bugs: ProtocolBugs{
+ SendServerHelloVersion: 0x1234,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":UNSUPPORTED_PROTOCOL:",
+ })
+
// Test TLS 1.3's downgrade signal.
testCases = append(testCases, testCase{
name: "Downgrade-TLS12-Client",
diff --git a/ssl/tls_method.c b/ssl/tls_method.c
index 61dc9f6..155d09a 100644
--- a/ssl/tls_method.c
+++ b/ssl/tls_method.c
@@ -56,6 +56,7 @@
#include <openssl/ssl.h>
+#include <assert.h>
#include <string.h>
#include <openssl/buf.h>
@@ -63,11 +64,35 @@
#include "internal.h"
-static uint16_t ssl3_version_from_wire(uint16_t wire_version) {
- return wire_version;
+static int ssl3_version_from_wire(uint16_t *out_version,
+ uint16_t wire_version) {
+ switch (wire_version) {
+ case SSL3_VERSION:
+ case TLS1_VERSION:
+ case TLS1_1_VERSION:
+ case TLS1_2_VERSION:
+ case TLS1_3_VERSION:
+ *out_version = wire_version;
+ return 1;
+ }
+
+ return 0;
}
-static uint16_t ssl3_version_to_wire(uint16_t version) { return version; }
+static uint16_t ssl3_version_to_wire(uint16_t version) {
+ switch (version) {
+ case SSL3_VERSION:
+ case TLS1_VERSION:
+ case TLS1_1_VERSION:
+ case TLS1_2_VERSION:
+ case TLS1_3_VERSION:
+ return version;
+ }
+
+ /* It is an error to use this function with an invalid version. */
+ assert(0);
+ return 0;
+}
static int ssl3_set_read_state(SSL *ssl, SSL_AEAD_CTX *aead_ctx) {
if (ssl->s3->rrec.length != 0) {
diff --git a/tool/client.cc b/tool/client.cc
index 27084fc..f8d314e 100644
--- a/tool/client.cc
+++ b/tool/client.cc
@@ -169,7 +169,9 @@
args_map["-max-version"].c_str());
return false;
}
- SSL_CTX_set_max_version(ctx.get(), version);
+ if (!SSL_CTX_set_max_version(ctx.get(), version)) {
+ return false;
+ }
}
if (args_map.count("-min-version") != 0) {
@@ -179,7 +181,9 @@
args_map["-min-version"].c_str());
return false;
}
- SSL_CTX_set_min_version(ctx.get(), version);
+ if (!SSL_CTX_set_min_version(ctx.get(), version)) {
+ return false;
+ }
}
if (args_map.count("-select-next-proto") != 0) {
diff --git a/tool/server.cc b/tool/server.cc
index e0aeb13..b4a4eb1 100644
--- a/tool/server.cc
+++ b/tool/server.cc
@@ -133,7 +133,9 @@
args_map["-max-version"].c_str());
return false;
}
- SSL_CTX_set_max_version(ctx, version);
+ if (!SSL_CTX_set_max_version(ctx, version)) {
+ return false;
+ }
}
if (args_map.count("-min-version") != 0) {
@@ -143,7 +145,9 @@
args_map["-min-version"].c_str());
return false;
}
- SSL_CTX_set_min_version(ctx, version);
+ if (!SSL_CTX_set_min_version(ctx, version)) {
+ return false;
+ }
}
if (args_map.count("-ocsp-response") != 0 &&
