Use ClientHello.legacy_version for the RSA pre-master no matter what
authorMatt Caswell <matt@openssl.org>
Wed, 23 Nov 2016 13:56:15 +0000 (13:56 +0000)
committerMatt Caswell <matt@openssl.org>
Wed, 23 Nov 2016 17:01:33 +0000 (17:01 +0000)
Don't use what is in supported_versions for the RSA pre-master

Reviewed-by: Emilia Käsper <emilia@openssl.org>
ssl/ssl_locl.h
ssl/statem/statem_clnt.c
ssl/statem/statem_lib.c

index d269595..e909cad 100644 (file)
@@ -1020,7 +1020,10 @@ struct ssl_st {
     int max_proto_version;
     size_t max_cert_list;
     int first_packet;
     int max_proto_version;
     size_t max_cert_list;
     int first_packet;
-    /* what was passed, used for SSLv3/TLS rollback check */
+    /*
+     * What was passed in ClientHello.legacy_version. Used for RSA pre-master
+     * secret and SSLv3/TLS (<=1.2) rollback check
+     */
     int client_version;
     /*
      * If we're using more than one pipeline how should we divide the data
     int client_version;
     /*
      * If we're using more than one pipeline how should we divide the data
index 1f4e630..ba873ee 100644 (file)
@@ -849,7 +849,6 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
     SSL_COMP *comp;
 #endif
     SSL_SESSION *sess = s->session;
     SSL_COMP *comp;
 #endif
     SSL_SESSION *sess = s->session;
-    int client_version;
 
     if (!WPACKET_set_max_size(pkt, SSL3_RT_MAX_PLAIN_LENGTH)) {
         /* Should not happen */
 
     if (!WPACKET_set_max_size(pkt, SSL3_RT_MAX_PLAIN_LENGTH)) {
         /* Should not happen */
@@ -930,8 +929,7 @@ int tls_construct_client_hello(SSL *s, WPACKET *pkt)
      * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the
      * supported_versions extension for the real supported versions.
      */
      * For TLS 1.3 we always set the ClientHello version to 1.2 and rely on the
      * supported_versions extension for the real supported versions.
      */
-    client_version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->client_version;
-    if (!WPACKET_put_bytes_u16(pkt, client_version)
+    if (!WPACKET_put_bytes_u16(pkt, s->client_version)
             || !WPACKET_memcpy(pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         return 0;
             || !WPACKET_memcpy(pkt, s->s3->client_random, SSL3_RANDOM_SIZE)) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
         return 0;
index a971c51..a736a09 100644 (file)
@@ -1077,8 +1077,6 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello)
              * wheter to ignore versions <TLS1.2 in supported_versions. At the
              * moment we honour them if present. To be reviewed later
              */
              * wheter to ignore versions <TLS1.2 in supported_versions. At the
              * moment we honour them if present. To be reviewed later
              */
-            if ((int)candidate_vers > s->client_version)
-                s->client_version = candidate_vers;
             if (version_cmp(s, candidate_vers, best_vers) <= 0)
                 continue;
             for (vent = table;
             if (version_cmp(s, candidate_vers, best_vers) <= 0)
                 continue;
             for (vent = table;
@@ -1299,7 +1297,7 @@ int ssl_get_client_min_max_version(const SSL *s, int *min_version,
 
 /*
  * ssl_set_client_hello_version - Work out what version we should be using for
 
 /*
  * ssl_set_client_hello_version - Work out what version we should be using for
- * the initial ClientHello.
+ * the initial ClientHello.legacy_version field.
  *
  * @s: client SSL handle.
  *
  *
  * @s: client SSL handle.
  *
@@ -1314,6 +1312,12 @@ int ssl_set_client_hello_version(SSL *s)
     if (ret != 0)
         return ret;
 
     if (ret != 0)
         return ret;
 
-    s->client_version = s->version = ver_max;
+    s->version = ver_max;
+
+    /* TLS1.3 always uses TLS1.2 in the legacy_version field */
+    if (!SSL_IS_DTLS(s) && ver_max > TLS1_2_VERSION)
+        ver_max = TLS1_2_VERSION;
+
+    s->client_version = ver_max;
     return 0;
 }
     return 0;
 }