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 <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(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 c6f936b7040b62253ae5c847db798290593b135b..27461ff6454cdf8d009eea5c1ae4df304350b5a2 100644 (file)
--- 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 8ba96dbf200381ddbb8df353e6e6dc078840ecca..1c44d3f688856c20e2d39e3cfbce9e266a6cd4b3 100644 (file)
--- 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 26471c3784bfee06aacc2b0fc504f514114bed88..67123c7372194ae4ae1a7010a65bdb24d65b1b96 100644 (file)
--- 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 b885214f3c23c0a6ae3ca61736d6734d26037737..138bca220cf87d08bd9129850f51aff4926dea99 100644 (file)
--- 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 28ef97922e687825d0deff9b3f9024b78a3d2069..167e8a12bfc1bec4150560a1bfb675b16d939bf1 100644 (file)
--- 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 79b76b8dbc023f64fd3a8b9adef13b4b89a059db..afdd227fc6569866cdcae2c7544149f6e1e301e3 100644 (file)
--- 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 95861eb3f4b2aa509496c31f9ce4e1a45f29daea..1c7fd93240ed740c6090cedc7795e43b76ae6518 100644 (file)
--- 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));