From 57389a3261075cc1266218742434aa749cf3733e Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 10 Feb 2017 17:43:09 +0000 Subject: [PATCH] Actually update the keys when a KeyUpdate message is sent or received Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2609) --- include/openssl/ssl.h | 1 + ssl/ssl_err.c | 1 + ssl/ssl_locl.h | 3 + ssl/statem/statem_clnt.c | 3 + ssl/statem/statem_lib.c | 16 +++- ssl/statem/statem_srvr.c | 7 ++ ssl/tls13_enc.c | 162 +++++++++++++++++++++++++++------------ 7 files changed, 142 insertions(+), 51 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 4456de19c8..72f835460a 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2096,6 +2096,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_DANE_CTX_ENABLE 347 # define SSL_F_DANE_MTYPE_SET 393 # define SSL_F_DANE_TLSA_ADD 394 +# define SSL_F_DERIVE_SECRET_KEY_AND_IV 514 # define SSL_F_DO_DTLS1_WRITE 245 # define SSL_F_DO_SSL3_WRITE 104 # define SSL_F_DTLS1_BUFFER_RECORD 247 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 2a8cd0a817..ec2b41dabf 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -28,6 +28,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_DANE_CTX_ENABLE), "dane_ctx_enable"}, {ERR_FUNC(SSL_F_DANE_MTYPE_SET), "dane_mtype_set"}, {ERR_FUNC(SSL_F_DANE_TLSA_ADD), "dane_tlsa_add"}, + {ERR_FUNC(SSL_F_DERIVE_SECRET_KEY_AND_IV), "derive_secret_key_and_iv"}, {ERR_FUNC(SSL_F_DO_DTLS1_WRITE), "do_dtls1_write"}, {ERR_FUNC(SSL_F_DO_SSL3_WRITE), "do_ssl3_write"}, {ERR_FUNC(SSL_F_DTLS1_BUFFER_RECORD), "dtls1_buffer_record"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 8a3f573bf9..12eba2130e 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -986,6 +986,8 @@ struct ssl_st { unsigned char client_finished_secret[EVP_MAX_MD_SIZE]; unsigned char server_finished_secret[EVP_MAX_MD_SIZE]; unsigned char server_finished_hash[EVP_MAX_MD_SIZE]; + unsigned char client_app_traffic_secret[EVP_MAX_MD_SIZE]; + unsigned char server_app_traffic_secret[EVP_MAX_MD_SIZE]; EVP_CIPHER_CTX *enc_read_ctx; /* cryptographic state */ unsigned char read_iv[EVP_MAX_IV_LENGTH]; /* TLSv1.3 static read IV */ EVP_MD_CTX *read_hash; /* used for mac generation */ @@ -2163,6 +2165,7 @@ __owur int tls13_setup_key_block(SSL *s); __owur size_t tls13_final_finish_mac(SSL *s, const char *str, size_t slen, unsigned char *p); __owur int tls13_change_cipher_state(SSL *s, int which); +__owur int tls13_update_key(SSL *s, int write); __owur int tls13_hkdf_expand(SSL *s, const EVP_MD *md, const unsigned char *secret, const unsigned char *label, size_t labellen, diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 909b2f0953..88d7608567 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -735,6 +735,9 @@ WORK_STATE ossl_statem_client_post_work(SSL *s, WORK_STATE wst) case TLS_ST_CW_KEY_UPDATE: if (statem_flush(s) != 1) return WORK_MORE_A; + + if (!tls13_update_key(s, 1)) + return WORK_ERROR; break; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index d65feba2e2..809fe6fd50 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -513,16 +513,16 @@ int tls_construct_key_update(SSL *s, WPACKET *pkt) MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt) { + int al; unsigned int updatetype; if (!PACKET_get_1(pkt, &updatetype) || PACKET_remaining(pkt) != 0 || (updatetype != SSL_KEY_UPDATE_NOT_REQUESTED && updatetype != SSL_KEY_UPDATE_REQUESTED)) { - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_KEY_UPDATE, SSL_R_BAD_KEY_UPDATE); - ossl_statem_set_error(s); - return MSG_PROCESS_ERROR; + goto err; } /* @@ -533,7 +533,17 @@ MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt) if (updatetype == SSL_KEY_UPDATE_REQUESTED) s->key_update = SSL_KEY_UPDATE_NOT_REQUESTED; + if (!tls13_update_key(s, 0)) { + al = SSL_AD_INTERNAL_ERROR; + SSLerr(SSL_F_TLS_PROCESS_KEY_UPDATE, ERR_R_INTERNAL_ERROR); + goto err; + } + return MSG_PROCESS_FINISHED_READING; + err: + ssl3_send_alert(s, SSL3_AL_FATAL, al); + ossl_statem_set_error(s); + return MSG_PROCESS_ERROR; } #ifndef OPENSSL_NO_NEXTPROTONEG diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 3007088b9b..8f3841cc84 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -841,6 +841,13 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) break; case TLS_ST_SW_KEY_UPDATE: + if (statem_flush(s) != 1) + return WORK_MORE_A; + + if (!tls13_update_key(s, 1)) + return WORK_ERROR; + break; + case TLS_ST_SW_SESSION_TICKET: if (SSL_IS_TLS13(s) && statem_flush(s) != 1) return WORK_MORE_A; diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index ebfeecdfb2..3d8562bbf9 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -242,6 +242,74 @@ int tls13_setup_key_block(SSL *s) return 1; } +static int derive_secret_key_and_iv(SSL *s, int write, + const unsigned char *insecret, + const unsigned char *hash, + const unsigned char *label, + size_t labellen, unsigned char *secret, + unsigned char *iv, EVP_CIPHER_CTX *ciph_ctx) +{ + unsigned char key[EVP_MAX_KEY_LENGTH]; + size_t ivlen, keylen, taglen; + const EVP_MD *md = ssl_handshake_md(s); + size_t hashlen = EVP_MD_size(md); + const EVP_CIPHER *ciph = s->s3->tmp.new_sym_enc; + + if (!tls13_hkdf_expand(s, md, insecret, label, labellen, hash, secret, + hashlen)) { + SSLerr(SSL_F_DERIVE_SECRET_KEY_AND_IV, ERR_R_INTERNAL_ERROR); + goto err; + } + + /* TODO(size_t): convert me */ + keylen = EVP_CIPHER_key_length(ciph); + if (EVP_CIPHER_mode(ciph) == EVP_CIPH_CCM_MODE) { + ivlen = EVP_CCM_TLS_IV_LEN; + if (s->s3->tmp.new_cipher->algorithm_enc + & (SSL_AES128CCM8 | SSL_AES256CCM8)) + taglen = EVP_CCM8_TLS_TAG_LEN; + else + taglen = EVP_CCM_TLS_TAG_LEN; + } else { + ivlen = EVP_CIPHER_iv_length(ciph); + taglen = 0; + } + + if (!tls13_derive_key(s, secret, key, keylen) + || !tls13_derive_iv(s, secret, iv, ivlen)) { + SSLerr(SSL_F_DERIVE_SECRET_KEY_AND_IV, ERR_R_INTERNAL_ERROR); + goto err; + } + + if (EVP_CipherInit_ex(ciph_ctx, ciph, NULL, NULL, NULL, write) <= 0 + || !EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_IVLEN, ivlen, NULL) + || (taglen != 0 && !EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_TAG, + taglen, NULL)) + || EVP_CipherInit_ex(ciph_ctx, NULL, NULL, key, NULL, -1) <= 0) { + SSLerr(SSL_F_DERIVE_SECRET_KEY_AND_IV, ERR_R_EVP_LIB); + goto err; + } + +#ifdef OPENSSL_SSL_TRACE_CRYPTO + if (s->msg_callback) { + int wh = write ? TLS1_RT_CRYPTO_WRITE : 0; + + if (ciph->key_len) + s->msg_callback(2, s->version, wh | TLS1_RT_CRYPTO_KEY, + key, ciph->key_len, s, s->msg_callback_arg); + + wh |= TLS1_RT_CRYPTO_IV; + s->msg_callback(2, s->version, wh, iv, ivlen, s, + s->msg_callback_arg); + } +#endif + + return 1; + err: + OPENSSL_cleanse(key, sizeof(key)); + return 0; +} + int tls13_change_cipher_state(SSL *s, int which) { static const unsigned char client_handshake_traffic[] = @@ -254,7 +322,6 @@ int tls13_change_cipher_state(SSL *s, int which) "server application traffic secret"; static const unsigned char resumption_master_secret[] = "resumption master secret"; - unsigned char key[EVP_MAX_KEY_LENGTH]; unsigned char *iv; unsigned char secret[EVP_MAX_MD_SIZE]; unsigned char hashval[EVP_MAX_MD_SIZE]; @@ -263,8 +330,7 @@ int tls13_change_cipher_state(SSL *s, int which) unsigned char *finsecret = NULL; const char *log_label = NULL; EVP_CIPHER_CTX *ciph_ctx; - const EVP_CIPHER *ciph = s->s3->tmp.new_sym_enc; - size_t ivlen, keylen, taglen, finsecretlen = 0; + size_t finsecretlen = 0; const unsigned char *label; size_t labellen, hashlen = 0; int ret = 0; @@ -350,12 +416,6 @@ int tls13_change_cipher_state(SSL *s, int which) if (label == server_application_traffic) memcpy(s->server_finished_hash, hashval, hashlen); - if (!tls13_hkdf_expand(s, ssl_handshake_md(s), insecret, label, labellen, - hash, secret, hashlen)) { - SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); - goto err; - } - if (label == client_application_traffic) { /* * We also create the resumption master secret, but this time use the @@ -371,64 +431,70 @@ int tls13_change_cipher_state(SSL *s, int which) s->session->master_key_length = hashlen; } - /* TODO(size_t): convert me */ - keylen = EVP_CIPHER_key_length(ciph); - if (EVP_CIPHER_mode(ciph) == EVP_CIPH_CCM_MODE) { - ivlen = EVP_CCM_TLS_IV_LEN; - if (s->s3->tmp.new_cipher->algorithm_enc - & (SSL_AES128CCM8 | SSL_AES256CCM8)) - taglen = EVP_CCM8_TLS_TAG_LEN; - else - taglen = EVP_CCM_TLS_TAG_LEN; - } else { - ivlen = EVP_CIPHER_iv_length(ciph); - taglen = 0; + if (!derive_secret_key_and_iv(s, which & SSL3_CC_WRITE, insecret, hash, + label, labellen, secret, iv, ciph_ctx)) { + goto err; } + if (label == server_application_traffic) + memcpy(s->server_app_traffic_secret, secret, hashlen); + else if (label == client_application_traffic) + memcpy(s->client_app_traffic_secret, secret, hashlen); + if (!ssl_log_secret(s, log_label, secret, hashlen)) { SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err; } - if (!tls13_derive_key(s, secret, key, keylen) - || !tls13_derive_iv(s, secret, iv, ivlen) - || (finsecret != NULL && !tls13_derive_finishedkey(s, - ssl_handshake_md(s), - secret, - finsecret, - finsecretlen))) { + if (finsecret != NULL + && !tls13_derive_finishedkey(s, ssl_handshake_md(s), secret, + finsecret, finsecretlen)) { SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_INTERNAL_ERROR); goto err; } - if (EVP_CipherInit_ex(ciph_ctx, ciph, NULL, NULL, NULL, - (which & SSL3_CC_WRITE)) <= 0 - || !EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_IVLEN, ivlen, NULL) - || (taglen != 0 && !EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_TAG, - taglen, NULL)) - || EVP_CipherInit_ex(ciph_ctx, NULL, NULL, key, NULL, -1) <= 0) { - SSLerr(SSL_F_TLS13_CHANGE_CIPHER_STATE, ERR_R_EVP_LIB); - goto err; - } + ret = 1; + err: + OPENSSL_cleanse(secret, sizeof(secret)); + return ret; +} -#ifdef OPENSSL_SSL_TRACE_CRYPTO - if (s->msg_callback) { - int wh = which & SSL3_CC_WRITE ? TLS1_RT_CRYPTO_WRITE : 0; +int tls13_update_key(SSL *s, int write) +{ + static const unsigned char application_traffic[] = + "application traffic secret"; + const EVP_MD *md = ssl_handshake_md(s); + size_t hashlen = EVP_MD_size(md); + unsigned char *insecret, *iv; + unsigned char secret[EVP_MAX_MD_SIZE]; + EVP_CIPHER_CTX *ciph_ctx; + int ret = 0; - if (ciph->key_len) - s->msg_callback(2, s->version, wh | TLS1_RT_CRYPTO_KEY, - key, ciph->key_len, s, s->msg_callback_arg); + if (s->server == write) + insecret = s->server_app_traffic_secret; + else + insecret = s->client_app_traffic_secret; - wh |= TLS1_RT_CRYPTO_IV; - s->msg_callback(2, s->version, wh, iv, ivlen, s, - s->msg_callback_arg); + if (write) { + iv = s->write_iv; + ciph_ctx = s->enc_write_ctx; + RECORD_LAYER_reset_write_sequence(&s->rlayer); + } else { + iv = s->read_iv; + ciph_ctx = s->enc_read_ctx; + RECORD_LAYER_reset_read_sequence(&s->rlayer); } -#endif + + if (!derive_secret_key_and_iv(s, write, insecret, NULL, application_traffic, + sizeof(application_traffic) - 1, secret, iv, + ciph_ctx)) + goto err; + + memcpy(insecret, secret, hashlen); ret = 1; err: OPENSSL_cleanse(secret, sizeof(secret)); - OPENSSL_cleanse(key, sizeof(key)); return ret; } -- 2.25.1