Ensure that we write out alerts correctly after early_data

If we sent early_data and then received back an HRR, the enc_write_ctx
was stale resulting in errors if an alert needed to be sent.

Thanks to Quarkslab for reporting this.

In any case it makes little sense to encrypt alerts using the
client_early_traffic_secret, so we add special handling for alerts sent
after early_data. All such alerts are sent in plaintext.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6887)

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 1628ac8f9a8dcfcb503bde6237fd055bc63ea51e..10ac0936ed96ad345805de21cd6191ac6a4a8fa3 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -829,7 +829,10 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
          * In TLSv1.3, once encrypting, we always use application data for the
          * record type
          */
-        if (SSL_TREAT_AS_TLS13(s) && s->enc_write_ctx != NULL)
+        if (SSL_TREAT_AS_TLS13(s)
+                && s->enc_write_ctx != NULL
+                && (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
+                    || type != SSL3_RT_ALERT))
             rectype = SSL3_RT_APPLICATION_DATA;
         else
             rectype = type;
@@ -892,7 +895,10 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
             SSL3_RECORD_reset_input(&wr[j]);
         }
 
-        if (SSL_TREAT_AS_TLS13(s) && s->enc_write_ctx != NULL) {
+        if (SSL_TREAT_AS_TLS13(s)
+                && s->enc_write_ctx != NULL
+                && (s->statem.enc_write_state != ENC_WRITE_STATE_WRITE_PLAIN_ALERTS
+                    || type != SSL3_RT_ALERT)) {
             size_t rlen, max_send_fragment;
 
             if (!WPACKET_put_bytes_u8(thispkt, type)) {
@@ -981,8 +987,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
         SSL3_RECORD_set_length(thiswr, len);
     }
 
-    if (s->early_data_state == SSL_EARLY_DATA_WRITING
-            || s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY) {
+    if (s->statem.enc_write_state == ENC_WRITE_STATE_WRITE_PLAIN_ALERTS) {
         /*
          * We haven't actually negotiated the version yet, but we're trying to
          * send early data - so we need to use the tls13enc function.

diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c
index 8822ca25c36976a3c3ef088c8719fb2951306b08..cbf6a652e7050c06f0fa91f9eea50caa3f817715 100644 (file)
--- a/ssl/record/ssl3_record_tls13.c
+++ b/ssl/record/ssl3_record_tls13.c
@@ -52,7 +52,10 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
         seq = RECORD_LAYER_get_read_sequence(&s->rlayer);
     }
 
-    if (ctx == NULL) {
+    if (ctx == NULL
+            || (rec->type == SSL3_RT_ALERT
+                && s->statem.enc_write_state
+                   == ENC_WRITE_STATE_WRITE_PLAIN_ALERTS)) {
         memmove(rec->data, rec->input, rec->length);
         rec->input = rec->data;
         return 1;

diff --git a/ssl/s3_enc.c b/ssl/s3_enc.c
index 6d78aa1a2e95bdb5814b028d30e7538745c2bf36..5f403817b4d5814fee70a7f7c0d518011043b9ab 100644 (file)
--- a/ssl/s3_enc.c
+++ b/ssl/s3_enc.c
@@ -155,7 +155,7 @@ int ssl3_change_cipher_state(SSL *s, int which)
         RECORD_LAYER_reset_read_sequence(&s->rlayer);
         mac_secret = &(s->s3->read_mac_secret[0]);
     } else {
-        s->statem.invalid_enc_write_ctx = 1;
+        s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
         if (s->enc_write_ctx != NULL) {
             reuse_dd = 1;
         } else if ((s->enc_write_ctx = EVP_CIPHER_CTX_new()) == NULL) {
@@ -238,7 +238,7 @@ int ssl3_change_cipher_state(SSL *s, int which)
         goto err;
     }
 
-    s->statem.invalid_enc_write_ctx = 0;
+    s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
     OPENSSL_cleanse(exp_key, sizeof(exp_key));
     OPENSSL_cleanse(exp_iv, sizeof(exp_iv));
     return 1;

diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c
index 7f1017d8f55279da1c9e977925fa485b5fbce8b8..d75f9ea036086478e06d0dbff0ebf8850b08c344 100644 (file)
--- a/ssl/statem/statem.c
+++ b/ssl/statem/statem.c
@@ -123,7 +123,8 @@ void ossl_statem_fatal(SSL *s, int al, int func, int reason, const char *file,
     s->statem.in_init = 1;
     s->statem.state = MSG_FLOW_ERROR;
     ERR_put_error(ERR_LIB_SSL, func, reason, file, line);
-    if (al != SSL_AD_NO_ALERT && !s->statem.invalid_enc_write_ctx)
+    if (al != SSL_AD_NO_ALERT
+            && s->statem.enc_write_state != ENC_WRITE_STATE_INVALID)
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
 }
 

diff --git a/ssl/statem/statem.h b/ssl/statem/statem.h
index 95dd881208406fbe1d0c74c15b6c777689ba6446..0799870178b306235daed76b30efbaaabc95f32d 100644 (file)
--- a/ssl/statem/statem.h
+++ b/ssl/statem/statem.h
@@ -71,6 +71,15 @@ 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;
+
 /*****************************************************************************
  *                                                                           *
  * This structure should be considered "opaque" to anything outside of the   *
@@ -100,7 +109,7 @@ struct ossl_statem_st {
     /* Should we skip the CertificateVerify message? */
     unsigned int no_cert_verify;
     int use_timer;
-    int invalid_enc_write_ctx;
+    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 23d3efb0247ff8eb846cb35eaa704561fc5a2313..2db913fb0687fd010efccd5744b2fece94df999c 100644 (file)
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -154,7 +154,7 @@ int tls1_change_cipher_state(SSL *s, int which)
         mac_secret = &(s->s3->read_mac_secret[0]);
         mac_secret_size = &(s->s3->read_mac_secret_size);
     } else {
-        s->statem.invalid_enc_write_ctx = 1;
+        s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
         if (s->ext.use_etm)
             s->s3->flags |= TLS1_FLAGS_ENCRYPT_THEN_MAC_WRITE;
         else
@@ -316,7 +316,7 @@ int tls1_change_cipher_state(SSL *s, int which)
                  ERR_R_INTERNAL_ERROR);
         goto err;
     }
-    s->statem.invalid_enc_write_ctx = 0;
+    s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
 
 #ifdef SSL_DEBUG
     printf("which = %04X\nkey=", which);

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index 48990fd65c3bb77f2a47b8514eacd5768e6ef0a4..22db2f8237333292bc2a8644690679f53fbd8f4e 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -425,7 +425,7 @@ int tls13_change_cipher_state(SSL *s, int which)
 
         RECORD_LAYER_reset_read_sequence(&s->rlayer);
     } else {
-        s->statem.invalid_enc_write_ctx = 1;
+        s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
         if (s->enc_write_ctx != NULL) {
             EVP_CIPHER_CTX_reset(s->enc_write_ctx);
         } else {
@@ -648,7 +648,10 @@ int tls13_change_cipher_state(SSL *s, int which)
         goto err;
     }
 
-    s->statem.invalid_enc_write_ctx = 0;
+    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;
     ret = 1;
  err:
     OPENSSL_cleanse(secret, sizeof(secret));
@@ -671,7 +674,7 @@ int tls13_update_key(SSL *s, int sending)
         insecret = s->client_app_traffic_secret;
 
     if (sending) {
-        s->statem.invalid_enc_write_ctx = 1;
+        s->statem.enc_write_state = ENC_WRITE_STATE_INVALID;
         iv = s->write_iv;
         ciph_ctx = s->enc_write_ctx;
         RECORD_LAYER_reset_write_sequence(&s->rlayer);
@@ -692,7 +695,7 @@ int tls13_update_key(SSL *s, int sending)
 
     memcpy(insecret, secret, hashlen);
 
-    s->statem.invalid_enc_write_ctx = 0;
+    s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
     ret = 1;
  err:
     OPENSSL_cleanse(secret, sizeof(secret));