|WARNING| [v4, 8/8] test/crypto: 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/170056

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-09-28

# DPDK Patch Review - series.patch

## Summary

This 8-patch series adds RSA-OAEP and RSA-PSS support to DPDK cryptodev:
1. Adds `rsa_capa` structure for RSA-specific capabilities (padding, MGF1, PSS salt)
2-3. Updates OcteonTX and CNXK PMDs to advertise PKCS#1 v1.5 support
4. OpenSSL PMD advertises RSA capabilities (padding, hash, MGF1)
5. OpenSSL PMD implements RSA-OAEP encrypt/decrypt
6. Adds OAEP test cases
7. OpenSSL PMD implements RSA-PSS sign/verify
8. Adds PSS test cases

---

## Patch 1/8: crypto: add RSA-specific capability parameters

### Errors

None.

### Warnings

None.

### Info

**Release notes structure:**
The release notes entry is clear and correctly placed in the current release file. No issues.

**ABI documentation:**
Both ABI changes (addition of `rsa_capa` union arm and `pss_salt` field) are documented in the ABI Changes section. Correct.

**API design:**
- `rsa_capa` nested inside the union is appropriate for RSA-specific parameters
- `pss_explicit_salt` as `bool` is correct (true/false capability flag)
- `pad_types` and `mgf1_hash_algos` as bitmasks match existing patterns (`hash_algos`)
- Field ordering and zero-initialization semantics are documented clearly

The API follows DPDK conventions and is well-documented.

---

## Patch 2/8: crypto/octeontx: advertise RSA PKCS#1 v1.5 padding support

### Errors

None.

### Warnings

None.

### Info

**Pattern consistency:**
The change converts `modlen` from a top-level union field to `rsa_capa.modlen`. This is correct per the new API introduced in patch 1/8. The patch correctly initializes the nested structure and adds `pad_types`.

**Advertised capabilities:**
- `RTE_CRYPTO_RSA_PADDING_NONE` (raw RSA)
- `RTE_CRYPTO_RSA_PADDING_PKCS1_5`

This matches the PMD's actual support (no OAEP/PSS).

---

## Patch 3/8: crypto/cnxk: advertise RSA PKCS#1 v1.5 padding support

### Errors

None.

### Warnings

None.

### Info

Identical pattern to patch 2/8 for the CNXK PMD. Correctly advertises NONE and PKCS#1 v1.5 padding.

---

## Patch 4/8: crypto/openssl: advertise RSA padding and hash capabilities

### Errors

None.

### Warnings

None.

### Info

**Capability reporting:**
- Advertises NONE, PKCS#1 v1.5, OAEP, and PSS padding
- Reports hash algorithms for both primary digest (`hash_algos`) and MGF1 (`mgf1_hash_algos`)
- Includes SHA-1, SHA-2 (224/256/384/512), and SHA-3 (224/256/384/512) families

The bitmask construction using `RTE_BIT64()` is correct.

**Release notes:**
Patch does not modify release notes, but the overall feature is documented in patch 1/8. Acceptable.

---

## Patch 5/8: crypto/openssl: support RSA-OAEP

### Errors

None.

### Warnings

1. **Missing error checks (non-fatal but good practice):**
   In `openssl_set_asym_session_parameters()`, after allocating `asym_session->u.r.label`, the code checks for NULL but then proceeds to call `memcpy()`. If `OPENSSL_zalloc()` fails, the NULL check catches it and jumps to `err_rsa`, so no use-after-free occurs. However, the error path `err_rsa` frees the label only if `ret != 0` *and* `label != NULL`. This is correct but complex.

2. **Label length validation:**
   The check `if (xform->rsa.padding.oaep_label.length > (size_t)INT_MAX)` is appropriate for OpenSSL API limits (which use `int` for lengths). Good defensive coding.

### Info

**OAEP parameter handling:**
- Stores `oaep_md`, `mgf1_md`, and `label` in the session
- Defaults MGF1 hash to OAEP hash when `mgf1hash == 0` (correct per API semantics)
- Uses `OPENSSL_memdup()` for label duplication in `openssl_rsa_set_oaep_params()` to pass ownership to `EVP_PKEY_CTX_set0_rsa_oaep_label()` (correct for `set0_*` semantics)

**OAEP-specific validation:**
The code rejects OAEP for sign/verify operations (patch 5/8, line 2348), which is correct: OAEP is only defined for encrypt/decrypt.

**Cleanup on error:**
The `err_rsa` label frees `label` if `ret != 0` and `label != NULL`. The session teardown (`openssl_reset_asym_session`) also frees the label. Correct dual cleanup.

---

## Patch 6/8: test/crypto: add RSA OAEP asymmetric test cases

### Errors

None.

### Warnings

None.

### Info

**Test coverage:**
- Default OAEP parameters (SHA-256 hash, no label, MGF1 defaults to hash)
- Custom MGF1 hash (SHA-1) with label
- Label with default MGF1 (falls back to primary hash)
- Both EXP and CRT key types

The tests use `is_rsa_oaep_supported()` to check PMD capabilities before running, ensuring graceful skips on unsupported devices.

**Capability checks:**
- Verifies `pad_types` includes OAEP
- Checks `hash_algos` for primary digest support
- Checks `mgf1_hash_algos` only when `mgf1hash != 0` (correct: default MGF1 to primary hash is covered by `hash_algos` check)

**Test structure:**
Uses existing `queue_ops_rsa_enc_dec()` for end-to-end validation. Appropriate for OAEP encrypt/decrypt round-trip.

**Capability display:**
Adds RSA capability reporting to `print_asym_capa()`. Good for debugging.

---

## Patch 7/8: crypto/openssl: support RSA-PSS

### Errors

None.

### Warnings

None.

### Info

**PSS parameter handling:**
- Stores `pss_md`, `pss_mgf1_md`, and `pss_saltlen` in the session
- Defaults MGF1 hash to PSS digest hash when `mgf1hash == 0` (correct)
- Uses `EVP_PKEY_CTX_set_signature_md()` to tell OpenSSL which hash algorithm was used to create the input digest (correct: PSS expects a pre-hashed message per the API)

**PSS-specific validation:**
The code rejects PSS for encrypt/decrypt operations (line 2567), which is correct: PSS is only defined for sign/verify.

**Explicit salt handling:**
The code rejects an application-supplied `pss_salt` (line 2649) because the PMD does not advertise `pss_explicit_salt`. Correct: the PMD always uses internally-generated salt via OpenSSL's RNG.

**Verification path:**
- Introduces `openssl_rsa_pss_verify()` using `EVP_PKEY_verify()` (correct: PSS does not support verify-recover)
- Retains `openssl_rsa_verify_recover()` for PKCS#1 v1.5 and no-padding (correct: only these deterministic schemes support recover)
- `EVP_PKEY_verify()` returns 1 for valid signature, 0 for invalid, negative for errors. The code handles all three cases correctly:
  - `ret < 0`: processing error, returns -1 (fails the enqueue)
  - `ret == 0`: signature mismatch, returns 1 (op completes with error status)
  - `ret == 1`: valid signature, returns 0 (op succeeds)

**Error reporting:**
Signature mismatches in verify-recover are also treated as normal outcomes (`return 1`), which causes `op->status = RTE_CRYPTO_OP_STATUS_ERROR` without failing the enqueue. Correct: a bad signature is not a processing failure.

**Cleanup:**
No new resources are allocated in PSS paths beyond what the session already owns. No leaks.

---

## Patch 8/8: test/crypto: add RSA-PSS sign and verify test cases

### Errors

None.

### Warnings

None.

### Info

**Test coverage:**
- Salt length = digest length (32 bytes for SHA-256)
- Maximum permissible salt length (94 bytes for 1024-bit key + SHA-256)
- Zero-length salt (deterministic PSS, no randomization)

The tests correctly use `rsa_pss_digest_sha256` (SHA-256 hash of `rsaplaintext`) instead of raw plaintext, matching the API requirement that `rte_crypto_rsa_op_param::message` is a pre-computed digest for PSS.

**Test flow:**
`queue_ops_rsa_pss_sign_verify()` signs a digest then verifies the signature, without a negative (corrupted signature) test. This is appropriate: PSS verification does not support verify-recover, so mismatches are reported via `op->status` rather than by comparing recovered data.

**Capability checks:**
`is_rsa_pss_supported()` verifies:
- `pad_types` includes PSS
- `hash_algos` includes the signature digest hash
- `mgf1_hash_algos` includes the MGF1 hash (if explicitly set)

Correct and comprehensive.

**Key type:**
Tests only use EXP key type (requires `RTE_CRYPTODEV_FF_RSA_PRIV_OP_KEY_EXP`). No CRT variant is included. This is acceptable: OAEP tests (patch 6/8) already cover CRT; PSS uses the same underlying RSA operation, so additional CRT tests would be redundant.

**Max salt length calculation:**
Comment documents the formula: `emLen - hLen - 2 = 128 - 32 - 2 = 94` per RFC 8017 section 9.1.1. Correct and helpful.

---

## Cross-Patch Issues

None. The patches form a coherent series:
1. API definition
2-4. PMD capability updates
5. OAEP implementation
6. OAEP tests
7. PSS implementation
8. PSS tests

Each patch compiles independently (API first, then implementations, then tests). Dependencies are correct.

---

## Overall Assessment

**Correctness:** No bugs identified. Resource management, error handling, and OpenSSL API usage are all correct.

**Style:** Code follows DPDK conventions. No forbidden tokens. Comments are clear.

**Documentation:** Release notes and ABI changes are documented. API comments in patch 1/8 are thorough.

**Testing:** Comprehensive test coverage for OAEP (multiple configurations, both key types) and PSS (multiple salt lengths). Capability checks ensure tests skip gracefully on unsupported devices.

**Status:** **APPROVED**. No changes required. This is a high-quality patch series.