patch 'crypto/openssl: fix GCM and CCM thread unsafe contexts' has been queued to stable release 23.11.2
Xueming Li
xuemingl at nvidia.com
Mon Aug 12 14:48:22 CEST 2024
Hi,
FYI, your patch has been queued to stable release 23.11.2
Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 08/14/24. So please
shout if anyone has objections.
Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.
Queued patches are on a temporary branch at:
https://git.dpdk.org/dpdk-stable/log/?h=23.11-staging
This queued commit can be viewed at:
https://git.dpdk.org/dpdk-stable/commit/?h=23.11-staging&id=eb6a1a85e6fedeaac5c3aca29db91173e5ebaa92
Thanks.
Xueming Li <xuemingl at nvidia.com>
---
>From eb6a1a85e6fedeaac5c3aca29db91173e5ebaa92 Mon Sep 17 00:00:00 2001
From: Jack Bond-Preston <jack.bond-preston at foss.arm.com>
Date: Wed, 3 Jul 2024 13:45:47 +0000
Subject: [PATCH] crypto/openssl: fix GCM and CCM thread unsafe contexts
Cc: Xueming Li <xuemingl at nvidia.com>
[ upstream commit 78d7765f0acbb23168b7b25e25d775bea22c48ab ]
Commit 67ab783b5d70 ("crypto/openssl: use local copy for session
contexts") introduced a fix for concurrency bugs which could occur when
using one OpenSSL PMD session across multiple cores simultaneously. The
solution was to clone the EVP contexts per-buffer to avoid them being
used concurrently.
However, part of commit 75adf1eae44f ("crypto/openssl: update HMAC
routine with 3.0 EVP API") reverted this fix, only for combined ops
(AES-GCM and AES-CCM).
Fix the concurrency issue by cloning EVP contexts per-buffer. An extra
workaround is required for OpenSSL versions which are >= 3.0.0, and
<= 3.2.0. This is because, prior to OpenSSL 3.2.0, EVP_CIPHER_CTX_copy()
is not implemented for AES-GCM or AES-CCM. When using these OpenSSL
versions, create and initialise the context from scratch, per-buffer.
Throughput performance uplift measurements for AES-GCM-128 encrypt on
Ampere Altra Max platform:
1 worker lcore
| buffer sz (B) | prev (Gbps) | optimised (Gbps) | uplift |
|-----------------+---------------+--------------------+----------|
| 64 | 2.60 | 1.31 | -49.5% |
| 256 | 7.69 | 4.45 | -42.1% |
| 1024 | 15.33 | 11.30 | -26.3% |
| 2048 | 18.74 | 15.37 | -18.0% |
| 4096 | 21.11 | 18.80 | -10.9% |
8 worker lcores
| buffer sz (B) | prev (Gbps) | optimised (Gbps) | uplift |
|-----------------+---------------+--------------------+----------|
| 64 | 19.94 | 2.83 | -85.8% |
| 256 | 58.84 | 11.00 | -81.3% |
| 1024 | 119.71 | 42.46 | -64.5% |
| 2048 | 147.69 | 80.91 | -45.2% |
| 4096 | 167.39 | 121.25 | -27.6% |
Fixes: 75adf1eae44f ("crypto/openssl: update HMAC routine with 3.0 EVP API")
Signed-off-by: Jack Bond-Preston <jack.bond-preston at foss.arm.com>
Acked-by: Kai Ji <kai.ji at intel.com>
Reviewed-by: Wathsala Vithanage <wathsala.vithanage at arm.com>
---
drivers/crypto/openssl/rte_openssl_pmd.c | 84 ++++++++++++++++++------
1 file changed, 64 insertions(+), 20 deletions(-)
diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index e8cb09defc..3e547c2039 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -350,7 +350,8 @@ get_aead_algo(enum rte_crypto_aead_algorithm sess_algo, size_t keylen,
static int
openssl_set_sess_aead_enc_param(struct openssl_session *sess,
enum rte_crypto_aead_algorithm algo,
- uint8_t tag_len, const uint8_t *key)
+ uint8_t tag_len, const uint8_t *key,
+ EVP_CIPHER_CTX **ctx)
{
int iv_type = 0;
unsigned int do_ccm;
@@ -378,7 +379,7 @@ openssl_set_sess_aead_enc_param(struct openssl_session *sess,
}
sess->cipher.mode = OPENSSL_CIPHER_LIB;
- sess->cipher.ctx = EVP_CIPHER_CTX_new();
+ *ctx = EVP_CIPHER_CTX_new();
if (get_aead_algo(algo, sess->cipher.key.length,
&sess->cipher.evp_algo) != 0)
@@ -388,19 +389,19 @@ openssl_set_sess_aead_enc_param(struct openssl_session *sess,
sess->chain_order = OPENSSL_CHAIN_COMBINED;
- if (EVP_EncryptInit_ex(sess->cipher.ctx, sess->cipher.evp_algo,
+ if (EVP_EncryptInit_ex(*ctx, sess->cipher.evp_algo,
NULL, NULL, NULL) <= 0)
return -EINVAL;
- if (EVP_CIPHER_CTX_ctrl(sess->cipher.ctx, iv_type, sess->iv.length,
+ if (EVP_CIPHER_CTX_ctrl(*ctx, iv_type, sess->iv.length,
NULL) <= 0)
return -EINVAL;
if (do_ccm)
- EVP_CIPHER_CTX_ctrl(sess->cipher.ctx, EVP_CTRL_CCM_SET_TAG,
+ EVP_CIPHER_CTX_ctrl(*ctx, EVP_CTRL_CCM_SET_TAG,
tag_len, NULL);
- if (EVP_EncryptInit_ex(sess->cipher.ctx, NULL, NULL, key, NULL) <= 0)
+ if (EVP_EncryptInit_ex(*ctx, NULL, NULL, key, NULL) <= 0)
return -EINVAL;
return 0;
@@ -410,7 +411,8 @@ openssl_set_sess_aead_enc_param(struct openssl_session *sess,
static int
openssl_set_sess_aead_dec_param(struct openssl_session *sess,
enum rte_crypto_aead_algorithm algo,
- uint8_t tag_len, const uint8_t *key)
+ uint8_t tag_len, const uint8_t *key,
+ EVP_CIPHER_CTX **ctx)
{
int iv_type = 0;
unsigned int do_ccm = 0;
@@ -437,7 +439,7 @@ openssl_set_sess_aead_dec_param(struct openssl_session *sess,
}
sess->cipher.mode = OPENSSL_CIPHER_LIB;
- sess->cipher.ctx = EVP_CIPHER_CTX_new();
+ *ctx = EVP_CIPHER_CTX_new();
if (get_aead_algo(algo, sess->cipher.key.length,
&sess->cipher.evp_algo) != 0)
@@ -447,24 +449,54 @@ openssl_set_sess_aead_dec_param(struct openssl_session *sess,
sess->chain_order = OPENSSL_CHAIN_COMBINED;
- if (EVP_DecryptInit_ex(sess->cipher.ctx, sess->cipher.evp_algo,
+ if (EVP_DecryptInit_ex(*ctx, sess->cipher.evp_algo,
NULL, NULL, NULL) <= 0)
return -EINVAL;
- if (EVP_CIPHER_CTX_ctrl(sess->cipher.ctx, iv_type,
+ if (EVP_CIPHER_CTX_ctrl(*ctx, iv_type,
sess->iv.length, NULL) <= 0)
return -EINVAL;
if (do_ccm)
- EVP_CIPHER_CTX_ctrl(sess->cipher.ctx, EVP_CTRL_CCM_SET_TAG,
+ EVP_CIPHER_CTX_ctrl(*ctx, EVP_CTRL_CCM_SET_TAG,
tag_len, NULL);
- if (EVP_DecryptInit_ex(sess->cipher.ctx, NULL, NULL, key, NULL) <= 0)
+ if (EVP_DecryptInit_ex(*ctx, NULL, NULL, key, NULL) <= 0)
return -EINVAL;
return 0;
}
+static int openssl_aesni_ctx_clone(EVP_CIPHER_CTX **dest,
+ struct openssl_session *sess)
+{
+#if (OPENSSL_VERSION_NUMBER >= 0x30200000L)
+ *dest = EVP_CIPHER_CTX_dup(sess->ctx);
+ return 0;
+#elif (OPENSSL_VERSION_NUMBER >= 0x30000000L)
+ /* OpenSSL versions 3.0.0 <= V < 3.2.0 have no dupctx() implementation
+ * for AES-GCM and AES-CCM. In this case, we have to create new empty
+ * contexts and initialise, as we did the original context.
+ */
+ if (sess->auth.algo == RTE_CRYPTO_AUTH_AES_GMAC)
+ sess->aead_algo = RTE_CRYPTO_AEAD_AES_GCM;
+
+ if (sess->cipher.direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT)
+ return openssl_set_sess_aead_enc_param(sess, sess->aead_algo,
+ sess->auth.digest_length, sess->cipher.key.data,
+ dest);
+ else
+ return openssl_set_sess_aead_dec_param(sess, sess->aead_algo,
+ sess->auth.digest_length, sess->cipher.key.data,
+ dest);
+#else
+ *dest = EVP_CIPHER_CTX_new();
+ if (EVP_CIPHER_CTX_copy(*dest, sess->cipher.ctx) != 1)
+ return -EINVAL;
+ return 0;
+#endif
+}
+
/** Set session cipher parameters */
static int
openssl_set_session_cipher_parameters(struct openssl_session *sess,
@@ -623,12 +655,14 @@ openssl_set_session_auth_parameters(struct openssl_session *sess,
return openssl_set_sess_aead_enc_param(sess,
RTE_CRYPTO_AEAD_AES_GCM,
xform->auth.digest_length,
- xform->auth.key.data);
+ xform->auth.key.data,
+ &sess->cipher.ctx);
else
return openssl_set_sess_aead_dec_param(sess,
RTE_CRYPTO_AEAD_AES_GCM,
xform->auth.digest_length,
- xform->auth.key.data);
+ xform->auth.key.data,
+ &sess->cipher.ctx);
break;
case RTE_CRYPTO_AUTH_MD5:
@@ -770,10 +804,12 @@ openssl_set_session_aead_parameters(struct openssl_session *sess,
/* Select cipher direction */
if (xform->aead.op == RTE_CRYPTO_AEAD_OP_ENCRYPT)
return openssl_set_sess_aead_enc_param(sess, xform->aead.algo,
- xform->aead.digest_length, xform->aead.key.data);
+ xform->aead.digest_length, xform->aead.key.data,
+ &sess->cipher.ctx);
else
return openssl_set_sess_aead_dec_param(sess, xform->aead.algo,
- xform->aead.digest_length, xform->aead.key.data);
+ xform->aead.digest_length, xform->aead.key.data,
+ &sess->cipher.ctx);
}
/** Parse crypto xform chain and set private session parameters */
@@ -1590,6 +1626,12 @@ process_openssl_combined_op
return;
}
+ EVP_CIPHER_CTX *ctx;
+ if (openssl_aesni_ctx_clone(&ctx, sess) != 0) {
+ op->status = RTE_CRYPTO_OP_STATUS_ERROR;
+ return;
+ }
+
iv = rte_crypto_op_ctod_offset(op, uint8_t *,
sess->iv.offset);
if (sess->auth.algo == RTE_CRYPTO_AUTH_AES_GMAC) {
@@ -1623,12 +1665,12 @@ process_openssl_combined_op
status = process_openssl_auth_encryption_gcm(
mbuf_src, offset, srclen,
aad, aadlen, iv,
- dst, tag, sess->cipher.ctx);
+ dst, tag, ctx);
else
status = process_openssl_auth_encryption_ccm(
mbuf_src, offset, srclen,
aad, aadlen, iv,
- dst, tag, taglen, sess->cipher.ctx);
+ dst, tag, taglen, ctx);
} else {
if (sess->auth.algo == RTE_CRYPTO_AUTH_AES_GMAC ||
@@ -1636,14 +1678,16 @@ process_openssl_combined_op
status = process_openssl_auth_decryption_gcm(
mbuf_src, offset, srclen,
aad, aadlen, iv,
- dst, tag, sess->cipher.ctx);
+ dst, tag, ctx);
else
status = process_openssl_auth_decryption_ccm(
mbuf_src, offset, srclen,
aad, aadlen, iv,
- dst, tag, taglen, sess->cipher.ctx);
+ dst, tag, taglen, ctx);
}
+ EVP_CIPHER_CTX_free(ctx);
+
if (status != 0) {
if (status == (-EFAULT) &&
sess->auth.operation ==
--
2.34.1
---
Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- - 2024-08-12 20:44:03.392082430 +0800
+++ 0025-crypto-openssl-fix-GCM-and-CCM-thread-unsafe-context.patch 2024-08-12 20:44:01.955069267 +0800
@@ -1 +1 @@
-From 78d7765f0acbb23168b7b25e25d775bea22c48ab Mon Sep 17 00:00:00 2001
+From eb6a1a85e6fedeaac5c3aca29db91173e5ebaa92 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit 78d7765f0acbb23168b7b25e25d775bea22c48ab ]
@@ -43 +45,0 @@
-Cc: stable at dpdk.org
More information about the stable
mailing list