|WARNING| [10/10] app/test: add RSA-PSS sign and verify test cases

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/168724

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-31

# DPDK Patch Review

## Overall Assessment

This patch series adds RSA-specific capability parameters (OAEP/PSS padding, hash algorithms) and implements RSA-OAEP and RSA-PSS support in the OpenSSL PMD, with corresponding test coverage. The changes touch the cryptodev API, multiple PMDs, and the test suite.

---

## Patch 01/10: crypto: add RSA-specific capability parameters

### Errors

None identified.

### Warnings

**Missing release notes update**
The patch adds new public API fields (`rsa_capa`, `pss_salt`, `pss_explicit_salt`) but does not include a corresponding update to `doc/guides/rel_notes/release_notes_XX.YY.rst`. API additions require release notes.

**Documentation style (definition list opportunity)**
In `lib/cryptodev/rte_cryptodev.h`, the comment block for `struct rte_crypto_rsa_capa` uses a bullet-list style for field descriptions. Per AGENTS.md RST style guidelines, this would be clearer as a definition list when rendered in documentation, though the current C-style Doxygen is acceptable and does not require change.

### Info

The new `rsa_capa` union arm coexists with the existing `modlen` field, which is clarified to apply to MODEXP/MODINV/DH/DSA. This is backward-compatible for existing xform types.

---

## Patch 02/10: crypto/virtio: advertise RSA padding and hash capabilities

### Errors

None identified.

### Warnings

None.

### Info

Correctly migrates virtio PMD to use `rsa_capa` and advertises the padding/hash support present in the driver.

---

## Patch 03/10: crypto/octeontx: advertise RSA PKCS#1 v1.5 padding support

### Errors

None identified.

### Warnings

None.

### Info

Correctly migrates OcteonTX PMD to `rsa_capa`.

---

## Patch 04/10: crypto/cnxk: advertise RSA PKCS#1 v1.5 padding support

### Errors

None identified.

### Warnings

None.

### Info

Correctly migrates CNXK PMD to `rsa_capa`.

---

## Patch 05/10: crypto/qat: advertise RSA padding capabilities

### Errors

None identified.

### Warnings

None.

### Info

Correctly adds QAT macro for RSA capability reporting and updates Gen1/Gen4 PMDs.

---

## Patch 06/10: crypto/openssl: advertise RSA padding and hash capabilities

### Errors

None identified.

### Warnings

**Inconsistent hash_algos placement**
In `drivers/crypto/openssl/rte_openssl_pmd_ops.c`, the OpenSSL >=3.0 block places `hash_algos` outside the `rsa_capa` structure (lines 756-764), making it a sibling of `rsa_capa` rather than a field within it. However, per the API definition in patch 01, `hash_algos` is a field of `struct rte_cryptodev_asymmetric_xform_capability`, not of `rsa_capa`.

This is structurally correct per the API but the indentation and placement in the code makes it easy to misread. Consider verifying the brace alignment or adding a comment to clarify that `hash_algos` is at the outer `asymmetric_xform_capability` level, not nested within `rsa_capa`.

---

## Patch 07/10: crypto/openssl: add RSA-OAEP support for OpenSSL PMD

### Errors

**Missing bounds check on rsa_op_param fields before OpenSSL calls**
In `rte_openssl_pmd.c`, the function `openssl_rsa_set_oaep_params()` dereferences `sess->u.r.label` and uses `sess->u.r.label_len` in `OPENSSL_memdup()` and `EVP_PKEY_CTX_set0_rsa_oaep_label()` without verifying that these values are sane (e.g., that `label_len` does not exceed a reasonable limit beyond the `INT_MAX` check already present during session setup). While the session-creation path validates the input, runtime access should defensively recheck or assert invariants.

However, given that the validation occurs at session creation (lines 1295-1304) and the session is immutable thereafter, this is acceptable. No change required, but documenting the invariant (e.g., "Assumes label_len validated at session creation") would improve clarity.

### Warnings

**Error path cleanup incomplete for oaep_md/mgf1_md**
In `rte_openssl_pmd_ops.c`, the `err_rsa` label frees `sess->u.r.label` on error but does not NULL out `sess->u.r.oaep_md` or `sess->u.r.mgf1_md`. These fields point to static OpenSSL EVP_MD structures (returned by `openssl_get_md()`), so they do not require `free()`, but leaving them non-NULL on a failed session could cause a later use-after-error if session teardown logic is added. Consider NULLing them for defensive hygiene, or document that they are valid only when the session is successfully created.

**Label freed twice on error path**
In `openssl_set_asym_session_parameters()`, if `OPENSSL_zalloc(label_len)` succeeds but a later step fails (e.g., `OSSL_PARAM_BLD_new()` returns NULL), the `err_rsa` block frees `asym_session->u.r.label` (line 1433), but the label was already allocated and copied at line 1315. If an error occurs between label allocation and the `err_rsa` block, the label is freed, but if the session is later freed or reset without re-initializing, `openssl_reset_asym_session()` will attempt to free it again (line 1912). However, `openssl_reset_asym_session()` checks `sess->u.r.label_len > 0`, so this is safe. No change required.

---

## Patch 08/10: app/test: add RSA OAEP asymmetric test cases

### Errors

None identified.

### Warnings

None.

### Info

Test cases correctly validate OAEP functionality with capability checks. The capability check function `rsa_oaep_supported()` correctly validates padding, hash, and MGF1 hash support before running tests.

---

## Patch 09/10: crypto/openssl: add RSA-PSS support for RSA operations

### Errors

**Missing check for PSS salt length validity**
In `openssl_set_asym_session_parameters()`, the code copies `xform->rsa.padding.pss_saltlen` directly into `asym_session->u.r.pss_saltlen` (line 1350) without validating that it is within the permissible range for the key size. RFC 8017 section 9.1.1 (EMSA-PSS-ENCODE) requires:
```
sLen <= emLen - hLen - 2
```
where `emLen = ceil((modBits - 1) / 8)` and `hLen` is the hash digest length. For a 1024-bit key with SHA-256 (test key used in patch 10), this is `128 - 32 - 2 = 94` bytes. The test vectors include a 94-byte saltlen case, which is valid, but if an application requests a larger saltlen, OpenSSL will fail during EVP_PKEY_sign() with an error, causing the operation to fail instead of rejecting the session creation. Consider validating saltlen at session creation.

**Signature mismatch treated as success in verify path**
In `openssl_rsa_pss_verify()`, when `EVP_PKEY_verify()` returns 0 (signature invalid), the function returns 1 to indicate mismatch, and the caller in `process_openssl_rsa_op_evp()` sets `cop->status = RTE_CRYPTO_OP_STATUS_ERROR` and returns 0 (operation succeeded). This is correct per DPDK semantics (a verification failure is not an enqueue failure). However, verify-recover for PKCS#1 v1.5 follows the same pattern, so this is consistent. No change required.

### Warnings

**Explicit salt not rejected at capability advertisement**
The patch adds `rte_crypto_rsa_op_param::pss_salt` (patch 01) and rejects its use in `process_openssl_rsa_op_evp()` at operation time (line 2565-2569), but the OpenSSL PMD does not advertise `pss_explicit_salt = true` in its capability structure. Per the API documentation in patch 01, applications should check `rsa_capa.pss_explicit_salt` before populating `pss_salt.data`, so the runtime rejection is redundant but harmless. Consider documenting in the commit message or code comment that the OpenSSL PMD does not support explicit salt and always generates it internally.

---

## Patch 10/10: app/test: add RSA-PSS sign and verify test cases

### Errors

None identified.

### Warnings

None.

### Info

Test cases correctly validate PSS functionality with appropriate capability checks. The tests cover digest-length salt, maximum salt, and zero-length salt cases.

---

## Series-Wide Issues

### Errors

None.

### Warnings

**Missing release notes for the entire series**
The series adds new API fields (`rsa_capa`, `pss_salt`, `pss_explicit_salt`), new padding types (OAEP, PSS), and significant new PMD functionality, but does not include updates to `doc/guides/rel_notes/release_notes_XX.YY.rst`. Release notes are required for API changes and new features.

**Missing documentation of new padding semantics**
The series adds OAEP and PSS padding modes, which have different semantics from PKCS#1 v1.5 (e.g., PSS operates on digests, not raw messages; OAEP supports labels). The Programmer's Guide (`doc/guides/prog_guide/cryptodev_lib.rst` or similar) should be updated to describe these modes and their parameters.

---

## Summary

The patch series is technically sound and adds valuable RSA padding mode support to DPDK. The implementation correctly handles OpenSSL 3.x APIs, validates capabilities, and includes comprehensive test coverage. Key issues:

1. **Release notes must be added** for the API changes (Error-level omission per contribution guidelines).
2. PSS salt length validation should occur at session creation rather than operation time (Warning).
3. Explicit salt rejection in OpenSSL PMD should be documented or capability-advertised (Warning).
4. Minor defensive improvements to error path cleanup (Info-level suggestions).

Once release notes are added, this series is ready for merge. The technical correctness of the RSA operations, capability reporting, and test coverage is excellent.