Fix TSP message imprint algorithm/digest length consistency

[yosuke-wolfssl]

Problem

f-7075: the TspRequest message imprint was configured by two setters that both wrote imprint.hashSz with no cross-validation, so the field did not distinguish "a digest was supplied" from "the algorithm implies this length".

• Forgotten digest. wc_TspRequest_Init() + wc_TspRequest_SetHashType(WC_HASH_TYPE_SHA256) left hashSz = 32 with imprint.hash still all zero. wc_TspRequest_Encode() only checked hashSz == 0, so it returned 0 and encoded a TimeStampReq whose messageImprint was 32 zero bytes — the caller obtains a token over a fixed, attacker-known digest instead of over their data.
• Either call order. SetHash(sha1Digest, 20) then SetHashType(SHA-256) raised hashSz to 32, encoding 20 real bytes plus 12 zero bytes tagged SHA-256; the reverse order encoded a 20-byte imprint tagged SHA-256. Neither was rejected.

wc_TspTstInfo_SetMsgImprint() and wc_TspTstInfo_Encode() had the same hashSz != 0-only check. Medium severity, unsafe default configuration. TSP is unreleased (added after v5.9.2), so the setter contract is tightened rather than kept compatible.

Fix (wolfcrypt/src/tsp.c, wolfcrypt/src/asn_tsp.c, wolfcrypt/src/hash.c)

hashSz now means only "a digest was supplied"; the expected length is derived from imprint.hashAlgOID via wc_HashGetDigestSize(wc_OidGetHash(...)). No new struct member, no ABI change.

Function	Change
wc_TspRequest_SetHashType()	Sets the OID and invalidates any digest (imprint.hash zeroed, hashSz = 0) instead of resizing it
wc_TspRequest_SetHash()	HASH_TYPE_E when the algorithm is unset/unavailable, BUFFER_E when hashSz is not its digest size
wc_TspTstInfo_SetMsgImprint()	BUFFER_E on a length mismatch; lenient for an unknown/unavailable OID so a TSA can echo an imprint it cannot compute
wc_TspRequest_Encode(), wc_TspTstInfo_Encode()	Reject hashSz == 0 (never-supplied digest) and a determinable length mismatch
wc_OidGetHash()	Added the missing SHA512_224h/SHA512_256h cases so those digest lengths are determinable

The length check applies only when the digest size is determinable, so an unknown OID still yields ASN_UNKNOWN_OID_E. Decoders are unchanged: a verifier still sees what arrived, and a malformed imprint fails closed at wc_TspTstInfo_Encode() / wc_TspTstInfo_CheckRequest(). The OpenSSL compat layer writes the imprint fields directly, so order-independent TS_MSG_IMPRINT_set_algo/_set_msg semantics are preserved and it gains only the fail-closed encode check.

The wc_OidGetHash() gap came out of review: wc_HashGetOID() has mapped those hash types to those OIDs since #4257 (2021), but the reverse function, written in 2018, was never updated. Deriving the expected length through it therefore made SHA-512/224 and SHA-512/256 imprints unbuildable (SetHash() always returned HASH_TYPE_E) and silently skipped the encode-time length check for them. The two cases use guards mirroring wc_HashGetOID(); being the shared reverse lookup, this also unbreaks those algorithms for PKCS#7/CMS digest algorithms, OCSP CertID hashes, the PKCS#12 MAC, and the OpenSSL-compat RSA-PSS path.

Closes f-7075.

Tests

tests/api/test_tsp.c: forgotten digest fails to encode; SetHash() then SetHashType() discards the digest and fails to encode; SetHash() with no algorithm → HASH_TYPE_E; wrong length → BUFFER_E for both setters; encode-time length mismatch for request and TSTInfo; SHA-512/224 imprint set and encoded end to end. tests/api/test_hash.c: both OIDs added to the supported and not-supported lists of test_wc_OidGetHash().

The SHA-512/224 case is gated on the same condition wc_HashGetDigestSize() uses for that algorithm — (!HAVE_FIPS || FIPS_VERSION3_GE(7,0,0)) && !HAVE_SELFTEST on top of the WOLFSSL_SHA512/WOLFSSL_NOSHA512_224 checks — so it is never compiled into a build where wc_TspRequest_SetHashType() reports the algorithm unavailable.

Verification

• Build clean under -Werror. Also builds clean with WOLFSSL_TSP_REQUESTER alone, WOLFSSL_TSP_RESPONDER alone, and -DWOLFSSL_NOSHA512_224 -DWOLFSSL_NOSHA512_256.
• make check: 6 passed, 4 skipped, 0 failed (includes scripts/tsp.test and the full unit.test). tsp 59/59, ossl_tsp 18 passed, hash 13 passed.
• Negative controls: restoring hashSz = digestSz fails the forgotten-digest test; dropping the encode length term fails both mismatch tests; removing the SHA512_224h case fails the new SHA-512/224 and wc_OidGetHash() tests.

[Copilot]

Pull request overview

This PR fixes a correctness/security issue in the RFC 3161 TSP messageImprint handling by making the digest length consistently derived from the configured hash algorithm, and by ensuring encoders fail closed when a digest was never provided or is the wrong length.

Changes:

• Tighten TspRequest setter semantics: SetHashType() now invalidates any previously-set digest, and SetHash() requires an algorithm to be set and enforces the expected digest length.
• Add encoder-side validation in wc_TspRequest_Encode() and wc_TspTstInfo_Encode() to reject missing digests and determinable length mismatches.
• Update documentation and extend API tests to cover the previously unsafe/ambiguous configurations (forgotten digest, wrong length, and call-order interactions).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
wolfcrypt/src/tsp.c	Updates requester/responder imprint setters to enforce algorithm-before-digest and digest-length consistency.
wolfcrypt/src/asn_tsp.c	Adds fail-closed encode-time imprint length validation based on the algorithm’s digest size when determinable.
tests/api/test_tsp.c	Adds/updates tests covering missing digest, setter ordering, and length mismatch behaviors.
doc/dox_comments/header_files/tsp.h	Updates public API documentation/examples to use the setters and document the ordering/length contract.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m4-rsa-only

• FLASH: .text +64 B (+0.0%, 328,176 B / 1,048,576 B, total: 31% used)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-openssl-compat
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11153

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

doc/dox_comments/header_files/tsp.h:56

• This doc example passes sizeof(hash) into wc_TspRequest_SetHash() even though the API expects a word32. Other examples in this file cast sizeof(...) to word32, so doing the same here keeps the documentation consistent and avoids potential conversion warnings for users compiling with strict flags.

    wc_TspRequest_Init(&req);
    wc_TspRequest_SetHashType(&req, WC_HASH_TYPE_SHA256);
    wc_TspRequest_SetHash(&req, hash, sizeof(hash));
    req.certReq = 1;

doc/dox_comments/header_files/tsp.h:23

• In the documentation examples, wc_TspRequest_SetHash() takes a word32 size but the example passes sizeof(hash) (a size_t). Elsewhere in this same doc file sizes are explicitly cast to word32 (e.g., word32 hashSz = (word32)sizeof(hash);), so keeping the cast here avoids signed/size conversion warnings in downstream builds.

This issue also appears on line 53 of the same file.

    wc_TspRequest_Init(&req);
    wc_TspRequest_SetHashType(&req, WC_HASH_TYPE_SHA256);
    wc_TspRequest_SetHash(&req, hash, sizeof(hash));
    req.certReq = 1;

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11153

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 3
2 finding(s) posted as inline comments (see file-level comments below)

Low (1)

Stale SHA-512/224/256 skip in wc_OidGetHash round-trip coverage

File: tests/api/test_hash.c:1109
Function: test_wc_HashFeatureCoverage
Category: Missing edge-case coverage on a function the PR also changed

The comment claims wc_OidGetHash() has no case for SHA512_224h/SHA512_256h, which this PR makes false, and the guard still skips the wc_HashGetOID → wc_OidGetHash round-trip assertion for exactly the two mappings the PR adds. supportedHash[] only contains those types when the mapping is present (!HAVE_FIPS && !HAVE_SELFTEST), so the skip is now permanently dead.

Recommendation: Drop the two SHA512_224/SHA512_256 exclusions and the stale comment so the round trip is asserted for every entry of supportedHash[].

Referenced code: tests/api/test_hash.c:1109-1113 (5 lines)

---

This review was generated automatically by Fenrir. Findings are non-blocking.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #11153

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfssl-bugs
Failed targets: wolfcrypt-src, wolfssl-src

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.