git: 5c0dac0b7a01 - main - ossl: Keep mutable AES-GCM state on the stack

The branch main has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=5c0dac0b7a012f326edab06ad85aee5ad68ff120

commit 5c0dac0b7a012f326edab06ad85aee5ad68ff120
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-11-29 17:51:55 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-11-29 17:55:51 +0000

    ossl: Keep mutable AES-GCM state on the stack
    
    ossl(4)'s AES-GCM implementation keeps mutable state in the session
    structure, together with the key schedule.  This was done for
    convenience, as both are initialized together.  However, some OCF
    consumers, particularly ZFS, assume that requests may be dispatched to
    the same session in parallel.  Without serialization, this results in
    incorrect output.
    
    Fix the problem by explicitly copying per-session state onto the stack
    at the beginning of each operation.
    
    PR:             275306
    Reviewed by:    jhb
    Fixes:          9a3444d91c70 ("ossl: Add a VAES-based AES-GCM implementation for amd64")
    MFC after:      3 days
    Differential Revision:  https://reviews.freebsd.org/D42783
---
 sys/crypto/openssl/ossl_aes.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/sys/crypto/openssl/ossl_aes.c b/sys/crypto/openssl/ossl_aes.c
index c0cb8ba08d35..65b6f126736e 100644
--- a/sys/crypto/openssl/ossl_aes.c
+++ b/sys/crypto/openssl/ossl_aes.c
@@ -167,10 +167,9 @@ static int
 ossl_aes_gcm(struct ossl_session_cipher *s, struct cryptop *crp,
     const struct crypto_session_params *csp)
 {
-	struct ossl_cipher_context key;
+	struct ossl_gcm_context ctx;
 	struct crypto_buffer_cursor cc_in, cc_out;
 	unsigned char iv[AES_BLOCK_LEN], tag[AES_BLOCK_LEN];
-	struct ossl_gcm_context *ctx;
 	const unsigned char *inseg;
 	unsigned char *outseg;
 	size_t inlen, outlen, seglen;
@@ -182,24 +181,25 @@ ossl_aes_gcm(struct ossl_session_cipher *s, struct cryptop *crp,
 	if (crp->crp_cipher_key != NULL) {
 		if (encrypt)
 			error = s->cipher->set_encrypt_key(crp->crp_cipher_key,
-			    8 * csp->csp_cipher_klen, &key);
+			    8 * csp->csp_cipher_klen,
+			    (struct ossl_cipher_context *)&ctx);
 		else
 			error = s->cipher->set_decrypt_key(crp->crp_cipher_key,
-			    8 * csp->csp_cipher_klen, &key);
+			    8 * csp->csp_cipher_klen,
+			    (struct ossl_cipher_context *)&ctx);
 		if (error)
 			return (error);
-		ctx = (struct ossl_gcm_context *)&key;
 	} else if (encrypt) {
-		ctx = (struct ossl_gcm_context *)&s->enc_ctx;
+		memcpy(&ctx, &s->enc_ctx, sizeof(struct ossl_gcm_context));
 	} else {
-		ctx = (struct ossl_gcm_context *)&s->dec_ctx;
+		memcpy(&ctx, &s->dec_ctx, sizeof(struct ossl_gcm_context));
 	}
 
 	crypto_read_iv(crp, iv);
-	ctx->ops->setiv(ctx, iv, csp->csp_ivlen);
+	ctx.ops->setiv(&ctx, iv, csp->csp_ivlen);
 
 	if (crp->crp_aad != NULL) {
-		if (ctx->ops->aad(ctx, crp->crp_aad, crp->crp_aad_length) != 0)
+		if (ctx.ops->aad(&ctx, crp->crp_aad, crp->crp_aad_length) != 0)
 			return (EINVAL);
 	} else {
 		crypto_cursor_init(&cc_in, &crp->crp_buf);
@@ -208,7 +208,7 @@ ossl_aes_gcm(struct ossl_session_cipher *s, struct cryptop *crp,
 		    alen -= seglen) {
 			inseg = crypto_cursor_segment(&cc_in, &inlen);
 			seglen = MIN(alen, inlen);
-			if (ctx->ops->aad(ctx, inseg, seglen) != 0)
+			if (ctx.ops->aad(&ctx, inseg, seglen) != 0)
 				return (EINVAL);
 			crypto_cursor_advance(&cc_in, seglen);
 		}
@@ -229,10 +229,10 @@ ossl_aes_gcm(struct ossl_session_cipher *s, struct cryptop *crp,
 		seglen = MIN(plen, MIN(inlen, outlen));
 
 		if (encrypt) {
-			if (ctx->ops->encrypt(ctx, inseg, outseg, seglen) != 0)
+			if (ctx.ops->encrypt(&ctx, inseg, outseg, seglen) != 0)
 				return (EINVAL);
 		} else {
-			if (ctx->ops->decrypt(ctx, inseg, outseg, seglen) != 0)
+			if (ctx.ops->decrypt(&ctx, inseg, outseg, seglen) != 0)
 				return (EINVAL);
 		}
 
@@ -242,18 +242,19 @@ ossl_aes_gcm(struct ossl_session_cipher *s, struct cryptop *crp,
 
 	error = 0;
 	if (encrypt) {
-		ctx->ops->tag(ctx, tag, GMAC_DIGEST_LEN);
+		ctx.ops->tag(&ctx, tag, GMAC_DIGEST_LEN);
 		crypto_copyback(crp, crp->crp_digest_start, GMAC_DIGEST_LEN,
 		    tag);
 	} else {
 		crypto_copydata(crp, crp->crp_digest_start, GMAC_DIGEST_LEN,
 		    tag);
-		if (ctx->ops->finish(ctx, tag, GMAC_DIGEST_LEN) != 0)
+		if (ctx.ops->finish(&ctx, tag, GMAC_DIGEST_LEN) != 0)
 			error = EBADMSG;
 	}
 
 	explicit_bzero(iv, sizeof(iv));
 	explicit_bzero(tag, sizeof(tag));
+	explicit_bzero(&ctx, sizeof(ctx));
 
 	return (error);
 }