Remove the separation betweeen enc_read_ctx and enc_write_ctx
authorMatt Caswell <matt@openssl.org>
Wed, 25 May 2022 16:19:33 +0000 (17:19 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 18 Aug 2022 15:38:13 +0000 (16:38 +0100)
Similarly with read_hash and write_hash. In the new model we have a
separate record layer object for reading and writing. Therefore we don't
need to distinguish between reading and writing inside the record layer
object in the encryption and md ctxs.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/18132)

ssl/record/methods/recmethod_local.h
ssl/record/methods/ssl3_meth.c
ssl/record/methods/tls13_meth.c
ssl/record/methods/tls1_meth.c
ssl/record/methods/tls_common.c

index 9284783b368363391979e4090086a8604e3bd5a1..2c677205a816e5e3361d24254e5693f6115e0a27 100644 (file)
@@ -130,10 +130,11 @@ struct ossl_record_layer_st
     size_t empty_record_count;
 
     /* cryptographic state */
-    EVP_CIPHER_CTX *enc_read_ctx;
+    EVP_CIPHER_CTX *enc_ctx;
 
     /* used for mac generation */
-    EVP_MD_CTX *read_hash;
+    EVP_MD_CTX *md_ctx;
+
     /* uncompress */
     COMP_CTX *expand;
 
index 7e8219a094a6ecad46ebc83a77a96f7132b41ba9..c841bacce737cd955dd4ca271fe16721d7ad1219 100644 (file)
@@ -34,14 +34,14 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-    if ((rl->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) {
+    if ((rl->enc_ctx = EVP_CIPHER_CTX_new()) == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
-    ciph_ctx = rl->enc_read_ctx;
+    ciph_ctx = rl->enc_ctx;
 
-    rl->read_hash = EVP_MD_CTX_new();
-    if (rl->read_hash == NULL) {
+    rl->md_ctx = EVP_MD_CTX_new();
+    if (rl->md_ctx == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
@@ -100,19 +100,12 @@ static int ssl3_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *inrecs, size_t n_recs
      */
     if (n_recs != 1)
         return 0;
-    if (sending) {
-        ds = s->enc_write_ctx;
-        if (s->enc_write_ctx == NULL)
-            enc = NULL;
-        else
-            enc = EVP_CIPHER_CTX_get0_cipher(s->enc_write_ctx);
-    } else {
-        ds = rl->enc_read_ctx;
-        if (rl->enc_read_ctx == NULL)
-            enc = NULL;
-        else
-            enc = EVP_CIPHER_CTX_get0_cipher(rl->enc_read_ctx);
-    }
+
+    ds = rl->enc_ctx;
+    if (rl->enc_ctx == NULL)
+        enc = NULL;
+    else
+        enc = EVP_CIPHER_CTX_get0_cipher(rl->enc_ctx);
 
     provided = (EVP_CIPHER_get0_provider(enc) != NULL);
 
@@ -222,13 +215,8 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
     size_t npad;
     int t;
 
-    if (sending) {
-        mac_sec = &(ssl->s3.write_mac_secret[0]);
-        hash = ssl->write_hash;
-    } else {
-        mac_sec = &(rl->mac_secret[0]);
-        hash = rl->read_hash;
-    }
+    mac_sec = &(rl->mac_secret[0]);
+    hash = rl->md_ctx;
 
     t = EVP_MD_CTX_get_size(hash);
     if (t < 0)
@@ -237,7 +225,7 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
     npad = (48 / md_size) * md_size;
 
     if (!sending
-        && EVP_CIPHER_CTX_get_mode(rl->enc_read_ctx) == EVP_CIPH_CBC_MODE
+        && EVP_CIPHER_CTX_get_mode(rl->enc_ctx) == EVP_CIPH_CBC_MODE
         && ssl3_cbc_record_digest_supported(hash)) {
 #ifdef OPENSSL_NO_DEPRECATED_3_0
         return 0;
index d5a8af4f8d42c67c53ab107c80d94f3fe094a6be..35270e641470cc076f97ddaacfe5627c6f54ba7c 100644 (file)
@@ -35,7 +35,7 @@ static int tls13_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
     }
     memcpy(rl->iv, iv, ivlen);
 
-    ciph_ctx = rl->enc_read_ctx = EVP_CIPHER_CTX_new();
+    ciph_ctx = rl->enc_ctx = EVP_CIPHER_CTX_new();
     if (ciph_ctx == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
@@ -78,13 +78,8 @@ static int tls13_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
         return 0;
     }
 
-    if (sending) {
-        ctx = s->enc_write_ctx;
-        staticiv = s->write_iv;
-    } else {
-        ctx = rl->enc_read_ctx;
-        staticiv = rl->iv;
-    }
+    ctx = rl->enc_ctx;
+    staticiv = rl->iv;
 
     cipher = EVP_CIPHER_CTX_get0_cipher(ctx);
     if (cipher == NULL) {
index 8d189bcb89d3bd5fcac9adac76309e1bd239744d..7b64793f8768e1ba473488d8fa7493c1adfabd2a 100644 (file)
@@ -34,15 +34,15 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
     if (level != OSSL_RECORD_PROTECTION_LEVEL_APPLICATION)
         return OSSL_RECORD_RETURN_FATAL;
 
-    if ((rl->enc_read_ctx = EVP_CIPHER_CTX_new()) == NULL) {
+    if ((rl->enc_ctx = EVP_CIPHER_CTX_new()) == NULL) {
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE);
         return OSSL_RECORD_RETURN_FATAL;
     }
 
-    ciph_ctx = rl->enc_read_ctx;
+    ciph_ctx = rl->enc_ctx;
 
-    rl->read_hash = EVP_MD_CTX_new();
-    if (rl->read_hash == NULL) {
+    rl->md_ctx = EVP_MD_CTX_new();
+    if (rl->md_ctx == NULL) {
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
@@ -75,7 +75,7 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
                                            (int)mackeylen);
         }
         if (mac_key == NULL
-            || EVP_DigestSignInit_ex(rl->read_hash, NULL, EVP_MD_get0_name(md),
+            || EVP_DigestSignInit_ex(rl->md_ctx, NULL, EVP_MD_get0_name(md),
                                      rl->libctx, rl->propq, mac_key,
                                      NULL) <= 0) {
             EVP_PKEY_free(mac_key);
@@ -160,23 +160,25 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
         return 0;
     }
 
-    if (sending) {
-        int ivlen;
 
-        if (EVP_MD_CTX_get0_md(s->write_hash)) {
-            int n = EVP_MD_CTX_get_size(s->write_hash);
-            if (!ossl_assert(n >= 0)) {
-                RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-        }
-        ds = s->enc_write_ctx;
-        if (!ossl_assert(s->enc_write_ctx)) {
+    if (EVP_MD_CTX_get0_md(rl->md_ctx)) {
+        int n = EVP_MD_CTX_get_size(rl->md_ctx);
+        if (!ossl_assert(n >= 0)) {
             RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
             return 0;
         }
+    }
+    ds = rl->enc_ctx;
+    if (!ossl_assert(rl->enc_ctx)) {
+        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    enc = EVP_CIPHER_CTX_get0_cipher(rl->enc_ctx);
+
+    if (sending) {
+        int ivlen;
 
-        enc = EVP_CIPHER_CTX_get0_cipher(s->enc_write_ctx);
         /* For TLSv1.1 and later explicit IV */
         if (RLAYER_USE_EXPLICIT_IV(s)
             && EVP_CIPHER_get_mode(enc) == EVP_CIPH_CBC_MODE)
@@ -192,30 +194,14 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
                         */
                     RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                     return 0;
-                } else if (RAND_bytes_ex(sctx->libctx, recs[ctr].input,
-                                         ivlen, 0) <= 0) {
+                } else if (RAND_bytes_ex(rl->libctx, recs[ctr].input,
+                                            ivlen, 0) <= 0) {
                     RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                     return 0;
                 }
             }
         }
-    } else {
-        if (EVP_MD_CTX_get0_md(rl->read_hash)) {
-            int n = EVP_MD_CTX_get_size(rl->read_hash);
-            if (!ossl_assert(n >= 0)) {
-                RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-        }
-        ds = rl->enc_read_ctx;
-        if (!ossl_assert(rl->enc_read_ctx)) {
-            RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-            return 0;
-        }
-
-        enc = EVP_CIPHER_CTX_get0_cipher(rl->enc_read_ctx);
     }
-
     if ((s->session == NULL) || (enc == NULL)) {
         RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
@@ -474,10 +460,7 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
     int t;
     int ret = 0;
 
-    if (sending)
-        hash = ssl->write_hash;
-    else
-        hash = rl->read_hash;
+    hash = rl->md_ctx;
 
     t = EVP_MD_CTX_get_size(hash);
     if (!ossl_assert(t >= 0))
@@ -519,7 +502,7 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
     header[12] = (unsigned char)(rec->length & 0xff);
 
     if (!sending && !rl->use_etm
-        && EVP_CIPHER_CTX_get_mode(rl->enc_read_ctx) == EVP_CIPH_CBC_MODE
+        && EVP_CIPHER_CTX_get_mode(rl->enc_ctx) == EVP_CIPH_CBC_MODE
         && ssl3_cbc_record_digest_supported(mac_ctx)) {
         OSSL_PARAM tls_hmac_params[2], *p = tls_hmac_params;
 
index a5a0b08af0fa64b183ae3b11abbc7350ac79319e..49ca086d814e104aa20376d917b83fd12f279162 100644 (file)
@@ -616,8 +616,8 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
     } while (num_recs < max_recs
              && thisrr->type == SSL3_RT_APPLICATION_DATA
              && RLAYER_USE_EXPLICIT_IV(rl)
-             && rl->enc_read_ctx != NULL
-             && (EVP_CIPHER_get_flags(EVP_CIPHER_CTX_get0_cipher(rl->enc_read_ctx))
+             && rl->enc_ctx != NULL
+             && (EVP_CIPHER_get_flags(EVP_CIPHER_CTX_get0_cipher(rl->enc_ctx))
                  & EVP_CIPH_FLAG_PIPELINE) != 0
              && tls_record_app_data_waiting(rl));
 
@@ -652,8 +652,8 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
         return OSSL_RECORD_RETURN_SUCCESS;
     }
 
-    if (rl->read_hash != NULL) {
-        const EVP_MD *tmpmd = EVP_MD_CTX_get0_md(rl->read_hash);
+    if (rl->md_ctx != NULL) {
+        const EVP_MD *tmpmd = EVP_MD_CTX_get0_md(rl->md_ctx);
 
         if (tmpmd != NULL) {
             imac_size = EVP_MD_get_size(tmpmd);
@@ -669,7 +669,7 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
      * If in encrypt-then-mac mode calculate mac from encrypted record. All
      * the details below are public so no timing details can leak.
      */
-    if (rl->use_etm && rl->read_hash) {
+    if (rl->use_etm && rl->md_ctx) {
         unsigned char *mac;
 
         for (j = 0; j < num_recs; j++) {
@@ -754,10 +754,10 @@ static int tls_get_more_records(OSSL_RECORD_LAYER *rl,
     } OSSL_TRACE_END(TLS);
 
     /* r->length is now the compressed data plus mac */
-    if (rl->enc_read_ctx != NULL
+    if (rl->enc_ctx != NULL
             && !rl->use_etm
-            && EVP_MD_CTX_get0_md(rl->read_hash) != NULL) {
-        /* rl->read_hash != NULL => mac_size != -1 */
+            && EVP_MD_CTX_get0_md(rl->md_ctx) != NULL) {
+        /* rl->md_ctx != NULL => mac_size != -1 */
 
         for (j = 0; j < num_recs; j++) {
             SSL_MAC_BUF *thismb = &macbufs[j];
@@ -1262,8 +1262,8 @@ static void tls_int_free(OSSL_RECORD_LAYER *rl)
     BIO_free(rl->next);
     SSL3_BUFFER_release(&rl->rbuf);
 
-    EVP_CIPHER_CTX_free(rl->enc_read_ctx);
-    EVP_MD_CTX_free(rl->read_hash);
+    EVP_CIPHER_CTX_free(rl->enc_ctx);
+    EVP_MD_CTX_free(rl->md_ctx);
 #ifndef OPENSSL_NO_COMP
     COMP_CTX_free(rl->expand);
 #endif