[openssl-commits] [openssl] OpenSSL_1_1_0-stable update

[Matt Caswell]

The branch OpenSSL_1_1_0-stable has been updated
       via  c088325b42cee1236f7b4996dd71f93ecc95bd5d (commit)
      from  0de0fb887b464a9f0f20ddcf98e7980ae4b0fd9c (commit)


- Log -----------------------------------------------------------------
commit c088325b42cee1236f7b4996dd71f93ecc95bd5d
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Jan 24 11:12:03 2017 +0000

    Fix SSL_get0_raw_cipherlist()
    
    SSL_get0_raw_cipherlist() was a little too "raw" in the case of an SSLv2
    compat ClientHello. In 1.0.2 and below, during version negotiation, if
    we received an SSLv2 compat ClientHello but actually wanted to do SSLv3+
    then we would construct a "fake" SSLv3+ ClientHello. This "fake" ClientHello
    would have its ciphersuite list converted to the SSLv3+ format. It was
    this "fake" raw list that got saved away to later be returned by a call to
    SSL_get0_raw_cipherlist().
    
    In 1.1.0+ version negotiation works differently and we process an SSLv2
    compat ClientHello directly without the need for an intermediary "fake"
    ClientHello. This meant that the raw ciphersuite list being saved was in
    the SSLv2 format. Any caller of this function would not expect that and
    potentially overread the returned buffer by one byte.
    
    Fixes #2189
    
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/2280)
    (cherry picked from commit 07afdf3c3ac97af4f2b4eec22a97f7230f8227e0)

-----------------------------------------------------------------------

Summary of changes:
 ssl/statem/statem_srvr.c | 63 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 46 insertions(+), 17 deletions(-)

diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 41215da..d367c21 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -3256,20 +3256,51 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
         return NULL;
     }
 
-    if ((skp == NULL) || (*skp == NULL)) {
-        sk = sk_SSL_CIPHER_new_null(); /* change perhaps later */
-        if (sk == NULL) {
-            SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
-            *al = SSL_AD_INTERNAL_ERROR;
-            return NULL;
-        }
-    } else {
-        sk = *skp;
-        sk_SSL_CIPHER_zero(sk);
+    sk = sk_SSL_CIPHER_new_null();
+    if (sk == NULL) {
+        SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
+        *al = SSL_AD_INTERNAL_ERROR;
+        return NULL;
     }
 
-    if (!PACKET_memdup(cipher_suites, &s->s3->tmp.ciphers_raw,
-                       &s->s3->tmp.ciphers_rawlen)) {
+    if (sslv2format) {
+        size_t numciphers = PACKET_remaining(cipher_suites) / n;
+        PACKET sslv2ciphers = *cipher_suites;
+        unsigned int leadbyte;
+        unsigned char *raw;
+
+        /*
+         * We store the raw ciphers list in SSLv3+ format so we need to do some
+         * preprocessing to convert the list first. If there are any SSLv2 only
+         * ciphersuites with a non-zero leading byte then we are going to
+         * slightly over allocate because we won't store those. But that isn't a
+         * problem.
+         */
+        raw = s->s3->tmp.ciphers_raw = OPENSSL_malloc(numciphers * n);
+        if (raw == NULL) {
+            *al = SSL_AD_INTERNAL_ERROR;
+            goto err;
+        }
+        for (s->s3->tmp.ciphers_rawlen = 0;
+             PACKET_remaining(&sslv2ciphers) > 0;
+             raw += TLS_CIPHER_LEN) {
+            if (!PACKET_get_1(&sslv2ciphers, &leadbyte)
+                    || (leadbyte == 0
+                        && !PACKET_copy_bytes(&sslv2ciphers, raw,
+                                              TLS_CIPHER_LEN))
+                    || (leadbyte != 0
+                        && !PACKET_forward(&sslv2ciphers, TLS_CIPHER_LEN))) {
+                *al = SSL_AD_INTERNAL_ERROR;
+                OPENSSL_free(raw);
+                s->s3->tmp.ciphers_raw = NULL;
+                s->s3->tmp.ciphers_rawlen = 0;
+                goto err;
+            }
+            if (leadbyte == 0)
+                s->s3->tmp.ciphers_rawlen += TLS_CIPHER_LEN;
+        }
+    } else if (!PACKET_memdup(cipher_suites, &s->s3->tmp.ciphers_raw,
+                           &s->s3->tmp.ciphers_rawlen)) {
         *al = SSL_AD_INTERNAL_ERROR;
         goto err;
     }
@@ -3330,11 +3361,9 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
         goto err;
     }
 
-    if (skp != NULL)
-        *skp = sk;
-    return (sk);
+    *skp = sk;
+    return sk;
  err:
-    if ((skp == NULL) || (*skp == NULL))
-        sk_SSL_CIPHER_free(sk);
+    sk_SSL_CIPHER_free(sk);
     return NULL;
 }