Validator verdict review

An independent read of the path validator's verdict logic (src/OneCrypto/Validation/*, behaviour in docs/validation.md) against RFC 5280 §6, RFC 6960 and the Baseline Requirements, written by someone who did not write it. Method: the code was read rule by rule; adversarial chains were built with the library's own builders; every finding has a test that asserts the behaviour the RFC asks for, so it fails today and carries Skip = "review finding Rn". Tests: tests/OneCrypto.Tests/Validation/ReviewFindingTests.cs (findings), ReviewCoverageTests.cs (what was found correct, running), ReviewSupport.cs (chain helpers). Nothing under src/ was changed. Un-skipping a test is how a fix is accepted.

Status. All eight findings and O1 are fixed, and the finding tests run (no Skip). R1: candidates that cannot issue are tried last and a search stopped by a limit with no valid path is Indeterminate; R2: the CRL signer is validated to the path's own anchor; R3: a listed entry counts whatever its reason; R4: a critical unknown entry extension makes the CRL unusable (a critical holdInstructionCode too, since it is not processed); R5: SmtpUTF8Mailbox is compared as rfc822Name; R6: one trailing dot and percent-encoding are normalised before comparison; R7: the required purpose is checked on an anchor target; R8: *. matches an A-label host. O1: UnknownRevocationIsFatal now defaults to true. O7: the citations are corrected in validation.md, and the 33 excluded-subtree cases are explained there. The text below is the review as written.

Findings: 8. By severity class: accept-invalid 6 (R2–R7), reject-valid 2 (R1, R8). R1 is also a denial-of-service shape. No wrong-reason or report-inconsistency findings; see "Reports".

Findings

R1: an untrusted pool can exhaust the path budget and turn a valid chain into Invalid. Engine.Candidates orders issuers by key identifier, validity at the reference time, then later notAfter first. The pool is attacker-supplied in the usual setting (certificates embedded in a signature or message). Forty extra certificates for the genuine CA's name and key, issued by the trusted root with a later notAfter but no keyCertSign, each complete a path that fails; the search stops at MaxPathsValidated (32) and the genuine CA (which expires sooner) is never tried. Assemble then reports Invalid because a path completed, although LimitReached is set and a valid path exists. The same steering works through MaxCandidatesPerCertificate (64) with decoys that merely fail signature checks. Observed: verdict Invalid, deciding rule KeyUsage, LimitReached = true, 16 reported paths. Fix: when a limit stopped the search and no path won, the verdict is Indeterminate, never Invalid; and rank or drop candidates that fail the cheap issuer rules (CA basicConstraints, keyCertSign, validity, same subject-and-key already seen) before descending, so duplicates of one subject-and-key pair cost one candidate.

R2: the CRL signer is validated against any anchor, not the certificate's anchor. UsableCrl/ValidateSigner build the signer's path with all of TrustAnchors. RFC 5280 §6.3.3 (f) requires the trust anchor of the CRL issuer's path to be the one used for the target. Any second trusted root can certify a key for the first CA's name and sign an empty CRL; the validator reports Good for a certificate whose CA never vouched. In the test two anchors are trusted, RootB issues a CRL-signing certificate named like RootA, and a CRL signed by it makes a RootA leaf Good ("no applicable CRL lists the certificate…"). The risk is real wherever the anchor set is large (an OS store) and revocation data arrives from the other party. Note that with UnknownRevocationIsFatal = true the attack is blocked only by accident (the signer's own revocation check finds the forged CRL inapplicable); the default is non-fatal. Fix: construct the signer's sub-validation with the selected path's anchor only.

R3: a listed entry whose reason is outside the CRL's reason scope counts as good. StatusIn returns Good ("listed with a reason outside the CRL's scope") when the entry's reason is not in the CRL's onlySomeReasons/DP-reasons coverage, and another CRL then supplies the missing coverage. RFC 5280 §6.3.3 (i)–(j) set cert_status from the entry found, with no reason test; the reason masks only decide when the answer is complete. Test: one CRL scoped to superseded lists the leaf as keyCompromise, a second CRL covers the other reasons and lists nothing; the verdict is Valid. The code comment cites §6.3.3 (a); that step is about CRL selection. Fix: any entry found is Revoked; keep coverage only for deciding Good.

R4: critical CRL entry extensions are ignored. RFC 5280 §5.3: a CRL with a critical entry extension the application cannot process must not be used for any certificate. UsableCrl checks only CRL-level extensions. A CRL carrying an unknown critical extension on an unrelated entry still yields Good for the certificate. Fix: unusable when any entry has a critical extension other than the ones read (reasonCode, invalidityDate, certificateIssuer, holdInstructionCode); critical certificateIssuer only on an indirect CRL.

R5: SmtpUTF8Mailbox bypasses rfc822Name constraints. NameConstraintState.CheckName sends OtherName to the "form we cannot compare" branch, which fails only when a critical constraint of the same form exists. RFC 8398 §6 extends rfc822Name constraints to SmtpUTF8Mailbox subjectAltNames. A CA excluding excluded.test can still be used for an otherName mailbox at that domain. Only matters to consumers that read the mailbox identity (S/MIME). Fix: compare the otherName value of id-on-SmtpUTF8Mailbox (1.3.6.1.5.5.7.8.9) as an rfc822Name.

R6: fully qualified hosts escape name constraints. UriInside compares the URI host text exactly, so https://example.test./x is outside an excluded example.test (the trailing dot names the same host, RFC 3986 §3.2.2). DnsInside has the same blind spot for a dNSName with a trailing dot when StrictStructure is off (preferred-syntax check is the only guard). Percent-encoded reg-names were not tested. Fix: strip one trailing dot, and decode percent-encoding, in the URI host before comparison; strip a trailing dot in DnsInside.

R7: RequiredExtendedKeyUsage is skipped when the target is itself an anchor. The targetIsAnchor branch of ValidatePath runs structure, profile, critical-extension and key-usage checks but not the extKeyUsage check. A self-signed anchor certificate whose EKU is serverAuth only is Valid for codeSigning. Fix: run the same EKU check in that branch.

R8: a wildcard never matches an xn-- host. DnsMatches rejects any host starting with xn-- against *.example.test. RFC 9525 §6.3 forbids wildcards within an IDN label (xn--*, b*cher); a whole left-most label wildcard stands for exactly one label whatever it holds, and browsers match it. The behaviour is not in docs/validation.md. Fix: drop the xn-- clause, or document it as a deliberate deviation.

Observations (not findings; no test)

• O1, default is fail-open for revocation. With defaults (RevocationRequired = None, UnknownRevocationIsFatal = false) the verdict is Valid and RevocationSummary is NotChecked; with RevocationRequired set and no data it is Valid with Unknown. This is documented and tested (IsValid is the path verdict). It does not meet "offline never reads as valid" for a caller that only reads IsValid. Suggest: RevocationRequired != None implies unknown is Indeterminate unless the caller opts out, or rename the flag so the opt-out is explicit.
• O2, a stale CRL that lists the certificate is ignored. A CRL past nextUpdate that lists the certificate gives Unknown, not Revoked. Defensible under §6.3.3 (a) but the report should say the stale data named the certificate.
• O3, delegated OCSP responder revocation is not checked. RFC 6960 §4.2.2.2.1 lets local policy decide when the responder certificate has no nocheck, CRL or AIA pointer; if it has a pointer the client should follow it. The code never checks. Documented as leniency; report-only suggestion.
• O4, removeFromCRL lifts any base entry. RFC 5280 §5.3.1 defines it for certificateHold only. The delta is CA-signed, so the exposure is a CA error; a check is cheap.
• O5, work limits are per engine. ValidateSigner builds a fresh Engine (own step and signature budgets) per CRL signer, up to three levels deep. The amplification was not measured. The pool is also deduplicated quadratically and scanned linearly per search step (Candidates), and every pool certificate is parsed eagerly; a pool of tens of thousands was not tried.
• O6, the accepted-policy set in PolicyReport lists policies from every tree level. The RFC 5280 §6.1 output sets are defined on the final tree; the verdict is unaffected.
• O7, documentation. docs/validation.md, in the Go-differential paragraph, cites §6.1.4(h) for the self-issued exemptions (§6.1.4(h) only skips the counter decrements). The name-constraint exemption is in §6.1.3 (b)/(c) and §4.2.1.10; the path-length one is §6.1.4 (l). The 33 "excluded subtree violation" differential cases are stated as not inspected one by one; they remain unexplained until someone does.

Checked and correct

Each item was read against the RFC text; those marked (T) also have a running test in ReviewCoverageTests.

• §6.1.3 basic path checks: issuer-name chaining, signature over the received TBS octets, validity at the second-truncated reference time (both ends inclusive), unrecognised critical extensions on every certificate including the last (T), duplicate and malformed extensions, order of checks (a)–(f).
• Constraints: basicConstraints cA on every issuing certificate, pathLenConstraint arithmetic including self-issued exemption and the anchor's own constraint (§6.1.4 (k)–(m)), keyCertSign (n). Name constraints: intersection of permitted sets and union of excluded sets across CAs, self-issued exemption except for the last certificate, subject DN prefix match, subject emailAddress as rfc822Name, dNSName label boundaries (T), IP masks of both families (T), wildcard SAN versus excluded subtree (T), URI host rules for ordinary hosts, critical constraint on an incomparable form fails, comparison budget.
• Policy (§6.1.3 (d)–(f), §6.1.4 (a),(b),(h)–(j), §6.1.5): tree construction, anyPolicy under inhibitAnyPolicy and the self-issued rule, mappings (anyPolicy rejected, inhibited mapping deletes nodes and prunes), requireExplicitPolicy/inhibitPolicyMapping counters, the wrap-up intersection with the initial policy set including replacement of the anyPolicy leaf, the node bound. PKITS 4.8–4.12 agree in full.
• Extended key usage: required purpose allowed by every EKU in the chain, anyExtendedKeyUsage accepted, absent EKU unrestricted.
• CRLs: issuer match, IDP scope flags (onlyContainsUserCerts/CACerts/AttributeCerts, onlySomeReasons) (T), distribution point matching for full and relative names, indirect CRLs with certificateIssuer carry-over, reason coverage must be complete for Good, nextUpdate mandatory and freshness, cRLSign on the signer (T), a CRL signed by another key of the same CA accepted only through a valid, cRLSign-bearing certificate (T), a forged CRL from an untrusted same-name certificate rejected (T), delta CRL matching (issuer, IDP equality, BaseCRLNumber, number ordering, signature, freshness), serial numbers compared as integers.
• OCSP: CertID hash algorithm limited to SHA-1/SHA-2/SHA-3 (no MD5), name and key hash computed over the issuer's received name and key bits, serial compared as integers, responder authorisation by CA, trusted responder or delegated responder with id-kp-OCSPSigning issued by the CA and valid (T), non-signer certificate of the CA refused (T), expired responder refused (T), thisUpdate in the future and stale responses ignored, a CertID over another issuer key does not match (T), Revoked from any source wins.
• Past-time model: by reason — superseded, certificateHold and cessation after T are good at T; keyCompromise, cACompromise, unspecified and absent reason after T, and any reason at or before T, are revoked; a "good" answer needs data issued at or after T (T).
• Unknown handling: no data, stale data and inapplicable data are Unknown; a required Unknown is Indeterminate when fatal; the summary is the weakest status (T).
• Path building: an invalid path never produces Valid; a pool certificate is never trusted (only anchors confer trust); loops and mutual cross-certification removed by subject-and-key; a pool cannot make the builder accept something RFC 5280 §6.1 rejects, only cost work or crowd out the valid path (R1); the lattice and policy-tree explosions are bounded (existing tests).
• Reports: Build derives the verdict from the first Fail over path-level then per-certificate results, with Fail outranking Indeterminate; Assemble returns Valid only with a winner whose results contain no Fail or Indeterminate. AssertConsistent in the coverage tests checks this on every scenario it runs: no Valid report has a failing or undecided rule, and every non-valid report has a deciding rule of the matching outcome.
• Server profile: host matching is subjectAltName only; the commonName is never a host name (T); IP addresses by octets; wildcard only as a whole left-most label with two labels after it; profile rules are additional failures, never a second validator.
• Divergence classes in docs/validation.md: the cross-signed-root and public-suffix deviations follow RFC 5937 §2 and the missing Public Suffix List respectively. The OpenSSL common-name subtree class is correct by RFC 5280 §4.2.1.10 (constrained names are the subject DN and subjectAltName entries) and RFC 9525 §2 for server identity. The alternative-path classes (issuer not found, purpose, invalid CA, expired alternate) are correct by §6.1 (any valid path suffices). The notAfter-exact class relies on whole-second truncation, which is a documented, bounded leniency of under one second.

Not examined

Signature-algorithm policy wiring (being replaced), CMS signer-chain use, the ValidateAsync fetch loop beyond its option copying (With carries every option), attribute certificates, Brainpool/PQ key handling inside the validator, and the performance of the pool scan (O5). No OpenSSL or Go experiment was needed: every finding is determined by RFC text, which is quoted above by clause.