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)
diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c
index 1ff2a29..c1934af 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 @@
}
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 @@
* 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 f8db563..5be237b 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 @@
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 8587c41..03d06f8 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 @@
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 73233a2..8cce02b 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 @@
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 23f191f..0941210 100644
--- a/providers/implementations/ciphers/ciphercommon.c
+++ b/providers/implementations/ciphers/ciphercommon.c
@@ -434,14 +434,24 @@
* 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 c034528..a6071c2 100644
--- a/providers/implementations/include/prov/ciphercommon.h
+++ b/providers/implementations/include/prov/ciphercommon.h
@@ -61,9 +61,10 @@
* 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)
*/
