From 643d65b6e9fb780188211b156383980513f491f7 Mon Sep 17 00:00:00 2001 From: jsing Date: Sun, 25 Apr 2021 13:15:22 +0000 Subject: [PATCH] Clean up derivation of finished/peer finished. Make this process more readable by having specific client/server functions, calling the correct one based on s->server. This allows to remove various SSL_ST_ACCEPT/SSL_ST_CONNECT checks, along with duplicate code. ok inoguchi@ tb@ --- lib/libssl/Makefile | 3 +- lib/libssl/ssl_both.c | 77 ++++++++++++----------------------- lib/libssl/ssl_clnt.c | 5 +-- lib/libssl/ssl_locl.h | 13 ++++-- lib/libssl/ssl_pkt.c | 38 ++++++----------- lib/libssl/ssl_srvr.c | 8 ++-- lib/libssl/t1_enc.c | 27 +------------ lib/libssl/tls12_lib.c | 92 ++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 147 insertions(+), 116 deletions(-) create mode 100644 lib/libssl/tls12_lib.c diff --git a/lib/libssl/Makefile b/lib/libssl/Makefile index e4ad5f36f7c..d6730a5e04f 100644 --- a/lib/libssl/Makefile +++ b/lib/libssl/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.68 2020/10/03 18:01:55 jsing Exp $ +# $OpenBSD: Makefile,v 1.69 2021/04/25 13:15:22 jsing Exp $ .include .ifndef NOMAN @@ -67,6 +67,7 @@ SRCS= \ ssl_versions.c \ t1_enc.c \ t1_lib.c \ + tls12_lib.c \ tls12_record_layer.c \ tls13_buffer.c \ tls13_client.c \ diff --git a/lib/libssl/ssl_both.c b/lib/libssl/ssl_both.c index ad9b0ee2578..fe04f809b07 100644 --- a/lib/libssl/ssl_both.c +++ b/lib/libssl/ssl_both.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_both.c,v 1.28 2021/04/19 16:51:56 jsing Exp $ */ +/* $OpenBSD: ssl_both.c,v 1.29 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -164,42 +164,39 @@ ssl3_do_write(SSL *s, int type) } int -ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen) +ssl3_send_finished(SSL *s, int state_a, int state_b) { CBB cbb, finished; - int md_len; memset(&cbb, 0, sizeof(cbb)); - if (S3I(s)->hs.state == a) { - md_len = TLS1_FINISH_MAC_LENGTH; - OPENSSL_assert(md_len <= EVP_MAX_MD_SIZE); - - if (tls1_final_finish_mac(s, sender, slen, - S3I(s)->hs.finished) != md_len) - return (0); - S3I(s)->hs.finished_len = md_len; + if (S3I(s)->hs.state == state_a) { + if (!tls12_derive_finished(s)) + goto err; /* Copy finished so we can use it for renegotiation checks. */ if (!s->server) { memcpy(S3I(s)->previous_client_finished, - S3I(s)->hs.finished, md_len); - S3I(s)->previous_client_finished_len = md_len; + S3I(s)->hs.finished, S3I(s)->hs.finished_len); + S3I(s)->previous_client_finished_len = + S3I(s)->hs.finished_len; } else { memcpy(S3I(s)->previous_server_finished, - S3I(s)->hs.finished, md_len); - S3I(s)->previous_server_finished_len = md_len; + S3I(s)->hs.finished, S3I(s)->hs.finished_len); + S3I(s)->previous_server_finished_len = + S3I(s)->hs.finished_len; } if (!ssl3_handshake_msg_start(s, &cbb, &finished, SSL3_MT_FINISHED)) goto err; - if (!CBB_add_bytes(&finished, S3I(s)->hs.finished, md_len)) + if (!CBB_add_bytes(&finished, S3I(s)->hs.finished, + S3I(s)->hs.finished_len)) goto err; if (!ssl3_handshake_msg_finish(s, &cbb)) goto err; - S3I(s)->hs.state = b; + S3I(s)->hs.state = state_b; } return (ssl3_handshake_write(s)); @@ -210,36 +207,6 @@ ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen) return (-1); } -/* - * ssl3_take_mac calculates the Finished MAC for the handshakes messages seen - * so far. - */ -static void -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 (S3I(s)->hs.cipher == NULL) - return; - - if (S3I(s)->hs.state & SSL_ST_CONNECT) { - sender = TLS_MD_SERVER_FINISH_CONST; - slen = TLS_MD_SERVER_FINISH_CONST_SIZE; - } else { - sender = TLS_MD_CLIENT_FINISH_CONST; - slen = TLS_MD_CLIENT_FINISH_CONST_SIZE; - } - - S3I(s)->hs.peer_finished_len = - tls1_final_finish_mac(s, sender, slen, - S3I(s)->hs.peer_finished); -} - int ssl3_get_finished(SSL *s, int a, int b) { @@ -544,10 +511,16 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) n -= i; } - /* If receiving Finished, record MAC of prior handshake messages for - * Finished verification. */ - if (*s->internal->init_buf->data == SSL3_MT_FINISHED) - ssl3_take_mac(s); + /* + * If receiving Finished, record MAC of prior handshake messages for + * Finished verification. + */ + if (*s->internal->init_buf->data == SSL3_MT_FINISHED) { + if (S3I(s)->hs.cipher != NULL) { + if (!tls12_derive_peer_finished(s)) + goto err; + } + } /* Feed this message into MAC computation. */ if (s->internal->mac_packet) { @@ -566,7 +539,7 @@ ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok) fatal_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); -err: + err: *ok = 0; return (-1); } diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c index 7f69b8ba980..c129bb6d660 100644 --- a/lib/libssl/ssl_clnt.c +++ b/lib/libssl/ssl_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_clnt.c,v 1.92 2021/04/21 19:27:56 jsing Exp $ */ +/* $OpenBSD: ssl_clnt.c,v 1.93 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -497,8 +497,7 @@ ssl3_connect(SSL *s) if (SSL_is_dtls(s) && !s->internal->hit) dtls1_start_timer(s); ret = ssl3_send_finished(s, SSL3_ST_CW_FINISHED_A, - SSL3_ST_CW_FINISHED_B, TLS_MD_CLIENT_FINISH_CONST, - TLS_MD_CLIENT_FINISH_CONST_SIZE); + SSL3_ST_CW_FINISHED_B); if (ret <= 0) goto end; if (!SSL_is_dtls(s)) diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index 27397308efa..a9cab69ee09 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.337 2021/04/21 19:27:56 jsing Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.338 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1219,7 +1219,7 @@ int ssl3_do_write(SSL *s, int type); int ssl3_send_alert(SSL *s, int level, int desc); int ssl3_get_req_cert_types(SSL *s, CBB *cbb); long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int *ok); -int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen); +int ssl3_send_finished(SSL *s, int state_a, int state_b); int ssl3_num_ciphers(void); const SSL_CIPHER *ssl3_get_cipher(unsigned int u); const SSL_CIPHER *ssl3_get_cipher_by_id(unsigned int id); @@ -1371,10 +1371,14 @@ void tls1_transcript_freeze(SSL *s); void tls1_transcript_unfreeze(SSL *s); int tls1_transcript_record(SSL *s, const unsigned char *buf, size_t len); +int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len, + const void *seed1, size_t seed1_len, const void *seed2, size_t seed2_len, + const void *seed3, size_t seed3_len, const void *seed4, size_t seed4_len, + const void *seed5, size_t seed5_len, unsigned char *out, size_t out_len); + void tls1_cleanup_key_block(SSL *s); int tls1_change_cipher_state(SSL *s, int which); int tls1_setup_key_block(SSL *s); -int tls1_final_finish_mac(SSL *s, const char *str, int slen, unsigned char *p); int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, int len); int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen, @@ -1383,6 +1387,9 @@ int tls1_export_keying_material(SSL *s, unsigned char *out, size_t olen, int tls1_alert_code(int code); int ssl_ok(SSL *s); +int tls12_derive_finished(SSL *s); +int tls12_derive_peer_finished(SSL *s); + int ssl_using_ecc_cipher(SSL *s); int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s); diff --git a/lib/libssl/ssl_pkt.c b/lib/libssl/ssl_pkt.c index a760f90a3a0..6e0cfe21029 100644 --- a/lib/libssl/ssl_pkt.c +++ b/lib/libssl/ssl_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_pkt.c,v 1.40 2021/03/29 16:46:09 jsing Exp $ */ +/* $OpenBSD: ssl_pkt.c,v 1.41 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1155,13 +1155,6 @@ int ssl3_do_change_cipher_spec(SSL *s) { int i; - const char *sender; - int slen; - - if (S3I(s)->hs.state & SSL_ST_ACCEPT) - i = SSL3_CHANGE_CIPHER_SERVER_READ; - else - i = SSL3_CHANGE_CIPHER_CLIENT_READ; if (S3I(s)->hs.tls12.key_block == NULL) { if (s->session == NULL || s->session->master_key_length == 0) { @@ -1175,27 +1168,20 @@ ssl3_do_change_cipher_spec(SSL *s) return (0); } + if (S3I(s)->hs.state & SSL_ST_ACCEPT) + i = SSL3_CHANGE_CIPHER_SERVER_READ; + else + i = SSL3_CHANGE_CIPHER_CLIENT_READ; + if (!tls1_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 (S3I(s)->hs.state & SSL_ST_CONNECT) { - sender = TLS_MD_SERVER_FINISH_CONST; - slen = TLS_MD_SERVER_FINISH_CONST_SIZE; - } else { - sender = TLS_MD_CLIENT_FINISH_CONST; - slen = TLS_MD_CLIENT_FINISH_CONST_SIZE; - } - - i = tls1_final_finish_mac(s, sender, slen, - S3I(s)->hs.peer_finished); - if (i == 0) { - SSLerror(s, ERR_R_INTERNAL_ERROR); - return 0; - } - S3I(s)->hs.peer_finished_len = i; + /* + * We have to record the message digest at this point so we can get it + * before we read the finished message. + */ + if (!tls12_derive_peer_finished(s)) + return (0); return (1); } diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index c85a25158f8..2c15081f450 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.103 2021/04/21 19:27:56 jsing Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.104 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -666,10 +666,8 @@ ssl3_accept(SSL *s) case SSL3_ST_SW_FINISHED_A: case SSL3_ST_SW_FINISHED_B: - ret = ssl3_send_finished(s, - SSL3_ST_SW_FINISHED_A, SSL3_ST_SW_FINISHED_B, - TLS_MD_SERVER_FINISH_CONST, - TLS_MD_SERVER_FINISH_CONST_SIZE); + ret = ssl3_send_finished(s, SSL3_ST_SW_FINISHED_A, + SSL3_ST_SW_FINISHED_B); if (ret <= 0) goto end; S3I(s)->hs.state = SSL3_ST_SW_FLUSH; diff --git a/lib/libssl/t1_enc.c b/lib/libssl/t1_enc.c index 6b3d40d8ec4..3f93bcecf5c 100644 --- a/lib/libssl/t1_enc.c +++ b/lib/libssl/t1_enc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_enc.c,v 1.138 2021/04/19 17:26:39 jsing Exp $ */ +/* $OpenBSD: t1_enc.c,v 1.139 2021/04/25 13:15:22 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -144,11 +144,6 @@ #include #include -int tls1_PRF(SSL *s, const unsigned char *secret, size_t secret_len, - const void *seed1, size_t seed1_len, const void *seed2, size_t seed2_len, - const void *seed3, size_t seed3_len, const void *seed4, size_t seed4_len, - const void *seed5, size_t seed5_len, unsigned char *out, size_t out_len); - void tls1_cleanup_key_block(SSL *s) { @@ -470,26 +465,6 @@ tls1_setup_key_block(SSL *s) return (ret); } -int -tls1_final_finish_mac(SSL *s, const char *str, int str_len, unsigned char *out) -{ - unsigned char buf[EVP_MAX_MD_SIZE]; - size_t hash_len; - - if (str_len < 0) - return 0; - - if (!tls1_transcript_hash_value(s, buf, sizeof(buf), &hash_len)) - return 0; - - if (!tls1_PRF(s, s->session->master_key, s->session->master_key_length, - str, str_len, buf, hash_len, NULL, 0, NULL, 0, NULL, 0, - out, TLS1_FINISH_MAC_LENGTH)) - return 0; - - return TLS1_FINISH_MAC_LENGTH; -} - int tls1_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p, int len) diff --git a/lib/libssl/tls12_lib.c b/lib/libssl/tls12_lib.c new file mode 100644 index 00000000000..520f41678d6 --- /dev/null +++ b/lib/libssl/tls12_lib.c @@ -0,0 +1,92 @@ +/* $OpenBSD: tls12_lib.c,v 1.1 2021/04/25 13:15:23 jsing Exp $ */ +/* + * Copyright (c) 2021 Joel Sing + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#include "ssl_locl.h" + +static int +tls12_finished_verify_data(SSL *s, const char *finished_label, + size_t finished_label_len, uint8_t *verify_data, size_t verify_data_len, + size_t *out_len) +{ + uint8_t transcript_hash[EVP_MAX_MD_SIZE]; + size_t transcript_hash_len; + + *out_len = 0; + + if (verify_data_len < TLS1_FINISH_MAC_LENGTH) + return 0; + + if (!tls1_transcript_hash_value(s, transcript_hash, + sizeof(transcript_hash), &transcript_hash_len)) + return 0; + + if (!tls1_PRF(s, s->session->master_key, s->session->master_key_length, + finished_label, finished_label_len, transcript_hash, + transcript_hash_len, NULL, 0, NULL, 0, NULL, 0, verify_data, + TLS1_FINISH_MAC_LENGTH)) + return 0; + + *out_len = TLS1_FINISH_MAC_LENGTH; + + return 1; +} + +static int +tls12_client_finished_verify_data(SSL *s, uint8_t *verify_data, + size_t verify_data_len, size_t *out_len) +{ + return tls12_finished_verify_data(s, TLS_MD_CLIENT_FINISH_CONST, + TLS_MD_CLIENT_FINISH_CONST_SIZE, verify_data, verify_data_len, + out_len); +} + +static int +tls12_server_finished_verify_data(SSL *s, uint8_t *verify_data, + size_t verify_data_len, size_t *out_len) +{ + return tls12_finished_verify_data(s, TLS_MD_SERVER_FINISH_CONST, + TLS_MD_SERVER_FINISH_CONST_SIZE, verify_data, verify_data_len, + out_len); +} + +int +tls12_derive_finished(SSL *s) +{ + if (!s->server) { + return tls12_client_finished_verify_data(s, + S3I(s)->hs.finished, sizeof(S3I(s)->hs.finished), + &S3I(s)->hs.finished_len); + } else { + return tls12_server_finished_verify_data(s, + S3I(s)->hs.finished, sizeof(S3I(s)->hs.finished), + &S3I(s)->hs.finished_len); + } +} + +int +tls12_derive_peer_finished(SSL *s) +{ + if (s->server) { + return tls12_client_finished_verify_data(s, + S3I(s)->hs.peer_finished, sizeof(S3I(s)->hs.peer_finished), + &S3I(s)->hs.peer_finished_len); + } else { + return tls12_server_finished_verify_data(s, + S3I(s)->hs.peer_finished, sizeof(S3I(s)->hs.peer_finished), + &S3I(s)->hs.peer_finished_len); + } +} -- 2.20.1