diff --git a/webauthn-server-attestation/build.gradle.kts b/webauthn-server-attestation/build.gradle.kts index 1748835d2..590e42ad7 100644 --- a/webauthn-server-attestation/build.gradle.kts +++ b/webauthn-server-attestation/build.gradle.kts @@ -48,7 +48,12 @@ dependencies { testImplementation("org.scalatestplus:junit-4-13_2.13") testImplementation("org.scalatestplus:scalacheck-1-16_2.13") - testImplementation("org.slf4j:slf4j-api") + testImplementation("org.slf4j:slf4j-api") { + version { + strictly("[1.7.25,1.8-a)") // Pre-1.8 version required by slf4j-test + } + } + testRuntimeOnly("uk.org.lidalia:slf4j-test") } val integrationTest = task("integrationTest") { @@ -58,9 +63,6 @@ val integrationTest = task("integrationTest") { testClassesDirs = sourceSets["integrationTest"].output.classesDirs classpath = sourceSets["integrationTest"].runtimeClasspath shouldRunAfter(tasks.test) - - // Required for processing CRL distribution points extension - systemProperty("com.sun.security.enableCRLDP", "true") } tasks["check"].dependsOn(integrationTest) diff --git a/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala b/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala index 937a0db8c..a2d01fc09 100644 --- a/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala +++ b/webauthn-server-attestation/src/integrationTest/scala/com/yubico/fido/metadata/FidoMetadataDownloaderIntegrationTest.scala @@ -1,5 +1,7 @@ package com.yubico.fido.metadata +import com.yubico.internal.util.CertificateParser +import com.yubico.webauthn.data.ByteArray import org.junit.runner.RunWith import org.scalatest.BeforeAndAfter import org.scalatest.funspec.AnyFunSpec @@ -8,7 +10,8 @@ import org.scalatest.tags.Network import org.scalatest.tags.Slow import org.scalatestplus.junit.JUnitRunner -import java.util.Optional +import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.OptionConverters.RichOption import scala.util.Success import scala.util.Try @@ -21,6 +24,9 @@ class FidoMetadataDownloaderIntegrationTest with BeforeAndAfter { describe("FidoMetadataDownloader with default settings") { + // Cache downloaded items to avoid cause unnecessary load on remote servers + var trustRootCache: Option[ByteArray] = None + var blobCache: Option[ByteArray] = None val downloader = FidoMetadataDownloader .builder() @@ -28,17 +34,46 @@ class FidoMetadataDownloaderIntegrationTest "Retrieval and use of this BLOB indicates acceptance of the appropriate agreement located at https://fidoalliance.org/metadata/metadata-legal-terms/" ) .useDefaultTrustRoot() - .useTrustRootCache(() => Optional.empty(), _ => {}) + .useTrustRootCache( + () => trustRootCache.toJava, + trustRoot => { trustRootCache = Some(trustRoot) }, + ) .useDefaultBlob() - .useBlobCache(() => Optional.empty(), _ => {}) + .useBlobCache( + () => blobCache.toJava, + blob => { blobCache = Some(blob) }, + ) .build() it("downloads and verifies the root cert and BLOB successfully.") { - // This test requires the system property com.sun.security.enableCRLDP=true val blob = Try(downloader.loadCachedBlob) blob shouldBe a[Success[_]] blob.get should not be null } + + it( + "does not encounter any CRLDistributionPoints entries in unknown format." + ) { + val blob = Try(downloader.loadCachedBlob) + blob shouldBe a[Success[_]] + val trustRootCert = + CertificateParser.parseDer(trustRootCache.get.getBytes) + val certChain = downloader + .fetchHeaderCertChain( + trustRootCert, + FidoMetadataDownloader.parseBlob(blobCache.get).getBlob.getHeader, + ) + .asScala :+ trustRootCert + for { cert <- certChain } { + withClue( + s"Unknown CRLDistributionPoints structure in cert [${cert.getSubjectX500Principal}] : ${new ByteArray(cert.getEncoded)}" + ) { + CertificateParser + .parseCrlDistributionPointsExtension(cert) + .isAnyDistributionPointUnsupported should be(false) + } + } + } } } diff --git a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java index 0dffab20f..90d550285 100644 --- a/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java +++ b/webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java @@ -29,6 +29,7 @@ import com.yubico.fido.metadata.FidoMetadataDownloaderException.Reason; import com.yubico.internal.util.BinaryUtil; import com.yubico.internal.util.CertificateParser; +import com.yubico.internal.util.OptionalUtil; import com.yubico.webauthn.data.ByteArray; import com.yubico.webauthn.data.exception.Base64UrlException; import com.yubico.webauthn.data.exception.HexException; @@ -54,6 +55,7 @@ import java.security.Signature; import java.security.SignatureException; import java.security.cert.CRL; +import java.security.cert.CRLException; import java.security.cert.CertPath; import java.security.cert.CertPathValidator; import java.security.cert.CertPathValidatorException; @@ -1131,13 +1133,18 @@ private MetadataBLOB verifyBlob(ParseResult parseResult, X509Certificate trustRo if (certStore != null) { pathParams.addCertStore(certStore); } + + // Parse CRLDistributionPoints ourselves so users don't have to set the + // `com.sun.security.enableCRLDP=true` system property + fetchCrlDistributionPoints(certChain, certFactory).ifPresent(pathParams::addCertStore); + pathParams.setDate(Date.from(clock.instant())); cpv.validate(blobCertPath, pathParams); return parseResult.blob; } - private static ParseResult parseBlob(ByteArray jwt) throws IOException, Base64UrlException { + static ParseResult parseBlob(ByteArray jwt) throws IOException, Base64UrlException { Scanner s = new Scanner(new ByteArrayInputStream(jwt.getBytes())).useDelimiter("\\."); final ByteArray jwtHeader = ByteArray.fromBase64Url(s.next()); final ByteArray jwtPayload = ByteArray.fromBase64Url(s.next()); @@ -1176,7 +1183,7 @@ private static ByteArray verifyHash(ByteArray contents, Set acceptedC } @Value - private static class ParseResult { + static class ParseResult { private MetadataBLOB blob; private ByteArray jwtHeader; private ByteArray jwtPayload; @@ -1213,4 +1220,68 @@ List fetchHeaderCertChain( return Collections.singletonList(trustRootCertificate); } } + + /** + * Parse the CRLDistributionPoints extension of each certificate, fetch each distribution point + * and assemble them into a {@link CertStore} ready to be injected into {@link + * PKIXParameters#addCertStore(CertStore)} to provide CRLs for the verification procedure. + * + *

We do this ourselves so that users don't have to set the `com.sun.security.enableCRLDP=true` + * system property. This is required by the default SUN provider in order to enable + * CRLDistributionPoints resolution. + * + *

Any CRLDistributionPoints entries in unknown format are ignored and log a warning. + */ + private Optional fetchCrlDistributionPoints( + List certChain, CertificateFactory certFactory) + throws InvalidAlgorithmParameterException, NoSuchAlgorithmException { + final List crlDistributionPointUrls = + certChain.stream() + .flatMap( + cert -> { + log.debug( + "Attempting to parse CRLDistributionPoints extension of cert: {}", + cert.getSubjectX500Principal()); + try { + return CertificateParser.parseCrlDistributionPointsExtension(cert) + .getDistributionPoints() + .stream(); + } catch (Exception e) { + log.warn( + "Failed to parse CRLDistributionPoints extension of cert: {}", + cert.getSubjectX500Principal(), + e); + return Stream.empty(); + } + }) + .collect(Collectors.toList()); + + if (crlDistributionPointUrls.isEmpty()) { + return Optional.empty(); + + } else { + final List crldpCrls = + crlDistributionPointUrls.stream() + .map( + crldpUrl -> { + log.debug("Attempting to download CRL distribution point: {}", crldpUrl); + try { + return Optional.of( + certFactory.generateCRL( + new ByteArrayInputStream(download(crldpUrl).getBytes()))); + } catch (CRLException e) { + log.warn("Failed to import CRL from distribution point: {}", crldpUrl, e); + return Optional.empty(); + } catch (Exception e) { + log.warn("Failed to download CRL distribution point: {}", crldpUrl, e); + return Optional.empty(); + } + }) + .flatMap(OptionalUtil::stream) + .collect(Collectors.toList()); + + return Optional.of( + CertStore.getInstance("Collection", new CollectionCertStoreParameters(crldpCrls))); + } + } }