commit 2dc0204603f777597e2f97662e42887d1af5013f
author David Benjamin <davidben@google.com> Mon Sep 19 19:57:37 2016 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Wed Sep 21 19:51:45 2016 +0000
tree 51be5e0d7c4e766c6a5d08bccaa7c961338d0c15
parent c027999c28db2f448ea5795798080f6a5aaa01d6

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>

ssl/t1_lib.c

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;
   }