Skip to content

REF-16: Make CRLCheckerTest reliable (force OCSP POST, use a truly revoked cert, tag integration tests) - #90

Open
thomasnymand wants to merge 2 commits into
masterfrom
feature/REF-16-ocsp-force-post-in-tests
Open

REF-16: Make CRLCheckerTest reliable (force OCSP POST, use a truly revoked cert, tag integration tests)#90
thomasnymand wants to merge 2 commits into
masterfrom
feature/REF-16-ocsp-force-post-in-tests

Conversation

@thomasnymand

Copy link
Copy Markdown
Collaborator

Fixes the flaky/failing CRLCheckerTest uncovered while reviewing the test suite.

Problems

  1. testOcspCheckOnValidCertificate failed (expected: <1> but was: <0>). The JDK OCSP client uses the RFC 5019 GET form (request base64-encoded in the URL path) for small requests. The NemLog-in test OCSP responder at ca1.cti-gov.dk returns HTTP 404 for that GET form (it only serves POST), so validation failed with UNDETERMINED_REVOCATION_STATUS and the valid certificate was dropped.
  2. The revoked tests passed for the wrong reason. With OCSP unreachable, testOcspCheckOnRevokedCertificate returned an empty set and happened to satisfy its assertEquals(0, …) — i.e. it was green due to the network failure, not actual revocation. The revoked certificate's real status was never exercised.

Changes

  • Force OCSP POST in tests: set com.sun.security.ocsp.useget=false in BaseServiceTest.beforeAll, so the JDK POSTs the OCSP request (request in body). Verified end-to-end that the responder then returns good/revoked and the cert path validates.
  • Use a genuinely revoked certificate for TestConstants.REVOKED_CERTIFICATE. Verified against the live test CA: OCSP reports revoked and the serial is present in the issuing CRL. Validity 2026-07-06 .. 2029-07-05 (no near-term expiry).
  • Tag the revoked tests @Tag("integration") (both OCSP and CRL variants) since they depend on the live test-CA revocation infrastructure. No group filtering is configured, so they still run by default; the tag allows excluding them offline via -DexcludedGroups=integration.

Verification

mvn -pl oiosaml clean test: all four CRLCheckerTest tests pass (present, not skipped). The only remaining failure is the unrelated OIOBPPUtilTest (JDK 26 JAXB incompatibility), which also fails on master.

Notes

  • com.sun.security.ocsp.useget is an OpenJDK/Oracle-internal property (sun.security.provider.certpath.OCSP); reliable on HotSpot-derived JVMs and JVM-global in scope.
  • These revocation tests remain live-network dependent by nature; the integration tag now makes that explicit. A fully hermetic (MockServer-based) rewrite remains a possible follow-up.

🤖 Generated with Claude Code

thomasnymand and others added 2 commits July 6, 2026 15:46
…ach the responder

The JDK OCSP client uses the RFC 5019 GET form (request base64-encoded in the
URL path) for small requests. The NemLog-in test OCSP responder at
ca1.cti-gov.dk returns HTTP 404 for that GET form (it only serves POST), so
CRLCheckerTest's OCSP validation failed with UNDETERMINED_REVOCATION_STATUS
and dropped the otherwise-valid certificate.

Set com.sun.security.ocsp.useget=false in BaseServiceTest.beforeAll so the JDK
POSTs the OCSP request (request in the body) instead. Verified end-to-end that
this makes the responder return "good" and the certificate validate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n tests

Replace TestConstants.REVOKED_CERTIFICATE with a certificate that is actually
revoked at the NemLog-in test CA (verified: OCSP reports "revoked" and the
serial is present in the issuing CRL; valid 2026-07-06 .. 2029-07-05). The
revoked OCSP/CRL tests now pass because the certificate is genuinely revoked,
rather than incidentally due to an unreachable responder.

Tag both testOcspCheckOnRevokedCertificate and testCrlCheckOnRevokedCertificate
with @tag("integration") since they depend on the live test CA revocation
infrastructure. No group filtering is configured, so they still run by default;
the tag allows excluding them from offline runs via -DexcludedGroups=integration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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