[openssl-commits] [openssl] OpenSSL_1_1_0-stable update
Matt Caswell
matt at openssl.org
Fri Feb 9 15:37:01 UTC 2018
The branch OpenSSL_1_1_0-stable has been updated
via 622ddb57798bb5c895b0fe40e3fd89fdb4cdbc65 (commit)
from 30562caa34de5f23dead9b246aaf284748e184bf (commit)
- Log -----------------------------------------------------------------
commit 622ddb57798bb5c895b0fe40e3fd89fdb4cdbc65
Author: Matt Caswell <matt at 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 <appro at 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
More information about the openssl-commits
mailing list