Simplify version configuration.

OpenSSL's SSL_OP_NO_* flags allow discontinuous version ranges. This is a
nuisance for two reasons. First it makes it unnecessarily difficult to answer
"are any versions below TLS 1.3 enabled?". Second the protocol does not allow
discontinuous version ranges on the client anyway. OpenSSL instead picks the
first continous range of enabled versions on the client, but not the server.

This is bizarrely inconsistent. It also doesn't quite do this as the
ClientHello sending logic does this, but not the ServerHello processing logic.
So we actually break some invariants slightly. The logic is also cumbersome in
DTLS which kindly inverts the comparison logic.

First, switch min_version/max_version's storage to normalized versions. Next
replace all the ad-hoc version-related functions with a single
ssl_get_version_range function. Client and server now consistently pick a
contiguous range of versions. Note this is a slight behavior change for
servers. Version-range-sensitive logic is rewritten to use this new function.

BUG=66

Change-Id: Iad0d64f2b7a917603fc7da54c9fc6656c5fbdb24
Reviewed-on: https://boringssl-review.googlesource.com/8513
Reviewed-by: David Benjamin <davidben@google.com>

include/openssl/ssl.h

@@ -3626,10 +3626,12 @@ struct ssl_ctx_st {
   /* lock is used to protect various operations on this object. */
   CRYPTO_MUTEX lock;
 
-  /* max_version is the maximum acceptable wire protocol version. */
+  /* max_version is the maximum acceptable protocol version. Note this version
+   * is normalized in DTLS. */
   uint16_t max_version;
 
-  /* min_version is the minimum acceptable wire protocol version. */
+  /* min_version is the minimum acceptable protocol version. Note this version
+   * is normalized in DTLS. */
   uint16_t min_version;
 
   struct ssl_cipher_preference_list_st *cipher_list;
@@ -3873,10 +3875,12 @@ struct ssl_st {
   /* version is the protocol version. */
   int version;
 
-  /* max_version is the maximum acceptable wire protocol version. */
+  /* max_version is the maximum acceptable protocol version. Note this version
+   * is normalized in DTLS. */
   uint16_t max_version;
 
-  /* min_version is the minimum acceptable wire protocol version. */
+  /* min_version is the minimum acceptable protocol version. Note this version
+   * is normalized in DTLS. */
   uint16_t min_version;
 
   /* method is the method table corresponding to the current protocol (DTLS or

ssl/d1_meth.c

@@ -56,11 +56,44 @@
 
 #include <openssl/ssl.h>
 
+#include <assert.h>
+
 #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);
+  }
+  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;
+}
+
+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;
+  }
+  return ~(version - 0x0201);
+}
+
 static const SSL_PROTOCOL_METHOD DTLS_protocol_method = {
     1 /* is_dtls */,
+    TLS1_1_VERSION,
+    TLS1_2_VERSION,
+    dtls1_version_from_wire,
+    dtls1_version_to_wire,
     dtls1_new,
     dtls1_free,
     dtls1_get_message,

ssl/handshake_client.c

@@ -573,7 +573,7 @@ static int ssl3_write_client_cipher_list(SSL *ssl, CBB *out) {
     /* TODO(davidben): Also check |SSL_CIPHER_get_max_version| against the
      * minimum enabled version. See https://crbug.com/boringssl/66. */
     if (SSL_CIPHER_get_min_version(cipher) >
-        ssl3_version_from_wire(ssl, ssl->client_version)) {
+        ssl->method->version_from_wire(ssl->client_version)) {
       continue;
     }
     any_enabled = 1;
@@ -622,30 +622,31 @@ static int ssl3_send_client_hello(SSL *ssl) {
   CBB cbb;
   CBB_zero(&cbb);
 
+  uint16_t min_version, max_version;
+  if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
+    goto err;
+  }
+
   assert(ssl->state == SSL3_ST_CW_CLNT_HELLO_A);
   if (!ssl->s3->have_version) {
-    uint16_t max_version = ssl3_get_max_client_version(ssl);
-    /* Disabling all versions is silly: return an error. */
-    if (max_version == 0) {
-      OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION);
-      goto err;
-    }
-
-    ssl->version = max_version;
+    ssl->version = ssl->method->version_to_wire(max_version);
     /* Only set |ssl->client_version| on the initial handshake. Renegotiations,
      * although locked to a version, reuse the value. When using the plain RSA
      * key exchange, the ClientHello version is checked in the premaster secret.
      * Some servers fail when this value changes. */
-    ssl->client_version = max_version;
+    ssl->client_version = ssl->version;
   }
 
   /* If the configured session has expired or was created at a disabled
    * version, drop it. */
-  if (ssl->session != NULL &&
-      (ssl->session->session_id_length == 0 || ssl->session->not_resumable ||
-       ssl->session->timeout < (long)(time(NULL) - ssl->session->time) ||
-       !ssl3_is_version_enabled(ssl, ssl->session->ssl_version))) {
-    SSL_set_session(ssl, NULL);
+  if (ssl->session != NULL) {
+    uint16_t session_version =
+        ssl->method->version_from_wire(ssl->session->ssl_version);
+    if (ssl->session->session_id_length == 0 || ssl->session->not_resumable ||
+        ssl->session->timeout < (long)(time(NULL) - ssl->session->time) ||
+        session_version < min_version || session_version > max_version) {
+      SSL_set_session(ssl, NULL);
+    }
   }
 
   /* If resending the ClientHello in DTLS after a HelloVerifyRequest, don't
@@ -747,7 +748,7 @@ static int ssl3_get_server_hello(SSL *ssl) {
   int al = SSL_AD_INTERNAL_ERROR, ok;
   long n;
   CBS server_hello, server_random, session_id;
-  uint16_t server_version, cipher_suite;
+  uint16_t server_wire_version, server_version, cipher_suite;
   uint8_t compression_method;
 
   n = ssl->method->ssl_get_message(ssl, SSL3_MT_SERVER_HELLO, ssl_hash_message,
@@ -770,7 +771,7 @@ static int ssl3_get_server_hello(SSL *ssl) {
 
   CBS_init(&server_hello, ssl->init_msg, n);
 
-  if (!CBS_get_u16(&server_hello, &server_version) ||
+  if (!CBS_get_u16(&server_hello, &server_wire_version) ||
       !CBS_get_bytes(&server_hello, &server_random, SSL3_RANDOM_SIZE) ||
       !CBS_get_u8_length_prefixed(&server_hello, &session_id) ||
       CBS_len(&session_id) > SSL3_SESSION_ID_SIZE ||
@@ -781,20 +782,24 @@ static int ssl3_get_server_hello(SSL *ssl) {
     goto f_err;
   }
 
+  server_version = ssl->method->version_from_wire(server_wire_version);
+
   assert(ssl->s3->have_version == ssl->s3->initial_handshake_complete);
   if (!ssl->s3->have_version) {
-    if (!ssl3_is_version_enabled(ssl, server_version)) {
+    uint16_t min_version, max_version;
+    if (!ssl_get_version_range(ssl, &min_version, &max_version) ||
+        server_version < min_version || server_version > max_version) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
       al = SSL_AD_PROTOCOL_VERSION;
       goto f_err;
     }
-    ssl->version = server_version;
+    ssl->version = server_wire_version;
     ssl->s3->enc_method = ssl3_get_enc_method(server_version);
     assert(ssl->s3->enc_method != NULL);
     /* At this point, the connection's version is known and ssl->version is
      * fixed. Begin enforcing the record-layer version. */
     ssl->s3->have_version = 1;
-  } else if (server_version != ssl->version) {
+  } else if (server_wire_version != ssl->version) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION);
     al = SSL_AD_PROTOCOL_VERSION;
     goto f_err;

ssl/handshake_server.c

@@ -719,7 +719,7 @@ static int ssl3_get_client_hello(SSL *ssl) {
   STACK_OF(SSL_CIPHER) *ciphers = NULL;
   struct ssl_early_callback_ctx early_ctx;
   CBS client_hello;
-  uint16_t client_version;
+  uint16_t client_wire_version;
   CBS client_random, session_id, cipher_suites, compression_methods;
   SSL_SESSION *session = NULL;
 
@@ -782,7 +782,7 @@ static int ssl3_get_client_hello(SSL *ssl) {
   }
 
   CBS_init(&client_hello, ssl->init_msg, n);
-  if (!CBS_get_u16(&client_hello, &client_version) ||
+  if (!CBS_get_u16(&client_hello, &client_wire_version) ||
       !CBS_get_bytes(&client_hello, &client_random, SSL3_RANDOM_SIZE) ||
       !CBS_get_u8_length_prefixed(&client_hello, &session_id) ||
       CBS_len(&session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH) {
@@ -791,9 +791,11 @@ static int ssl3_get_client_hello(SSL *ssl) {
     goto f_err;
   }
 
+  uint16_t client_version = ssl->method->version_from_wire(client_wire_version);
+
   /* use version from inside client hello, not from record header (may differ:
    * see RFC 2246, Appendix E, second paragraph) */
-  ssl->client_version = client_version;
+  ssl->client_version = client_wire_version;
 
   /* Load the client random. */
   memcpy(ssl->s3->client_random, CBS_data(&client_random), SSL3_RANDOM_SIZE);
@@ -809,27 +811,35 @@ static int ssl3_get_client_hello(SSL *ssl) {
     }
   }
 
+  uint16_t min_version, max_version;
+  if (!ssl_get_version_range(ssl, &min_version, &max_version)) {
+    al = SSL_AD_PROTOCOL_VERSION;
+    goto f_err;
+  }
+
   /* Note: This codepath may run twice if |ssl_get_prev_session| completes
    * asynchronously.
    *
    * TODO(davidben): Clean up the order of events around ClientHello
    * processing. */
   if (!ssl->s3->have_version) {
-    /* Select version to use */
-    uint16_t version = ssl3_get_mutual_version(ssl, client_version);
-    if (version == 0) {
+    /* Select the version to use. */
+    uint16_t version = client_version;
+    if (version > max_version) {
+      version = max_version;
+    }
+    if (version < min_version) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
       al = SSL_AD_PROTOCOL_VERSION;
       goto f_err;
     }
-    ssl->version = version;
+    ssl->version = ssl->method->version_to_wire(version);
     ssl->s3->enc_method = ssl3_get_enc_method(version);
     assert(ssl->s3->enc_method != NULL);
     /* At this point, the connection's version is known and |ssl->version| is
      * fixed. Begin enforcing the record-layer version. */
     ssl->s3->have_version = 1;
-  } else if (SSL_IS_DTLS(ssl) ? (ssl->client_version > ssl->version)
-                            : (ssl->client_version < ssl->version)) {
+  } else if (client_version < ssl3_protocol_version(ssl)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_VERSION_NUMBER);
     al = SSL_AD_PROTOCOL_VERSION;
     goto f_err;
@@ -916,7 +926,7 @@ static int ssl3_get_client_hello(SSL *ssl) {
     goto f_err;
   }
 
-  ciphers = ssl_bytes_to_cipher_list(ssl, &cipher_suites);
+  ciphers = ssl_bytes_to_cipher_list(ssl, &cipher_suites, max_version);
   if (ciphers == NULL) {
     goto err;
   }

ssl/internal.h

@@ -809,6 +809,21 @@ struct ssl_method_st {
 struct ssl_protocol_method_st {
   /* is_dtls is one if the protocol is DTLS and zero otherwise. */
   char is_dtls;
+  /* min_version is the minimum implemented version. */
+  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_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);
   int (*ssl_new)(SSL *ssl);
   void (*ssl_free)(SSL *ssl);
   long (*ssl_get_message)(SSL *ssl, int msg_type,
@@ -954,7 +969,8 @@ enum ssl_session_result_t ssl_get_prev_session(
     SSL *ssl, SSL_SESSION **out_session, int *out_send_ticket,
     const struct ssl_early_callback_ctx *ctx);
 
-STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *ssl, const CBS *cbs);
+STACK_OF(SSL_CIPHER) *
+    ssl_bytes_to_cipher_list(SSL *ssl, const CBS *cbs, uint16_t max_version);
 void ssl_cipher_preference_list_free(
     struct ssl_cipher_preference_list_st *cipher_list);
 struct ssl_cipher_preference_list_st *ssl_get_cipher_preferences(SSL *ssl);
@@ -1167,33 +1183,10 @@ int ssl3_can_false_start(const SSL *ssl);
  * |version|. */
 const SSL3_ENC_METHOD *ssl3_get_enc_method(uint16_t version);
 
-/* ssl3_get_max_server_version returns the maximum SSL/TLS version number
- * supported by |ssl| as a server, or zero if all versions are disabled. */
-uint16_t ssl3_get_max_server_version(const SSL *ssl);
-
-/* ssl3_get_mutual_version selects the protocol version on |ssl| for a client
- * which advertises |client_version|. If no suitable version exists, it returns
- * zero. */
-uint16_t ssl3_get_mutual_version(SSL *ssl, uint16_t client_version);
-
-/* ssl3_get_max_client_version returns the maximum protocol version configured
- * for the client. It is guaranteed that the set of allowed versions at or below
- * this maximum version is contiguous. If all versions are disabled, it returns
- * zero. */
-uint16_t ssl3_get_max_client_version(SSL *ssl);
-
-/* ssl3_is_version_enabled returns one if |version| is an enabled protocol
- * version for |ssl| and zero otherwise. */
-int ssl3_is_version_enabled(SSL *ssl, uint16_t version);
-
-/* ssl3_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 ssl3_version_from_wire(const SSL *ssl, uint16_t wire_version);
+/* ssl_get_version_range sets |*out_min_version| and |*out_max_version| to the
+ * minimum and maximum enabled protocol versions, respectively. */
+int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version,
+                          uint16_t *out_max_version);
 
 /* ssl3_protocol_version returns |ssl|'s protocol version. It is an error to
  * call this function before the version is determined. */

ssl/s3_meth.c

@@ -59,8 +59,18 @@
 #include "internal.h"
 
 
+static uint16_t ssl3_version_from_wire(uint16_t wire_version) {
+  return wire_version;
+}
+
+static uint16_t ssl3_version_to_wire(uint16_t version) { return version; }
+
 static const SSL_PROTOCOL_METHOD TLS_protocol_method = {
     0 /* is_dtls */,
+    SSL3_VERSION,
+    TLS1_3_VERSION,
+    ssl3_version_from_wire,
+    ssl3_version_to_wire,
     ssl3_new,
     ssl3_free,
     ssl3_get_message,

ssl/ssl_lib.c

@@ -292,19 +292,17 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *method) {
     ret->options |= SSL_OP_NO_TICKET;
   }
 
+  ret->min_version = ret->method->min_version;
+  ret->max_version = ret->method->max_version;
+
   /* Lock the SSL_CTX to the specified version, for compatibility with legacy
    * uses of SSL_METHOD. */
   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) {
-    /* 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. */
+  } else if (!method->method->is_dtls) {
+    /* TODO(svaldez): Enable TLS 1.3 by default once fully implemented. */
     SSL_CTX_set_max_version(ret, TLS1_2_VERSION);
-    SSL_CTX_set_min_version(ret, SSL3_VERSION);
   }
 
   return ret;
@@ -842,19 +840,19 @@ int SSL_get_error(const SSL *ssl, int ret_code) {
 }
 
 void SSL_CTX_set_min_version(SSL_CTX *ctx, uint16_t version) {
-  ctx->min_version = version;
+  ctx->min_version = ctx->method->version_from_wire(version);
 }
 
 void SSL_CTX_set_max_version(SSL_CTX *ctx, uint16_t version) {
-  ctx->max_version = version;
+  ctx->max_version = ctx->method->version_from_wire(version);
 }
 
 void SSL_set_min_version(SSL *ssl, uint16_t version) {
-  ssl->min_version = version;
+  ssl->min_version = ssl->method->version_from_wire(version);
 }
 
 void SSL_set_max_version(SSL *ssl, uint16_t version) {
-  ssl->max_version = version;
+  ssl->max_version = ssl->method->version_from_wire(version);
 }
 
 uint32_t SSL_CTX_set_options(SSL_CTX *ctx, uint32_t options) {
@@ -1494,7 +1492,8 @@ int SSL_set_cipher_list(SSL *ssl, const char *str) {
   return 1;
 }
 
-STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *ssl, const CBS *cbs) {
+STACK_OF(SSL_CIPHER) *
+    ssl_bytes_to_cipher_list(SSL *ssl, const CBS *cbs, uint16_t max_version) {
   CBS cipher_suites = *cbs;
   const SSL_CIPHER *c;
   STACK_OF(SSL_CIPHER) *sk;
@@ -1536,9 +1535,7 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *ssl, const CBS *cbs) {
 
     /* Check for FALLBACK_SCSV. */
     if (ssl->s3 && cipher_suite == (SSL3_CK_FALLBACK_SCSV & 0xffff)) {
-      uint16_t max_version = ssl3_get_max_server_version(ssl);
-      if (SSL_IS_DTLS(ssl) ? (uint16_t)ssl->version > max_version
-                         : (uint16_t)ssl->version < max_version) {
+      if (ssl3_protocol_version(ssl) < max_version) {
         OPENSSL_PUT_ERROR(SSL, SSL_R_INAPPROPRIATE_FALLBACK);
         ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL3_AD_INAPPROPRIATE_FALLBACK);
         goto err;
@@ -2515,8 +2512,6 @@ const SSL3_ENC_METHOD *ssl3_get_enc_method(uint16_t version) {
     case TLS1_1_VERSION:
     case TLS1_2_VERSION:
     case TLS1_3_VERSION:
-    case DTLS1_VERSION:
-    case DTLS1_2_VERSION:
       return &TLSv1_enc_data;
 
     default:
@@ -2524,96 +2519,41 @@ const SSL3_ENC_METHOD *ssl3_get_enc_method(uint16_t version) {
   }
 }
 
-uint16_t ssl3_get_max_server_version(const SSL *ssl) {
-  uint16_t max_version;
+const struct {
+  uint16_t version;
+  uint32_t flag;
+} kVersions[] = {
+    {SSL3_VERSION, SSL_OP_NO_SSLv3},
+    {TLS1_VERSION, SSL_OP_NO_TLSv1},
+    {TLS1_1_VERSION, SSL_OP_NO_TLSv1_1},
+    {TLS1_2_VERSION, SSL_OP_NO_TLSv1_2},
+    {TLS1_3_VERSION, SSL_OP_NO_TLSv1_3},
+};
 
-  if (SSL_IS_DTLS(ssl)) {
-    max_version = (ssl->max_version != 0) ? ssl->max_version : DTLS1_2_VERSION;
-    if (!(ssl->options & SSL_OP_NO_DTLSv1_2) &&
-        DTLS1_2_VERSION >= max_version) {
-      return DTLS1_2_VERSION;
-    }
-    if (!(ssl->options & SSL_OP_NO_DTLSv1) && DTLS1_VERSION >= max_version) {
-      return DTLS1_VERSION;
-    }
-    return 0;
-  }
-
-  max_version = (ssl->max_version != 0) ? ssl->max_version : TLS1_3_VERSION;
-  if (!(ssl->options & SSL_OP_NO_TLSv1_3) && TLS1_3_VERSION <= max_version) {
-    return TLS1_3_VERSION;
-  }
-  if (!(ssl->options & SSL_OP_NO_TLSv1_2) && TLS1_2_VERSION <= max_version) {
-    return TLS1_2_VERSION;
-  }
-  if (!(ssl->options & SSL_OP_NO_TLSv1_1) && TLS1_1_VERSION <= max_version) {
-    return TLS1_1_VERSION;
-  }
-  if (!(ssl->options & SSL_OP_NO_TLSv1) && TLS1_VERSION <= max_version) {
-    return TLS1_VERSION;
-  }
-  if (!(ssl->options & SSL_OP_NO_SSLv3) && SSL3_VERSION <= max_version) {
-    return SSL3_VERSION;
-  }
-  return 0;
-}
-
-uint16_t ssl3_get_mutual_version(SSL *ssl, uint16_t client_version) {
-  uint16_t version = 0;
+static const size_t kVersionsLen = sizeof(kVersions) / sizeof(kVersions[0]);
 
+int ssl_get_version_range(const SSL *ssl, uint16_t *out_min_version,
+                          uint16_t *out_max_version) {
+  /* For historical reasons, |SSL_OP_NO_DTLSv1| aliases |SSL_OP_NO_TLSv1|, but
+   * DTLS 1.0 should be mapped to TLS 1.1. */
+  uint32_t options = ssl->options;
   if (SSL_IS_DTLS(ssl)) {
-    /* Clamp client_version to max_version. */
-    if (client_version < ssl->max_version) {
-      client_version = ssl->max_version;
-    }
-
-    if (client_version <= DTLS1_2_VERSION &&
-        !(ssl->options & SSL_OP_NO_DTLSv1_2)) {
-      version = DTLS1_2_VERSION;
-    } else if (client_version <= DTLS1_VERSION &&
-               !(ssl->options & SSL_OP_NO_DTLSv1)) {
-      version = DTLS1_VERSION;
-    }
-
-    /* Check against min_version. */
-    if (version != 0 && version > ssl->min_version) {
-      return 0;
-    }
-    return version;
-  } else {
-    /* Clamp client_version to max_version. */
-    if (client_version > ssl->max_version) {
-      client_version = ssl->max_version;
+    options &= ~SSL_OP_NO_TLSv1_1;
+    if (options & SSL_OP_NO_DTLSv1) {
+      options |= SSL_OP_NO_TLSv1_1;
     }
+  }
 
-    if (client_version >= TLS1_3_VERSION &&
-        !(ssl->options & SSL_OP_NO_TLSv1_3)) {
-      version = TLS1_3_VERSION;
-    } else if (client_version >= TLS1_2_VERSION &&
-               !(ssl->options & SSL_OP_NO_TLSv1_2)) {
-      version = TLS1_2_VERSION;
-    } else if (client_version >= TLS1_1_VERSION &&
-               !(ssl->options & SSL_OP_NO_TLSv1_1)) {
-      version = TLS1_1_VERSION;
-    } else if (client_version >= TLS1_VERSION &&
-               !(ssl->options & SSL_OP_NO_TLSv1)) {
-      version = TLS1_VERSION;
-    } else if (client_version >= SSL3_VERSION &&
-               !(ssl->options & SSL_OP_NO_SSLv3)) {
-      version = SSL3_VERSION;
-    }
+  uint16_t min_version = ssl->min_version;
+  uint16_t max_version = ssl->max_version;
 
-    /* Check against min_version. */
-    if (version != 0 && version < ssl->min_version) {
-      return 0;
-    }
-    return version;
+  /* Bound the range to only those implemented in this protocol. */
+  if (min_version < ssl->method->min_version) {
+    min_version = ssl->method->min_version;
+  }
+  if (max_version > ssl->method->max_version) {
+    max_version = ssl->method->max_version;
   }
-}
-
-uint16_t ssl3_get_max_client_version(SSL *ssl) {
-  uint32_t options = ssl->options;
-  uint16_t version = 0;
 
   /* OpenSSL's API for controlling versions entails blacklisting individual
    * protocols. This has two problems. First, on the client, the protocol can
@@ -2624,117 +2564,48 @@ uint16_t ssl3_get_max_client_version(SSL *ssl) {
    * To account for both of these, OpenSSL interprets the client-side bitmask
    * as a min/max range by picking the lowest contiguous non-empty range of
    * enabled protocols. Note that this means it is impossible to set a maximum
-   * version of TLS 1.2 in a future-proof way.
-   *
-   * By this scheme, the maximum version is the lowest version V such that V is
-   * enabled and V+1 is disabled or unimplemented. */
-  if (SSL_IS_DTLS(ssl)) {
-    if (!(options & SSL_OP_NO_DTLSv1_2)) {
-      version = DTLS1_2_VERSION;
-    }
-    if (!(options & SSL_OP_NO_DTLSv1) && (options & SSL_OP_NO_DTLSv1_2)) {
-      version = DTLS1_VERSION;
-    }
-    if (version != 0 && version < ssl->max_version) {
-      version = ssl->max_version;
-    }
-  } else {
-    if (!(options & SSL_OP_NO_TLSv1_3)) {
-      version = TLS1_3_VERSION;
-    }
-    if (!(options & SSL_OP_NO_TLSv1_2) && (options & SSL_OP_NO_TLSv1_3)) {
-      version = TLS1_2_VERSION;
-    }
-    if (!(options & SSL_OP_NO_TLSv1_1) && (options & SSL_OP_NO_TLSv1_2)) {
-      version = TLS1_1_VERSION;
-    }
-    if (!(options & SSL_OP_NO_TLSv1) && (options & SSL_OP_NO_TLSv1_1)) {
-      version = TLS1_VERSION;
-    }
-    if (!(options & SSL_OP_NO_SSLv3) && (options & SSL_OP_NO_TLSv1)) {
-      version = SSL3_VERSION;
-    }
-    if (version != 0 && version > ssl->max_version) {
-      version = ssl->max_version;
-    }
-  }
-
-  return version;
-}
-
-int ssl3_is_version_enabled(SSL *ssl, uint16_t version) {
-  if (SSL_IS_DTLS(ssl)) {
-    if (version < ssl->max_version) {
-      return 0;
+   * version of the higest supported TLS version in a future-proof way. */
+  size_t i;
+  int any_enabled = 0;
+  for (i = 0; i < kVersionsLen; i++) {
+    /* Only look at the versions already enabled. */
+    if (min_version > kVersions[i].version) {
+      continue;
     }
-    if (version > ssl->min_version) {
-      return 0;
+    if (max_version < kVersions[i].version) {
+      break;
     }
 
-    switch (version) {
-      case DTLS1_VERSION:
-        return !(ssl->options & SSL_OP_NO_DTLSv1);
-
-      case DTLS1_2_VERSION:
-        return !(ssl->options & SSL_OP_NO_DTLSv1_2);
-
-      default:
-        return 0;
-    }
-  } else {
-    if (version > ssl->max_version) {
-      return 0;
-    }
-    if (version < ssl->min_version) {
-      return 0;
+    if (!(options & kVersions[i].flag)) {
+      /* The minimum version is the first enabled version. */
+      if (!any_enabled) {
+        any_enabled = 1;
+        min_version = kVersions[i].version;
+      }
+      continue;
     }
 
-    switch (version) {
-      case SSL3_VERSION:
-        return !(ssl->options & SSL_OP_NO_SSLv3);
-
-      case TLS1_VERSION:
-        return !(ssl->options & SSL_OP_NO_TLSv1);
-
-      case TLS1_1_VERSION:
-        return !(ssl->options & SSL_OP_NO_TLSv1_1);
-
-      case TLS1_2_VERSION:
-        return !(ssl->options & SSL_OP_NO_TLSv1_2);
-
-      case TLS1_3_VERSION:
-        return !(ssl->options & SSL_OP_NO_TLSv1_3);
-
-      default:
-        return 0;
+    /* If there is a disabled version after the first enabled one, all versions
+     * after it are implicitly disabled. */
+    if (any_enabled) {
+      max_version = kVersions[i-1].version;
+      break;
     }
   }
-}
 
-uint16_t ssl3_version_from_wire(const SSL *ssl, uint16_t wire_version) {
-  if (!SSL_IS_DTLS(ssl)) {
-    return wire_version;
+  if (!any_enabled) {
+    OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_SSL_VERSION);
+    return 0;
   }
 
-  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);
-  }
-  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;
+  *out_min_version = min_version;
+  *out_max_version = max_version;
+  return 1;
 }
 
 uint16_t ssl3_protocol_version(const SSL *ssl) {
   assert(ssl->s3->have_version);
-  return ssl3_version_from_wire(ssl, ssl->version);
+  return ssl->method->version_from_wire(ssl->version);
 }
 
 int SSL_is_server(SSL *ssl) { return ssl->server; }

ssl/ssl_test.cc

@@ -695,7 +695,12 @@ static bool TestDefaultVersion(uint16_t min_version, uint16_t max_version,
   if (!ctx) {
     return false;
   }
-  return ctx->min_version == min_version && ctx->max_version == max_version;
+  if (ctx->min_version != min_version || ctx->max_version != max_version) {
+    fprintf(stderr, "Got min %04x, max %04x; wanted min %04x, max %04x\n",
+            ctx->min_version, ctx->max_version, min_version, max_version);
+    return false;
+  }
+  return true;
 }
 
 static bool CipherGetRFCName(std::string *out, uint16_t value) {
@@ -1364,9 +1369,9 @@ int main() {
       !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) ||
+      !TestDefaultVersion(TLS1_1_VERSION, TLS1_2_VERSION, &DTLS_method) ||
+      !TestDefaultVersion(TLS1_1_VERSION, TLS1_1_VERSION, &DTLSv1_method) ||
+      !TestDefaultVersion(TLS1_2_VERSION, TLS1_2_VERSION, &DTLSv1_2_method) ||
       !TestCipherGetRFCName() ||
       !TestPaddingExtension() ||
       !TestClientCAList() ||

ssl/t1_lib.c

@@ -1034,7 +1034,7 @@ 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 (ssl3_version_from_wire(ssl, ssl->client_version) < TLS1_2_VERSION) {
+  if (ssl->method->version_from_wire(ssl->client_version) < TLS1_2_VERSION) {
     return 1;
   }