Skip to content

fix: [SIW-4981] Align iOS and Android X.509 revocation to a per-certificate CRL policy - #55

Merged
ale-mazz merged 1 commit into
mainfrom
SIW-4981
Sep 8, 2026
Merged

ale-mazz merged 1 commit into
mainfrom
SIW-4981

Conversation

@ale-mazz

@ale-mazz ale-mazz commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Short description

Chain verification was broken and inconsistent across platforms. A chain whose leaf publishes no CRL Distribution Point — the case in the current IT-Wallet PKI — failed on Android in every configuration, and passed on iOS only because iOS never checked revocation.

Both platforms now apply the same per-certificate policy: a certificate is checked against its CRL only when it publishes a CDP. No CDP means not checkable, not a failure. A revoked certificate always fails, including with requireCrl: false. An undetermined status fails only with requireCrl: true.

Root causes:

  • Android delegated revocation to PKIX, which demands a CRL for every certificate on the path, so the leaf produced UNDETERMINED_REVOCATION_STATUS. requireCrl: false did not opt out.
  • iOS only read the leaf's CDP, so the sub-CA — the one certificate actually covered by a published CRL — was never checked, and a revoked sub-CA was reported VALID.

This does not remove the need for the PKI to publish a CDP on the leaf plus the matching CRL signed by the sub-CA. Leaf revocation is unverifiable by any conforming client today (openssl verify -crl_checkunable to get certificate CRL at depth 0). Once published, the new policy picks them up with no code change.

List of changes proposed in this pull request

  • Revocation is evaluated per certificate across the chain, not on the leaf alone. A CRL is usable only if issued by the certificate's own issuer, correctly signed by it, and current. The issuer is resolved positionally, not by DN — required here, where leaf and sub-CA share a Subject DN.
  • Android: PKIX now does path validation only (isRevocationEnabled = false); the policy lives in checkRevocationPerCertificate / checkCertificateAgainstCrls. Path validation runs before any CRL fetch. Removed fetchCrlsForChain, findIssuerCertificate, CrlFetchException, hasCrlDistributionPoint.
  • iOS: the CRL check now runs on any otherwise-trusted chain, not only when requireCrl is set — this closes the revoked-sub-CA hole. All CDP URIs are extracted (not just the first), filtered to HTTP/HTTPS, tried in order. fetchCRL checks the HTTP status, so a 404 body no longer reaches the DER parser. -2 now maps to CRL_PARSE_FAILED (previously unreachable), -6 to CRL_FETCH_FAILED. Removed inference of revocation from generic SecTrust failures, which produced false CERTIFICATE_REVOKED results.
  • iOS crash: an empty certChainBase64 reached a first! force-unwrap and trapped. Now returns INVALID_CHAIN_PATH, with the module rejecting the promise as Android does.
  • iOS/Android divergence: iOS rejected chains containing the trust anchor with VALIDATION_ERROR; it now truncates like Android, comparing the anchor by DER instead of object identity. The "trust anchor alone" special case was dropped, as the general path covers it identically.

How to test

Case iOS before iOS after Android after
[leaf, sub], requireCrl=false VALID (unchecked) VALID VALID
[leaf, sub], requireCrl=true CRL_REQUIRED_BUT_MISSING_CDP VALID VALID
[leaf, sub, root] VALIDATION_ERROR VALID VALID
[root] alone VALID VALID VALID
[leaf] (no connection) INVALID_TRUST_ANCHOR INVALID_TRUST_ANCHOR INVALID_TRUST_ANCHOR
empty chain crash, SIGTRAP INVALID_CHAIN_PATH INVALID_CHAIN_PATH

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Jira Pull Request Link

This Pull Request refers to Jira issues:

@ale-mazz ale-mazz self-assigned this Sep 7, 2026
@ale-mazz ale-mazz added bug Something isn't working enhancement New feature or request labels Sep 7, 2026
@ale-mazz
ale-mazz marked this pull request as ready for review September 7, 2026 13:17
@ale-mazz
ale-mazz requested a review from a team as a code owner September 7, 2026 13:17

@gispada gispada left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested on Android.

@ale-mazz
ale-mazz merged commit 500ff37 into main Sep 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants