commit 0e9e0ba18c56160d2e25f958b83a5119aace89c3
author Matthew Braithwaite <mab@google.com> Tue Apr 10 15:40:12 2018 -0700
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri May 04 18:27:34 2018 +0000
tree a5e581115cd0fc7df81a3376b4ce67e8641a775a
parent 537553ff7f8af593aadb0e9bbc642ae243034d63

SSL_apply_handback: check that SSL version is valid.

Along the way, check the version against the cipher to make sure the
combination is possible.

(Found by fuzzing: a bad version trips an assert.)

Change-Id: Ib0a284fd5fd9b7ba5ceba63aa6224966282a2cb7
Reviewed-on: https://boringssl-review.googlesource.com/27265
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>

ssl/handoff.cc

diff --git a/ssl/handoff.cc b/ssl/handoff.cc
index 89fca2f..5ba1d11 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -252,10 +252,16 @@
   }
 
   ssl->version = session->ssl_version;
+  s3->have_version = true;
+  if (!ssl_method_supports_version(ssl->method, ssl->version) ||
+      session->cipher != s3->hs->new_cipher ||
+      ssl_protocol_version(ssl) < SSL_CIPHER_get_min_version(session->cipher) ||
+      SSL_CIPHER_get_max_version(session->cipher) < ssl_protocol_version(ssl)) {
+    return false;
+  }
   ssl->do_handshake = ssl_server_handshake;
   ssl->server = true;
 
-  s3->have_version = true;
   s3->hs->state = CBS_len(&transcript) == 0 ? state12_finish_server_handshake
                                             : state12_read_client_certificate;
   s3->session_reused = session_reused;

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 9840aa4..be1b9dd 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -378,6 +378,10 @@
 // ssl_supports_version returns whether |hs| supports |version|.
 bool ssl_supports_version(SSL_HANDSHAKE *hs, uint16_t version);
 
+// ssl_method_supports_version returns whether |method| supports |version|.
+bool ssl_method_supports_version(const SSL_PROTOCOL_METHOD *method,
+                                 uint16_t version);
+
 // ssl_add_supported_versions writes the supported versions of |hs| to |cbb|, in
 // decreasing preference order.
 bool ssl_add_supported_versions(SSL_HANDSHAKE *hs, CBB *cbb);

ssl/ssl_versions.cc

diff --git a/ssl/ssl_versions.cc b/ssl/ssl_versions.cc
index 0e28620..a09efa1 100644
--- a/ssl/ssl_versions.cc
+++ b/ssl/ssl_versions.cc
@@ -81,8 +81,8 @@
   }
 }
 
-static bool method_supports_version(const SSL_PROTOCOL_METHOD *method,
-                                    uint16_t version) {
+bool ssl_method_supports_version(const SSL_PROTOCOL_METHOD *method,
+                                 uint16_t version) {
   const uint16_t *versions;
   size_t num_versions;
   get_method_versions(method, &versions, &num_versions);
@@ -164,7 +164,7 @@
 static bool set_version_bound(const SSL_PROTOCOL_METHOD *method, uint16_t *out,
                               uint16_t version) {
   if (!api_version_to_wire(&version, version) ||
-      !method_supports_version(method, version) ||
+      !ssl_method_supports_version(method, version) ||
       !ssl_protocol_version_from_wire(out, version)) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_SSL_VERSION);
     return false;
@@ -292,7 +292,7 @@
 bool ssl_supports_version(SSL_HANDSHAKE *hs, uint16_t version) {
   SSL *const ssl = hs->ssl;
   uint16_t protocol_version;
-  if (!method_supports_version(ssl->method, version) ||
+  if (!ssl_method_supports_version(ssl->method, version) ||
       !ssl_protocol_version_from_wire(&protocol_version, version) ||
       hs->min_version > protocol_version ||
       protocol_version > hs->max_version) {