diff --git a/src/client.rs b/src/client.rs index e4e7b22..c9b2eeb 100644 --- a/src/client.rs +++ b/src/client.rs @@ -133,8 +133,16 @@ pub fn select_server_verifier( /// to the built-in `webpki-roots` if the platform store is empty (e.g. in a /// container with no system CA bundle) — alknet ADR-088 §5. pub fn load_platform_root_cert_store() -> Result { + fill_root_store_from_native(&rustls_native_certs::load_native_certs()) +} + +/// The seam behind [`load_platform_root_cert_store`], parameterized on the +/// native-cert load so the webpki-roots fallback is deterministically +/// testable without root or platform-store control (alknet ADR-088 §5). +fn fill_root_store_from_native( + result: &rustls_native_certs::CertificateResult, +) -> Result { let mut roots = rustls::RootCertStore::empty(); - let result = rustls_native_certs::load_native_certs(); for err in &result.errors { tracing::warn!(error = ?err, "failed to load a native root cert"); } @@ -420,6 +428,19 @@ mod tests { .expect("DigitallySignedStruct decodes from its wire encoding") } + fn dss_with_scheme( + scheme: rustls::SignatureScheme, + signature: Vec, + ) -> rustls::DigitallySignedStruct { + use rustls::internal::msgs::codec::{Codec, Reader}; + let mut encoded = Vec::new(); + scheme.encode(&mut encoded); + (signature.len() as u16).encode(&mut encoded); + encoded.extend_from_slice(&signature); + rustls::DigitallySignedStruct::read(&mut Reader::init(&encoded)) + .expect("DigitallySignedStruct decodes from its wire encoding") + } + #[test] fn fingerprint_pin_verifier_matches_correct_ed25519_fingerprint() { let sk = Ed25519SecretKey::generate(); @@ -535,6 +556,51 @@ mod tests { ); } + #[test] + fn fingerprint_pin_verifier_verify_tls12_signature_routes_x509_through_standard_path() { + use rustls::client::danger::ServerCertVerifier; + + let key_pair = rcgen::KeyPair::generate().expect("ECDSA P-256 key gen"); + let cert = rcgen::CertificateParams::default() + .self_signed(&key_pair) + .expect("self-signed cert"); + let cert_der = cert.der().clone(); + let supported = aws_lc_rs_provider().signature_verification_algorithms; + let pin = fingerprint_from_cert_der(cert_der.as_ref()).expect("SHA256 pin"); + let verifier = FingerprintPinVerifier::new(pin, supported); + + let signing_key = rustls::crypto::aws_lc_rs::sign::any_ecdsa_type( + &rustls::pki_types::PrivateKeyDer::Pkcs8(rustls::pki_types::PrivatePkcs8KeyDer::from( + key_pair.serialize_der(), + )), + ) + .expect("ECDSA signing key loads"); + let message = b"alktls tls12 x509 signature routing"; + let signature = signing_key + .choose_scheme(&[rustls::SignatureScheme::ECDSA_NISTP256_SHA256]) + .expect("ECDSA_NISTP256_SHA256 must be offered") + .sign(message) + .expect("signing must succeed"); + let dss = dss_with_scheme(rustls::SignatureScheme::ECDSA_NISTP256_SHA256, signature); + + let result = verifier.verify_tls12_signature(message, &cert_der, &dss); + assert!( + result.is_ok(), + "TLS 1.2 signature for a non-Ed25519 (X.509 ECDSA) cert must route \ + through the standard verification path and verify, got: {result:?}" + ); + + let forged = dss_with_scheme( + rustls::SignatureScheme::ECDSA_NISTP256_SHA256, + vec![0u8; 70], + ); + let tampered = verifier.verify_tls12_signature(message, &cert_der, &forged); + assert!( + tampered.is_err(), + "a forged TLS 1.2 signature must fail the handshake signature check" + ); + } + #[test] fn select_server_verifier_returns_ca_verifier_for_none() { let provider = aws_lc_rs_provider(); @@ -725,6 +791,71 @@ mod tests { ); } + fn native_result( + certs: Vec>, + errors: usize, + ) -> rustls_native_certs::CertificateResult { + let mut result = rustls_native_certs::CertificateResult::default(); + result.certs = certs; + for i in 0..errors { + result.errors.push(rustls_native_certs::Error { + context: "test", + kind: rustls_native_certs::ErrorKind::Os( + std::io::Error::other(format!("native load failure {i}")).into(), + ), + }); + } + result + } + + #[test] + fn empty_platform_store_deterministically_falls_back_to_webpki_roots() { + let empty = native_result(Vec::new(), 0); + let roots = fill_root_store_from_native(&empty).expect("fallback must not fail"); + assert_eq!( + roots.len(), + webpki_roots::TLS_SERVER_ROOTS.len(), + "an empty platform store must deterministically pull in every \ + webpki-roots anchor (alknet ADR-088 §5)" + ); + assert!(!roots.is_empty()); + } + + #[test] + fn native_cert_errors_are_logged_and_valid_certs_still_load() { + let key_pair = rcgen::KeyPair::generate().expect("key gen"); + let cert = rcgen::CertificateParams::default() + .self_signed(&key_pair) + .expect("self-signed cert"); + let der = cert.der().to_owned(); + + let partial = native_result(vec![der], 2); + let roots = fill_root_store_from_native(&partial).expect("partial load must not fail"); + assert_eq!( + roots.len(), + 1, + "the valid cert must load despite the sibling errors" + ); + } + + #[test] + fn native_certs_fill_the_store_without_the_fallback() { + let key_pair = rcgen::KeyPair::generate().expect("key gen"); + let cert = rcgen::CertificateParams::default() + .self_signed(&key_pair) + .expect("self-signed cert"); + let der = cert.der().to_owned(); + + let full = native_result(vec![der], 0); + let roots = fill_root_store_from_native(&full).expect("native load must not fail"); + assert_eq!( + roots.len(), + 1, + "a non-empty platform store must be used verbatim — no webpki-roots \ + fallback on top" + ); + } + #[test] fn verifier_selection_matrix_over_client_config() { let sk = Ed25519SecretKey::generate(); @@ -814,10 +945,9 @@ mod tests { directory: crate::identity::AcmeDirectory::Production, contact: vec![], }); - let err = match TlsClientConfig::new(&acme, b"alk/call") { - Ok(_) => panic!("Acme client auth must fail config construction"), - Err(e) => e, - }; + let err = TlsClientConfig::new(&acme, b"alk/call") + .err() + .expect("Acme client auth must fail config construction"); assert!( matches!(err, TlsError::AcmeConfig(_)), "Acme local identity must map to TlsError::AcmeConfig, got: {err:?}" diff --git a/src/fingerprint.rs b/src/fingerprint.rs index c7f0392..9ee4ad7 100644 --- a/src/fingerprint.rs +++ b/src/fingerprint.rs @@ -402,7 +402,7 @@ mod tests { .into_iter() .chain([0u8; 31].iter().copied()) .collect(); - let mut spki = vec![0x30u8, 0x2b]; + let mut spki = vec![0x30u8, 0x29]; spki.extend_from_slice(&alg_id); spki.extend_from_slice(&bit_string_32); assert_eq!( @@ -415,7 +415,7 @@ mod tests { .into_iter() .chain([0u8; 33].iter().copied()) .collect(); - let mut spki = vec![0x30u8, 0x2d]; + let mut spki = vec![0x30u8, 0x2b]; spki.extend_from_slice(&alg_id); spki.extend_from_slice(&bit_string_34); assert_eq!( @@ -428,7 +428,7 @@ mod tests { .into_iter() .chain([0u8; 32].iter().copied()) .collect(); - let mut spki = vec![0x30u8, 0x2d]; + let mut spki = vec![0x30u8, 0x2a]; spki.extend_from_slice(&alg_id); spki.extend_from_slice(&bit_string_unused_bits); assert_eq!( @@ -438,6 +438,32 @@ mod tests { ); } + #[test] + fn wrong_tag_before_oid_and_missing_bit_string_extract_nothing() { + let non_oid_alg_id: Vec = vec![0x30, 0x05, 0x04, 0x03, 0x2b, 0x65, 0x70]; + let bit_string: Vec = vec![0x03, 0x21, 0x00] + .into_iter() + .chain([0u8; 32].iter().copied()) + .collect(); + let mut spki = vec![0x30u8, 0x2a]; + spki.extend_from_slice(&non_oid_alg_id); + spki.extend_from_slice(&bit_string); + assert_eq!( + extract_ed25519_raw_key_from_spki(&spki), + None, + "a non-OID tag (0x04) inside AlgorithmIdentifier must be rejected" + ); + + let alg_id: Vec = vec![0x30, 0x05, 0x06, 0x03, 0x2b, 0x65, 0x70]; + let mut spki = vec![0x30u8, 0x07]; + spki.extend_from_slice(&alg_id); + assert_eq!( + extract_ed25519_raw_key_from_spki(&spki), + None, + "a well-formed SPKI with no BIT STRING after the alg-id must be rejected" + ); + } + #[test] fn malformed_der_still_falls_back_to_sha256_fingerprint() { let malformed = [0x30u8, 0x81, 0x05, 0x01]; diff --git a/src/pem.rs b/src/pem.rs index 1d386a4..86a4d86 100644 --- a/src/pem.rs +++ b/src/pem.rs @@ -48,6 +48,23 @@ mod tests { ); } + #[test] + fn load_private_key_parse_error_yields_cert_load_error() { + let dir = tempfile::tempdir().unwrap(); + let garbage = dir.path().join("garbage.key"); + std::fs::write( + &garbage, + b"-----BEGIN PRIVATE KEY-----\n!!!\n-----END PRIVATE KEY-----\n", + ) + .unwrap(); + let err = load_private_key(&garbage); + assert!( + matches!(err, Err(TlsError::CertLoad(_))), + "malformed base64 in a keyed section is a parse error (not 'no key \ + found') and must yield CertLoad, got {err:?}" + ); + } + #[test] fn load_private_key_returns_error_when_file_missing() { let err = load_private_key(Path::new("/nonexistent/alktls-coverage/missing.key")); diff --git a/src/server.rs b/src/server.rs index a431be9..acef93e 100644 --- a/src/server.rs +++ b/src/server.rs @@ -695,6 +695,129 @@ mod tests { assert!(verifier.root_hint_subjects().is_empty()); } + fn dss_with_scheme( + scheme: rustls::SignatureScheme, + signature: Vec, + ) -> rustls::DigitallySignedStruct { + use rustls::internal::msgs::codec::{Codec, Reader}; + let mut encoded = Vec::new(); + scheme.encode(&mut encoded); + (signature.len() as u16).encode(&mut encoded); + encoded.extend_from_slice(&signature); + rustls::DigitallySignedStruct::read(&mut Reader::init(&encoded)) + .expect("DigitallySignedStruct decodes from its wire encoding") + } + + #[test] + fn verify_presented_cert_verifier_tls12_signature_routes_ed25519_spki_through_raw_key_path() { + use rustls::server::danger::ClientCertVerifier; + + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let verifier = VerifyPresentedCertVerifier::new(&provider); + + let sk = Ed25519SecretKey::generate(); + let raw_key = sk.public().to_bytes(); + let spki_der = + rustls::sign::public_key_to_spki(&rustls::pki_types::alg_id::ED25519, raw_key).to_vec(); + let message = b"alktls verify-presented tls12 raw-key routing"; + let signature = sk.sign(message).to_bytes().to_vec(); + let dss = dss_with_scheme(rustls::SignatureScheme::ED25519, signature); + let cert = rustls::pki_types::CertificateDer::from(spki_der); + + let result = verifier.verify_tls12_signature(message, &cert, &dss); + assert!( + result.is_ok(), + "TLS 1.2 signature for an Ed25519 SPKI presentation must route \ + through the raw-key path and verify, got: {result:?}" + ); + + let forged = dss_with_scheme(rustls::SignatureScheme::ED25519, vec![0u8; 64]); + let tampered = verifier.verify_tls12_signature(b"tampered", &cert, &forged); + assert!( + tampered.is_err(), + "a signature that does not verify must fail the possession check" + ); + } + + #[test] + fn verify_presented_cert_verifier_tls12_signature_routes_x509_through_standard_path() { + use rustls::server::danger::ClientCertVerifier; + + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let verifier = VerifyPresentedCertVerifier::new(&provider); + + let key_pair = rcgen::KeyPair::generate().expect("ECDSA P-256 key gen"); + let cert = rcgen::CertificateParams::default() + .self_signed(&key_pair) + .expect("self-signed cert"); + let cert_der = cert.der().clone(); + + let signing_key = rustls::crypto::aws_lc_rs::sign::any_ecdsa_type( + &rustls::pki_types::PrivateKeyDer::Pkcs8(rustls::pki_types::PrivatePkcs8KeyDer::from( + key_pair.serialize_der(), + )), + ) + .expect("ECDSA signing key loads"); + let message = b"alktls verify-presented tls12 x509 routing"; + let signature = signing_key + .choose_scheme(&[rustls::SignatureScheme::ECDSA_NISTP256_SHA256]) + .expect("ECDSA_NISTP256_SHA256 must be offered") + .sign(message) + .expect("signing must succeed"); + let dss = dss_with_scheme(rustls::SignatureScheme::ECDSA_NISTP256_SHA256, signature); + + let result = verifier.verify_tls12_signature(message, &cert_der, &dss); + assert!( + result.is_ok(), + "TLS 1.2 signature for an X.509 presentation must route through the \ + standard verification path and verify, got: {result:?}" + ); + + let wrong_key = rcgen::KeyPair::generate().expect("other key gen"); + let other_signing_key = rustls::crypto::aws_lc_rs::sign::any_ecdsa_type( + &rustls::pki_types::PrivateKeyDer::Pkcs8(rustls::pki_types::PrivatePkcs8KeyDer::from( + wrong_key.serialize_der(), + )), + ) + .expect("other ECDSA signing key loads"); + let wrong_signature = other_signing_key + .choose_scheme(&[rustls::SignatureScheme::ECDSA_NISTP256_SHA256]) + .expect("ECDSA_NISTP256_SHA256 must be offered") + .sign(message) + .expect("signing must succeed"); + let wrong_key_dss = dss_with_scheme( + rustls::SignatureScheme::ECDSA_NISTP256_SHA256, + wrong_signature, + ); + let tampered = verifier.verify_tls12_signature(message, &cert_der, &wrong_key_dss); + assert!( + tampered.is_err(), + "a valid signature under a different key must fail the possession check" + ); + } + + #[test] + fn verify_presented_cert_verifier_tls12_signature_rejects_mismatched_message() { + use rustls::server::danger::ClientCertVerifier; + + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let verifier = VerifyPresentedCertVerifier::new(&provider); + + let sk = Ed25519SecretKey::generate(); + let raw_key = sk.public().to_bytes(); + let spki_der = + rustls::sign::public_key_to_spki(&rustls::pki_types::alg_id::ED25519, raw_key).to_vec(); + let signature = sk.sign(b"the real message").to_bytes().to_vec(); + let dss = dss_with_scheme(rustls::SignatureScheme::ED25519, signature); + let cert = rustls::pki_types::CertificateDer::from(spki_der); + + let result = verifier.verify_tls12_signature(b"a different message", &cert, &dss); + assert!( + result.is_err(), + "a signature over a different message must fail the possession check" + ); + } + #[test] fn server_verifiers_keep_requires_raw_public_keys_default_false() { use rustls::server::danger::ClientCertVerifier; @@ -751,6 +874,35 @@ mod tests { assert!(s.contains("AcceptAnyCertVerifier")); } + #[test] + fn accept_any_cert_verifier_tls12_signature_asserts_without_possession_check() { + use rustls::server::danger::ClientCertVerifier; + + let verifier = AcceptAnyCertVerifier; + let cert = rustls::pki_types::CertificateDer::from(b"not even a cert".to_vec()); + let dss = dss_with_scheme(rustls::SignatureScheme::ED25519, vec![0u8; 64]); + let result = verifier.verify_tls12_signature(b"any message", &cert, &dss); + assert!( + result.is_ok(), + "the escape hatch must assert TLS 1.2 signature validity \ + unconditionally (the documented no-pop posture), got: {result:?}" + ); + } + + #[test] + fn verify_presented_cert_verifier_supported_schemes_are_the_nine_pinned() { + use rustls::server::danger::ClientCertVerifier; + + let provider = Arc::new(rustls::crypto::aws_lc_rs::default_provider()); + let verifier = VerifyPresentedCertVerifier::new(&provider); + assert_eq!( + verifier.supported_verify_schemes(), + nine_supported_verify_schemes(), + "the default verifier must report the same nine-scheme list as the \ + escape hatch (the load-bearing list)" + ); + } + #[test] fn raw_key_cert_resolver_debug_is_implemented() { let sk = Ed25519SecretKey::generate(); @@ -873,10 +1025,10 @@ mod tests { directory: crate::identity::AcmeDirectory::Staging, contact: vec!["mailto:dev@example.com".to_string()], }; - let err = match TlsServerConfig::new(&identity, &[b"alktls/test".to_vec()]).await { - Ok(_) => panic!("empty domain list must not construct an ACME config"), - Err(e) => e, - }; + let err = TlsServerConfig::new(&identity, &[b"alktls/test".to_vec()]) + .await + .err() + .expect("empty domain list must not construct an ACME config"); assert!( matches!(err, TlsError::AcmeConfig(_)), "empty domains must surface as TlsError::AcmeConfig, got {err:?}" diff --git a/tasks/coverage-cheap-closes.md b/tasks/coverage-cheap-closes.md index 1ed4f83..60fb058 100644 --- a/tasks/coverage-cheap-closes.md +++ b/tasks/coverage-cheap-closes.md @@ -1,7 +1,7 @@ --- id: coverage-cheap-closes name: Cheap coverage closes — non-Ed25519 pin arms, PEM parse-error arm, fallback seam, escape-hatch methods (U-2, re-baselined) -status: pending +status: completed depends_on: [] scope: narrow risk: low @@ -97,18 +97,18 @@ are exercised by the handshake suites) and `RawKeyCertResolver::resolve` ## Verification -- [ ] `cargo llvm-cov --all-features` shows the six groups covered +- [x] `cargo llvm-cov --all-features` shows the six groups covered (client.rs 316/145-148, pem.rs 30, server.rs 349-366/468-475, fingerprint.rs 67) -- [ ] The fallback test deterministically exercises the push loop +- [x] The fallback test deterministically exercises the push loop (platform-store-independent) -- [ ] `cargo test`, `cargo test --all-features`, clippy, fmt green +- [x] `cargo test`, `cargo test --all-features`, clippy, fmt green ## Acceptance Criteria -- [ ] Line coverage ≥ 98% (from 95.81% currently; 95.32% at +- [x] Line coverage ≥ 98% (from 95.81% currently; 95.32% at decomposition) with every load-bearing uncovered group closed -- [ ] No public-API growth +- [x] No public-API growth ## References @@ -124,6 +124,100 @@ are exercised by the handshake suites) and `RawKeyCertResolver::resolve` > Agent fills this during implementation. +Work notes (2026-09-12, verified against the current tree before and +after): + +- **Line drift since the re-baseline**: the two doc-pin commits + (`9bdc32d`, `7713a6e`) shifted the ADR-008 ranges. Current ground + truth at task start: client.rs TLS 1.2 pin else-arm = 334; + server.rs `VerifyPresentedCertVerifier::verify_tls12_signature` = + 414-431; `AcceptAnyCertVerifier::verify_tls12_signature` = 538-545 + (`supported_verify_schemes()` at 547-549 was already covered). +- **Group 1** (client.rs:334): the else-arm needs a parseable X.509 + cert — `verify_tls12_signature` maps the cert through + `webpki::EndEntityCert`. Test: rcgen ECDSA-P256 cert + + `any_ecdsa_type` signer + `ECDSA_NISTP256_SHA256` DSS → ok; forged + sig → err. (rcgen's `KeyPair::generate()` is already P-256; its + message-signing is `pub(crate)`, so the signer comes from + `rustls::crypto::aws_lc_rs::sign::any_ecdsa_type`.) +- **Group 2** (server.rs:414-431): three direct unit-call tests — + Ed25519 SPKI → raw-key path ok + forged → err; X.509 → standard + path ok + valid-sig-under-different-key → err; Ed25519 sig over a + mismatched message → err. +- **Group 3** (pem.rs:30): the prescribed garbage-but-keyed input + verified against rustls-pemfile 2.2.0 first (returns + `Err(InvalidCharacter(33))`) — works as prescribed. +- **Group 4**: seam extracted as `fill_root_store_from_native(&CertificateResult)` + (plain private fn, no API growth). Tests construct + `CertificateResult::default()` and push into its public `certs` / + `errors` fields (probed: construction works despite + `#[non_exhaustive]` — it derives `Default` and `Error`'s fields are + public). Three tests: empty → all webpki-roots anchors + (count-checked, deterministic); errors + valid cert → valid cert + loads (also covers the client.rs:139 `tracing::warn!` pocket); + valid cert → used verbatim, no fallback. +- **Group 5** (server.rs:538-545): `verify_tls12_signature` asserts + unconditionally (pinned with garbage cert + garbage sig → Ok); + added the nine-scheme pin for `VerifyPresentedCertVerifier:: + supported_verify_schemes()` (its list delegates to + `nine_supported_verify_schemes()`; `AcceptAnyCertVerifier`'s list + was already pinned). +- **Group 6** (fingerprint.rs:67): the task's second-disjunct concern + was already half-covered, but the existing + `bad_bit_string_lengths_extract_nothing` passed **vacuously** — its + crafted SPKIs declared outer SEQUENCE lengths longer than the + actual bytes (0x2b/0x2d/0x2d vs actual 0x29/0x2b/0x2a), so parsing + failed at the outer TLV and never reached the bit-string checks. + Fixed the three declared lengths and added a tag-matrix test + (non-OID tag inside AlgorithmIdentifier; well-formed SPKI with no + BIT STRING after the alg-id). +- **Coverage leftovers, documented as out of scope**: the two + `panic!` arms of the error-path tests were converted to + `.err().expect(..)` (closes client.rs:949 and server.rs:1029 — the + config types are deliberately not `Debug`, so `.err()` avoids + `expect_err`'s `T: Debug` bound). fingerprint.rs 125/143 (the + `} else { None }` tail arms of `expect_sequence` / + `expect_bit_string`) remain "uncovered" in llvm-cov output — proven + (temporary eprintln + region dump) to be an llvm-cov region + attribution artifact: the else arms execute (region counts 19 and + 1) but llvm attributes the region to the `read_tlv()?` line, not + the `None` literal. The parser is behavior-preservation code; + restructuring it for a tooling artifact is not warranted. +- Remaining uncovered lines crate-wide are the ACME event loop + (owned by `acme-event-loop-test`) and the ACME-only task arms. + ## Summary -> Agent fills this on completion. \ No newline at end of file +> Agent fills this on completion. + +Closed all six re-baselined U-2 coverage groups. Tests added: + +- `client.rs`: `fingerprint_pin_verifier_verify_tls12_signature_routes_x509_through_standard_path` + (ECDSA-P256 → standard TLS 1.2 path ok + forged → err); + `empty_platform_store_deterministically_falls_back_to_webpki_roots`, + `native_cert_errors_are_logged_and_valid_certs_still_load`, + `native_certs_fill_the_store_without_the_fallback` (deterministic + fallback pins over the new `fill_root_store_from_native` seam) + + `dss_with_scheme` / `native_result` test helpers. +- `server.rs`: three `VerifyPresentedCertVerifier::verify_tls12_signature` + tests (Ed25519 raw-key route ok/forged-err, X.509 route + ok/wrong-key-err, mismatched-message err), the escape hatch's + `verify_tls12_signature` no-pop pin, and the + `VerifyPresentedCertVerifier` nine-scheme list pin + `dss_with_scheme` + helper. +- `pem.rs`: `load_private_key_parse_error_yields_cert_load_error` + (garbage-but-keyed input → `TlsError::CertLoad`). +- `fingerprint.rs`: corrected the vacuous + `bad_bit_string_lengths_extract_nothing` (wrong outer lengths) so + the line-67 disjunct genuinely executes, and added + `wrong_tag_before_oid_and_missing_bit_string_extract_nothing`. + +Only non-test change: `load_platform_root_cert_store` now delegates +to a private `fill_root_store_from_native(&CertificateResult)` seam — +no public-API growth. + +Verification: `cargo llvm-cov --all-features` line coverage **98.26%** +(from 95.90%), all six groups covered; `cargo test` (80 lib + 13 +integration), `cargo test --all-features` (89 lib + 29 integration), +`cargo clippy --all-targets -- -D warnings` (default and +`--all-features`), `cargo fmt --check` — all green. \ No newline at end of file