Skip to content

Replace the reflective revocation checker with the JDK's PKIX checker - #1948

Draft
kevinherron wants to merge 4 commits into
integration/1.2from
fix/pkix-revocation-checker
Draft

Replace the reflective revocation checker with the JDK's PKIX checker#1948
kevinherron wants to merge 4 commits into
integration/1.2from
fix/pkix-revocation-checker

Conversation

@kevinherron

Copy link
Copy Markdown
Contributor

Revocation checking in CertificateValidationUtil relied on OpcUaCertificateRevocationChecker, a reflective subclass of the JDK's internal RevocationChecker. It only worked when the JVM was started with --add-opens java.base/sun.security.provider.certpath=ALL-UNNAMED. Without that flag, validation silently fell back to the default checker, REVOCATION_LISTS had no effect, and until #1944 a certificate that a supplied CRL listed as revoked could be accepted.

This replaces the reflective checker with the public PKIXRevocationChecker API. The checker is configured for CRLs only (PREFER_CRLS, NO_FALLBACK), fed the trust list's CRLs after the existing same-name issuer selection, and isolated from the JVM-wide ocsp.* security properties the JDK otherwise reads even in CRL-only mode. A revocation established by an applicable CRL is always enforced. REVOCATION_LISTS now decides only whether an unknown status is rejected or tolerated; tolerated results are read back from the checker's soft-fail list and logged at WARN with the certificate, issuer, and reason. ValidationCheck.REVOCATION is deprecated for removal since it no longer has an effect.

One consequence worth knowing about: when the supplied CRLs do not cover an issuer, the JDK checker fetches from a CRL distribution point named in the certificate before treating the status as unknown, and the public API offers no way to turn that off. It runs on the validating thread (the channel's event loop) and waits up to the com.sun.security.crl.timeout and crl.readtimeout defaults per distribution point when the host is unreachable. The package docs and the 1.2.0 migration guide describe this and the mitigation (supply the issuer's CRL through the trust list). If you would rather keep the 1.1.x no-network default, the only option is to skip the checker under the tolerant policy when no applicable CRL exists, which I did not do here.

A second commit fixes a related reporting gap: the PKIX path builder rejects an expired CA-issued certificate before validation runs, and that surfaced as Bad_SecurityChecksFailed. buildCertPath now checks the presented chain's validity on builder failure so the peer receives Bad_CertificateTimeInvalid (or the issuer variant). This is an observable status code change.

The never-implemented OpcUaCertPathValidator, the unused OpcUaCertificateValidityChecker, the surefire --add-opens argument, and the sun.security.* OSGi import are removed. The migration guide gains a section on the fixed meaning of the revocation flags and the one deployment shape that changes behavior on upgrade.

Verification: spotless:check and clean compile, plus the stack-core test classes CertificateRevocationTest (new, including a child-JVM test for the ocsp.* isolation and a local distribution point server), CertificateValidationUtilTest, DefaultCertificateValidatorTest, and BrainpoolCertificateValidationTest. Not yet exercised against the CTT; that is planned separately before this leaves draft.

OpcUaCertificateRevocationChecker subclassed the JDK's internal
RevocationChecker through reflection, so it only worked when the JVM was
started with --add-opens java.base/sun.security.provider.certpath. Without
that flag validation silently fell back to the default checker, which
ignored REVOCATION_LISTS and, until recently, could accept a certificate
that a supplied CRL listed as revoked.

Configure the public PKIXRevocationChecker instead. It evaluates the
trust list's CRLs only (PREFER_CRLS, NO_FALLBACK) and is isolated from
the JVM-wide ocsp.* security properties another component may set, which
the JDK otherwise reads even in CRL-only mode. A revocation established
by an applicable CRL is always enforced. REVOCATION_LISTS decides whether
an unknown status is rejected or tolerated; tolerated results are read
back from the checker's soft-fail list and logged with the certificate,
issuer, and reason. REVOCATION is deprecated for removal because it no
longer has an effect.

The JDK checker fetches from a certificate's CRL distribution point when
the supplied CRLs do not cover its issuer, and the public API offers no
way to turn that off. Document the timeouts and the event-loop cost
rather than hide it.

Remove the never-implemented OpcUaCertPathValidator and the unused
OpcUaCertificateValidityChecker, the --add-opens surefire argument, and
the sun.security OSGi import that only existed for the reflective access.
The PKIX path builder rejects a certificate outside its validity period
before validation runs, and its exception does not say which rule
failed, so an expired CA-issued certificate surfaced as the generic
Bad_SecurityChecksFailed. Check the presented chain's validity when path
building fails so the peer receives the Part 4 6.1.3 status code and can
tell a renewal is needed.
Add a migration section covering the fixed meaning of the revocation
flags, the deprecation of ValidationCheck.REVOCATION, the removed
--add-opens requirement, the distribution-point retrieval the JDK
checker performs when a CRL is missing, and the one deployment shape
that changes behavior on upgrade.
Allow unknown revocation status in fixtures without CRLs so validation
reaches the compatibility check. Assert Bad_CertificateUseNotAllowed
instead of accepting any UaException.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant