Remove redundant copies of the Finished messages. We only need one copy, not two. This trims 130 bytes of per-connection memory. Change-Id: I334aa7b1f8608e72426986bfa68534d416f3bda9 Reviewed-on: https://boringssl-review.googlesource.com/11569 Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_both.c b/ssl/s3_both.c index e0d97dc..0e0c059 100644 --- a/ssl/s3_both.c +++ b/ssl/s3_both.c
@@ -240,12 +240,12 @@ return ssl->method->write_message(ssl); } - int n = ssl->s3->enc_method->final_finish_mac(ssl, ssl->server, - ssl->s3->tmp.finish_md); - if (n == 0) { + uint8_t finished[EVP_MAX_MD_SIZE]; + size_t finished_len = + ssl->s3->enc_method->final_finish_mac(ssl, ssl->server, finished); + if (finished_len == 0) { return 0; } - ssl->s3->tmp.finish_md_len = n; /* Log the master secret, if logging is enabled. */ if (!ssl_log_secret(ssl, "CLIENT_RANDOM", @@ -254,21 +254,18 @@ return 0; } - /* Copy the finished so we can use it for renegotiation checks */ + /* Copy the finished so we can use it for renegotiation checks. */ if (ssl->server) { - assert(n <= EVP_MAX_MD_SIZE); - memcpy(ssl->s3->previous_server_finished, ssl->s3->tmp.finish_md, n); - ssl->s3->previous_server_finished_len = n; + memcpy(ssl->s3->previous_server_finished, finished, finished_len); + ssl->s3->previous_server_finished_len = finished_len; } else { - assert(n <= EVP_MAX_MD_SIZE); - memcpy(ssl->s3->previous_client_finished, ssl->s3->tmp.finish_md, n); - ssl->s3->previous_client_finished_len = n; + memcpy(ssl->s3->previous_client_finished, finished, finished_len); + ssl->s3->previous_client_finished_len = finished_len; } CBB cbb, body; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_FINISHED) || - !CBB_add_bytes(&body, ssl->s3->tmp.finish_md, - ssl->s3->tmp.finish_md_len) || + !CBB_add_bytes(&body, finished, finished_len) || !ssl->method->finish_message(ssl, &cbb)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); CBB_cleanup(&cbb); @@ -279,21 +276,7 @@ return ssl->method->write_message(ssl); } -/* ssl3_take_mac calculates the Finished MAC for the handshakes messages seen - * so far. */ -static void ssl3_take_mac(SSL *ssl) { - /* If no new cipher setup then return immediately: other functions will set - * the appropriate error. */ - if (ssl->s3->tmp.new_cipher == NULL) { - return; - } - - ssl->s3->tmp.peer_finish_md_len = ssl->s3->enc_method->final_finish_mac( - ssl, !ssl->server, ssl->s3->tmp.peer_finish_md); -} - int ssl3_get_finished(SSL *ssl) { - int al; int ret = ssl->method->ssl_get_message(ssl, SSL3_MT_FINISHED, ssl_dont_hash_message); if (ret <= 0) { @@ -301,44 +284,35 @@ } /* Snapshot the finished hash before incorporating the new message. */ - ssl3_take_mac(ssl); - if (!ssl->method->hash_current_message(ssl)) { - goto err; + uint8_t finished[EVP_MAX_MD_SIZE]; + size_t finished_len = + ssl->s3->enc_method->final_finish_mac(ssl, !ssl->server, finished); + if (finished_len == 0 || + !ssl->method->hash_current_message(ssl)) { + return -1; } - size_t finished_len = ssl->s3->tmp.peer_finish_md_len; - int finished_ok = ssl->init_num == finished_len && - CRYPTO_memcmp(ssl->init_msg, ssl->s3->tmp.peer_finish_md, - finished_len) == 0; + CRYPTO_memcmp(ssl->init_msg, finished, finished_len) == 0; #if defined(BORINGSSL_UNSAFE_FUZZER_MODE) finished_ok = 1; #endif if (!finished_ok) { - al = SSL_AD_DECRYPT_ERROR; + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR); OPENSSL_PUT_ERROR(SSL, SSL_R_DIGEST_CHECK_FAILED); - goto f_err; + return -1; } - /* Copy the finished so we can use it for renegotiation checks */ + /* Copy the finished so we can use it for renegotiation checks. */ if (ssl->server) { - assert(finished_len <= EVP_MAX_MD_SIZE); - memcpy(ssl->s3->previous_client_finished, ssl->s3->tmp.peer_finish_md, - finished_len); + memcpy(ssl->s3->previous_client_finished, finished, finished_len); ssl->s3->previous_client_finished_len = finished_len; } else { - assert(finished_len <= EVP_MAX_MD_SIZE); - memcpy(ssl->s3->previous_server_finished, ssl->s3->tmp.peer_finish_md, - finished_len); + memcpy(ssl->s3->previous_server_finished, finished, finished_len); ssl->s3->previous_server_finished_len = finished_len; } return 1; - -f_err: - ssl3_send_alert(ssl, SSL3_AL_FATAL, al); -err: - return 0; } int ssl3_send_change_cipher_spec(SSL *ssl) {
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index c91fe81..1dd748d 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c
@@ -1233,6 +1233,15 @@ return 1; } +static size_t copy_finished(void *out, size_t out_len, const uint8_t *in, + size_t in_len) { + if (out_len > in_len) { + out_len = in_len; + } + memcpy(out, in, out_len); + return in_len; +} + size_t SSL_get_finished(const SSL *ssl, void *buf, size_t count) { if (!ssl->s3->initial_handshake_complete || ssl3_protocol_version(ssl) < TLS1_VERSION || @@ -1240,12 +1249,13 @@ return 0; } - size_t ret = ssl->s3->tmp.finish_md_len; - if (count > ret) { - count = ret; + if (ssl->server) { + return copy_finished(buf, count, ssl->s3->previous_server_finished, + ssl->s3->previous_server_finished_len); } - memcpy(buf, ssl->s3->tmp.finish_md, count); - return ret; + + return copy_finished(buf, count, ssl->s3->previous_client_finished, + ssl->s3->previous_client_finished_len); } size_t SSL_get_peer_finished(const SSL *ssl, void *buf, size_t count) { @@ -1255,12 +1265,13 @@ return 0; } - size_t ret = ssl->s3->tmp.peer_finish_md_len; - if (count > ret) { - count = ret; + if (ssl->server) { + return copy_finished(buf, count, ssl->s3->previous_client_finished, + ssl->s3->previous_client_finished_len); } - memcpy(buf, ssl->s3->tmp.peer_finish_md, count); - return ret; + + return copy_finished(buf, count, ssl->s3->previous_server_finished, + ssl->s3->previous_server_finished_len); } int SSL_get_verify_mode(const SSL *ssl) { return ssl->verify_mode; }