[PWCI] "[v2] crypto/openssl: Add support for SHAKE algorithms"#608
Open
[PWCI] "[v2] crypto/openssl: Add support for SHAKE algorithms"#608
Conversation
OpenSSL 3.X has support for SHAKE, Hence adding SHAKE-128 and SHAKE-256 support to the OpenSSL PMD. Signed-off-by: Emma Finn <emma.finn@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Reviewer's GuideAdds SHAKE_128 and SHAKE_256 authentication support to the OpenSSL crypto PMD, wiring them into capability reporting, session configuration, digest processing (with OpenSSL 3.x XOF handling), and test coverage. Sequence diagram for SHAKE authentication processing in OpenSSL PMDsequenceDiagram
participant App as App_thread
participant Dev as rte_cryptodev
participant QP as openssl_qp
participant Sess as openssl_session
participant AuthOp as process_openssl_auth_op
participant AuthProc as process_openssl_auth
participant EVP as OpenSSL_EVP
App->>Dev: rte_cryptodev_enqueue_burst(crypto_op with SHAKE_128/256)
Dev->>QP: enqueue op to queue pair
QP->>Sess: lookup openssl_session
Note over Sess: sess->auth.algo = RTE_CRYPTO_AUTH_SHAKE_128/256
Sess->>Sess: get_auth_algo(SHAKE_128/256, evp_algo=EVP_shake128/256)
Sess->>Sess: sess->auth.mode = OPENSSL_AUTH_AS_AUTH
Sess->>Sess: sess->auth.digest_length = xform->auth.digest_length
QP->>AuthOp: process_openssl_auth_op(qp, op, sess)
AuthOp->>AuthProc: process_openssl_auth(mbuf_src, dst,
AuthOp->>AuthProc: data.offset, iv=NULL, pkey=NULL,
AuthOp->>AuthProc: srclen, ctx_a,
AuthOp->>AuthProc: sess->auth.auth.evp_algo,
AuthOp->>AuthProc: sess->auth.digest_length)
AuthProc->>EVP: EVP_DigestInit_ex(ctx, algo=EVP_shake128/256)
AuthProc->>EVP: EVP_DigestUpdate(ctx, src_chunks)
AuthProc->>EVP: EVP_MD_CTX_ctrl(ctx, EVP_MD_CTRL_XOF_LEN,
AuthProc->>EVP: digest_length, NULL)
EVP-->>AuthProc: success
AuthProc->>EVP: EVP_DigestFinalXOF(ctx, dst, digest_length)
EVP-->>AuthProc: success
AuthProc-->>AuthOp: 0 (success)
AuthOp-->>QP: auth status = SUCCESS
QP-->>Dev: completed op
Dev-->>App: rte_cryptodev_dequeue_burst() returns op with digest
Class diagram for updated OpenSSL PMD auth structures and functions with SHAKE supportclassDiagram
class openssl_session {
+AuthConfig auth
}
class AuthConfig {
+int mode
+AuthAlgoConfig auth
+int digest_length
}
class AuthAlgoConfig {
+const EVP_MD *evp_algo
}
class rte_cryptodev_capabilities {
+int op
+SymCapability sym
}
class SymCapability {
+int xform_type
+AuthCapability auth
}
class AuthCapability {
+int algo
+int block_size
+SizeRange key_size
+SizeRange digest_size
+SizeRange iv_size
}
class SizeRange {
+int min
+int max
+int increment
}
class openssl_qp {
+int id
}
class rte_crypto_op {
+SymOp *sym
}
class SymOp {
+AuthXform auth
}
class AuthXform {
+int algo
+int data_offset
+int digest_length
}
class EVP_MD_CTX
class EVP_MD
class get_auth_algo {
+int get_auth_algo(int sessalgo, const EVP_MD **algo)
}
class process_openssl_auth {
+int process_openssl_auth(rte_mbuf *mbuf_src, uint8_t *dst, int offset, uint8_t *iv, EVP_PKEY *pkey, int srclen, EVP_MD_CTX *ctx, const EVP_MD *algo, int digest_length)
}
class process_openssl_auth_op {
+int process_openssl_auth_op(openssl_qp *qp, rte_crypto_op *op, openssl_session *sess, uint8_t *dst, int srclen)
}
%% Relationships
openssl_session --> AuthConfig : has
AuthConfig --> AuthAlgoConfig : has
AuthConfig --> EVP_MD : uses evp_algo
rte_cryptodev_capabilities --> SymCapability : has
SymCapability --> AuthCapability : has
AuthCapability --> SizeRange : has
rte_crypto_op --> SymOp : has
SymOp --> AuthXform : has
process_openssl_auth_op --> openssl_session : uses
process_openssl_auth_op --> openssl_qp : uses
process_openssl_auth_op --> rte_crypto_op : uses
process_openssl_auth_op --> process_openssl_auth : calls
process_openssl_auth --> EVP_MD_CTX : uses
process_openssl_auth --> EVP_MD : uses
get_auth_algo --> EVP_MD : returns
%% New algorithms
class RTE_CRYPTO_AUTH_SHAKE_128
class RTE_CRYPTO_AUTH_SHAKE_256
AuthCapability --> RTE_CRYPTO_AUTH_SHAKE_128 : algo
AuthCapability --> RTE_CRYPTO_AUTH_SHAKE_256 : algo
get_auth_algo --> RTE_CRYPTO_AUTH_SHAKE_128 : case
get_auth_algo --> RTE_CRYPTO_AUTH_SHAKE_256 : case
class EVP_shake128
class EVP_shake256
get_auth_algo --> EVP_shake128 : sets algo
get_auth_algo --> EVP_shake256 : sets algo
process_openssl_auth --> EVP_shake128 : XOF path
process_openssl_auth --> EVP_shake256 : XOF path
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughSupport for SHAKE-based authentication algorithms (SHAKE_128 and SHAKE_256) has been added to the OpenSSL crypto driver. Changes include registering new algorithm capabilities, mapping them in the auth algorithm handler, implementing XOF-based finalization with OpenSSL 3.0+ conditional compilation, and extending test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new SHAKE capabilities and
get_auth_algo()cases are unconditionally compiled, but the XOF handling is gated on OpenSSL 3.0+; consider guarding capability advertisement and EVP_shake* usage with the same version checks (or failing session creation earlier) to avoid advertising algorithms that will only fail at runtime on older OpenSSL versions. process_openssl_auth()detects SHAKE by comparingalgoagainstEVP_shake128()/EVP_shake256(), which is fragile if the EVP_MD comes from a different provider or wrapper; using the session'sxform->auth.algo(or storing the enum in the session) would make the SHAKE/XOF branch selection more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new SHAKE capabilities and `get_auth_algo()` cases are unconditionally compiled, but the XOF handling is gated on OpenSSL 3.0+; consider guarding capability advertisement and EVP_shake* usage with the same version checks (or failing session creation earlier) to avoid advertising algorithms that will only fail at runtime on older OpenSSL versions.
- `process_openssl_auth()` detects SHAKE by comparing `algo` against `EVP_shake128()`/`EVP_shake256()`, which is fragile if the EVP_MD comes from a different provider or wrapper; using the session's `xform->auth.algo` (or storing the enum in the session) would make the SHAKE/XOF branch selection more robust.
## Individual Comments
### Comment 1
<location> `drivers/crypto/openssl/rte_openssl_pmd_ops.c:272-271` </location>
<code_context>
}, }
}, }
},
+ { /* SHAKE_128 */
+ .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
+ {.sym = {
+ .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
+ {.auth = {
+ .algo = RTE_CRYPTO_AUTH_SHAKE_128,
+ .block_size = 168,
+ .key_size = {
+ .min = 0,
+ .max = 0,
+ .increment = 0
+ },
+ .digest_size = {
+ .min = 1,
+ .max = 256,
+ .increment = 1
+ },
+ .iv_size = { 0 }
+ }, }
+ }, }
+ },
+ { /* SHAKE_256 */
+ .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
</code_context>
<issue_to_address>
**issue (bug_risk):** SHAKE capabilities are advertised even when runtime support is compiled out for OpenSSL < 3.0, which can cause consistent runtime failures.
The capabilities table now lists SHAKE_128/256 unconditionally, but `process_openssl_auth()` always rejects SHAKE for OpenSSL < 3.0 (log + error path). So the PMD advertises SHAKE, yet all such ops fail on older OpenSSL. Please either conditionally expose these capabilities only for `OPENSSL_VERSION_NUMBER >= 0x30000000L` or add a fallback implementation for older versions, so advertised features match actual runtime support.
</issue_to_address>
### Comment 2
<location> `drivers/crypto/openssl/rte_openssl_pmd.c:273-277` </location>
<code_context>
case RTE_CRYPTO_AUTH_SHA512_HMAC:
*algo = EVP_sha512();
break;
+ case RTE_CRYPTO_AUTH_SHAKE_128:
+ *algo = EVP_shake128();
+ break;
+ case RTE_CRYPTO_AUTH_SHAKE_256:
+ *algo = EVP_shake256();
+ break;
default:
</code_context>
<issue_to_address>
**issue (bug_risk):** Using EVP_shake128/256 without version guards may break builds with older OpenSSL headers.
These new `get_auth_algo()` cases call `EVP_shake128()`/`EVP_shake256()` unconditionally, which will fail to compile with OpenSSL versions that don’t declare them. Please wrap these cases in the same `OPENSSL_VERSION_NUMBER` checks you use for SHAKE elsewhere, and return `-EINVAL` on unsupported versions to keep builds working across the supported OpenSSL range.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @drivers/crypto/openssl/rte_openssl_pmd.c:
- Around line 273-278: The SHAKE algorithm cases must be guarded by OpenSSL
version checks: wrap the RTE_CRYPTO_AUTH_SHAKE_128 and RTE_CRYPTO_AUTH_SHAKE_256
handling in get_auth_algo() and the corresponding registration in
openssl_set_session_auth_parameters() with a compile-time guard (e.g., #if
(OPENSSL_VERSION_NUMBER >= 0x30000000L)) so these cases are omitted when
building against pre-3.0 OpenSSL; update both switch-case locations (symbols
get_auth_algo and openssl_set_session_auth_parameters, specifically the cases
for RTE_CRYPTO_AUTH_SHAKE_128 and RTE_CRYPTO_AUTH_SHAKE_256) to prevent
selecting/registering SHAKE on unsupported versions.
🧹 Nitpick comments (1)
drivers/crypto/openssl/rte_openssl_pmd.c (1)
1448-1464: The EVP_MD_CTRL_XOF_LEN control approach is valid, but redundant.Both
EVP_MD_CTX_ctrl(ctx, EVP_MD_CTRL_XOF_LEN, digest_length, NULL)and passing the length directly toEVP_DigestFinalXOF(ctx, dst, digest_length)are supported methods in OpenSSL 3.0+. The current code uses both simultaneously, which is redundant. SinceEVP_DigestFinalXOFalready accepts the output length as a parameter, the precedingEVP_MD_CTX_ctrlcall can be removed for simplicity.♻️ Optional simplification
/* SHAKE algorithms are XOFs and require EVP_DigestFinalXOF */ if (algo == EVP_shake128() || algo == EVP_shake256()) { #if (OPENSSL_VERSION_NUMBER >= 0x30000000L) - /* Set XOF output length before calling EVP_DigestFinalXOF */ - if (EVP_MD_CTX_ctrl(ctx, EVP_MD_CTRL_XOF_LEN, digest_length, NULL) <= 0) - goto process_auth_err; if (EVP_DigestFinalXOF(ctx, dst, digest_length) <= 0) goto process_auth_err; #else
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/test/test_cryptodev_blockcipher.cdrivers/crypto/openssl/rte_openssl_pmd.cdrivers/crypto/openssl/rte_openssl_pmd_ops.c
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (4)
drivers/crypto/openssl/rte_openssl_pmd_ops.c (1)
272-313: LGTM! SHAKE capability definitions are correct.The block sizes are accurate (SHAKE-128 rate = 168 bytes, SHAKE-256 rate = 136 bytes), and the variable digest size range (1-256) is appropriate for XOF algorithms. The keyless and IV-less configuration is correct for these hash functions.
drivers/crypto/openssl/rte_openssl_pmd.c (2)
668-669: LGTM!SHAKE algorithms are correctly added to the digest authentication path using
OPENSSL_AUTH_AS_AUTHmode.
2019-2023: LGTM!The
digest_lengthparameter is correctly passed from the session configuration to enable variable-length XOF output for SHAKE algorithms.app/test/test_cryptodev_blockcipher.c (1)
1177-1179: SHAKE algorithms properly added to auth capability checks with existing test vectors.The additions correctly extend the Auth Only test suite to check for SHAKE-128 and SHAKE-256 capabilities. Test vectors for both algorithms are properly defined in test_cryptodev_hash_test_vectors.h with comprehensive test cases including Digest and Verify operations.
Comment on lines
+273
to
+278
| case RTE_CRYPTO_AUTH_SHAKE_128: | ||
| *algo = EVP_shake128(); | ||
| break; | ||
| case RTE_CRYPTO_AUTH_SHAKE_256: | ||
| *algo = EVP_shake256(); | ||
| break; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
OpenSSL EVP_shake128 EVP_shake256 version introduced history
💡 Result:
Short answer
- EVP_shake128 and EVP_shake256 were added as part of OpenSSL's SHA‑3 support in OpenSSL 1.1.1 (released 11 Sept 2018). [1][2]
Relevant details / later changes
- OpenSSL 1.1.1 release notes list “SHA3” (which includes SHAKE) among new algorithms. [1]
- The EVP SHAKE manpages document these functions (EVP_shake128 / EVP_shake256) in the OpenSSL docs. [2]
- Later OpenSSL 3.x work added XOF-related APIs/behavior (e.g. EVP_MD_CTRL_XOF_LEN / EVP_DigestFinalXOF and EVP_DigestSqueeze) and changes to default XOF handling in the 3.x series (notes in 3.3 / 3.4 documentation). [3][4]
Sources
[1] OpenSSL 1.1.1 release notes (SHA3 added).
[2] OpenSSL EVP SHA-3 / EVP_shake128 / EVP_shake256 manpage.
[3] OpenSSL 3.3 release announcement (EVP_DigestSqueeze etc.).
[4] EVP_MD-SHAKE manpage (notes about XOF defaults / history).
🏁 Script executed:
# Find version checks and OPENSSL_VERSION requirements in the codebase
rg -B2 -A2 "OPENSSL_VERSION_NUMBER" drivers/crypto/openssl/rte_openssl_pmd.c | head -50Repository: ovsrobot/dpdk
Length of output: 1189
🏁 Script executed:
# Check for any documentation about minimum OpenSSL version
find . -type f \( -name "*.txt" -o -name "*.md" -o -name "*.rst" -o -name "*.cfg" \) | xargs grep -l "openssl.*version\|minimum.*openssl" 2>/dev/null | head -5Repository: ovsrobot/dpdk
Length of output: 39
🏁 Script executed:
# Check meson.build or CMakeLists for OpenSSL version requirements
find . -type f \( -name "meson.build" -o -name "CMakeLists.txt" \) | xargs grep -i "openssl" 2>/dev/null | head -20Repository: ovsrobot/dpdk
Length of output: 900
🏁 Script executed:
# Get context around lines 273-278 and check for version guards
sed -n '260,290p' drivers/crypto/openssl/rte_openssl_pmd.cRepository: ovsrobot/dpdk
Length of output: 668
🏁 Script executed:
# Check the broader function context and any guards
sed -n '240,300p' drivers/crypto/openssl/rte_openssl_pmd.cRepository: ovsrobot/dpdk
Length of output: 1391
🏁 Script executed:
# Check lines 668-669 for the auth parameters handling
sed -n '660,680p' drivers/crypto/openssl/rte_openssl_pmd.cRepository: ovsrobot/dpdk
Length of output: 637
🏁 Script executed:
# Check the XOF finalization at 1448-1465
sed -n '1440,1470p' drivers/crypto/openssl/rte_openssl_pmd.cRepository: ovsrobot/dpdk
Length of output: 1016
Add version guards for SHAKE algorithm registration on pre-3.0 OpenSSL.
While EVP_shake128() and EVP_shake256() are available in OpenSSL 1.1.1+, the code allows SHAKE algorithms to be registered unconditionally at lines 668-669, but finalization fails at runtime on OpenSSL versions prior to 3.0 with an error message. This creates a broken user experience where the algorithm can be selected but fails during operation.
Add version guards (e.g., #if (OPENSSL_VERSION_NUMBER >= 0x30000000L)) around the RTE_CRYPTO_AUTH_SHAKE_128 and RTE_CRYPTO_AUTH_SHAKE_256 cases in both get_auth_algo() (lines 273-278) and openssl_set_session_auth_parameters() (lines 668-669) to prevent registration on unsupported OpenSSL versions, or ensure minimum OpenSSL version requirements are documented.
🤖 Prompt for AI Agents
In @drivers/crypto/openssl/rte_openssl_pmd.c around lines 273 - 278, The SHAKE
algorithm cases must be guarded by OpenSSL version checks: wrap the
RTE_CRYPTO_AUTH_SHAKE_128 and RTE_CRYPTO_AUTH_SHAKE_256 handling in
get_auth_algo() and the corresponding registration in
openssl_set_session_auth_parameters() with a compile-time guard (e.g., #if
(OPENSSL_VERSION_NUMBER >= 0x30000000L)) so these cases are omitted when
building against pre-3.0 OpenSSL; update both switch-case locations (symbols
get_auth_algo and openssl_set_session_auth_parameters, specifically the cases
for RTE_CRYPTO_AUTH_SHAKE_128 and RTE_CRYPTO_AUTH_SHAKE_256) to prevent
selecting/registering SHAKE on unsupported versions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOTE: This is an auto submission for "[v2] crypto/openssl: Add support for SHAKE algorithms".
See "http://patchwork.dpdk.org/project/dpdk/list/?series=36995" for details.
Summary by Sourcery
Add SHAKE extendable-output function (XOF) authentication support to the OpenSSL crypto PMD and its tests.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.