|WARNING| [v9] examples: add Wycheproof validation app

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-13

# DPDK Wycheproof Validation Patch Review

## ERRORS

### 1. Sensitive key material not zeroed before free (Error)
**Line 338 (free_vector):** `free_vector()` calls `rte_free_sensitive()` on the key but uses plain `rte_free()` on other fields that may contain sensitive data.

```c
static void
free_vector(struct wycheproof_data *vector)
{
	rte_free_sensitive(vector->key);
	rte_free(vector->iv);           /* IV is sensitive */
	rte_free(vector->aad);
	rte_free(vector->msg);
	rte_free(vector->ct);
	rte_free(vector->tag);           /* Tag may be secret (HMAC key derivation) */
	memset(vector, 0, sizeof(*vector));
}
```

IVs used in cipher modes (especially stream ciphers and CTR-like modes) should be wiped before free to avoid leaking keystream correlation data. While AAD, plaintext msg, and ciphertext ct are typically not secret, the IV should use `rte_free_sensitive()`.

**Suggested fix:**
```c
static void
free_vector(struct wycheproof_data *vector)
{
	rte_free_sensitive(vector->key);
	rte_free_sensitive(vector->iv);   /* IV may reveal keystream info */
	rte_free(vector->aad);
	rte_free(vector->msg);
	rte_free(vector->ct);
	rte_free(vector->tag);
	memset(vector, 0, sizeof(*vector));
}
```

### 2. Resource leak on multiple error paths (Error)
**Line 530-544 (run_aead):** When `rte_pktmbuf_append()` fails (input_len exceeds tailroom), the function sets `ret = -EMSGSIZE` and jumps to `out:`, but at that point `*digest` has already been allocated. The `out:` label only frees `*output` and `*digest` when `ret != 0`, so the digest buffer leaks on this path.

```c
	*digest = rte_malloc(NULL, vector->tag_len, RTE_CACHE_LINE_SIZE);
	if (*digest == NULL && vector->tag_len != 0)
		goto out;
	/* ... */
	if (input_len > rte_pktmbuf_tailroom(mbuf)) {
		ret = -EMSGSIZE;
		goto out;   /* leaks *digest allocated above */
	}
```

The same pattern appears in `run_hmac` (lines 614-650), `run_gmac` (lines 692-728), `compute_hash` (lines 947-988), and asymmetric operation functions.

**Suggested fix:** Allocate `*digest` after all size checks, or ensure the error path reaches the cleanup that frees it. The `out:` label already has the correct cleanup, so the issue is that `*digest` is allocated before the tailroom check.

### 3. Use of `rte_malloc()` instead of `rte_zmalloc_socket()` for queue-related buffers (Warning)
**Throughout main.c:** All `rte_malloc()` and `rte_zmalloc()` calls use `rte_socket_id()` or the default NUMA node, but do not use the `_socket` variants consistently. For queue-related control structures (session pools, op pools), `rte_zmalloc_socket()` is preferred to ensure NUMA-local allocation and zero-initialization.

However, the patch does use `rte_zmalloc()` in line 1491 for ECDH operand buffers. The guideline recommends `rte_zmalloc_socket()` for queue control structures, but this patch allocates per-operation temporary buffers, so plain `rte_zmalloc()` or `rte_malloc()` is acceptable here.

**No change required** for per-operation buffers. The mempool creation in `app_init()` already uses `rte_socket_id()`, which is correct.

### 4. ECDH private key not wiped (Error)
**Line 1491-1492 (run_ecdh_ecpoint):** The private key `priv_buf` is a local stack buffer containing sensitive ECC scalar material. When the function returns (both normal and error paths), this buffer is not explicitly wiped.

```c
	uint8_t priv_buf[MAX_EC_BYTESIZE];
	/* ... */
	if (normalize_operand(priv, priv_len, priv_buf, bytesize) != 0)
		return -EINVAL;
	/* ... uses priv_buf ... */
	return ret;   /* priv_buf not wiped */
}
```

**Suggested fix:** Add `rte_memzero_explicit(priv_buf, sizeof(priv_buf));` before every `return` and `goto out`.

### 5. ECDSA private key material (wx, wy) not wiped (Error)
**Line 1719-1720 (run_ecdsa_verify):** Local stack buffers `wx_buf` and `wy_buf` contain normalized EC public key coordinates. While public keys are not secret, the guideline requires wiping all key-related buffers that go through crypto operations. If these are truly public (which they are for ECDSA verify), they do not need wiping. However, the `priv_buf` analogy from ECDH suggests consistency.

**No change required** (public key coordinates are not secret).

### 6. DSA verify digest buffer contains hash output but is not wiped (Info - pre-existing pattern)
**Line 1070-1072 (run_dsa_verify):** The `digest` buffer contains the SHA hash of the message and is used directly in the DSA verify operation. After the operation completes, the function returns without wiping `digest`. However, message hashes are typically not considered secret (the message itself is public in signature verification), so this is acceptable.

**No change required** (message digest for signature verification is not secret).

### 7. ECDSA digest buffer not wiped (Info)
**Line 1719 (run_ecdsa_verify):** Same as DSA: `digest` buffer holds the message hash and is not wiped. Message hashes for signature verification are not secret.

**No change required**.

### 8. Missing release notes for internal example application (Warning)
**Line 163-169 (doc/guides/rel_notes/release_26_11.rst):** The patch adds release notes for the new Wycheproof validation example. The guideline states "Internal API and helper functions (not exported to applications) do NOT require release notes." However, this is an example application (under `examples/`), not an internal API. Example applications are user-facing and should be documented in release notes.

**The existing release notes entry is correct.** No change required.

---

## WARNINGS

### 1. Missing `__rte_experimental` tag on asymmetric operation casting (Warning)
**Line 1109, 1507, 1748:** The code casts `RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE` to `(enum rte_crypto_asym_op_type)`. This is a workaround for a type mismatch where `RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE` is an `rte_crypto_asym_ke_type` but `check_optype` expects an `rte_crypto_asym_op_type`. This suggests a potential API design issue, but the application is correctly working around it.

```c
	if (rte_cryptodev_asym_xform_capability_check_optype(capability,
			(enum rte_crypto_asym_op_type)
			RTE_CRYPTO_ASYM_KE_SHARED_SECRET_COMPUTE) == 0)
		return -ENOTSUP;
```

The cast itself is not wrong, but it may indicate an API inconsistency. However, this is an application workaround for a library design, not a bug in the patch.

**No change required** (application correctly works around library API).

### 2. Hardcoded AES-CCM AAD offset constant (Warning)
**Line 32:** `#define AES_CCM_AAD_OFFSET 18` is a magic number. The comment explains it ("B0 block + length"), but the value should ideally be derived from the CCM specification constants or at least have a more descriptive name.

```c
/* CCM AAD is prefixed with an 18-byte reserved area (B0 block + length). */
#define AES_CCM_AAD_OFFSET 18
```

The comment is clear, so this is acceptable for an example application. No change required, but a symbolic constant like `RTE_CRYPTO_CCM_B0_SIZE + RTE_CRYPTO_CCM_AAD_LEN_SIZE` would be more maintainable if such constants existed in DPDK.

**No change required** (well-documented magic number in example code).

### 3. Example application uses `ALLOW_EXPERIMENTAL_API` in Makefile (Warning)
**Line 25 (examples/crypto_validation/wycheproof_validation/Makefile):**
```makefile
CFLAGS += -O3 -DALLOW_EXPERIMENTAL_API $(shell $(PKGCONF) --cflags libdpdk jansson)
```

The guideline states: "Do NOT use `-DALLOW_EXPERIMENTAL_API` in lib/drivers/app." However, this is an `examples/` application, not part of the core `lib/`, `drivers/`, or `app/test/` directories. Example applications are allowed to use experimental APIs to demonstrate new features.

**No change required** (examples are exempt from the experimental API restriction).

### 4. Meson file also enables experimental APIs (Warning)
**Line 9 (examples/crypto_validation/wycheproof_validation/meson.build):**
```python
allow_experimental_apis = true
```

Same reasoning as above: examples are allowed to use experimental APIs.

**No change required**.

---

## INFO

### 1. Large example application in a single file (Info)
**main.c is 2097 lines.** For maintainability, consider splitting into separate modules (e.g., `aead.c`, `mac.c`, `asymmetric.c`, `json_parser.c`). However, example applications often prioritize self-contained simplicity over modular architecture.

**No change required** (acceptable for an example application).

### 2. JSON error handling could be more defensive (Info)
**Throughout main.c:** The code uses `json_object_get()` and `json_string_value()` without always checking for NULL before using the result. For example, line 1015:
```c
sha = json_string_value(json_object_get(group, "sha"));
if (sha == NULL || map_sha(sha, &dsa.hash, &dsa.digest_len) != 0) {
```

This checks for NULL, which is correct. However, some other paths (e.g., line 433 `vector.result = json_string_value(...)`) assume the JSON is well-formed. Since the vectors come from Google Wycheproof, this assumption is reasonable.

**No change required** (acceptable for an example application processing known-good JSON).

---

## SUMMARY

This patch introduces a valuable crypto validation tool. The main correctness issues are:

1. **Sensitive key material (IVs) not wiped** in `free_vector()`
2. **Resource leak** of `*digest` buffer on `-EMSGSIZE` error path in multiple functions
3. **ECDH private key not wiped** from stack buffer `priv_buf` in `run_ecdh_ecpoint()`

All other patterns are either acceptable for an example application or follow existing DPDK conventions.

Review-Result: ERROR