Fix RC4-MD5 based ciphersuites

The RC4-MD5 ciphersuites were not removing the length of the MAC when
calculating the length of decrypted TLS data. Since RC4 is a streamed
cipher that doesn't use padding we separate out the concepts of fixed
length TLS data to be removed, and TLS padding.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/13378)

6 files changed, 24 insertions, 10 deletions

diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
index 1ff2a29590..c1934afac5 100644
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
@@ -184,7 +184,7 @@ static int aes_set_ctx_params(void *vctx, const OSSL_PARAM params[])
         }
         if (ctx->base.tlsversion == SSL3_VERSION
                 || ctx->base.tlsversion == TLS1_VERSION) {
-            if (!ossl_assert(ctx->base.removetlspad >= AES_BLOCK_SIZE)) {
+            if (!ossl_assert(ctx->base.removetlsfixed >= AES_BLOCK_SIZE)) {
                 ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR);
                 return 0;
             }
@@ -192,7 +192,7 @@ static int aes_set_ctx_params(void *vctx, const OSSL_PARAM params[])
              * There is no explicit IV with these TLS versions, so don't attempt
              * to remove it.
              */
-            ctx->base.removetlspad -= AES_BLOCK_SIZE;
+            ctx->base.removetlsfixed -= AES_BLOCK_SIZE;
         }
     }
     return ret;
diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha1_hw.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha1_hw.c
index f8db563d18..5be237b485 100644
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha1_hw.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha1_hw.c
@@ -60,7 +60,8 @@ static int aesni_cbc_hmac_sha1_init_key(PROV_CIPHER_CTX *vctx,
 
     ctx->payload_length = NO_PAYLOAD_LENGTH;
 
-    vctx->removetlspad = SHA_DIGEST_LENGTH + AES_BLOCK_SIZE;
+    vctx->removetlspad = 1;
+    vctx->removetlsfixed = SHA_DIGEST_LENGTH + AES_BLOCK_SIZE;
 
     return ret < 0 ? 0 : 1;
 }
diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha256_hw.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha256_hw.c
index 8587c414cd..03d06f8870 100644
--- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha256_hw.c
+++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha256_hw.c
@@ -62,7 +62,8 @@ static int aesni_cbc_hmac_sha256_init_key(PROV_CIPHER_CTX *vctx,
 
     ctx->payload_length = NO_PAYLOAD_LENGTH;
 
-    vctx->removetlspad = SHA256_DIGEST_LENGTH + AES_BLOCK_SIZE;
+    vctx->removetlspad = 1;
+    vctx->removetlsfixed = SHA256_DIGEST_LENGTH + AES_BLOCK_SIZE;
 
     return ret < 0 ? 0 : 1;
 }
diff --git a/providers/implementations/ciphers/cipher_rc4_hmac_md5_hw.c b/providers/implementations/ciphers/cipher_rc4_hmac_md5_hw.c
index 73233a2de7..8cce02b1c5 100644
--- a/providers/implementations/ciphers/cipher_rc4_hmac_md5_hw.c
+++ b/providers/implementations/ciphers/cipher_rc4_hmac_md5_hw.c
@@ -42,6 +42,7 @@ static int cipher_hw_rc4_hmac_md5_initkey(PROV_CIPHER_CTX *bctx,
     ctx->tail = ctx->head;
     ctx->md = ctx->head;
     ctx->payload_length = NO_PAYLOAD_LENGTH;
+    bctx->removetlsfixed = MD5_DIGEST_LENGTH;
     return 1;
 }
 
diff --git a/providers/implementations/ciphers/ciphercommon.c b/providers/implementations/ciphers/ciphercommon.c
index 23f191fbbf..0941210f20 100644
--- a/providers/implementations/ciphers/ciphercommon.c
+++ b/providers/implementations/ciphers/ciphercommon.c
@@ -434,14 +434,24 @@ int ossl_cipher_generic_stream_update(void *vctx, unsigned char *out,
         * Remove any TLS padding. Only used by cipher_aes_cbc_hmac_sha1_hw.c and
         * cipher_aes_cbc_hmac_sha256_hw.c
         */
-        if (ctx->removetlspad > 0) {
+        if (ctx->removetlspad) {
+            /*
+             * We should have already failed in the cipher() call above if this
+             * isn't true.
+             */
+            if (!ossl_assert(*outl >= (size_t)(out[inl - 1] + 1)))
+                return 0;
             /* The actual padding length */
             *outl -= out[inl - 1] + 1;
-
-            /* MAC and explicit IV */
-            *outl -= ctx->removetlspad;
         }
 
+        /* TLS MAC and explicit IV if relevant. We should have already failed
+         * in the cipher() call above if *outl is too short.
+         */
+        if (!ossl_assert(*outl >= ctx->removetlsfixed))
+            return 0;
+        *outl -= ctx->removetlsfixed;
+
         /* Extract the MAC if there is one */
         if (ctx->tlsmacsize > 0) {
             if (*outl < ctx->tlsmacsize)
diff --git a/providers/implementations/include/prov/ciphercommon.h b/providers/implementations/include/prov/ciphercommon.h
index c034528448..a6071c28b7 100644
--- a/providers/implementations/include/prov/ciphercommon.h
+++ b/providers/implementations/include/prov/ciphercommon.h
@@ -61,9 +61,10 @@ struct prov_cipher_ctx_st {
                               * points into the user buffer.
                               */
     size_t tlsmacsize;       /* Size of the TLS MAC */
-    size_t removetlspad;     /*
+    int removetlspad;        /* Whether TLS padding should be removed or not */
+    size_t removetlsfixed;   /*
                               * Length of the fixed size data to remove when
-                              * removing TLS padding (equals mac size plus
+                              * processing TLS data (equals mac size plus
                               * IV size if applicable)
                               */