diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index c68dc12c..aee297c8 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -565,20 +565,20 @@ OPENSSL_EXPORT int DTLSv1_handle_timeout(SSL *ssl); #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 e2e17265..9d791b59 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 4e4cf5c6..d78d0a4f 100644 --- a/ssl/handshake_client.c +++ b/ssl/handshake_client.c @@ -609,9 +609,11 @@ static int ssl_write_client_cipher_list(SSL *ssl, CBB *out, 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 @@ static int ssl3_send_client_hello(SSL *ssl) { /* 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 @@ static int ssl3_get_server_hello(SSL *ssl) { 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 @@ static int ssl3_get_server_hello(SSL *ssl) { 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 3790762a..d57735a9 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -564,12 +564,30 @@ static int negotiate_version( 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 @@ static int negotiate_version( 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 bdb392c9..eff56723 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1090,14 +1090,10 @@ struct ssl_protocol_method_st { 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 3e27f37d..0e8b3442 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -949,20 +949,20 @@ int SSL_get_error(const SSL *ssl, int ret_code) { 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 @@ int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version, 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 8d45acea..4c4c0f47 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -1523,11 +1523,11 @@ static bool TestGetPeerCertificate() { bssl::UniquePtr 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 @@ static bool TestRetainOnlySHA256OfCerts() { bssl::UniquePtr 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 TestRetainOnlySHA256OfCerts() { static bool ClientHelloMatches(uint16_t version, const uint8_t *expected, size_t expected_len) { bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); - if (!ctx) { - 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")) { + 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; } + bssl::UniquePtr ssl(SSL_new(ctx.get())); if (!ssl) { return false; @@ -1872,16 +1871,15 @@ static bool TestSessionIDContext() { !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 session = @@ -1933,16 +1931,16 @@ static bool TestSessionTimeout() { bssl::UniquePtr 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 @@ static bool TestSNICallback() { // 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 bool TestSNICallback() { } 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 @@ static bool TestEarlyCallbackVersionSwitch() { bssl::UniquePtr 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 client, server; @@ -2083,6 +2082,58 @@ static bool TestEarlyCallbackVersionSwitch() { return true; } +static bool TestSetVersion() { + bssl::UniquePtr 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 @@ int main() { !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 5871be23..a89f5cfd 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1038,12 +1038,14 @@ static int ext_ticket_add_clienthello(SSL *ssl, CBB *out) { * 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 @@ static int ext_ticket_add_serverhello(SSL *ssl, CBB *out) { * 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 @@ static int ext_pre_shared_key_add_clienthello(SSL *ssl, CBB *out) { 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 7196e49e..55b6599d 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -814,9 +814,10 @@ static bssl::UniquePtr SetupCtx(const TestConfig *config) { 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 @@ static bool DoExchange(bssl::UniquePtr *out_session, !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 039cd5c4..99d7fac5 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -4221,6 +4221,17 @@ func addVersionNegotiationTests() { 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 61dc9f62..155d09a2 100644 --- a/ssl/tls_method.c +++ b/ssl/tls_method.c @@ -56,6 +56,7 @@ #include +#include #include #include @@ -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 27084fcb..f8d314ea 100644 --- a/tool/client.cc +++ b/tool/client.cc @@ -169,7 +169,9 @@ bool Client(const std::vector &args) { 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 @@ bool Client(const std::vector &args) { 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 e0aeb134..b4a4eb13 100644 --- a/tool/server.cc +++ b/tool/server.cc @@ -133,7 +133,9 @@ bool Server(const std::vector &args) { 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 @@ bool Server(const std::vector &args) { 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 &&