[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  be9c8deb7de92feb5e5300f2e46d3516bcc43c00 (commit)
       via  8aac5d2e5a4c5d1c13e0f671d263a9eb59031514 (commit)
       via  de0717ebccd7daf6e68b6aae09e900c9cd9ad37d (commit)
       via  6da573921503126f3f4f4f48dedce41e5b0ea780 (commit)
      from  5507b9610af8ba521ac10488b5cb6d6c6f1c69fa (commit)


- Log -----------------------------------------------------------------
commit be9c8deb7de92feb5e5300f2e46d3516bcc43c00
Author: Matt Caswell <matt at openssl.org>
Date:   Tue May 17 15:27:09 2016 +0100

    Add a comment to explain the use of |num_recs|
    
    In the SSLV2ClientHello processing code in ssl3_get_record, the value of
    |num_recs| will always be 0. This isn't obvious from the code so a comment
    is added to explain it.
    
    Reviewed-by: Viktor Dukhovni <viktor at openssl.org>

commit 8aac5d2e5a4c5d1c13e0f671d263a9eb59031514
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Apr 26 16:28:26 2016 +0100

    Fix RSA dasync engine bug
    
    When RSA went opaque a bug was introduced into the dasync engine where
    the wrong function was being set for the rsa_priv_dec operation.
    
    Reviewed-by: Viktor Dukhovni <viktor at openssl.org>

commit de0717ebccd7daf6e68b6aae09e900c9cd9ad37d
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Apr 26 16:07:17 2016 +0100

    Use the current record offset in ssl3_get_record
    
    The function ssl3_get_record() can obtain multiple records in one go
    as long as we are set up for pipelining and all the records are app
    data records. The logic in the while loop which reads in each record is
    supposed to only continue looping if the last record we read was app data
    and we have an app data record waiting in the buffer to be processed. It
    was actually checking that the first record had app data and we have an
    app data record waiting. This actually amounts to the same thing so wasn't
    wrong - but it looks a bit odd because it uses the |rr| array without an
    offset.
    
    Reviewed-by: Viktor Dukhovni <viktor at openssl.org>

commit 6da573921503126f3f4f4f48dedce41e5b0ea780
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Apr 26 16:00:09 2016 +0100

    There is only one read buffer
    
    Pipelining introduced the concept of multiple records being read in one
    go. Therefore we work with an array of SSL3_RECORD objects. The pipelining
    change erroneously made a change in ssl3_get_record() to apply the current
    record offset to the SSL3_BUFFER we are using for reading. This is wrong -
    there is only ever one read buffer. This reverts that change. In practice
    this should make little difference because the code block in question is
    only ever used when we are processing a single record.
    
    Reviewed-by: Viktor Dukhovni <viktor at openssl.org>

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

Summary of changes:
 engines/e_dasync.c       |  2 +-
 ssl/record/ssl3_record.c | 15 ++++++++++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/engines/e_dasync.c b/engines/e_dasync.c
index 27a5602..c307998 100644
--- a/engines/e_dasync.c
+++ b/engines/e_dasync.c
@@ -228,7 +228,7 @@ static int bind_dasync(ENGINE *e)
         || RSA_meth_set_pub_enc(dasync_rsa_method, dasync_pub_enc) == 0
         || RSA_meth_set_pub_dec(dasync_rsa_method, dasync_pub_dec) == 0
         || RSA_meth_set_priv_enc(dasync_rsa_method, dasync_rsa_priv_enc) == 0
-        || RSA_meth_set_priv_enc(dasync_rsa_method, dasync_rsa_priv_dec) == 0
+        || RSA_meth_set_priv_dec(dasync_rsa_method, dasync_rsa_priv_dec) == 0
         || RSA_meth_set_mod_exp(dasync_rsa_method, dasync_rsa_mod_exp) == 0
         || RSA_meth_set_bn_mod_exp(dasync_rsa_method, BN_mod_exp_mont) == 0
         || RSA_meth_set_init(dasync_rsa_method, dasync_rsa_init) == 0
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index 3c28572..57fef4a 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -270,13 +270,21 @@ int ssl3_get_record(SSL *s)
             if (s->first_packet && s->server && !s->read_hash
                     && !s->enc_read_ctx
                     && (p[0] & 0x80) && (p[2] == SSL2_MT_CLIENT_HELLO)) {
-                /* SSLv2 style record */
+                /*
+                 *  SSLv2 style record
+                 *
+                 * |num_recs| here will actually always be 0 because
+                 * |num_recs > 0| only ever occurs when we are processing
+                 * multiple app data records - which we know isn't the case here
+                 * because it is an SSLv2ClientHello. We keep it using
+                 * |num_recs| for the sake of consistency
+                 */
                 rr[num_recs].type = SSL3_RT_HANDSHAKE;
                 rr[num_recs].rec_version = SSL2_VERSION;
 
                 rr[num_recs].length = ((p[0] & 0x7f) << 8) | p[1];
 
-                if (rr[num_recs].length > SSL3_BUFFER_get_len(&rbuf[num_recs])
+                if (rr[num_recs].length > SSL3_BUFFER_get_len(rbuf)
                                  - SSL2_RT_HEADER_LENGTH) {
                     al = SSL_AD_RECORD_OVERFLOW;
                     SSLerr(SSL_F_SSL3_GET_RECORD, SSL_R_PACKET_LENGTH_TOO_LONG);
@@ -420,7 +428,8 @@ int ssl3_get_record(SSL *s)
 
         /* we have pulled in a full packet so zero things */
         RECORD_LAYER_reset_packet_length(&s->rlayer);
-    } while (num_recs < max_recs && rr->type == SSL3_RT_APPLICATION_DATA
+    } while (num_recs < max_recs
+             && rr[num_recs-1].type == SSL3_RT_APPLICATION_DATA
              && SSL_USE_EXPLICIT_IV(s)
              && s->enc_read_ctx != NULL
              && (EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(s->enc_read_ctx))