Enforce rustls provider contracts and security hardening

[thieman]

Summary

The codebase review found several provider-contract and security-hardening gaps where this provider diverges from rustls/rustls-webpki behavior or leaves sensitive data/error cases insufficiently handled. This issue groups fixes that should be implemented with regression tests.

Problems to fix

1. RSA verification does not enforce rustls/webpki RSA key-size bounds

Evidence

• RSA verification imports whatever RsaPublicKey::try_from(public_key) parses and asks CNG to verify (src/verify.rs:228-279).
• import_rsa_public_key only populates BCRYPT_RSAKEY_BLOB.BitLength from the modulus length and does not reject small/oversized moduli (src/keys.rs:79-113).
• Locked rustls/rustls-webpki built-in providers use RSA verification algorithms bounded to 2048-8192 bits, e.g. RSA_PKCS1_2048_8192_* / RSA_PSS_2048_8192_* in rustls-webpki.
• Existing tests only exercise 2048-bit and larger Wycheproof cases (src/verify.rs tests use Rsa2048/Rsa3072/Rsa4096 cases).

Impact

If CNG accepts RSA-1024 or otherwise out-of-policy keys, this provider may accept certificate-chain or handshake signatures that rustls built-in providers reject. In FIPS mode, this also weakens the provider's FIPS posture.

Expected fix

• Reject RSA public keys with modulus sizes outside rustls/webpki's accepted 2048-8192-bit range before CNG import.
• Add negative tests for RSA-1024 verification and any oversized boundary practical to test.

2. RSA PKCS#1 certificate verification omits absent-parameters AlgorithmIdentifier variants

Evidence

• SUPPORTED_SIG_ALGS.all includes only present-parameters RSA PKCS#1 algorithm identifiers (src/verify.rs:24-60, src/verify.rs:63-88).
• rustls' built-in provider includes present- and absent-parameters variants because RFC4055 requires accepting absent parameters for sha256WithRSAEncryption and related OIDs.

Impact

Valid RSA PKCS#1-signed certificate chains with omitted NULL AlgorithmIdentifier parameters can fail verification with this provider even though rustls built-ins accept them.

Expected fix

• Add absent-parameters RSA PKCS#1 SHA-256/SHA-384/SHA-512 verification algorithm objects.
• Include them in SUPPORTED_SIG_ALGS.all for certificate-chain verification.
• Add certificate-chain or direct algorithm tests covering absent-parameter encodings.

3. TLS 1.2 decrypters do not reject oversized plaintext records

Evidence

• AES-GCM TLS 1.2 decrypt truncates and returns plaintext without checking max fragment size (src/tls12.rs:257-292).
• ChaCha20-Poly1305 TLS 1.2 decrypt does the same (src/tls12.rs:319-339).
• rustls built-in TLS 1.2 providers check plaintext length and return Error::PeerSentOversizedRecord.

Impact

A valid AEAD record with plaintext larger than TLS maximum can be accepted as normal TLS 1.2 plaintext, violating TLS record semantics and rustls provider expectations.

Expected fix

• Enforce the same max-fragment check as rustls built-in TLS 1.2 providers before returning plaintext.
• Add tests for AES-GCM and ChaCha20 TLS 1.2 decryption with oversized plaintext records.

4. TLS 1.2 ECDSA suites list Ed25519 despite no Ed25519 implementation

Evidence

• ECDSA_SCHEMES includes SignatureScheme::ED25519 (src/tls12.rs:24-29).
• Ed25519 signing code is commented out because CNG does not support Ed25519 (src/signer/ec.rs).
• Ed25519 verification is commented out of SUPPORTED_SIG_ALGS (src/verify.rs).

Impact

TLS 1.2 ECDSA suites advertise Ed25519 as compatible while the provider cannot load Ed25519 private keys or verify Ed25519 signatures, which can cause confusing negotiation/handshake failures.

Expected fix

• Remove SignatureScheme::ED25519 from TLS 1.2 ECDSA suite sign lists unless/until Ed25519 is fully implemented.
• Add tests/assertions that advertised TLS 1.2 sign schemes are supported by provider signing/verifying capabilities.

5. Private-key import blobs are not zeroized

Evidence

• RSA private-key import copies private primes into heap Vec blob and drops it without zeroization (src/keys.rs:30-75).
• EC private-key import appends the private scalar into a heap Vec and drops it without zeroization (src/keys.rs:149-169).
• The crate already uses zeroize for ECDH shared-secret stack storage (src/kx.rs:242-273).

Impact

Extra copies of private key material can remain in freed heap pages until allocator reuse, increasing exposure in crash dumps, memory disclosure bugs, or local process inspection.

Expected fix

• Use zeroize::Zeroizing<Vec<u8>> or explicit zeroize() for private import blobs.
• Avoid unnecessary temporary sensitive buffers where practical.

6. AEAD in-place CNG calls may violate Rust aliasing through overlapping &[u8] and &mut [u8]

Evidence

• AeadKey::seal creates an immutable input slice from data.as_ptr() and also passes data as mutable output to BCryptEncrypt (src/aead.rs:116-129).
• AeadKey::open similarly passes overlapping immutable and mutable slices to BCryptDecrypt (src/aead.rs:164-177).
• CNG supports in-place encryption/decryption, but Rust references do not permit live immutable and mutable references to the same bytes across a mutating call.

Impact

Potential undefined behavior at the Rust FFI boundary despite the underlying CNG API supporting in-place operation.

Expected fix

• Avoid materializing overlapping Rust references. Options:
  • use raw-pointer bindings (windows-sys or manual extern declarations) for these in-place CNG calls, or
  • copy input to a separate buffer before calling the current wrapper.
• Keep safety comments focused on both CNG's in-place contract and Rust aliasing avoidance.

7. Reduce avoidable runtime unwrap() panics in crypto paths

Evidence

Examples include src/fips.rs, src/hash.rs, src/hkdf.rs, src/prf.rs, src/tls12.rs, src/tls13.rs, and src/quic.rs.

Impact

Some rustls trait methods are infallible, but provider/OS/resource failures can currently terminate consumer processes. This is especially risky after any stale-handle or unsupported-provider condition.

Expected fix

• Where trait signatures return Result, return rustls::Error instead of panicking.
• Where trait signatures are infallible, preflight capabilities before advertising primitives or document unavoidable panic behavior.
• At minimum, fix panics that are reachable from peer-controlled or normal OS/provider failure paths.

Suggested validation

cargo fmt -- --check
cargo check --target x86_64-pc-windows-msvc
cargo clippy --target x86_64-pc-windows-msvc -- -D warnings
cargo test  # on Windows CI/workstation
cargo test --doc  # on Windows CI/workstation

Add targeted tests before fixes and verify they fail on current behavior where possible.

Acceptance criteria

• RSA verification rejects public keys outside 2048-8192 bits before CNG verification.
• RSA PKCS#1 absent-parameter signature AlgorithmIdentifier variants are supported and tested.
• TLS 1.2 AEAD decrypters reject oversized plaintext records with the same error semantics as rustls built-ins.
• TLS 1.2 ECDSA suite sign lists no longer advertise unsupported Ed25519.
• RSA/EC private-key import buffers containing private material are zeroized.
• AEAD in-place calls avoid overlapping Rust reference aliasing.
• Avoidable runtime unwrap() panics in the touched crypto paths are removed or justified by infallible trait constraints and preflight checks.
• Regression tests and Windows CI validate the changes.

[thieman]

[GPT 5.5] Scope update for PR #11 (fix(tls): enforce signature provider contracts): this PR has been narrowed to the minimal signature/provider-contract fixes.

Addressed in PR #11:

• RSA verification key-size policy (2048-8192 bits)

  • This aligns the CNG provider with rustls/webpki's accepted RSA verification bounds.
  • This is directly relevant to FIPS posture because accepting out-of-policy RSA key sizes would be surprising for a FIPS-sensitive provider.

• RSA PKCS#1 absent-parameter AlgorithmIdentifier variants

  • This adds compatibility with valid certificate signatures whose sha*WithRSAEncryption AlgorithmIdentifier omits the optional NULL parameters.
  • RFC 4055 requires accepting both the explicit-NULL and absent-parameter encodings, and rustls/webpki's built-in providers handle both.

• Remove unsupported Ed25519 from TLS 1.2 ECDSA cipher-suite signing schemes

  • The provider does not implement Ed25519 signing or verification, and the code already documents that CNG does not support Ed25519 here.
  • Removing it avoids advertising a signature scheme this provider cannot actually satisfy. This is also consistent with the FIPS-sensitive scope of the crate: Ed25519 is not needed for the FIPS provider path.

Deferred from PR #11:

• TLS 1.2 oversized plaintext rejection

  • This is a real provider-contract concern, but it changes record-decryption behavior and deserves a focused PR with full decrypt-path regression tests rather than helper-only tests.

• AEAD in-place aliasing / copy-based CNG calls

  • The original code intentionally used CNG's in-place encryption/decryption support. The aliasing concern is worth reviewing, but changing it adds allocations and touches hot crypto paths. This should be handled separately, ideally with a raw-pointer/in-place strategy review rather than a broad bundled change.

• Private-key import zeroization

  • This is useful hardening, but it is not required for the signature-provider contract fixes and is difficult to assert with a normal regression test. It should be a separate focused hardening PR.

• Runtime unwrap() / dbg! cleanup

  • These are cleanup/hardening items, but they are broader than the narrowed PR goal. Keeping them out avoids incidental behavior changes in a crypto-sensitive PR.

Net: PR #11 is now limited to signature algorithm correctness and compatibility. The remaining hardening items should become separate, smaller issues/PRs with their own tests and reviewer focus.