The branch OpenSSL_1_1_0-stable has been updated
       via  622ddb57798bb5c895b0fe40e3fd89fdb4cdbc65 (commit)
      from  30562caa34de5f23dead9b246aaf284748e184bf (commit)


- Log -----------------------------------------------------------------
commit 622ddb57798bb5c895b0fe40e3fd89fdb4cdbc65
Author: Matt Caswell <m...@openssl.org>
Date:   Thu Feb 8 14:48:51 2018 +0000

    Don't calculate the Finished MAC twice
    
    In <= TLSv1.2 a Finished message always comes immediately after a CCS
    except in the case of NPN where there is an additional message between
    the CCS and Finished. Historically we always calculated the Finished MAC
    when we processed the CCS. However to deal with NPN we also calculated it
    when we receive the Finished message. Really this should only have been
    done if we hand negotiated NPN.
    
    This simplifies the code to only calculate the MAC when we receive the
    Finished. In 1.1.1 we need to do it this way anyway because there is no
    CCS (except in middlebox compat mode) in TLSv1.3.
    
    Reviewed-by: Andy Polyakov <ap...@openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5286)

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

Summary of changes:
 include/openssl/ssl.h    |  1 +
 ssl/s3_msg.c             | 25 +------------------------
 ssl/ssl_err.c            |  3 ++-
 ssl/statem/statem_dtls.c |  9 +++++++++
 ssl/statem/statem_lib.c  | 34 +++++++++++++++++-----------------
 ssl/statem/statem_locl.h |  1 +
 6 files changed, 31 insertions(+), 42 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 9ff9b60..abe4406 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2131,6 +2131,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_SSL3_SETUP_KEY_BLOCK                       157
 # define SSL_F_SSL3_SETUP_READ_BUFFER                     156
 # define SSL_F_SSL3_SETUP_WRITE_BUFFER                    291
+# define SSL_F_SSL3_TAKE_MAC                              425
 # define SSL_F_SSL3_WRITE_BYTES                           158
 # define SSL_F_SSL3_WRITE_PENDING                         159
 # define SSL_F_SSL_ADD_CERT_CHAIN                         316
diff --git a/ssl/s3_msg.c b/ssl/s3_msg.c
index 82513d2..8b08ba0 100644
--- a/ssl/s3_msg.c
+++ b/ssl/s3_msg.c
@@ -13,8 +13,6 @@
 int ssl3_do_change_cipher_spec(SSL *s)
 {
     int i;
-    const char *sender;
-    int slen;
 
     if (s->server)
         i = SSL3_CHANGE_CIPHER_SERVER_READ;
@@ -36,28 +34,7 @@ int ssl3_do_change_cipher_spec(SSL *s)
     if (!s->method->ssl3_enc->change_cipher_state(s, i))
         return (0);
 
-    /*
-     * we have to record the message digest at this point so we can get it
-     * before we read the finished message
-     */
-    if (!s->server) {
-        sender = s->method->ssl3_enc->server_finished_label;
-        slen = s->method->ssl3_enc->server_finished_label_len;
-    } else {
-        sender = s->method->ssl3_enc->client_finished_label;
-        slen = s->method->ssl3_enc->client_finished_label_len;
-    }
-
-    i = s->method->ssl3_enc->final_finish_mac(s,
-                                              sender, slen,
-                                              s->s3->tmp.peer_finish_md);
-    if (i == 0) {
-        SSLerr(SSL_F_SSL3_DO_CHANGE_CIPHER_SPEC, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-    s->s3->tmp.peer_finish_md_len = i;
-
-    return (1);
+    return 1;
 }
 
 int ssl3_send_alert(SSL *s, int level, int desc)
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 96edd96..3c2ebe1 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -75,6 +75,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_SSL3_SETUP_KEY_BLOCK), "ssl3_setup_key_block"},
     {ERR_FUNC(SSL_F_SSL3_SETUP_READ_BUFFER), "ssl3_setup_read_buffer"},
     {ERR_FUNC(SSL_F_SSL3_SETUP_WRITE_BUFFER), "ssl3_setup_write_buffer"},
+    {ERR_FUNC(SSL_F_SSL3_TAKE_MAC), "ssl3_take_mac"},
     {ERR_FUNC(SSL_F_SSL3_WRITE_BYTES), "ssl3_write_bytes"},
     {ERR_FUNC(SSL_F_SSL3_WRITE_PENDING), "ssl3_write_pending"},
     {ERR_FUNC(SSL_F_SSL_ADD_CERT_CHAIN), "ssl_add_cert_chain"},
diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 37e7fea..22be871 100644
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -371,6 +371,15 @@ int dtls_get_message(SSL *s, int *mt, unsigned long *len)
         msg_len += DTLS1_HM_HEADER_LENGTH;
     }
 
+    /*
+     * If receiving Finished, record MAC of prior handshake messages for
+     * Finished verification.
+     */
+    if (*mt == SSL3_MT_FINISHED && !ssl3_take_mac(s)) {
+        /* SSLfatal() already called */
+        return 0;
+    }
+
     if (!ssl3_finish_mac(s, p, msg_len))
         return 0;
     if (s->msg_callback)
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 637c610..5702145 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -95,21 +95,15 @@ int tls_construct_finished(SSL *s, const char *sender, int 
slen)
     return 1;
 }
 
-#ifndef OPENSSL_NO_NEXTPROTONEG
 /*
  * ssl3_take_mac calculates the Finished MAC for the handshakes messages seen
  * to far.
  */
-static void ssl3_take_mac(SSL *s)
+int ssl3_take_mac(SSL *s)
 {
     const char *sender;
     int slen;
-    /*
-     * If no new cipher setup return immediately: other functions will set
-     * the appropriate error.
-     */
-    if (s->s3->tmp.new_cipher == NULL)
-        return;
+
     if (!s->server) {
         sender = s->method->ssl3_enc->server_finished_label;
         slen = s->method->ssl3_enc->server_finished_label_len;
@@ -118,12 +112,17 @@ static void ssl3_take_mac(SSL *s)
         slen = s->method->ssl3_enc->client_finished_label_len;
     }
 
-    s->s3->tmp.peer_finish_md_len = s->method->ssl3_enc->final_finish_mac(s,
-                                                                          
sender,
-                                                                          slen,
-                                                                          
s->s3->tmp.peer_finish_md);
+    s->s3->tmp.peer_finish_md_len =
+        s->method->ssl3_enc->final_finish_mac(s, sender, slen,
+                                              s->s3->tmp.peer_finish_md);
+
+    if (s->s3->tmp.peer_finish_md_len == 0) {
+        SSLerr(SSL_F_SSL3_TAKE_MAC, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    return 1;
 }
-#endif
 
 MSG_PROCESS_RETURN tls_process_change_cipher_spec(SSL *s, PACKET *pkt)
 {
@@ -465,14 +464,15 @@ int tls_get_message_body(SSL *s, unsigned long *len)
         n -= i;
     }
 
-#ifndef OPENSSL_NO_NEXTPROTONEG
     /*
      * If receiving Finished, record MAC of prior handshake messages for
      * Finished verification.
      */
-    if (*s->init_buf->data == SSL3_MT_FINISHED)
-        ssl3_take_mac(s);
-#endif
+    if (*(s->init_buf->data) == SSL3_MT_FINISHED && !ssl3_take_mac(s)) {
+        /* SSLfatal() already called */
+        *len = 0;
+        return 0;
+    }
 
     /* Feed this message into MAC computation. */
     if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) {
diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index a46e6c7..98e41fe 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -42,6 +42,7 @@ typedef enum {
 
 /* Flush the write BIO */
 int statem_flush(SSL *s);
+int ssl3_take_mac(SSL *s);
 
 /*
  * TLS/DTLS client state machine functions
_____
openssl-commits mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits

Reply via email to