From 8fbcd8802d77125dc7ff55d8f1503bff59b3e0a0 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 13 Aug 2026 16:23:33 +0200 Subject: [PATCH 1/6] REF-31: Verify revocation data before trusting it The CRL was fetched from the distribution point named in the certificate and consulted for the serial number without checking who signed it or how current it was, and the OCSP trust anchor was whatever certificate the issuer access location returned. Both locations come from the certificate being checked, so neither the absence of a serial number in a list nor a completed OCSP path validation meant anything on its own. A CRL is now required to be issued by the CA that issued the certificate, to verify against that CA's public key, and to be current within the configured clock skew. A list without a nextUpdate is refused, since it cannot be told apart from one superseded long ago. The certificate fetched from the issuer access location is required to be the issuer of the certificate being checked, by name and by signature, before it is used as OCSP trust anchor or CRL signer. CRL, issuer certificate and OCSP responder locations are restricted to http and https. The OCES CAs serve revocation data over plain http, which is fine as it is signed, but a certificate must not be able to point the SP at file:, jar: or any other protocol the JVM supports. Those fetches had no timeouts and now have connect and read timeouts. The issuer certificate cache was keyed by URL and never revisited, so a CA replaced at the same location would fail every check until the process restarted. A cached certificate is now reused only while it still is the issuer. Tested against a locally minted CA served from MockServer: a certificate absent from a valid list is accepted, one listed in it is rejected, and a list signed by another key, a stale list, a list without nextUpdate and a substituted CA are all refused. bcpkix is added as a test dependency to mint that material. --- oiosaml/pom.xml | 8 ++ .../dk/gov/oio/saml/service/CRLChecker.java | 132 ++++++++++++++++-- .../oio/saml/service/CRLCheckerLocalTest.java | 118 ++++++++++++++++ .../dk/gov/oio/saml/util/TestPkiUtil.java | 104 ++++++++++++++ 4 files changed, 349 insertions(+), 13 deletions(-) create mode 100644 oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java create mode 100644 oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java diff --git a/oiosaml/pom.xml b/oiosaml/pom.xml index 9f58a0c..2e13d6c 100644 --- a/oiosaml/pom.xml +++ b/oiosaml/pom.xml @@ -178,6 +178,14 @@ 2.3.0 + + + org.bouncycastle + bcpkix-jdk15on + 1.59 + test + + org.junit.jupiter junit-jupiter-engine 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 c42f518..af771f6 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 @@ -5,6 +5,8 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.net.URLConnection; +import java.security.GeneralSecurityException; import java.security.InvalidAlgorithmParameterException; import java.security.NoSuchAlgorithmException; import java.security.cert.CRLException; @@ -21,11 +23,13 @@ import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Collections; +import java.util.Date; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -59,6 +63,11 @@ public class CRLChecker { // (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"; + // Revocation data is fetched from locations named by the certificate being checked, a slow or + // unresponsive endpoint must not hold the revocation check open + private static final int CONNECT_TIMEOUT_MILLIS = 10000; + private static final int READ_TIMEOUT_MILLIS = 10000; + private static Map certificateMap = new HashMap(); /** @@ -156,6 +165,8 @@ private static boolean doOCSPCheck(X509Certificate certificate) throws Certifica throw new RuntimeException("No OCSP access location could be found"); } + URI responder = toHttpUri(ocspServer); + // try to retrieve issuing OCES CA certificate X509Certificate issuer = getIssuingCertificate(certificate); if (issuer == null) { @@ -179,7 +190,7 @@ private static boolean doOCSPCheck(X509Certificate certificate) throws Certifica // "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)); + revocationChecker.setOcspResponder(responder); // Fallback to CRL is handled by checkCertificate, according to the OIOSAML configuration revocationChecker.setOptions(EnumSet.of(PKIXRevocationChecker.Option.NO_FALLBACK)); @@ -236,28 +247,90 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate Iterator urlIt = issuingCaUrls.iterator(); while (urlIt.hasNext()) { Object caUrl = new UntrustedUrlInput(urlIt.next()); + String url = caUrl.toString(); + + // A cached certificate is only reused while it still is the issuer, so that a CA replaced at the + // same location is picked up instead of failing every check until the process restarts + X509Certificate cached = certificateMap.get(url); + if (cached != null && isIssuerOf(cached, certificate)) { + return cached; + } + + X509Certificate issuer = downloadCertificate(url); + if (issuer == null || !isIssuerOf(issuer, certificate)) { + return null; + } + + certificateMap.put(url, issuer); - return downloadCertificate(caUrl.toString()); + return issuer; } return null; } + + /** + * The AIA location is named by the certificate being checked, so the certificate downloaded from it can + * only serve as trust anchor or CRL signer once it is known to be the CA that issued that certificate. + */ + private static boolean isIssuerOf(X509Certificate issuer, X509Certificate certificate) { + if (!certificate.getIssuerX500Principal().equals(issuer.getSubjectX500Principal())) { + log.warn("Certificate fetched from the issuer location of {} is issued to somebody else", certificate.getSubjectDN()); + return false; + } + + try { + certificate.verify(issuer.getPublicKey()); + return true; + } + catch (GeneralSecurityException e) { + log.warn("Certificate fetched from the issuer location of {} did not issue it", certificate.getSubjectDN(), e); + return false; + } + } + + /** + * Open a stream to a location named by certificate content. + * + *

Only http and https are allowed. The OCES CAs serve revocation data over plain http, which is fine + * because the data is signed, but the scheme has to be constrained so that a certificate cannot make the + * SP read from file:, jar: or any other protocol the JVM happens to support.

+ */ + private static InputStream openStream(String url) throws IOException { + URL location = new URL(url); + + String protocol = location.getProtocol().toLowerCase(Locale.ROOT); + if (!"http".equals(protocol) && !"https".equals(protocol)) { + throw new IOException(String.format("Refusing to fetch revocation data over '%s'", protocol)); + } + + URLConnection connection = location.openConnection(); + connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); + connection.setReadTimeout(READ_TIMEOUT_MILLIS); + + return connection.getInputStream(); + } + + private static URI toHttpUri(String url) throws URISyntaxException { + URI uri = new URI(url); + + String scheme = (uri.getScheme() != null) ? uri.getScheme().toLowerCase(Locale.ROOT) : ""; + if (!"http".equals(scheme) && !"https".equals(scheme)) { + throw new RuntimeException(String.format("Refusing to use OCSP responder with scheme '%s'", scheme)); + } + + return uri; + } private static X509Certificate downloadCertificate(String url) { - if (certificateMap.containsKey(url)) { - return certificateMap.get(url); - } - try { CertificateFactory factory = CertificateFactory.getInstance("X.509"); - try (InputStream is = new URL(url).openStream()) { + try (InputStream is = openStream(url)) { X509Certificate certificate = (X509Certificate) factory.generateCertificate(is); if (certificate != null) { - certificateMap.put(url, certificate); - return certificate; } - + log.warn("Failed to parse certificate from {}", url); } catch (IOException ex) { @@ -345,7 +418,7 @@ private static List getOCSPUrls(AuthorityInformationAccess authInfoAcces return urls; } - private static boolean doCRLCheck(X509Certificate certificate) throws IOException, CertificateException, CRLException, InitializationException { + private static boolean doCRLCheck(X509Certificate certificate) throws IOException, GeneralSecurityException, InitializationException { boolean revoked = true; String url = getCRLUrl(certificate); @@ -353,13 +426,19 @@ private static boolean doCRLCheck(X509Certificate certificate) throws IOExceptio throw new RuntimeException("No CRL url could be found"); } - URL u = new URL(url); - try (InputStream is = u.openStream()) { + X509Certificate issuer = getIssuingCertificate(certificate); + if (issuer == null) { + throw new RuntimeException("CA Certificate for CRL check could not be retrieved!"); + } + + try (InputStream is = openStream(url)) { CertificateFactory cf = CertificateFactory.getInstance("X.509"); X509CRL crl = (X509CRL) cf.generateCRL(is); log.debug("CRL for {}: {}", url, crl); + validateCRL(crl, certificate, issuer); + X509CRLEntry revokedCertificate = crl.getRevokedCertificate(certificate.getSerialNumber()); if (revokedCertificate != null) { log.warn("Certificate found in revocation list " + certificate.getSubjectDN()); @@ -373,6 +452,33 @@ private static boolean doCRLCheck(X509Certificate certificate) throws IOExceptio return !revoked; } + /** + * A CRL is fetched from a location named by the certificate itself, so the absence of a serial number in + * it only means anything once the list is known to be signed by the issuing CA and to be current. + */ + private static void validateCRL(X509CRL crl, X509Certificate certificate, X509Certificate issuer) throws GeneralSecurityException { + if (!crl.getIssuerX500Principal().equals(certificate.getIssuerX500Principal())) { + throw new CRLException("CRL was not issued by the CA that issued the certificate"); + } + + crl.verify(issuer.getPublicKey()); + + long clockSkewMillis = 1000L * 60 * OIOSAML3Service.getConfig().getClockSkew(); + Date now = new Date(); + + Date thisUpdate = crl.getThisUpdate(); + if (thisUpdate == null || thisUpdate.getTime() - clockSkewMillis > now.getTime()) { + throw new CRLException("CRL is not valid yet, thisUpdate is " + thisUpdate); + } + + // RFC 5280 leaves nextUpdate optional, but a list that does not say when it is superseded cannot be + // told apart from one that was replaced long ago + Date nextUpdate = crl.getNextUpdate(); + if (nextUpdate == null || nextUpdate.getTime() + clockSkewMillis < now.getTime()) { + throw new CRLException("CRL is no longer current, nextUpdate is " + nextUpdate); + } + } + private static String getCRLUrl(X509Certificate certificate) throws IOException { log.debug("Attempting to extract distribution point from certificate {}", certificate.getSubjectDN()); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java new file mode 100644 index 0000000..4b9e68c --- /dev/null +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java @@ -0,0 +1,118 @@ +package dk.gov.oio.saml.service; + +import static org.mockserver.model.HttpRequest.request; +import static org.mockserver.model.HttpResponse.response; + +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.cert.X509CRL; +import java.security.cert.X509Certificate; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.mockserver.client.MockServerClient; + +import dk.gov.oio.saml.util.TestPkiUtil; + +/** + * Revocation checking against a locally served CA, so the CRL can be tampered with. The tests against the + * live NemLog-in test CA are in {@link CRLCheckerTest}. + */ +public class CRLCheckerLocalTest extends BaseServiceTest { + private static final String CRL_PATH = "/test-ca/crl"; + private static final String CA_ISSUER_PATH = "/test-ca/cacert"; + private static final BigInteger SERIAL_NUMBER = BigInteger.valueOf(4711); + + private KeyPair caKeyPair; + private X509Certificate caCertificate; + private X509Certificate certificate; + + @BeforeEach + public void beforeEach(MockServerClient mockServer) throws Exception { + OIOSAML3Service.getConfig().setCRLCheckEnabled(true); + OIOSAML3Service.getConfig().setOcspCheckEnabled(false); + + caKeyPair = TestPkiUtil.generateKeyPair(); + caCertificate = TestPkiUtil.createCaCertificate(caKeyPair, "OIOSAML test CA"); + certificate = TestPkiUtil.createCertificate(TestPkiUtil.generateKeyPair(), "OIOSAML test certificate", SERIAL_NUMBER, + caCertificate, caKeyPair, url(CRL_PATH), url(CA_ISSUER_PATH)); + + mockServer.reset(); + mockServer.when(request().withPath(CA_ISSUER_PATH)).respond(response().withBody(caCertificate.getEncoded())); + } + + @DisplayName("Test that a certificate absent from a valid CRL is accepted") + @Test + public void testAcceptCertificateAbsentFromCrl(MockServerClient mockServer) throws Exception { + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(1, checkCertificate().size()); + } + + @DisplayName("Test that a certificate listed in a valid CRL is rejected") + @Test + public void testRejectCertificateListedInCrl(MockServerClient mockServer) throws Exception { + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24), SERIAL_NUMBER)); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CRL signed by another key is not trusted") + @Test + public void testRejectCrlSignedByAnotherKey(MockServerClient mockServer) throws Exception { + // Same content as the accepted list above, only the signing key differs + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, TestPkiUtil.generateKeyPair(), TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CRL that is no longer current is not trusted") + @Test + public void testRejectStaleCrl(MockServerClient mockServer) throws Exception { + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(-12))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CRL without a nextUpdate is not trusted") + @Test + public void testRejectCrlWithoutNextUpdate(MockServerClient mockServer) throws Exception { + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, (Date) null)); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a certificate not issued by the CA at its issuer location is rejected") + @Test + public void testRejectCertificateNotIssuedByServedCa(MockServerClient mockServer) throws Exception { + // The CA certificate served at the issuer location belongs to a different CA + KeyPair otherCaKeyPair = TestPkiUtil.generateKeyPair(); + X509Certificate otherCaCertificate = TestPkiUtil.createCaCertificate(otherCaKeyPair, "OIOSAML test CA"); + + mockServer.reset(); + mockServer.when(request().withPath(CA_ISSUER_PATH)).respond(response().withBody(otherCaCertificate.getEncoded())); + serveCrl(mockServer, TestPkiUtil.createCrl(otherCaCertificate, otherCaKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + private void serveCrl(MockServerClient mockServer, X509CRL crl) throws Exception { + mockServer.when(request().withPath(CRL_PATH)).respond(response().withBody(crl.getEncoded())); + } + + private Set checkCertificate() throws Exception { + List certificates = Collections.singletonList(certificate); + + return CRLChecker.checkCertificates(certificates, null); + } + + private static String url(String path) { + return "http://localhost:8081" + path; + } +} diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java new file mode 100644 index 0000000..195e4c1 --- /dev/null +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java @@ -0,0 +1,104 @@ +package dk.gov.oio.saml.util; + +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.cert.X509CRL; +import java.security.cert.X509Certificate; +import java.util.Date; + +import org.bouncycastle.asn1.x500.X500Name; +import org.bouncycastle.asn1.x509.AccessDescription; +import org.bouncycastle.asn1.x509.AuthorityInformationAccess; +import org.bouncycastle.asn1.x509.BasicConstraints; +import org.bouncycastle.asn1.x509.CRLDistPoint; +import org.bouncycastle.asn1.x509.DistributionPoint; +import org.bouncycastle.asn1.x509.DistributionPointName; +import org.bouncycastle.asn1.x509.Extension; +import org.bouncycastle.asn1.x509.GeneralName; +import org.bouncycastle.asn1.x509.GeneralNames; +import org.bouncycastle.cert.X509v2CRLBuilder; +import org.bouncycastle.cert.jcajce.JcaX509CRLConverter; +import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter; +import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder; +import org.bouncycastle.operator.ContentSigner; +import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; + +/** + * Mints a CA, certificates and CRLs so revocation checking can be tested against locally served, and locally + * tampered with, revocation data instead of a live CA. + */ +public class TestPkiUtil { + private static final String SIGNATURE_ALGORITHM = "SHA256withRSA"; + + public static KeyPair generateKeyPair() throws Exception { + KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA"); + generator.initialize(2048); + + return generator.generateKeyPair(); + } + + /** + * Self signed CA certificate, the issuer of the certificates and CRLs below. + */ + public static X509Certificate createCaCertificate(KeyPair keyPair, String name) throws Exception { + X500Name subject = new X500Name("CN=" + name); + + JcaX509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( + subject, BigInteger.ONE, hoursFromNow(-1), hoursFromNow(24), subject, keyPair.getPublic()); + builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(0)); + + return convert(builder, keyPair); + } + + /** + * Certificate naming the given locations as its CRL distribution point and issuer access location, the + * way the OCES certificates do. + */ + public static X509Certificate createCertificate(KeyPair keyPair, String name, BigInteger serialNumber, X509Certificate caCertificate, KeyPair caKeyPair, String crlUrl, String caIssuerUrl) throws Exception { + JcaX509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( + caCertificate, serialNumber, hoursFromNow(-1), hoursFromNow(24), new X500Name("CN=" + name), keyPair.getPublic()); + + builder.addExtension(Extension.cRLDistributionPoints, false, new CRLDistPoint(new DistributionPoint[] { + new DistributionPoint(new DistributionPointName(new GeneralNames(uri(crlUrl))), null, null)})); + builder.addExtension(Extension.authorityInfoAccess, false, + new AuthorityInformationAccess(AccessDescription.id_ad_caIssuers, uri(caIssuerUrl))); + + return convert(builder, caKeyPair); + } + + /** + * CRL listing the given serial numbers as revoked. + * + * @param signingKeyPair key the list is signed with, which is not necessarily the CA key + * @param nextUpdate when the list is superseded, in the past for a list that is no longer current + */ + public static X509CRL createCrl(X509Certificate caCertificate, KeyPair signingKeyPair, Date nextUpdate, BigInteger... revokedSerialNumbers) throws Exception { + X509v2CRLBuilder builder = new X509v2CRLBuilder(new X500Name(caCertificate.getSubjectX500Principal().getName()), hoursFromNow(-1)); + if (nextUpdate != null) { + builder.setNextUpdate(nextUpdate); + } + + for (BigInteger serialNumber : revokedSerialNumbers) { + builder.addCRLEntry(serialNumber, hoursFromNow(-1), 0); + } + + ContentSigner signer = new JcaContentSignerBuilder(SIGNATURE_ALGORITHM).build(signingKeyPair.getPrivate()); + + return new JcaX509CRLConverter().getCRL(builder.build(signer)); + } + + public static Date hoursFromNow(int hours) { + return new Date(System.currentTimeMillis() + (hours * 60L * 60 * 1000)); + } + + private static X509Certificate convert(JcaX509v3CertificateBuilder builder, KeyPair signingKeyPair) throws Exception { + ContentSigner signer = new JcaContentSignerBuilder(SIGNATURE_ALGORITHM).build(signingKeyPair.getPrivate()); + + return new JcaX509CertificateConverter().getCertificate(builder.build(signer)); + } + + private static GeneralName uri(String url) { + return new GeneralName(GeneralName.uniformResourceIdentifier, url); + } +} From 7d84fa20f2c997ca61d71d9aca7e95a6c1341f4a Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 1 Sep 2026 11:36:14 +0200 Subject: [PATCH 2/6] Improved certificate validation, only fallback to CRL if OCSP validation is inconclusive (e.g. OCSP responder not available), additional tests for KeyUsage and BasicConstraints. --- .../dk/gov/oio/saml/service/CRLChecker.java | 150 ++++++++++++------ .../oio/saml/service/CRLCheckerLocalTest.java | 107 ++++++++++++- .../dk/gov/oio/saml/util/TestConstants.java | 7 +- .../dk/gov/oio/saml/util/TestPkiUtil.java | 60 ++++++- 4 files changed, 268 insertions(+), 56 deletions(-) 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 af771f6..463f805 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 @@ -15,6 +15,7 @@ import java.security.cert.CertPathValidatorException; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; +import java.security.cert.CertPathValidatorException.BasicReason; import java.security.cert.PKIXParameters; import java.security.cert.PKIXRevocationChecker; import java.security.cert.TrustAnchor; @@ -68,17 +69,20 @@ public class CRLChecker { private static final int CONNECT_TIMEOUT_MILLIS = 10000; private static final int READ_TIMEOUT_MILLIS = 10000; - private static Map certificateMap = new HashMap(); + private static final Map certificateMap = new HashMap(); + // Indexes of the keyCertSign and cRLSign bits in the KeyUsage extension, see RFC 5280 section 4.2.1.3 + private static final int KEY_CERT_SIGN = 5; + private static final int CRL_SIGN = 6; /** * 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. @@ -101,7 +105,7 @@ static void configureOcspTransport(Configuration configuration) { public static Set checkCertificates(List x509Certificates, DateTime lastCRLCheck) throws ExternalException, InternalException, InitializationException { Set result = new HashSet<>(); - if (x509Certificates == null || x509Certificates.size() == 0) { + if (x509Certificates == null || x509Certificates.isEmpty()) { return result; } @@ -110,8 +114,7 @@ public static Set checkCertificates(List x509C if (checkCertificate(certificate)) { result.add(certificate); log.debug("Certificate validated successfully: {}", certificate.getSubjectDN()); - } - else { + } else { log.warn("Certificate did not validate: {}", certificate.getSubjectDN()); } } @@ -121,35 +124,40 @@ public static Set checkCertificates(List x509C // OCSP first if configured, with fallback to CRL if configured private static boolean checkCertificate(X509Certificate certificate) { + if (!isWithinValidityPeriod(certificate)) { + log.warn("Certificate is outside its validity period: {}", certificate.getSubjectDN()); + return false; + } + boolean validated = false; Configuration config = OIOSAML3Service.getConfig(); if (config.isOcspCheckEnabled()) { try { validated = doOCSPCheck(certificate); - } - catch (Exception e) { + } catch (Exception e) { + if (!isRevocationDataUnavailable(e)) { + log.warn("Certificate rejected while validating it using OCSP: {}", certificate.getSubjectDN(), e); + return false; + } + log.warn("Unexpected error while validating certificate using OCSP.", e); if (config.isCRLCheckEnabled()) { try { validated = doCRLCheck(certificate); - } - catch (Exception ex) { + } catch (Exception ex) { log.warn("Unexpected error while validating certificate using CRL.", ex); - } + } } } - } - else if (config.isCRLCheckEnabled()) { + } else if (config.isCRLCheckEnabled()) { try { validated = doCRLCheck(certificate); - } - catch (Exception ex) { + } catch (Exception ex) { log.warn("Unexpected error while validating certificate using CRL.", ex); - } - } - else { + } + } else { log.warn("checkCertificate called, but both OCSP and CRL checking is disabled"); validated = true; } @@ -203,13 +211,11 @@ private static boolean doOCSPCheck(X509Certificate certificate) throws Certifica log.debug("Certificate successfully validated during OCSP check."); revoked = false; - } - catch (CertPathValidatorException cpve) { - if (cpve.getMessage() != null && cpve.getMessage().contains("Certificate has been revoked")) { + } catch (CertPathValidatorException cpve) { + if (BasicReason.REVOKED == cpve.getReason()) { revoked = true; log.info("Certificate revoked, cert[{}] : {}", cpve.getIndex(), cpve.getMessage()); - } - else { + } else { log.warn("Validation failure, cert[{}] : {}", cpve.getIndex(), cpve.getMessage()); throw cpve; } @@ -218,6 +224,22 @@ private static boolean doOCSPCheck(X509Certificate certificate) throws Certifica return (!revoked); } + /** + * Whether the OCSP check failed because the revocation status could not be obtained, rather than because + * the certificate itself failed validation. Only the former is a reason to ask the CRL instead: the CRL + * says nothing about the validity of a certificate, so letting it answer for a rejected certificate + * would turn that rejection into an acceptance. + */ + private static boolean isRevocationDataUnavailable(Exception e) { + if (e instanceof CertPathValidatorException) { + return BasicReason.UNDETERMINED_REVOCATION_STATUS == ((CertPathValidatorException) e).getReason(); + } + + // Everything else is a failure, such as a certificate naming no responder + // or an issuer certificate that could not be fetched + return true; + } + private static X509Certificate getIssuingCertificate(X509Certificate certificate) { log.debug("Attempting to extract issuing ca certifcate from certificate {}", certificate.getSubjectDN()); @@ -228,17 +250,16 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate try (ASN1InputStream aIn = new ASN1InputStream(bytes)) { ASN1OctetString octs = (ASN1OctetString) aIn.readObject(); - + try (ASN1InputStream aIn2 = new ASN1InputStream(octs.getOctets())) { ASN1Primitive auth_info_acc = aIn2.readObject(); - + if (auth_info_acc != null) { authInfoAcc = AuthorityInformationAccess.getInstance(auth_info_acc); } } } - } - catch (Exception e) { + } catch (Exception e) { log.debug("Cannot extract access location of issuing ca.", e); return null; } @@ -252,12 +273,12 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate // A cached certificate is only reused while it still is the issuer, so that a CA replaced at the // same location is picked up instead of failing every check until the process restarts X509Certificate cached = certificateMap.get(url); - if (cached != null && isIssuerOf(cached, certificate)) { + if (cached != null && isIssuedBy(cached, certificate)) { return cached; } X509Certificate issuer = downloadCertificate(url); - if (issuer == null || !isIssuerOf(issuer, certificate)) { + if (issuer == null || !isIssuedBy(issuer, certificate)) { return null; } @@ -271,19 +292,35 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate /** * The AIA location is named by the certificate being checked, so the certificate downloaded from it can - * only serve as trust anchor or CRL signer once it is known to be the CA that issued that certificate. + * only serve as trust anchor or CRL signer once it is known to be a CA allowed to issue certificates, + * and to be the one that issued that certificate. */ - private static boolean isIssuerOf(X509Certificate issuer, X509Certificate certificate) { + private static boolean isIssuedBy(X509Certificate issuer, X509Certificate certificate) { if (!certificate.getIssuerX500Principal().equals(issuer.getSubjectX500Principal())) { log.warn("Certificate fetched from the issuer location of {} is issued to somebody else", certificate.getSubjectDN()); return false; } + // Below zero means the basic constraints deny the certificate the CA role, or say nothing at all + if (issuer.getBasicConstraints() < 0) { + log.warn("Certificate fetched from the issuer location of {} is not a CA certificate", certificate.getSubjectDN()); + return false; + } + + if (!hasKeyUsage(issuer, KEY_CERT_SIGN)) { + log.warn("Certificate fetched from the issuer location of {} is not allowed to sign certificates", certificate.getSubjectDN()); + return false; + } + + if (!isWithinValidityPeriod(issuer)) { + log.warn("Certificate fetched from the issuer location of {} is outside its validity period", certificate.getSubjectDN()); + return false; + } + try { certificate.verify(issuer.getPublicKey()); return true; - } - catch (GeneralSecurityException e) { + } catch (GeneralSecurityException e) { log.warn("Certificate fetched from the issuer location of {} did not issue it", certificate.getSubjectDN(), e); return false; } @@ -321,7 +358,29 @@ private static URI toHttpUri(String url) throws URISyntaxException { return uri; } - + + /** + * Whether the certificate is within its validity period, allowing for the configured clock skew the way + * the freshness check on a CRL does. + */ + private static boolean isWithinValidityPeriod(X509Certificate certificate) { + long clockSkewMillis = 1000L * 60 * OIOSAML3Service.getConfig().getClockSkew(); + long now = System.currentTimeMillis(); + + return certificate.getNotBefore().getTime() - clockSkewMillis <= now + && certificate.getNotAfter().getTime() + clockSkewMillis >= now; + } + + /** + * A key is only usable for the purposes its certificate grants it, and a certificate stating no key + * usage at all grants none of them. + */ + private static boolean hasKeyUsage(X509Certificate certificate, int keyUsageBit) { + boolean[] keyUsage = certificate.getKeyUsage(); + + return keyUsage != null && keyUsage.length > keyUsageBit && keyUsage[keyUsageBit]; + } + private static X509Certificate downloadCertificate(String url) { try { CertificateFactory factory = CertificateFactory.getInstance("X.509"); @@ -332,12 +391,10 @@ private static X509Certificate downloadCertificate(String url) { } log.warn("Failed to parse certificate from {}", url); - } - catch (IOException ex) { + } catch (IOException ex) { log.warn("Failed to download intermediate CA certificate from {}", url); } - } - catch (CertificateException ex) { + } catch (CertificateException ex) { log.warn("Failed to generate certificate factory", ex); } @@ -353,17 +410,16 @@ private static String getOCSPUrl(X509Certificate certificate) { byte[] bytes = certificate.getExtensionValue(AUTH_INFO_ACCESS); try (ASN1InputStream aIn = new ASN1InputStream(bytes)) { ASN1OctetString octs = (ASN1OctetString) aIn.readObject(); - + try (ASN1InputStream aIn2 = new ASN1InputStream(octs.getOctets())) { ASN1Primitive auth_info_acc = aIn2.readObject(); - + if (auth_info_acc != null) { authInfoAcc = AuthorityInformationAccess.getInstance(auth_info_acc); } } } - } - catch (Exception e) { + } catch (Exception e) { log.debug("Cannot extract access location of OCSP responder.", e); return null; } @@ -419,7 +475,7 @@ private static List getOCSPUrls(AuthorityInformationAccess authInfoAcces } private static boolean doCRLCheck(X509Certificate certificate) throws IOException, GeneralSecurityException, InitializationException { - boolean revoked = true; + boolean revoked; String url = getCRLUrl(certificate); if (url == null) { @@ -443,8 +499,7 @@ private static boolean doCRLCheck(X509Certificate certificate) throws IOExceptio if (revokedCertificate != null) { log.warn("Certificate found in revocation list " + certificate.getSubjectDN()); revoked = true; - } - else { + } else { revoked = false; } } @@ -461,6 +516,11 @@ private static void validateCRL(X509CRL crl, X509Certificate certificate, X509Ce throw new CRLException("CRL was not issued by the CA that issued the certificate"); } + // A CA that is not allowed to sign CRLs cannot vouch for this list however well the signature verifies + if (!hasKeyUsage(issuer, CRL_SIGN)) { + throw new CRLException("CRL issuer is not allowed to sign CRLs, its key usage does not include cRLSign"); + } + crl.verify(issuer.getPublicKey()); long clockSkewMillis = 1000L * 60 * OIOSAML3Service.getConfig().getClockSkew(); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java index 4b9e68c..51a0d3f 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java @@ -12,6 +12,8 @@ import java.util.List; import java.util.Set; +import org.bouncycastle.asn1.x509.BasicConstraints; +import org.bouncycastle.asn1.x509.KeyUsage; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; @@ -27,7 +29,10 @@ public class CRLCheckerLocalTest extends BaseServiceTest { private static final String CRL_PATH = "/test-ca/crl"; private static final String CA_ISSUER_PATH = "/test-ca/cacert"; + private static final String OCSP_PATH = "/test-ca/ocsp"; private static final BigInteger SERIAL_NUMBER = BigInteger.valueOf(4711); + private static final String CA_NAME = "OIOSAML test CA"; + private static final KeyUsage CA_KEY_USAGE = new KeyUsage(KeyUsage.keyCertSign | KeyUsage.cRLSign); private KeyPair caKeyPair; private X509Certificate caCertificate; @@ -39,7 +44,7 @@ public void beforeEach(MockServerClient mockServer) throws Exception { OIOSAML3Service.getConfig().setOcspCheckEnabled(false); caKeyPair = TestPkiUtil.generateKeyPair(); - caCertificate = TestPkiUtil.createCaCertificate(caKeyPair, "OIOSAML test CA"); + caCertificate = TestPkiUtil.createCaCertificate(caKeyPair, CA_NAME); certificate = TestPkiUtil.createCertificate(TestPkiUtil.generateKeyPair(), "OIOSAML test certificate", SERIAL_NUMBER, caCertificate, caKeyPair, url(CRL_PATH), url(CA_ISSUER_PATH)); @@ -88,12 +93,92 @@ public void testRejectCrlWithoutNextUpdate(MockServerClient mockServer) throws E Assertions.assertEquals(0, checkCertificate().size()); } + @DisplayName("Test that a CRL is not trusted when the key usage of the CA does not allow it to sign CRLs") + @Test + public void testRejectCrlFromCaWithoutCrlSignKeyUsage(MockServerClient mockServer) throws Exception { + serveCaAndCertificate(mockServer, TestPkiUtil.createCaCertificate(caKeyPair, CA_NAME, new KeyUsage(KeyUsage.keyCertSign), new BasicConstraints(0))); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CA whose key usage does not allow it to sign certificates is not trusted") + @Test + public void testRejectCaWithoutKeyCertSignKeyUsage(MockServerClient mockServer) throws Exception { + serveCaAndCertificate(mockServer, TestPkiUtil.createCaCertificate(caKeyPair, CA_NAME, new KeyUsage(KeyUsage.cRLSign), new BasicConstraints(0))); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CA whose basic constraints do not give it the CA role is not trusted") + @Test + public void testRejectCaWithoutBasicConstraints(MockServerClient mockServer) throws Exception { + serveCaAndCertificate(mockServer, TestPkiUtil.createCaCertificate(caKeyPair, CA_NAME, CA_KEY_USAGE, null)); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a certificate that has expired is rejected") + @Test + public void testRejectExpiredCertificate(MockServerClient mockServer) throws Exception { + useCertificate(null, TestPkiUtil.hoursFromNow(-48), TestPkiUtil.hoursFromNow(-24)); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a certificate that is not valid yet is rejected") + @Test + public void testRejectCertificateNotValidYet(MockServerClient mockServer) throws Exception { + useCertificate(null, TestPkiUtil.hoursFromNow(24), TestPkiUtil.hoursFromNow(48)); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that a CA that has expired is not trusted") + @Test + public void testRejectExpiredCa(MockServerClient mockServer) throws Exception { + serveCaAndCertificate(mockServer, TestPkiUtil.createCaCertificate(caKeyPair, CA_NAME, CA_KEY_USAGE, + new BasicConstraints(0), TestPkiUtil.hoursFromNow(-48), TestPkiUtil.hoursFromNow(-24))); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + + @DisplayName("Test that the CRL answers when the OCSP responder cannot state a revocation status") + @Test + public void testFallBackToCrlWhenRevocationStatusIsUnavailable(MockServerClient mockServer) throws Exception { + OIOSAML3Service.getConfig().setOcspCheckEnabled(true); + useCertificate(url(OCSP_PATH), TestPkiUtil.hoursFromNow(-1), TestPkiUtil.hoursFromNow(24)); + + // Nothing is served at the OCSP location, so the responder cannot be reached + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(1, checkCertificate().size()); + } + + @DisplayName("Test that the CRL does not answer for a certificate the OCSP path rejected") + @Test + public void testDoNotFallBackToCrlWhenTheCertificateWasRejected(MockServerClient mockServer) throws Exception { + OIOSAML3Service.getConfig().setOcspCheckEnabled(true); + + // Expired, but so recently that the clock skew still lets it past the validity check, so the OCSP + // path is reached and rejects it where the CRL, which says nothing about validity, would not + useCertificate(url(OCSP_PATH), TestPkiUtil.hoursFromNow(-24), TestPkiUtil.minutesFromNow(-2)); + serveCrl(mockServer, TestPkiUtil.createCrl(caCertificate, caKeyPair, TestPkiUtil.hoursFromNow(24))); + + Assertions.assertEquals(0, checkCertificate().size()); + } + @DisplayName("Test that a certificate not issued by the CA at its issuer location is rejected") @Test public void testRejectCertificateNotIssuedByServedCa(MockServerClient mockServer) throws Exception { // The CA certificate served at the issuer location belongs to a different CA KeyPair otherCaKeyPair = TestPkiUtil.generateKeyPair(); - X509Certificate otherCaCertificate = TestPkiUtil.createCaCertificate(otherCaKeyPair, "OIOSAML test CA"); + X509Certificate otherCaCertificate = TestPkiUtil.createCaCertificate(otherCaKeyPair, CA_NAME); mockServer.reset(); mockServer.when(request().withPath(CA_ISSUER_PATH)).respond(response().withBody(otherCaCertificate.getEncoded())); @@ -102,6 +187,24 @@ public void testRejectCertificateNotIssuedByServedCa(MockServerClient mockServer Assertions.assertEquals(0, checkCertificate().size()); } + /** + * Replaces the CA, and the certificate being checked with one issued by it, so a CA differing from the + * one set up for every test in a single respect can be put in its place. + */ + private void serveCaAndCertificate(MockServerClient mockServer, X509Certificate replacementCaCertificate) throws Exception { + caCertificate = replacementCaCertificate; + certificate = TestPkiUtil.createCertificate(TestPkiUtil.generateKeyPair(), "OIOSAML test certificate", SERIAL_NUMBER, + caCertificate, caKeyPair, url(CRL_PATH), url(CA_ISSUER_PATH)); + + mockServer.reset(); + mockServer.when(request().withPath(CA_ISSUER_PATH)).respond(response().withBody(caCertificate.getEncoded())); + } + + private void useCertificate(String ocspUrl, Date notBefore, Date notAfter) throws Exception { + certificate = TestPkiUtil.createCertificate(TestPkiUtil.generateKeyPair(), "OIOSAML test certificate", SERIAL_NUMBER, + caCertificate, caKeyPair, url(CRL_PATH), url(CA_ISSUER_PATH), ocspUrl, notBefore, notAfter); + } + private void serveCrl(MockServerClient mockServer, X509CRL crl) throws Exception { mockServer.when(request().withPath(CRL_PATH)).respond(response().withBody(crl.getEncoded())); } 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 5424a13..8dd5359 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 @@ -147,8 +147,11 @@ public class TestConstants { " \n" + ""; public static final String BAD_SP_ASSERTION_CONSUMER_URL = "http://localhost:8080/sso"; - - 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=="; + + // The DevTest4 NemLog-in IdP signing certificate, taken from + // demo/src/main/resources/test-devtest4-idp-metadata.xml. The revocation tests validate against the + // live test CA, so this must be a certificate the CA still knows and has not expired. + public static final String VALID_CERTIFICATE = "MIIGjDCCBMCgAwIBAgIUaGLv8eOYx3cubJyI3ymai6tAXCowQQYJKoZIhvcNAQEKMDSgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQMEAgEFAKIDAgEgMGsxLTArBgNVBAMMJERlbiBEYW5za2UgU3RhdCBPQ0VTIHVkc3RlZGVuZGUtQ0EgMTETMBEGA1UECwwKVGVzdCAtIGN0aTEYMBYGA1UECgwPRGVuIERhbnNrZSBTdGF0MQswCQYDVQQGEwJESzAeFw0yNTA4MjAxMzQxNDBaFw0yODA4MTkxMzQxMzlaMIGhMR0wGwYDVQQDDBROZW1Mb2ctaW4gSWRQIC0gVGVzdDE3MDUGA1UEBRMuVUk6REstTzpHOmEwNDBmZTI2LWNlNzgtNDMyOC1hYzBlLTI5ZWViMTJmNDZkZjEhMB8GA1UECgwYRGlnaXRhbGlzZXJpbmdzc3R5cmVsc2VuMRcwFQYDVQRhDA5OVFJESy0zNDA1MTE3ODELMAkGA1UEBhMCREswggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQCVUzcmIfK0NbJEmAQ7kFOTmXs1mkSHF0LAEFq7Ne8vpNz2H+0F5IyWCwsBi3SByPTAEIClbIT6rLFBoMmIwjfRmAXd68zW7NYwR0N8PcR/hvtoCR1tW2JuKhQlKbWlaYLRvvvS3r5giIjHgEXpmv42caB76eYE3J2qWPaACOIRMXky1vCmbQpEwqJr0S9+QDtMYh5ypVk1zX9rrY2TBbienruISBF1090uuzEQZtfiq9nZaBQAJZFlcYLftepSZXW4QgJjWH6BhMrd4nvcz17WeabG3AhhKHvOUDukJgKMzQd/OcS1tY13P7soMeT8D8h5bCnNDoCfqM/5mgxgxZZXnjJhzOCY+MdrXXBRST2uQLNg0gl/LgqmEKk0izQaMtNauR+5fFHSJMN23Ga6fAmHMlyEj+5JhGeScRvIBp9tPERPhRF0MdEjobbcweMRWqurJYsdo9cIHwrL8WIgQGhQRUrKBu9VPZQTtVs3dn3cxZIJ3axu53hWEKRwqw4UoasCAwEAAaOCAYcwggGDMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUfyif2XGZQuJ159c1di5NCCVtdl4wewYIKwYBBQUHAQEEbzBtMEMGCCsGAQUFBzAChjdodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2Nlcy9pc3N1aW5nLzEvY2FjZXJ0L2lzc3VpbmcuY2VyMCYGCCsGAQUFBzABhhpodHRwOi8vY2ExLmN0aS1nb3YuZGsvb2NzcDAiBgNVHSAEGzAZMAgGBgQAj3oBATANBgsqgVCBKQEBAQMHATA7BggrBgEFBQcBAwQvMC0wKwYIKwYBBQUHCwIwHwYHBACL7EkBAjAUhhJodHRwczovL3VpZC5nb3YuZGswRQYDVR0fBD4wPDA6oDigNoY0aHR0cDovL2NhMS5jdGktZ292LmRrL29jZXMvaXNzdWluZy8xL2NybC9pc3N1aW5nLmNybDAdBgNVHQ4EFgQU9KSlGZd1HK7yFmo+zohr4oq6LLwwDgYDVR0PAQH/BAQDAgWgMEEGCSqGSIb3DQEBCjA0oA8wDQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiAwIBIAOCAYEAPt4QGIebeY3idMN6xPUQUBwYeiIjhtxqo2O9BHELEQ8Vp9fuBiExT286i1nNbd6TBgFFuUPh2kuwUqAj4YfaG7aSiCToJrTuaxBGlMI9nci2l5VDJ/1/lVVT/OkMLgR9xVXWSi8KFT8U9S+E1THE+G1YSNnt50yRDmqGNtG1XcaXQeu2Q4IcjMc2nf7NcGzGwidsKnBv0nNXwoYnh2ZLWPjQAgQzM3EYdZmEqaCrscYDFRbh4/T3TYjOo64N4rYZWTR+4/poz/29tyfVBWnr3rLuJH2q0kaiFVSXRHXoqOpCDUTVwsULShjmAHcPruQK7mRK1NQcwJtm0HQNWKUxizqc1pA38/9tU2vkQv+OxEEwHnCRK4XkLCJ8ZKCV5+c7AehhTLtDHqsA93+8afgrTLe+kWEO+xRgOJxKV9TnqkvnYiG8BtCOcb4+6yUMGAoOPULVKywIzutRsHYjZptCkuggBqNztSFU/DUd6SpSxf1MGfeh4SC4yPNvKQ+5fo7S"; 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"; } diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java index 195e4c1..4c2b9b2 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/TestPkiUtil.java @@ -17,6 +17,7 @@ import org.bouncycastle.asn1.x509.Extension; import org.bouncycastle.asn1.x509.GeneralName; import org.bouncycastle.asn1.x509.GeneralNames; +import org.bouncycastle.asn1.x509.KeyUsage; import org.bouncycastle.cert.X509v2CRLBuilder; import org.bouncycastle.cert.jcajce.JcaX509CRLConverter; import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter; @@ -39,14 +40,37 @@ public static KeyPair generateKeyPair() throws Exception { } /** - * Self signed CA certificate, the issuer of the certificates and CRLs below. + * Self signed CA certificate, the issuer of the certificates and CRLs below, holding the basic + * constraints and key usage a CA needs to issue both. */ public static X509Certificate createCaCertificate(KeyPair keyPair, String name) throws Exception { + return createCaCertificate(keyPair, name, new KeyUsage(KeyUsage.keyCertSign | KeyUsage.cRLSign), new BasicConstraints(0)); + } + + /** + * Self signed CA certificate with the given key usage and basic constraints, either of which is left out + * of the certificate when null, so a CA lacking what it takes to issue certificates or CRLs can be + * tested. + */ + public static X509Certificate createCaCertificate(KeyPair keyPair, String name, KeyUsage keyUsage, BasicConstraints basicConstraints) throws Exception { + return createCaCertificate(keyPair, name, keyUsage, basicConstraints, hoursFromNow(-1), hoursFromNow(24)); + } + + /** + * Self signed CA certificate valid in the given period, so a CA that has expired, or is not valid yet, + * can be tested. + */ + public static X509Certificate createCaCertificate(KeyPair keyPair, String name, KeyUsage keyUsage, BasicConstraints basicConstraints, Date notBefore, Date notAfter) throws Exception { X500Name subject = new X500Name("CN=" + name); JcaX509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( - subject, BigInteger.ONE, hoursFromNow(-1), hoursFromNow(24), subject, keyPair.getPublic()); - builder.addExtension(Extension.basicConstraints, true, new BasicConstraints(0)); + subject, BigInteger.ONE, notBefore, notAfter, subject, keyPair.getPublic()); + if (basicConstraints != null) { + builder.addExtension(Extension.basicConstraints, true, basicConstraints); + } + if (keyUsage != null) { + builder.addExtension(Extension.keyUsage, true, keyUsage); + } return convert(builder, keyPair); } @@ -56,17 +80,35 @@ public static X509Certificate createCaCertificate(KeyPair keyPair, String name) * way the OCES certificates do. */ public static X509Certificate createCertificate(KeyPair keyPair, String name, BigInteger serialNumber, X509Certificate caCertificate, KeyPair caKeyPair, String crlUrl, String caIssuerUrl) throws Exception { + return createCertificate(keyPair, name, serialNumber, caCertificate, caKeyPair, crlUrl, caIssuerUrl, null, hoursFromNow(-1), hoursFromNow(24)); + } + + /** + * Certificate naming an OCSP responder as well, and valid in the given period, so the OCSP path and a + * certificate that has expired, or is not valid yet, can be tested. No responder is named when the OCSP + * location is null. + */ + public static X509Certificate createCertificate(KeyPair keyPair, String name, BigInteger serialNumber, X509Certificate caCertificate, KeyPair caKeyPair, String crlUrl, String caIssuerUrl, String ocspUrl, Date notBefore, Date notAfter) throws Exception { JcaX509v3CertificateBuilder builder = new JcaX509v3CertificateBuilder( - caCertificate, serialNumber, hoursFromNow(-1), hoursFromNow(24), new X500Name("CN=" + name), keyPair.getPublic()); + caCertificate, serialNumber, notBefore, notAfter, new X500Name("CN=" + name), keyPair.getPublic()); builder.addExtension(Extension.cRLDistributionPoints, false, new CRLDistPoint(new DistributionPoint[] { new DistributionPoint(new DistributionPointName(new GeneralNames(uri(crlUrl))), null, null)})); - builder.addExtension(Extension.authorityInfoAccess, false, - new AuthorityInformationAccess(AccessDescription.id_ad_caIssuers, uri(caIssuerUrl))); + builder.addExtension(Extension.authorityInfoAccess, false, accessDescriptions(caIssuerUrl, ocspUrl)); return convert(builder, caKeyPair); } + private static AuthorityInformationAccess accessDescriptions(String caIssuerUrl, String ocspUrl) { + AccessDescription caIssuers = new AccessDescription(AccessDescription.id_ad_caIssuers, uri(caIssuerUrl)); + if (ocspUrl == null) { + return new AuthorityInformationAccess(caIssuers); + } + + return new AuthorityInformationAccess(new AccessDescription[] { + caIssuers, new AccessDescription(AccessDescription.id_ad_ocsp, uri(ocspUrl))}); + } + /** * CRL listing the given serial numbers as revoked. * @@ -89,7 +131,11 @@ public static X509CRL createCrl(X509Certificate caCertificate, KeyPair signingKe } public static Date hoursFromNow(int hours) { - return new Date(System.currentTimeMillis() + (hours * 60L * 60 * 1000)); + return minutesFromNow(hours * 60); + } + + public static Date minutesFromNow(int minutes) { + return new Date(System.currentTimeMillis() + (minutes * 60L * 1000)); } private static X509Certificate convert(JcaX509v3CertificateBuilder builder, KeyPair signingKeyPair) throws Exception { From b0cf392751fda67d0e4877ad9e4a16dc5649a37d Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 1 Sep 2026 11:44:19 +0200 Subject: [PATCH 3/6] Bump lombok in idp demo. --- idp/pom.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/idp/pom.xml b/idp/pom.xml index 7bf1573..784ca45 100644 --- a/idp/pom.xml +++ b/idp/pom.xml @@ -9,6 +9,9 @@ 11 UTF-8 + + 1.18.46 @@ -96,6 +99,22 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + + + + org.projectlombok + lombok + ${lombok.version} + + + + + org.springframework.boot spring-boot-maven-plugin From 41c464fd3ec1dc24fac2476256bc7e81ab570d71 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 1 Sep 2026 16:39:11 +0200 Subject: [PATCH 4/6] Use ConcurrentHashMap for issuing CA certificates. --- .../main/java/dk/gov/oio/saml/service/CRLChecker.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 463f805..0d4bb1f 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 @@ -26,13 +26,13 @@ import java.util.Collections; import java.util.Date; import java.util.EnumSet; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -69,7 +69,8 @@ public class CRLChecker { private static final int CONNECT_TIMEOUT_MILLIS = 10000; private static final int READ_TIMEOUT_MILLIS = 10000; - private static final Map certificateMap = new HashMap(); + private static final Map issuingCACertificatesByUrl = new ConcurrentHashMap(); + // Indexes of the keyCertSign and cRLSign bits in the KeyUsage extension, see RFC 5280 section 4.2.1.3 private static final int KEY_CERT_SIGN = 5; private static final int CRL_SIGN = 6; @@ -272,7 +273,7 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate // A cached certificate is only reused while it still is the issuer, so that a CA replaced at the // same location is picked up instead of failing every check until the process restarts - X509Certificate cached = certificateMap.get(url); + X509Certificate cached = issuingCACertificatesByUrl.get(url); if (cached != null && isIssuedBy(cached, certificate)) { return cached; } @@ -282,7 +283,7 @@ private static X509Certificate getIssuingCertificate(X509Certificate certificate return null; } - certificateMap.put(url, issuer); + issuingCACertificatesByUrl.put(url, issuer); return issuer; } From 1c0c4b1ed1a7576b975e8e7234dc40694326fee9 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 1 Sep 2026 16:46:07 +0200 Subject: [PATCH 5/6] Removed unused parameter in checkCertificates() --- .../src/main/java/dk/gov/oio/saml/model/IdPMetadata.java | 8 ++++---- .../src/main/java/dk/gov/oio/saml/service/CRLChecker.java | 3 +-- .../java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java | 2 +- .../test/java/dk/gov/oio/saml/service/CRLCheckerTest.java | 8 ++++---- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java b/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java index 1c274b1..5d56368 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/model/IdPMetadata.java @@ -179,22 +179,22 @@ private void doRevocationCheck() throws ExternalException, InternalException { if (lastCRLCheck == null || (lastUpdate != null && lastUpdate.isAfter(lastCRLCheck))) { try { // Encryption - Set validEncryptionCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.ENCRYPTION), getLastCRLCheck()); + Set validEncryptionCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.ENCRYPTION)); this.validEncryptionCertificates.clear(); if (validEncryptionCertificates != null) { this.validEncryptionCertificates.addAll(validEncryptionCertificates); } // Signing - Set validSigningCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.SIGNING), getLastCRLCheck()); + Set validSigningCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.SIGNING)); this.validSigningCertificates.clear(); if (validSigningCertificates != null) { this.validSigningCertificates.addAll(validSigningCertificates); } // Unspecified - Set validUnspecifiedCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.UNSPECIFIED), getLastCRLCheck()); - Set validNullCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(null), getLastCRLCheck()); + Set validUnspecifiedCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(UsageType.UNSPECIFIED)); + Set validNullCertificates = CRLChecker.checkCertificates(getAllX509CertificatesWithUsageType(null)); this.validUnspecifiedCertificates.clear(); if (validUnspecifiedCertificates != null) { 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 0d4bb1f..6ea59f2 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 @@ -49,7 +49,6 @@ import org.bouncycastle.asn1.x509.GeneralNames; import org.bouncycastle.i18n.filter.UntrustedUrlInput; import org.bouncycastle.x509.extension.X509ExtensionUtil; -import org.joda.time.DateTime; import org.opensaml.core.config.InitializationException; import dk.gov.oio.saml.config.Configuration; @@ -104,7 +103,7 @@ static void configureOcspTransport(Configuration configuration) { 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 { + public static Set checkCertificates(List x509Certificates) throws ExternalException, InternalException, InitializationException { Set result = new HashSet<>(); if (x509Certificates == null || x509Certificates.isEmpty()) { return result; diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java index 51a0d3f..8487eb5 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/service/CRLCheckerLocalTest.java @@ -212,7 +212,7 @@ private void serveCrl(MockServerClient mockServer, X509CRL crl) throws Exception private Set checkCertificate() throws Exception { List certificates = Collections.singletonList(certificate); - return CRLChecker.checkCertificates(certificates, null); + return CRLChecker.checkCertificates(certificates); } private static String url(String path) { 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 ab3848c..3dd2ef9 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 @@ -30,7 +30,7 @@ public class CRLCheckerTest extends BaseServiceTest { X509Certificate certificate = (X509Certificate) factory.generateCertificate(bis); List x509Certificates = Collections.singletonList(certificate); - Set validCertificates = CRLChecker.checkCertificates(x509Certificates, null); + Set validCertificates = CRLChecker.checkCertificates(x509Certificates); Assertions.assertEquals(1, validCertificates.size()); } @@ -47,7 +47,7 @@ public class CRLCheckerTest extends BaseServiceTest { X509Certificate certificate = (X509Certificate) factory.generateCertificate(bis); List x509Certificates = Collections.singletonList(certificate); - Set validCertificates = CRLChecker.checkCertificates(x509Certificates, null); + Set validCertificates = CRLChecker.checkCertificates(x509Certificates); Assertions.assertEquals(1, validCertificates.size()); } @@ -68,7 +68,7 @@ public class CRLCheckerTest extends BaseServiceTest { X509Certificate certificate = (X509Certificate) factory.generateCertificate(bis); List x509Certificates = Collections.singletonList(certificate); - Set validCertificates = CRLChecker.checkCertificates(x509Certificates, null); + Set validCertificates = CRLChecker.checkCertificates(x509Certificates); Assertions.assertNotNull(validCertificates); Assertions.assertEquals(0, validCertificates.size()); @@ -90,7 +90,7 @@ public class CRLCheckerTest extends BaseServiceTest { X509Certificate certificate = (X509Certificate) factory.generateCertificate(bis); List x509Certificates = Collections.singletonList(certificate); - Set validCertificates = CRLChecker.checkCertificates(x509Certificates, null); + Set validCertificates = CRLChecker.checkCertificates(x509Certificates); Assertions.assertNotNull(validCertificates); Assertions.assertEquals(0, validCertificates.size()); From 3065575c7414b334be3b250708e956d0d22ca2eb Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Tue, 1 Sep 2026 16:48:45 +0200 Subject: [PATCH 6/6] Fixed indention in pom-files. --- demo/pom.xml | 3 ++- idp/pom.xml | 3 ++- integrationtest/pom.xml | 3 ++- oiosaml/pom.xml | 53 ++++++++++++++++++++--------------------- pom.xml | 37 ++++++++++++++-------------- 5 files changed, 51 insertions(+), 48 deletions(-) diff --git a/demo/pom.xml b/demo/pom.xml index febee05..bfac6b7 100644 --- a/demo/pom.xml +++ b/demo/pom.xml @@ -1,5 +1,6 @@ - + oiosaml3-demo OIOSAML Demo for Java v3 4.0.0 diff --git a/idp/pom.xml b/idp/pom.xml index 784ca45..fb8bd50 100644 --- a/idp/pom.xml +++ b/idp/pom.xml @@ -1,4 +1,5 @@ - + 4.0.0 dk.digst oiosaml3-idp diff --git a/integrationtest/pom.xml b/integrationtest/pom.xml index 3154086..faed05e 100644 --- a/integrationtest/pom.xml +++ b/integrationtest/pom.xml @@ -1,5 +1,6 @@ - + oiosaml3-integrationtest OIOSAML Integrationtest Java v3 4.0.0 diff --git a/oiosaml/pom.xml b/oiosaml/pom.xml index f9a2191..b9f6a1a 100644 --- a/oiosaml/pom.xml +++ b/oiosaml/pom.xml @@ -1,6 +1,6 @@ + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> 4.0.0 dk.digst oiosaml3.java @@ -37,30 +37,29 @@ 1.8.0 false - - - - sign - - - - org.apache.maven.plugins - maven-gpg-plugin - 1.5 - - - sign-artifacts - verify - - sign - - - - - - - + + + sign + + + + org.apache.maven.plugins + maven-gpg-plugin + 1.5 + + + sign-artifacts + verify + + sign + + + + + + + @@ -87,7 +86,7 @@ true - + org.apache.maven.plugins maven-source-plugin @@ -101,7 +100,7 @@ - + org.apache.maven.plugins maven-javadoc-plugin diff --git a/pom.xml b/pom.xml index 3093c83..a3ef499 100644 --- a/pom.xml +++ b/pom.xml @@ -1,4 +1,5 @@ - + 4.0.0 dk.digst oiosaml3-parent @@ -11,19 +12,19 @@ true - - - - org.apache.maven.plugins - maven-compiler-plugin - 3.8.1 - - - 8 - - + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.8.1 + + + 8 + + org.apache.maven.plugins @@ -33,11 +34,11 @@ true true - - - + + + - + oiosaml demo integrationtest