The branch master has been updated
       via  5d671101739f9e9b259126375a9e8b2fa42ac45f (commit)
      from  368297d17352c7eb30efff443509caf7cf59f65f (commit)


- Log -----------------------------------------------------------------
commit 5d671101739f9e9b259126375a9e8b2fa42ac45f
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.
    
    Coincidentally, this commit also fixes the fact that no-nextprotoneg does
    not currently work in master.
    
    Reviewed-by: Andy Polyakov <ap...@openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5285)

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

Summary of changes:
 ssl/s3_msg.c             | 23 -----------------------
 ssl/statem/statem_dtls.c |  9 +++++++++
 ssl/statem/statem_lib.c  | 34 +++++++++++++++++-----------------
 ssl/statem/statem_locl.h |  1 +
 4 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/ssl/s3_msg.c b/ssl/s3_msg.c
index 5e6e7c4..6e102a1 100644
--- a/ssl/s3_msg.c
+++ b/ssl/s3_msg.c
@@ -12,9 +12,6 @@
 int ssl3_do_change_cipher_spec(SSL *s)
 {
     int i;
-    size_t finish_md_len;
-    const char *sender;
-    size_t slen;
 
     if (s->server)
         i = SSL3_CHANGE_CIPHER_SERVER_READ;
@@ -36,26 +33,6 @@ 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;
-    }
-
-    finish_md_len = s->method->ssl3_enc->final_finish_mac(s, sender, slen,
-                                            s->s3->tmp.peer_finish_md);
-    if (finish_md_len == 0) {
-        SSLerr(SSL_F_SSL3_DO_CHANGE_CIPHER_SPEC, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-    s->s3->tmp.peer_finish_md_len = finish_md_len;
-
     return 1;
 }
 
diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 8604d5b..0ac60cb 100644
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -376,6 +376,15 @@ int dtls_get_message(SSL *s, int *mt, size_t *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 f57f33c..82a7119 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -646,21 +646,15 @@ MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET 
*pkt)
     return MSG_PROCESS_FINISHED_READING;
 }
 
-#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;
     size_t 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;
@@ -669,12 +663,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) {
+        /* SSLfatal() already called */
+        return 0;
+    }
+
+    return 1;
 }
-#endif
 
 MSG_PROCESS_RETURN tls_process_change_cipher_spec(SSL *s, PACKET *pkt)
 {
@@ -1227,14 +1226,15 @@ int tls_get_message_body(SSL *s, size_t *len)
         n -= readbytes;
     }
 
-#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 af081eb..25e56e4 100644
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -54,6 +54,7 @@ typedef enum {
 
 typedef int (*confunc_f) (SSL *s, WPACKET *pkt);
 
+int ssl3_take_mac(SSL *s);
 int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups,
                   size_t num_groups, int checkallow);
 int create_synthetic_message_hash(SSL *s, const unsigned char *hashval,
_____
openssl-commits mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits

Reply via email to