Date: Tue, 5 Dec 2023 18:27:32 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 9fd62386ad6e - releng/14.0 - ossl: Keep mutable AES-GCM state on the stack Message-ID: <202312051827.3B5IRWpL009192@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch releng/14.0 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=9fd62386ad6e6f5c5298cda66c5c1894373e4379 commit 9fd62386ad6e6f5c5298cda66c5c1894373e4379 Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-11-29 17:51:55 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-12-04 14:02:05 +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 Approved by: so Security: FreeBSD-EN-23:17.ossl (cherry picked from commit 5c0dac0b7a012f326edab06ad85aee5ad68ff120) (cherry picked from commit 84ef0a84ecaa4f5d9bcfed3ce10c288953491e7e) --- 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 40162b6943df..800518e51205 100644 --- a/sys/crypto/openssl/ossl_aes.c +++ b/sys/crypto/openssl/ossl_aes.c @@ -168,10 +168,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; @@ -183,24 +182,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); @@ -209,7 +209,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); } @@ -230,10 +230,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); } @@ -243,18 +243,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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202312051827.3B5IRWpL009192>