Harden wolfSSL and wolfSSH validation
Enable validated ECC imports and X25519 all-zero rejection through PUBLIC build policy. Tighten wolfSSH parser bounds, overflow handling, and signature framing with guard-page and crypto vector contracts.
This commit is contained in:
@@ -0,0 +1,130 @@
|
||||
# Bounded wolfSSH parser contract
|
||||
|
||||
Run from the project root (installed pinned sources and a host C compiler required):
|
||||
|
||||
```sh
|
||||
CCACHE_DISABLE=1 python3 tests/wolfssh_parser_contract/run.py
|
||||
CCACHE_DISABLE=1 python3 tests/wolfssh_auth_contract/run.py --host-only
|
||||
CCACHE_DISABLE=1 python3 tests/sdk_security_overrides/run.py
|
||||
```
|
||||
|
||||
No download, PlatformIO, managed-component edit, production build-tree regeneration,
|
||||
or device operation is performed. The runner verifies the original `internal.c`
|
||||
SHA-256, calls the production `render_entry`, writes and reads back its generated
|
||||
bytes in a temporary directory, and extracts complete actual functions. C tests
|
||||
run with guard pages and UBSan trap instrumentation, both with and without
|
||||
`WOLFSSH_SMALL_STACK`. The existing SDK suite separately tests generation and
|
||||
CMake source replacement using fixtures. This is **not** a claim that an existing
|
||||
production generated file or firmware binary contains these edits.
|
||||
|
||||
## Reviewed upstream evidence and exact implementation scope
|
||||
|
||||
Official diffs fetched and inspected on 2026-09-15:
|
||||
|
||||
- https://github.com/wolfSSL/wolfssh/pull/892.diff
|
||||
- https://github.com/wolfSSL/wolfssh/pull/881.diff
|
||||
- https://github.com/wolfSSL/wolfssh/pull/899.diff
|
||||
- https://github.com/wolfSSL/wolfssh/pull/880.diff
|
||||
|
||||
These are PR URLs, not immutable commit pins. The authoritative local inputs remain
|
||||
original wolfSSH **1.4.20**, SHA-256
|
||||
`81ff1f9166708abd5c2911e9fe57c0aee01c88b5d3f68c909ee8a856d37f36a9`,
|
||||
plus the exact-once checked-in edits in `tools/security_overrides.py`. No repinning
|
||||
or wholesale upstream patch application occurs.
|
||||
|
||||
Covered:
|
||||
|
||||
- **892 subset:** `DoIgnore` calls `GetSkip`; `GetSkip` uses `GetSize` and accepts
|
||||
an empty string ending exactly at the payload boundary. `DoServiceRequest`
|
||||
validates the full string with `GetSize` before `GetString`, retaining the old
|
||||
strict `< WOLFSSH_MAX_NAMESZ` limit rather than upstream's truncation behavior.
|
||||
Failure leaves the caller index and client state untouched. Successful state
|
||||
transition remains exactly the old one; service-name semantic validation is
|
||||
not added. The original `GetSize` already uses bounded subtraction and needs
|
||||
no change. `GetString` now uses it and rejects zero output capacity before
|
||||
subtraction/copy; ordinary bounded truncation semantics remain unchanged.
|
||||
- **881 subset:** `DoChannelWindowAdjust` rejects addition exceeding the 32-bit
|
||||
maximum with `WS_OVERFLOW_E`, leaving the channel window unchanged. The parsed
|
||||
index still advances, as upstream does. No new include is needed for the
|
||||
explicit word32 maximum. Unknown channels and truncated fields stay rejected.
|
||||
- **880 subset:** both key/signature type checks in `DoUserAuthRequestEcc` and
|
||||
`DoUserAuthRequestEd25519` use OR.
|
||||
`GetSize` bounds the incoming span first; unequal lengths short-circuit before
|
||||
`memcmp`, and equal lengths compare exactly the expected span. Valid matching
|
||||
ECDSA/Ed25519 types follow the original crypto path. Existing error normalization
|
||||
remains (`WS_CRYPTO_FAILED` for key parsing, `WS_INVALID_ALGO_ID` for signature
|
||||
type mismatch).
|
||||
- **Local signature-framing correction:** the ECC r/s parser uses the checked
|
||||
end of the declared signature sub-blob, not the enclosing field size. The
|
||||
preceding `GetSize` establishes `sz <= signatureSz - i`, so calculating that
|
||||
end cannot wrap. Both mpints must exactly consume the sub-blob, and the
|
||||
sub-blob must exactly consume the enclosing signature field. Ed25519 likewise
|
||||
rejects bytes outside its declared signature string before starting message
|
||||
verification. These framing errors return `WS_BUFFER_E`. This intentionally
|
||||
rejects previously tolerated malformed trailing bytes; valid SSH signature
|
||||
framing and the crypto calls/digest/message construction are unchanged.
|
||||
This is a local correction verified against the pinned implementation and
|
||||
[key-validation review](../../docs/ssh_key_validation_review.md), not a claim
|
||||
that these framing edits came from PR 880.
|
||||
|
||||
Reachability evidence: the pinned `DoPacket` dispatches IGNORE, SERVICE_REQUEST,
|
||||
and CHANNEL_WINDOW_ADJUST to these handlers. `DoUserAuthRequestPublicKey` calls
|
||||
`DoUserAuthRequestEcc`/`DoUserAuthRequestEd25519` for ECDSA/Ed25519 authentication,
|
||||
both enabled in this server's reviewed profile. Advertisement is not treated as a parser dispatch filter.
|
||||
|
||||
## Explicitly deferred (not fixed by this slice)
|
||||
|
||||
- **892:** client `DoServiceAccept`, agent key preparation, daemon authentication,
|
||||
Windows terminal changes. Password framing/wiping is the existing local
|
||||
correction, intentionally not replaced with upstream's later formulation.
|
||||
- **899:** no hunks applied. `ParseRSAPubKey`/`ParseECCPubKey` skips require separate
|
||||
client/KEX reachability analysis (not the server's `DoUserAuthRequestEcc`).
|
||||
The old `DoChannelFailure` does not parse a channel ID at all; changing only its
|
||||
`len != 0` typo would not establish a bounded channel-ID parser. Its existing
|
||||
behavior is left unchanged rather than claiming the later parser contract.
|
||||
Windows port/terminal hunks are out of scope.
|
||||
- **880:** certificate RSA, agent, daemon, terminal, TPM and SCP changes are not
|
||||
applied. No complete PR-880 closure is claimed.
|
||||
- Message ordering/state machine (including CVE-2025-14942), service semantics,
|
||||
standalone ECC curve-name/key-blob semantic validation, other parsers, client
|
||||
behavior and broader crypto advisories are outside this slice. ECC point/import
|
||||
validation belongs to the separate crypto-policy owner and is not changed here.
|
||||
Existing account/key authorization, numeric r/s validity and Ed25519 raw
|
||||
signature-size/crypto validity checks remain owned by their existing layers.
|
||||
|
||||
## Test boundaries
|
||||
|
||||
The C matrix exercises zero/truncated/exact/oversized/wrapping lengths, invalid
|
||||
and nonzero offsets, zero-capacity output, copy canaries, window overflow boundary
|
||||
pairs, unknown channels, ECC equal-length mismatches, shorter/longer matching
|
||||
prefixes, empty types and every key/signature truncation. Expected ECC and
|
||||
Ed25519 type bytes end at a protected page, testing unequal-length short-circuit
|
||||
safety. Crypto and
|
||||
channel lookup are doubles; tests establish parser gating, not real signature or
|
||||
point validation. Numeric errors, context/channel layouts and name capacity are
|
||||
host doubles, not production ABI verification.
|
||||
|
||||
The follow-up matrix in `auth_framing.c` covers every truncated ECC sub-blob
|
||||
boundary with a complete r/s pair still available beyond that boundary, oversized
|
||||
and wrapping nested lengths, malformed r/s lengths, inner/outer trailing bytes,
|
||||
and physically guard-page-ended frames. Instrumented `ato32` also asserts that
|
||||
nested length reads cannot use accessible bytes outside the sub-blob. Signature
|
||||
input and surrounding canaries stay unchanged. Valid 32-byte and sign-padded
|
||||
33-byte r/s encodings reach conversion with their bytes/lengths intact. Ed25519
|
||||
covers both labels, all truncations, shortened/oversized/wrapping/trailing
|
||||
signature strings, exact raw-signature forwarding and unchanged streamed message
|
||||
bytes. Both paths retain crypto rejection behavior using doubles.
|
||||
|
||||
Validation: **3,124 cases per stack mode** (both pass with UBSan trap mode), plus
|
||||
**six guard-removal mutations rejected**: ECC nested read bound, inner/outer exact
|
||||
consumption, Ed25519 key/signature OR checks, and Ed25519 exact consumption. The
|
||||
mutation copies exist only in temporary test files; core dumps are disabled for
|
||||
those intentionally failing runs. These are framing-valid fixtures with crypto
|
||||
doubles, not independently verified real signatures.
|
||||
|
||||
The runner also compares complete password, packet dispatch, public-key dispatch and selected deferred
|
||||
functions against the pre-slice generated baseline to fence accidental changes.
|
||||
The separate auth suite executes its 135 password/control-flow cases, including
|
||||
payload wipe, callback framing and asynchronous pending retention. No whole-library
|
||||
fuzzing, real SSH exchange, firmware compile, hardware timing or security sign-off
|
||||
is implied.
|
||||
Reference in New Issue
Block a user