Move the SSLv3 crypto code into the new record layer
authorMatt Caswell <matt@openssl.org>
Thu, 15 Sep 2022 16:36:52 +0000 (17:36 +0100)
committerMatt Caswell <matt@openssl.org>
Wed, 5 Oct 2022 14:21:37 +0000 (15:21 +0100)
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19217)

ssl/record/methods/ssl3_meth.c
ssl/record/methods/tls_common.c
ssl/record/record.h
ssl/record/ssl3_record.c
ssl/s3_enc.c
ssl/s3_lib.c

index 6ff67df7d770cb98ee6bba52c24e6b3aa11dae27..e48ca5bc7000f9fabc44032387e59999a3590887 100644 (file)
@@ -24,6 +24,7 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
                                  COMP_METHOD *comp)
 {
     EVP_CIPHER_CTX *ciph_ctx;
+    int enc = (rl->direction == OSSL_RECORD_DIRECTION_WRITE) ? 1 : 0;
 
     if (md == NULL) {
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
@@ -41,6 +42,12 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
+
+    if ((md != NULL && EVP_DigestInit_ex(rl->md_ctx, md, NULL) <= 0)) {
+        ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
+        return OSSL_RECORD_RETURN_FATAL;
+    }
+
 #ifndef OPENSSL_NO_COMP
     if (comp != NULL) {
         rl->compctx = COMP_CTX_new(comp);
@@ -51,7 +58,7 @@ static int ssl3_set_crypto_state(OSSL_RECORD_LAYER *rl, int level,
     }
 #endif
 
-    if (!EVP_DecryptInit_ex(ciph_ctx, ciph, NULL, key, iv)) {
+    if (!EVP_CipherInit_ex(ciph_ctx, ciph, NULL, key, iv, enc)) {
         ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
         return OSSL_RECORD_RETURN_FATAL;
     }
index ef5f8e5e8fe48953d5290938dfd1776450590969..f07cd8ae8c80d4444c228ccde4cc2ac9fecd5dfb 100644 (file)
@@ -1521,11 +1521,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
     OSSL_RECORD_TEMPLATE *thistempl;
 
     /*
-     * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has
+     * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has
      *                 been moved to the new write record layer.
      */
-    if (rl->version == SSL3_VERSION
-            || rl->version == TLS1_3_VERSION
+    if (rl->version == TLS1_3_VERSION
             || rl->isdtls) {
         SSL_SESSION *sess = s->session;
 
@@ -1767,11 +1766,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
             unsigned char *mac;
 
             /*
-             * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has
+             * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has
              *                 been moved to the new write record layer.
              */
-            if (rl->version == SSL3_VERSION
-                    || rl->version == TLS1_3_VERSION
+            if (rl->version == TLS1_3_VERSION
                     || rl->isdtls) {
                 if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac)
                         || !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) {
@@ -1826,30 +1824,17 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
     } else {
         if (!using_ktls) {
             if (prefix) {
-                /*
-                * TODO(RECLAYER): Remove this once SSLv3 crypto has been moved
-                *                 to the new write record layer.
-                */
-                if (rl->version == SSL3_VERSION) {
-                    if (ssl->method->ssl3_enc->enc(s, wr, 1, 1, NULL, mac_size) < 1) {
-                        if (!ossl_statem_in_error(s))
-                            RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                        goto err;
-                    }
-                } else {
-                    if (rl->funcs->cipher(rl, wr, 1, 1, NULL, mac_size) < 1) {
-                        if (!ossl_statem_in_error(s))
-                            RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-                        goto err;
-                    }
+                if (rl->funcs->cipher(rl, wr, 1, 1, NULL, mac_size) < 1) {
+                    if (!ossl_statem_in_error(s))
+                        RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                    goto err;
                 }
             }
             /*
-             * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has
+             * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has
              *                 been moved to the new write record layer.
              */
-            if (rl->version == SSL3_VERSION
-                    || rl->version == TLS1_3_VERSION
+            if (rl->version == TLS1_3_VERSION
                     || rl->isdtls) {
                 if (ssl->method->ssl3_enc->enc(s, wr + prefix, numtempl, 1, NULL,
                                                mac_size) < 1) {
@@ -1894,11 +1879,10 @@ int tls_write_records_default(OSSL_RECORD_LAYER *rl,
             unsigned char *mac;
 
             /*
-             * TODO(RECLAYER): Remove this once SSLv3/TLSv1.3/DTLS crypto has
+             * TODO(RECLAYER): Remove this once TLSv1.3/DTLS crypto has
              *                 been moved to the new write record layer.
              */
-            if (rl->version == SSL3_VERSION
-                    || rl->version == TLS1_3_VERSION
+            if (rl->version == TLS1_3_VERSION
                     || rl->isdtls) {
                 if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac)
                         || !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) {
index 76b3314cb98b511f02228fba5f85076d80c5b58d..18a33d70dc0f954493ec462f881476b1a689922a 100644 (file)
@@ -226,10 +226,6 @@ __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type,
                            unsigned char *buf, size_t len, int peek,
                            size_t *readbytes);
 __owur int ssl3_setup_buffers(SSL_CONNECTION *s);
-__owur int ssl3_enc(SSL_CONNECTION *s, SSL3_RECORD *inrecs, size_t n_recs,
-                    int send, SSL_MAC_BUF *mac, size_t macsize);
-__owur int n_ssl3_mac(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md,
-                      int send);
 __owur int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs,
                     int sending, SSL_MAC_BUF *mac, size_t macsize);
 __owur int tls1_mac_old(SSL_CONNECTION *s, SSL3_RECORD *rec, unsigned char *md,
index fdf694a0e72dfe3f1a3b87fdf2eaf2db88b7c3bc..984b377cf75d5911bbe14b16ab85713b0ba4b516 100644 (file)
 #include "record_local.h"
 #include "internal/cryptlib.h"
 
-static const unsigned char ssl3_pad_1[48] = {
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36,
-    0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36, 0x36
-};
-
-static const unsigned char ssl3_pad_2[48] = {
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c,
-    0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c
-};
-
 /*
  * Clear the contents of an SSL3_RECORD but retain any memory allocated
  */
@@ -161,85 +143,6 @@ int ssl3_do_compress(SSL_CONNECTION *sc, SSL3_RECORD *wr)
     return 1;
 }
 
-/*-
- * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Calls SSLfatal on
- * internal error, but not otherwise. It is the responsibility of the caller to
- * report a bad_record_mac
- *
- * Returns:
- *    0: if the record is publicly invalid, or an internal error
- *    1: Success or Mac-then-encrypt decryption failed (MAC will be randomised)
- */
-int ssl3_enc(SSL_CONNECTION *s, SSL3_RECORD *inrecs, size_t n_recs, int sending,
-             SSL_MAC_BUF *mac, size_t macsize)
-{
-    SSL3_RECORD *rec;
-    EVP_CIPHER_CTX *ds;
-    size_t l, i;
-    size_t bs;
-    const EVP_CIPHER *enc;
-
-    assert(sending);
-    rec = inrecs;
-    /*
-     * We shouldn't ever be called with more than one record in the SSLv3 case
-     */
-    if (n_recs != 1)
-        return 0;
-
-    ds = s->enc_write_ctx;
-    if (s->enc_write_ctx == NULL)
-        enc = NULL;
-    else
-        enc = EVP_CIPHER_CTX_get0_cipher(s->enc_write_ctx);
-
-    if ((s->session == NULL) || (ds == NULL) || (enc == NULL)) {
-        memmove(rec->data, rec->input, rec->length);
-        rec->input = rec->data;
-    } else {
-        int provided = (EVP_CIPHER_get0_provider(enc) != NULL);
-
-        l = rec->length;
-        bs = EVP_CIPHER_CTX_get_block_size(ds);
-
-        /* COMPRESS */
-
-        if ((bs != 1) && !provided) {
-            /*
-             * We only do this for legacy ciphers. Provided ciphers add the
-             * padding on the provider side.
-             */
-            i = bs - (l % bs);
-
-            /* we need to add 'i-1' padding bytes */
-            l += i;
-            /*
-             * the last of these zero bytes will be overwritten with the
-             * padding length.
-             */
-            memset(&rec->input[rec->length], 0, i);
-            rec->length += i;
-            rec->input[l - 1] = (unsigned char)(i - 1);
-        }
-
-        if (EVP_CIPHER_get0_provider(enc) != NULL) {
-            int outlen;
-
-            if (!EVP_CipherUpdate(ds, rec->data, &outlen, rec->input,
-                                  (unsigned int)l))
-                return 0;
-            rec->length = outlen;
-        } else {
-            if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) {
-                /* Shouldn't happen */
-                SSLfatal(s, SSL_AD_BAD_RECORD_MAC, ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-        }
-    }
-    return 1;
-}
-
 #define MAX_PADDING 256
 /*-
  * tls1_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal
@@ -465,66 +368,6 @@ int tls1_enc(SSL_CONNECTION *s, SSL3_RECORD *recs, size_t n_recs, int sending,
     return 1;
 }
 
-int n_ssl3_mac(SSL_CONNECTION *sc, SSL3_RECORD *rec, unsigned char *md,
-               int sending)
-{
-    unsigned char *mac_sec, *seq;
-    const EVP_MD_CTX *hash;
-    unsigned char *p, rec_char;
-    size_t md_size;
-    size_t npad;
-    int t;
-    unsigned int md_size_u;
-    EVP_MD_CTX *md_ctx;
-
-    /*
-     * All read record layer operations should have been moved to the new
-     * record layer code
-     */
-    assert(sending);
-
-    mac_sec = &(sc->s3.write_mac_secret[0]);
-    seq = RECORD_LAYER_get_write_sequence(&sc->rlayer);
-    hash = sc->write_hash;
-
-    t = EVP_MD_CTX_get_size(hash);
-    if (t < 0)
-        return 0;
-    md_size = t;
-    npad = (48 / md_size) * md_size;
-
-    /* Chop the digest off the end :-) */
-    md_ctx = EVP_MD_CTX_new();
-
-    if (md_ctx == NULL)
-        return 0;
-
-    rec_char = rec->type;
-    p = md;
-    s2n(rec->length, p);
-    if (EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0
-        || EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0
-        || EVP_DigestUpdate(md_ctx, ssl3_pad_1, npad) <= 0
-        || EVP_DigestUpdate(md_ctx, seq, 8) <= 0
-        || EVP_DigestUpdate(md_ctx, &rec_char, 1) <= 0
-        || EVP_DigestUpdate(md_ctx, md, 2) <= 0
-        || EVP_DigestUpdate(md_ctx, rec->input, rec->length) <= 0
-        || EVP_DigestFinal_ex(md_ctx, md, NULL) <= 0
-        || EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0
-        || EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0
-        || EVP_DigestUpdate(md_ctx, ssl3_pad_2, npad) <= 0
-        || EVP_DigestUpdate(md_ctx, md, md_size) <= 0
-        || EVP_DigestFinal_ex(md_ctx, md, &md_size_u) <= 0) {
-        EVP_MD_CTX_free(md_ctx);
-        return 0;
-    }
-
-    EVP_MD_CTX_free(md_ctx);
-
-    ssl3_record_sequence_update(seq);
-    return 1;
-}
-
 int tls1_mac_old(SSL_CONNECTION *sc, SSL3_RECORD *rec, unsigned char *md,
                  int sending)
 {
index d575d28dcdfa0a1268a8a5881328daf69aca20ab..26471c3784bfee06aacc2b0fc504f514114bed88 100644 (file)
@@ -92,13 +92,11 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which)
     unsigned char *p, *mac_secret;
     size_t md_len;
     unsigned char *key, *iv;
-    EVP_CIPHER_CTX *dd;
     const EVP_CIPHER *ciph;
     const SSL_COMP *comp = NULL;
     const EVP_MD *md;
     int mdi;
     size_t n, iv_len, key_len;
-    int reuse_dd = 0;
     int direction = (which & SSL3_CC_READ) != 0 ? OSSL_RECORD_DIRECTION_READ
                                                 : OSSL_RECORD_DIRECTION_WRITE;
 
@@ -145,6 +143,9 @@ 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,
@@ -154,56 +155,6 @@ int ssl3_change_cipher_state(SSL_CONNECTION *s, int which)
         goto err;
     }
 
-    if (which & SSL3_CC_READ) {
-        s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
-        return 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) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_EVP_LIB);
-        goto err;
-    } else {
-        /*  make sure it's initialised in case we exit later with an error */
-        EVP_CIPHER_CTX_reset(s->enc_write_ctx);
-    }
-    dd = s->enc_write_ctx;
-    if (ssl_replace_hash(&s->write_hash, md) == NULL) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_SSL_LIB);
-        goto err;
-    }
-#ifndef OPENSSL_NO_COMP
-    /* COMPRESS */
-    COMP_CTX_free(s->compress);
-    s->compress = NULL;
-    if (comp != NULL) {
-        s->compress = COMP_CTX_new(comp->method);
-        if (s->compress == NULL) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                        SSL_R_COMPRESSION_LIBRARY_ERROR);
-            goto err;
-        }
-    }
-#endif
-    RECORD_LAYER_reset_write_sequence(&s->rlayer);
-    memcpy(&(s->s3.write_mac_secret[0]), mac_secret, md_len);
-
-    if (reuse_dd)
-        EVP_CIPHER_CTX_reset(dd);
-
-    if (!EVP_CipherInit_ex(dd, ciph, NULL, key, iv, (which & SSL3_CC_WRITE))) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    if (EVP_CIPHER_get0_provider(ciph) != NULL
-            && !tls_provider_set_tls_params(s, dd, ciph, md)) {
-        /* SSLfatal already called */
-        goto err;
-    }
-
     s->statem.enc_write_state = ENC_WRITE_STATE_VALID;
     return 1;
  err:
index 190f42cd0e01b2e850799991e5becadb28ebb53a..0896be023295f050ba08ccb23fa577ee695f945d 100644 (file)
@@ -3270,8 +3270,8 @@ static int sslcon_undefined_function_1(SSL_CONNECTION *sc, unsigned char *r,
 }
 
 const SSL3_ENC_METHOD SSLv3_enc_data = {
-    ssl3_enc,
-    n_ssl3_mac,
+    NULL,
+    NULL,
     ssl3_setup_key_block,
     ssl3_generate_master_secret,
     ssl3_change_cipher_state,