From 0bddac4f091c30106d3ac216ba224c18b1f8e7c1 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Mon, 6 Jul 2026 15:46:20 +0200 Subject: [PATCH 1/3] REF-16: Force OCSP requests to POST in tests so CRLCheckerTest can reach 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. --- .../test/java/dk/gov/oio/saml/service/BaseServiceTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java index b9d5132..cb87943 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java @@ -16,6 +16,12 @@ public class BaseServiceTest { @BeforeAll public static void beforeAll(MockServerClient idp) throws Exception { + // Force the JDK OCSP client to POST the request instead of using the RFC 5019 GET form + // (request base64-encoded in the URL path). The NemLog-in test OCSP responder returns + // HTTP 404 for the GET form, which otherwise makes CRLCheckerTest's OCSP checks fail with + // UNDETERMINED_REVOCATION_STATUS. Must be set before the first OCSP check. + System.setProperty("com.sun.security.ocsp.useget", "false"); + ClassLoader classLoader = AssertionServiceTest.class.getClassLoader(); String keystoreLocation = classLoader.getResource(TestConstants.SP_KEYSTORE_LOCATION).getFile(); From f77ad983895a4ec63f543db96cec24d5846c8a6e Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Mon, 6 Jul 2026 15:52:26 +0200 Subject: [PATCH 2/3] REF-16: Use a genuinely revoked certificate for the revoked revocation 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. --- .../java/dk/gov/oio/saml/service/CRLCheckerTest.java | 9 +++++++++ .../test/java/dk/gov/oio/saml/util/TestConstants.java | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerTest.java index d3ca909..ab3848c 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerTest.java @@ -11,6 +11,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import dk.gov.oio.saml.util.TestConstants; @@ -51,6 +52,10 @@ public class CRLCheckerTest extends BaseServiceTest { Assertions.assertEquals(1, validCertificates.size()); } + // Integration test: depends on the live NemLog-in test CA revocation infrastructure + // (the revoked certificate's real OCSP/CRL status). Tagged so it can be excluded from + // offline runs if needed, but it runs by default. + @Tag("integration") @DisplayName("Test revocation check on revoked certificate using OCSP") @Test public void testOcspCheckOnRevokedCertificate() throws Exception { OIOSAML3Service.getConfig().setCRLCheckEnabled(false); @@ -69,6 +74,10 @@ public class CRLCheckerTest extends BaseServiceTest { Assertions.assertEquals(0, validCertificates.size()); } + // Integration test: depends on the live NemLog-in test CA revocation infrastructure + // (the revoked certificate's real OCSP/CRL status). Tagged so it can be excluded from + // offline runs if needed, but it runs by default. + @Tag("integration") @DisplayName("Test revocation check on revoked certificate using CRL") @Test public void testCrlCheckOnRevokedCertificate() throws Exception { OIOSAML3Service.getConfig().setCRLCheckEnabled(true); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java index 87c2226..5424a13 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestConstants.java @@ -150,5 +150,5 @@ public class TestConstants { public static final String VALID_CERTIFICATE = "MIIGkzCCBMegAwIBAgIUdxCsIBOOtB5tqNtriG1TMHD9dqYwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yMzA4MTgxMDI2NThaFw0yNjA4MTcxMDI2NTdaMIGpMSUwIwYDVQQDDBxqYXZhLnJlZmVyZW5jZWltcGxlbWVudGVyaW5nMTcwNQYDVQQFEy5VSTpESy1POkc6MmRjZjc5MTktYjI4Mi00NGQxLWI5ODAtM2I3MzcwMGE3ZGQ0MSEwHwYDVQQKDBhEaWdpdGFsaXNlcmluZ3NzdHlyZWxzZW4xFzAVBgNVBGEMDk5UUkRLLTM0MDUxMTc4MQswCQYDVQQGEwJESzCCAaIwDQYJKoZIhvcNAQEBBQADggGPADCCAYoCggGBAKNAf9uAhuz3bEjPPFrBa39HCF6S64pSzGRr5yYm3lCBElYJvHzDr9lMKgbv8rKglIVgjWh+PzUjiwIlGjrqAbYa2Hg08Vw2H60GQSFP8rGsshgR+E5Ca2nb9kUcQXAQJl9ScG9squCPRNkdp8vSblRwv/3N0ksjxdZk1wdZ86bOqTsFEjpzhFdBXXSMl4tbhE7WOruKc0QqjUkzXJyp4qwyB2XA75+jsvtRHN/luOzCkUxLhEkFrbg+B6IWqjUuO132xC8d5+T8Y39K6rs4BYOIgQRJOg0OlA5844CC/WBLtAYgMiu1ucZ4mbVWOmm2F86WVRBdmwlN0CFORXihHiYNZfHpA0rPOSncDDMrrGZ7vuvvXxMfIiHlAniSw4eHaEqtaXqwDyNZbfcXgYkszQd7ZV7YMfAjkDo82Qn+Qz+Oc9qq0Syhd9pdUJ/Q26CjFDiaNSg+hDUUJTxowQAktX3AuwBcDeuMoc2yOGmg2xOf/3bIwJSNm8/b+y0wKfY2FwIDAQABo4IBhjCCAYIwDAYDVR0TAQH/BAIwADAfBgNVHSMEGDAWgBR/KJ/ZcZlC4nXn1zV2Lk0IJW12XjB7BggrBgEFBQcBAQRvMG0wQwYIKwYBBQUHMAKGN2h0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY2VzL2lzc3VpbmcvMS9jYWNlcnQvaXNzdWluZy5jZXIwJgYIKwYBBQUHMAGGGmh0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY3NwMCEGA1UdIAQaMBgwCAYGBACPegEBMAwGCiqBUIEpAQEBAwcwOwYIKwYBBQUHAQMELzAtMCsGCCsGAQUFBwsCMB8GBwQAi+xJAQIwFIYSaHR0cHM6Ly91aWQuZ292LmRrMEUGA1UdHwQ+MDwwOqA4oDaGNGh0dHA6Ly9jYTEuY3RpLWdvdi5kay9vY2VzL2lzc3VpbmcvMS9jcmwvaXNzdWluZy5jcmwwHQYDVR0OBBYEFNKFKxc70Coluez0ieqiVuK+a01oMA4GA1UdDwEB/wQEAwIFoDBBBgkqhkiG9w0BAQowNKAPMA0GCWCGSAFlAwQCAQUAoRwwGgYJKoZIhvcNAQEIMA0GCWCGSAFlAwQCAQUAogMCASADggGBAIkUbY8meqO9xQQ2gyMS4rfmqW3bV52YGs07DqG0zuVew7W7RMAJWqDLUj5ltMWK7wULcCBS1tjtxOrvMBCoAE42oQfF/EzLRYKr7VgsMyOgUiTk2t6LvyF5A1OGHOUP3lxQKX3viDURXUeoI4QZ3mxbHUg4sQXdXg2hOEhQOarOhWLdV3MzUkA9ZkwjmycXkbLBVdTbr/fODUU0jeDDlaixKXsGI66qg8Ou86nDkyW7wCxQ9QVwJ5YGogy9ZSc6sLt8XSv3+wFlXD/81EzWfqe5BdWX8cukLtSzdzg3SzJifB4IJ6GIQ58+NVLPEMezwZCLODzVkvdJfyWRxJrDijSVCza515qNW52yfYPYkTb+vdvKcFmwO1gCeK0vT21udVkp1grhNzwb8Cj/tq3OZ+IamZXkjL1go9GzSQQ31IbXHEI/oaPLEeX6j9E8X69wVtSti8SWPw0WgoeOglJM5A6fmlJIGhCBPk2klhH3IIU3+tjuz7iyFHZg7gbPhNHdow=="; - public static final String REVOKED_CERTIFICATE = "MIIGizCCBL+gAwIBAgIUNtX5gOHMima7NNCirR0xYtZD5dQwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yMzA3MDQwODQyMjZaFw0yNjA3MDMwODQyMjVaMIGDMRAwDgYDVQQDDAdSZXZva2VkMTcwNQYDVQQFEy5VSTpESy1POkc6Mjc1ODFhMzktYjYxNC00YzYxLWJlY2UtOWIyYjA5Zjg1NzJhMRAwDgYDVQQKDAdpZDIgQS9TMRcwFQYDVQRhDA5OVFJESy00NDE2MDMxNTELMAkGA1UEBhMCREswggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQDGFxjnzy3gKPLBt2Lj4ALK2uHAMFB8CZc4DaJDdn6YWNOdL4bvi2Ip0Nf87A9IZT3KzLsyju5pqQOaJ1MlwTB11Hlwne4DHUOo8EZhMAS2/oWtTf+scmbDXR0Edv05ecteReabO7tRyQcbxacirX20rT8jTVLtvWfzCHobvbCK9wWTXiaYYZXZykrHbCrRikuHz7DtyefznKRC36wC3sBzptA4ncdDnhiwOoDizZ46toMYahDLj2P1VDIh+2XyWKPV5rxTlZkY3b+H235tdRtjkX+kTfUDFBVt8PrU7/Pq1D1NHIQsvoHmSsUHwK0QqDGeCbj6qQMs5p5PhBksKdh7Xkfsoh6oTwvXRdbLmjcuc7rIoUB2QidfgltZ2bqSpGX+xvg4uQCKCgqn6TlXUCpDrLKKplE0MnYIyNuWrS/Grj11GJnmfTAsAVkXgM8dSXsBhf0JmMBMXLRTUuz1axxeNy3e29fXDEuoXPoTZ8a9EFTzgA/1l9n67qolWlZV5C8CAwEAAaOCAaQwggGgMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUfyif2XGZQuJ159c1di5NCCVtdl4wewYIKwYBBQUHAQEEbzBtMEMGCCsGAQUFBzAChjdodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY2FjZXJ0L2lzc3VpbmcuY2VyMCYGCCsGAQUFBzABhhpodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2NzcDAcBgNVHREEFTATgRF0aG9tYXNAbnltYW5kLm5ldDAhBgNVHSAEGjAYMAgGBgQAj3oBATAMBgoqgVCBKQEBAQMHMDsGCCsGAQUFBwEDBC8wLTArBggrBgEFBQcLAjAfBgcEAIvsSQECMBSGEmh0dHBzOi8vdWlkLmdvdi5kazBFBgNVHR8EPjA8MDqgOKA2hjRodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY3JsL2lzc3VpbmcuY3JsMB0GA1UdDgQWBBS6ZyVKq8su7B6OPmH0lkvc6nYpqzAOBgNVHQ8BAf8EBAMCBeAwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgA4IBgQCNt3wTWs0HKYutrfPxw06SmQdeS/bZDBtiallEYaE9HclkWPFIGii8V0gHYyvV6i90f+Sz9FWjAohhQCaMWVkImKG+8EU4mgAjkSRJa4/GpsB9KKQZs26dNayKTdbL/x9MajgaPBPp+kYuVPOjX6oXgLG8huaJ8GqsjQocGd9JPX4k/ir6S+E4Q4Q3PG89HCX9R+28XjT4R/SUwxIvjnf0eKapS/LZdOgk/e5pxTUDWOZhlmJLlXsDAcwXlQ+0vhX4Euk08RKqAi+jK2n2qKIiAg2UFfgdtg7Fu4hWkbLMH/KFf2U21vdmVZAgiOAPmipkuFOwiJV8Q6k5kIVf+OoxBlkUCK6Z9j3Cxuaa1bOif13Iyv003Gns2GCAULA4i+yloYkWHE1jPi1KaLhpcMbf7eRE9MEmS/JVBNVXpDi1M3Y6s6gltO4xbctJPPpnvopyHEhRQx0zQTxk7FcY5izmI+ld62eCLKtPEvV0oVGXVoUGkUqnz5IZmXpBTXVt+YA="; + public static final String REVOKED_CERTIFICATE = "MIIGbjCCBKKgAwIBAgIUSC6sLWUUKtMVPVEu8snwA47QlqgwQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yNjA3MDYxMzMyMzZaFw0yOTA3MDUxMzMyMzVaMIGDMRAwDgYDVQQDDAdSZXZva2VkMTcwNQYDVQQFEy5VSTpESy1POkc6Mjc1ODFhMzktYjYxNC00YzYxLWJlY2UtOWIyYjA5Zjg1NzJhMRAwDgYDVQQKDAdpZDIgQS9TMRcwFQYDVQRhDA5OVFJESy00NDE2MDMxNTELMAkGA1UEBhMCREswggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQDWBfNLzMmPCnFq9kRVStjsaGyiN6jO2vNRS/w1VTHW1UWPpdZZ07CsEOKFDgOy6Nd+Ra4wN/cnkE49+Bf/QDJQjZE2gtho+ajELFmrgrtPjscn4Htp+q9rd4X698LmFwc9J00bQMBgZ0Zr2KdWsfp5tYa5JP/Ro3w81FZx/xB7zOm/U1rw7be9gN9TvgWVZHK7z529lAnhVW5KSsJdrckeJvZtxX9EoGeWmeVY3OJM/QjJbXAUkouaizTt54Gfc0mpPXGv+MBK+ufGlQtJJZkdbjifS5HY7EgYD/+WM6SFNzRYeM5TU7+QMBZSlcaSoCCOLKN5/fjlXbkPRsUSx4coj37/LFmAhszkAV8CCotXe/Q6DlNtxWAAEnxmCSrKhMHCdRnEZaSDf4o7zBWl4tKpLbS/fqWC2+gV1euH4pV858uJKsoRmM1eSw5DU0NSnMenreZo0Q8VL9wz1AGnoCycqST5s+deRIJJgtB18NMlGv3LBsIuiEDB35zK8CnyOVUCAwEAAaOCAYcwggGDMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUfyif2XGZQuJ159c1di5NCCVtdl4wewYIKwYBBQUHAQEEbzBtMEMGCCsGAQUFBzAChjdodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY2FjZXJ0L2lzc3VpbmcuY2VyMCYGCCsGAQUFBzABhhpodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2NzcDAiBgNVHSAEGzAZMAgGBgQAj3oBATANBgsqgVCBKQEBAQMHATA7BggrBgEFBQcBAwQvMC0wKwYIKwYBBQUHCwIwHwYHBACL7EkBAjAUhhJodHRwczovL3VpZC5nb3YuZGswRQYDVR0fBD4wPDA6oDigNoY0aHR0cDovL2NhMS5jdGktZ292LmRrL29jZXMvaXNzdWluZy8xL2NybC9pc3N1aW5nLmNybDAdBgNVHQ4EFgQU3TbRHMKCpscjpajDI1TJoOzaPhIwDgYDVR0PAQH/BAQDAgWgMEEGCSqGSIb3DQEBCjA0oA8wDQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiAwIBIAOCAYEAK8HpHPPvXC1IPUVCIfkO0VwnvCBOLrCsr01pziBPg8AV0aVw2gEpyNi0xuIEqS3ipzObHwJu8s99e+rsrFjxCPPAG+Dq8tI1Hl85roZ5sanmLQMZLJMfQzKctImGT/3Q3WSxGGWkLkGObTPZcejLLKNjAfHd5Ufqx7q4wkU7UG/+OpJl8uS3AN2zZMi/39Lgnqraor0AgAmxA5DSv7h06YglgkVrLT4gzd/upTrODaYGia9l8UPGaP+tD0Jc/RRWe59vfGIz36QSu5ceIrgv/t1LQ2H0gl6pzruQ1aZDMYh4pygZbXROGv3UVURNyuftMFWwGE6snJquvlP8mPj9/HW75WD8kl2ART8YT41/BkOFbE6aC+Qgkl46Bx1WpktGElM6NGb4UlBDAwGp2Xin2tVITOuRBPKoAv3I2Fxkh7EBiMmWLZ58l8gCpW2/qOXZAdlnLiDkzc+qvXprYm81T2+Y3eA0LSOpNWOLPe77sjEj0/f92aZSb6+pviML7sDb"; } From 5bbfa6af8dfc50ef17f9ed4c53c9523a2be8d7d7 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 13 Aug 2026 11:31:45 +0200 Subject: [PATCH 3/3] REF-16: Send OCSP requests using POST from the library, not just in tests The JDK OCSP client uses the RFC 5019 GET form (request base64-encoded into the URL path) for requests of 255 characters or less, from JDK 12 onwards. The NemLog-in OCSP responders answer HTTP 404 to that form, so every OCSP check fails with UNDETERMINED_REVOCATION_STATUS: the SP silently degrades to the CRL fallback, or drops the IdP certificates entirely when CRL checking is disabled as well. Java 8 and 11 always POST and are unaffected. Forcing POST in the test base class only hid this from the test suite, it did not fix deployments. Set com.sun.security.ocsp.useget=false from OIOSAML3Service.init instead, gated by the new configuration property oiosaml.servlet.revocation.ocsp.post.enabled (default true). The JDK reads the property once, when sun.security.provider.certpath.OCSP is initialized, so it has to be set before the first OCSP check in the JVM. A value set explicitly by the deployer is left alone. POST support is mandatory for responders per RFC 6960, so forcing it is safe, the only cost is that GET responses are no longer HTTP cacheable for other OCSP users in the same JVM. Also replace the JVM global "ocsp.enable" and "ocsp.responderURL" security properties with a per validation PKIXRevocationChecker. The old code published the responder of the certificate being checked as a JVM wide default, which leaked into every other PKIX validation in the container and raced with concurrent checks. NO_FALLBACK keeps CRL fallback where it belongs, in checkCertificate, driven by the OIOSAML configuration. With the library doing this, BaseServiceTest no longer needs to set the system property, so CRLCheckerTest now exercises the production path. Verified: the four CRLCheckerTest cases pass, and re-running with -DargLine=-Dcom.sun.security.ocsp.useget=true makes the valid certificate OCSP test fail again, confirming both the 404 on GET and that the deployer override is honoured. --- demo/src/main/resources/oiosaml.properties | 5 ++ .../dk/gov/oio/saml/config/Configuration.java | 9 +++ .../dk/gov/oio/saml/service/CRLChecker.java | 62 ++++++++++++++++--- .../gov/oio/saml/service/OIOSAML3Service.java | 3 + .../oio/saml/servlet/DispatcherServlet.java | 5 ++ .../java/dk/gov/oio/saml/util/Constants.java | 1 + .../gov/oio/saml/service/BaseServiceTest.java | 6 -- 7 files changed, 77 insertions(+), 14 deletions(-) diff --git a/demo/src/main/resources/oiosaml.properties b/demo/src/main/resources/oiosaml.properties index aef3f81..f99f404 100644 --- a/demo/src/main/resources/oiosaml.properties +++ b/demo/src/main/resources/oiosaml.properties @@ -21,6 +21,11 @@ oiosaml.servlet.idp.metadata.file=test-devtest4-idp-metadata.xml oiosaml.servlet.revocation.crl.check.enabled=false oiosaml.servlet.revocation.ocsp.check.enabled=false +# Send OCSP requests using POST instead of the RFC 5019 GET form (default true). The NemLog-in OCSP +# responders answer HTTP 404 to the GET form used by JDK 12+, so only disable this if the responders +# in use handle GET and the JVM wide setting of 'com.sun.security.ocsp.useget' is unwanted. +#oiosaml.servlet.revocation.ocsp.post.enabled=true + # NemLogin-in2 does not return OIOSAML 3 profile valid assertions oiosaml.servlet.profile.validation.enabled=true diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java b/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java index 928bd2d..8a2d05a 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/config/Configuration.java @@ -62,6 +62,7 @@ public class Configuration { // Revocation check settings private boolean crlCheckEnabled = true; private boolean ocspCheckEnabled = true; + private boolean ocspPostEnabled = true; // AppSwitch return URL settings private String appSwitchReturnURLForAndroid; @@ -327,6 +328,14 @@ public void setOcspCheckEnabled(boolean ocspCheckEnabled) { this.ocspCheckEnabled = ocspCheckEnabled; } + public boolean isOcspPostEnabled() { + return ocspPostEnabled; + } + + public void setOcspPostEnabled(boolean ocspPostEnabled) { + this.ocspPostEnabled = ocspPostEnabled; + } + public String getAuditLoggerClassName() { return this.auditLoggerClassName; } diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/CRLChecker.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/CRLChecker.java index bcdcd05..c42f518 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/CRLChecker.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/CRLChecker.java @@ -2,10 +2,11 @@ import java.io.IOException; import java.io.InputStream; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; -import java.security.Security; import java.security.cert.CRLException; import java.security.cert.CertPath; import java.security.cert.CertPathValidator; @@ -13,12 +14,14 @@ import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.PKIXParameters; +import java.security.cert.PKIXRevocationChecker; import java.security.cert.TrustAnchor; import java.security.cert.X509CRL; import java.security.cert.X509CRLEntry; import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collections; +import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -51,8 +54,42 @@ public class CRLChecker { private static final Logger log = LoggerFactory.getLogger(CRLChecker.class); private static final String AUTH_INFO_ACCESS = Extension.authorityInfoAccess.getId(); + + // JDK system property deciding whether the built-in OCSP client may use the RFC 5019 GET form + // (request base64-encoded into the URL path) for small requests, see sun.security.provider.certpath.OCSP + private static final String JDK_OCSP_USE_GET_PROPERTY = "com.sun.security.ocsp.useget"; + private static Map certificateMap = new HashMap(); - + + /** + * Make the JDK OCSP client POST its requests instead of using the RFC 5019 GET form. + * + * JDK 12 and later default to GET for requests that fit in 255 characters, but the NemLog-in OCSP + * responders answer HTTP 404 to that form. The resulting UNDETERMINED_REVOCATION_STATUS makes every + * OCSP check fail, which silently degrades the SP to the CRL fallback, or drops the IdP certificates + * entirely when CRL checking is disabled as well. POST is mandatory for responders (RFC 6960), so + * forcing it is safe. Java 8 and 11 always POST and are unaffected. + * + * The JDK reads the property once, when sun.security.provider.certpath.OCSP is initialized, so this + * must run before the first OCSP check in the JVM, hence the call from {@link OIOSAML3Service#init}. + * An explicit setting made by the deployer (-Dcom.sun.security.ocsp.useget=...) always wins. + */ + static void configureOcspTransport(Configuration configuration) { + if (!configuration.isOcspPostEnabled()) { + log.info("Leaving '{}' untouched, OCSP POST is disabled in the OIOSAML configuration", JDK_OCSP_USE_GET_PROPERTY); + return; + } + + String existingValue = System.getProperty(JDK_OCSP_USE_GET_PROPERTY); + if (existingValue != null) { + log.info("Leaving '{}' at the value set by the deployer: {}", JDK_OCSP_USE_GET_PROPERTY, existingValue); + return; + } + + System.setProperty(JDK_OCSP_USE_GET_PROPERTY, "false"); + log.info("Setting '{}' to false, so OCSP requests are sent using POST", JDK_OCSP_USE_GET_PROPERTY); + } + public static Set checkCertificates(List x509Certificates, DateTime lastCRLCheck) throws ExternalException, InternalException, InitializationException { Set result = new HashSet<>(); if (x509Certificates == null || x509Certificates.size() == 0) { @@ -111,7 +148,7 @@ else if (config.isCRLCheckEnabled()) { return validated; } - private static boolean doOCSPCheck(X509Certificate certificate) throws CertificateException, CertPathValidatorException, InvalidAlgorithmParameterException, NoSuchAlgorithmException { + private static boolean doOCSPCheck(X509Certificate certificate) throws CertificateException, CertPathValidatorException, InvalidAlgorithmParameterException, NoSuchAlgorithmException, URISyntaxException { log.debug("Starting OCSP validation of certificate {}", certificate.getSubjectDN()); String ocspServer = getOCSPUrl(certificate); @@ -131,17 +168,26 @@ private static boolean doOCSPCheck(X509Certificate certificate) throws Certifica CertificateFactory cf = CertificateFactory.getInstance("X.509"); CertPath cp = cf.generateCertPath(certList); - Security.setProperty("ocsp.enable", "true"); - Security.setProperty("ocsp.responderURL", ocspServer); - boolean revoked; try { TrustAnchor anchor = new TrustAnchor(issuer, null); PKIXParameters params = new PKIXParameters(Collections.singleton(anchor)); - params.setRevocationEnabled(true); - // Validate and obtain results CertPathValidator cpv = CertPathValidator.getInstance("PKIX"); + + // Configure revocation checking on this validation only. The alternative, the JVM global + // "ocsp.enable"/"ocsp.responderURL" security properties, leaks the responder of the certificate + // being checked into every other PKIX validation in the JVM, and races with concurrent checks. + PKIXRevocationChecker revocationChecker = (PKIXRevocationChecker) cpv.getRevocationChecker(); + revocationChecker.setOcspResponder(new URI(ocspServer)); + + // Fallback to CRL is handled by checkCertificate, according to the OIOSAML configuration + revocationChecker.setOptions(EnumSet.of(PKIXRevocationChecker.Option.NO_FALLBACK)); + + params.setRevocationEnabled(false); + params.addCertPathChecker(revocationChecker); + + // Validate and obtain results cpv.validate(cp, params); log.debug("Certificate successfully validated during OCSP check."); diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/OIOSAML3Service.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/OIOSAML3Service.java index 4698deb..3151789 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/OIOSAML3Service.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/service/OIOSAML3Service.java @@ -45,6 +45,9 @@ public static void init(Configuration configuration) throws InitializationExcept OIOSAML3Service.sessionHandlerFactory = new InternalSessionHandlerFactory(); OIOSAML3Service.sessionHandlerFactory.configure(configuration); + // Must happen before the first OCSP check is made in this JVM + CRLChecker.configureOcspTransport(configuration); + initialized = true; } catch (Exception exception) { log.error("Unable to initialize OIOSAML",exception); diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java index 524bc35..d416aba 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/DispatcherServlet.java @@ -75,6 +75,11 @@ private void handleOptionalValues(Map config, Configuration conf configuration.setOcspCheckEnabled("true".equals(value)); } + value = config.get(Constants.OCSP_POST_ENABLED); + if (StringUtil.isNotEmpty(value)) { + configuration.setOcspPostEnabled("true".equals(value)); + } + value = config.get(Constants.METADATA_NAMEID_FORMAT); if (StringUtil.isNotEmpty(value)) { configuration.setNameIDFormat(value); diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java b/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java index 80366b7..c080683 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/util/Constants.java @@ -55,6 +55,7 @@ public class Constants { // Configuration constants for revocation check settings public static final String CRL_CHECK_ENABLED = "oiosaml.servlet.revocation.crl.check.enabled"; public static final String OCSP_CHECK_ENABLED = "oiosaml.servlet.revocation.ocsp.check.enabled"; + public static final String OCSP_POST_ENABLED = "oiosaml.servlet.revocation.ocsp.post.enabled"; // Configuration constants for AuthenticationFilter public static final String IS_PASSIVE = "oiosaml.filter.ispassive.enabled"; diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java index cb87943..b9d5132 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/BaseServiceTest.java @@ -16,12 +16,6 @@ public class BaseServiceTest { @BeforeAll public static void beforeAll(MockServerClient idp) throws Exception { - // Force the JDK OCSP client to POST the request instead of using the RFC 5019 GET form - // (request base64-encoded in the URL path). The NemLog-in test OCSP responder returns - // HTTP 404 for the GET form, which otherwise makes CRLCheckerTest's OCSP checks fail with - // UNDETERMINED_REVOCATION_STATUS. Must be set before the first OCSP check. - System.setProperty("com.sun.security.ocsp.useget", "false"); - ClassLoader classLoader = AssertionServiceTest.class.getClassLoader(); String keystoreLocation = classLoader.getResource(TestConstants.SP_KEYSTORE_LOCATION).getFile();