From: Matt Caswell Date: Fri, 23 Sep 2022 11:59:22 +0000 (+0100) Subject: Remove enc_write_state X-Git-Tag: openssl-3.2.0-alpha1~1934 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4bf610bdce3b0e474c5ce7db5be77e152f3649b6;p=thirdparty%2Fopenssl.git Remove enc_write_state This field was used to track whether a cipher ctx was valid for writing or not, and also whether we should write out plaintext alerts. With the new record layer design we no longer need to track whether a cipher ctx is valid since the whole record layer will be aborted if it is not. Also we have a different mechanism for tracking whether we should write out plaintext alerts. Therefore this field is removed from the SSL object. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/19343) --- diff --git a/ssl/record/methods/recmethod_local.h b/ssl/record/methods/recmethod_local.h index c6f936b7040..27461ff6454 100644 --- a/ssl/record/methods/recmethod_local.h +++ b/ssl/record/methods/recmethod_local.h @@ -140,6 +140,7 @@ struct ossl_record_layer_st /* Sequence number for the next record */ unsigned char sequence[SEQ_NUM_SIZE]; + /* Alert code to be used if an error occurs */ int alert; /* diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 8ba96dbf200..1c44d3f6888 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -1623,7 +1623,7 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, */ if (rl->version == TLS1_3_VERSION && rl->enc_ctx != NULL - && (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS + && (!rl->allow_plain_alerts || thistempl->type != SSL3_RT_ALERT)) rectype = SSL3_RT_APPLICATION_DATA; else @@ -1686,7 +1686,7 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, if (rl->version == TLS1_3_VERSION && !using_ktls && rl->enc_ctx != NULL - && (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS + && (!rl->allow_plain_alerts || thistempl->type != SSL3_RT_ALERT)) { size_t rlen, max_send_fragment; @@ -1779,7 +1779,7 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, if (!using_ktls) { if (prefix) { if (rl->funcs->cipher(rl, wr, 1, 1, NULL, mac_size) < 1) { - if (!ossl_statem_in_error(s)) { + if (rl->alert == SSL_AD_NO_ALERT) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); } goto err; @@ -1788,7 +1788,7 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl, if (rl->funcs->cipher(rl, wr + prefix, numtempl, 1, NULL, mac_size) < 1) { - if (!ossl_statem_in_error(s)) { + if (rl->alert == SSL_AD_NO_ALERT) { RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR); } goto err; diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c index 26471c3784b..67123c73721 100644 --- a/ssl/s3_enc.c +++ b/ssl/s3_enc.c @@ -143,9 +143,6 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) goto err; } - if ((which & SSL3_CC_WRITE) != 0) - s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; - if (!ssl_set_new_record_layer(s, SSL3_VERSION, direction, OSSL_RECORD_PROTECTION_LEVEL_APPLICATION, @@ -155,7 +152,6 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which) goto err; } - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; return 1; err: return 0; diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index b885214f3c2..138bca220cf 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -143,8 +143,7 @@ void ossl_statem_send_fatal(SSL_CONNECTION *s, int al) return; ossl_statem_set_in_init(s, 1); s->statem.state = MSG_FLOW_ERROR; - if (al != SSL_AD_NO_ALERT - && s->statem.enc_write_state != ENC_WRITE_STATE_INVALID) + if (al != SSL_AD_NO_ALERT) ssl3_send_alert(s, SSL3_AL_FATAL, al); } diff --git a/ssl/statem/statem.h b/ssl/statem/statem.h index 28ef97922e6..167e8a12bfc 100644 --- a/ssl/statem/statem.h +++ b/ssl/statem/statem.h @@ -71,15 +71,6 @@ typedef enum { WRITE_STATE_POST_WORK } WRITE_STATE; -typedef enum { - /* The enc_write_ctx can be used normally */ - ENC_WRITE_STATE_VALID, - /* The enc_write_ctx cannot be used */ - ENC_WRITE_STATE_INVALID, - /* Write alerts in plaintext, but otherwise use the enc_write_ctx */ - ENC_WRITE_STATE_WRITE_PLAIN_ALERTS -} ENC_WRITE_STATES; - typedef enum { CON_FUNC_ERROR = 0, CON_FUNC_SUCCESS, @@ -115,7 +106,6 @@ struct ossl_statem_st { /* Should we skip the CertificateVerify message? */ unsigned int no_cert_verify; int use_timer; - ENC_WRITE_STATES enc_write_state; }; typedef struct ossl_statem_st OSSL_STATEM; diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c index 79b76b8dbc0..afdd227fc65 100644 --- a/ssl/t1_enc.c +++ b/ssl/t1_enc.c @@ -250,7 +250,6 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which) /* TODO(RECLAYER): Temporary - remove me when DTLS write rlayer done*/ goto done; } else { - s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; if (s->ext.use_etm) s->s3.flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC_WRITE; else @@ -390,8 +389,6 @@ int tls1_change_cipher_state(SSL_CONNECTION *s, int which) } done: - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; - OSSL_TRACE_BEGIN(TLS) { BIO_printf(trc_out, "which = %04X, key:\n", which); BIO_dump_indent(trc_out, key, EVP_CIPHER_get_key_length(c), 4); diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c index 95861eb3f4b..1c7fd93240e 100644 --- a/ssl/tls13_enc.c +++ b/ssl/tls13_enc.c @@ -343,8 +343,7 @@ static int derive_secret_key_and_iv(SSL_CONNECTION *s, int sending, size_t labellen, unsigned char *secret, unsigned char *key, size_t *keylen, unsigned char *iv, size_t *ivlen, - size_t *taglen, - EVP_CIPHER_CTX *ciph_ctx) + size_t *taglen) { int hashleni = EVP_MD_get_size(md); size_t hashlen; @@ -409,17 +408,6 @@ static int derive_secret_key_and_iv(SSL_CONNECTION *s, int sending, return 0; } - if (sending) { - if (EVP_CipherInit_ex(ciph_ctx, ciph, NULL, NULL, NULL, sending) <= 0 - || EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_IVLEN, *ivlen, NULL) <= 0 - || (mode == EVP_CIPH_CCM_MODE - && EVP_CIPHER_CTX_ctrl(ciph_ctx, EVP_CTRL_AEAD_SET_TAG, *taglen, NULL) <= 0) - || EVP_CipherInit_ex(ciph_ctx, NULL, NULL, key, NULL, -1) <= 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_EVP_LIB); - return 0; - } - } - return 1; } @@ -449,7 +437,6 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) unsigned char *insecret; unsigned char *finsecret = NULL; const char *log_label = NULL; - EVP_CIPHER_CTX *ciph_ctx = NULL; size_t finsecretlen = 0; const unsigned char *label; size_t labellen, hashlen = 0; @@ -462,25 +449,11 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) int direction = (which & SSL3_CC_READ) != 0 ? OSSL_RECORD_DIRECTION_READ : OSSL_RECORD_DIRECTION_WRITE; - if (which & SSL3_CC_READ) { + if (which & SSL3_CC_READ) iv = s->read_iv; - } else { - s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; - if (s->enc_write_ctx != NULL) { - EVP_CIPHER_CTX_reset(s->enc_write_ctx); - } else { - s->enc_write_ctx = EVP_CIPHER_CTX_new(); - if (s->enc_write_ctx == NULL) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_EVP_LIB); - goto err; - } - } - ciph_ctx = s->enc_write_ctx; + else iv = s->write_iv; - RECORD_LAYER_reset_write_sequence(&s->rlayer); - } - if (((which & SSL3_CC_CLIENT) && (which & SSL3_CC_WRITE)) || ((which & SSL3_CC_SERVER) && (which & SSL3_CC_READ))) { if (which & SSL3_CC_EARLY) { @@ -658,7 +631,7 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) if (!derive_secret_key_and_iv(s, which & SSL3_CC_WRITE, md, cipher, insecret, hash, label, labellen, secret, key, - &keylen, iv, &ivlen, &taglen, ciph_ctx)) { + &keylen, iv, &ivlen, &taglen)) { /* SSLfatal() already called */ goto err; } @@ -695,10 +668,12 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which) goto err; } - if (!s->server && label == client_early_traffic) - s->statem.enc_write_state = ENC_WRITE_STATE_WRITE_PLAIN_ALERTS; - else - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; + if ((which & SSL3_CC_WRITE) != 0) { + if (!s->server && label == client_early_traffic) + s->rlayer.wrlmethod->set_plain_alerts(s->rlayer.wrl, 1); + else + s->rlayer.wrlmethod->set_plain_alerts(s->rlayer.wrl, 0); + } level = (which & SSL3_CC_EARLY) != 0 ? OSSL_RECORD_PROTECTION_LEVEL_EARLY @@ -735,7 +710,6 @@ int tls13_update_key(SSL_CONNECTION *s, int sending) unsigned char *insecret, *iv; unsigned char secret[EVP_MAX_MD_SIZE]; char *log_label; - EVP_CIPHER_CTX *ciph_ctx; size_t keylen, ivlen, taglen; int ret = 0, l; int direction = sending ? OSSL_RECORD_DIRECTION_WRITE @@ -752,21 +726,16 @@ int tls13_update_key(SSL_CONNECTION *s, int sending) else insecret = s->client_app_traffic_secret; - if (sending) { - s->statem.enc_write_state = ENC_WRITE_STATE_INVALID; + if (sending) iv = s->write_iv; - ciph_ctx = s->enc_write_ctx; - RECORD_LAYER_reset_write_sequence(&s->rlayer); - } else { + else iv = s->read_iv; - ciph_ctx = s->enc_read_ctx; - } if (!derive_secret_key_and_iv(s, sending, md, s->s3.tmp.new_sym_enc, insecret, NULL, application_traffic, sizeof(application_traffic) - 1, secret, key, - &keylen, iv, &ivlen, &taglen, ciph_ctx)) { + &keylen, iv, &ivlen, &taglen)) { /* SSLfatal() already called */ goto err; } @@ -789,8 +758,6 @@ int tls13_update_key(SSL_CONNECTION *s, int sending) /* SSLfatal() already called */ goto err; } - - s->statem.enc_write_state = ENC_WRITE_STATE_VALID; ret = 1; err: OPENSSL_cleanse(key, sizeof(key));