Add cachePolicy option to dynamically opt out of MDS cache fallback

Author: emlun

NEWS

@@ -59,6 +59,8 @@ New features:
   status, but returns an `ETag` response header matching the `"no"` of the
   cached BLOB, if any, this is now interpreted like a successful `304 Not
   Modified` response.
+* Added `.cachePolicy` setting to `FidoMetadataDownloader` to allow dynamically
+  opting out of falling back to cache when a BLOB download fails.
 
 Fixes:
 

webauthn-server-attestation/src/main/java/com/yubico/fido/metadata/FidoMetadataDownloader.java

@@ -78,6 +78,7 @@
 import java.util.Set;
 import java.util.UUID;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -122,6 +123,7 @@ public final class FidoMetadataDownloader {
   @NonNull private final Clock clock;
   private final KeyStore httpsTrustStore;
   private final boolean verifyDownloadsOnly;
+  private final Function<Exception, CachePolicyDecision> cachePolicy;
 
   /** For overriding JSON mapper settings in tests. */
   private final Supplier<ObjectMapper> makeHeaderJsonMapper;
@@ -158,6 +160,8 @@ public static class FidoMetadataDownloaderBuilder {
     @NonNull private Clock clock = Clock.systemUTC();
     private KeyStore httpsTrustStore = null;
     private boolean verifyDownloadsOnly = false;
+    private Function<Exception, CachePolicyDecision> cachePolicy =
+        (e) -> CachePolicyDecision.USE_CACHED;
 
     private Supplier<ObjectMapper> makeHeaderJsonMapper =
         FidoMetadataDownloader::defaultHeaderJsonMapper;
@@ -182,6 +186,7 @@ public FidoMetadataDownloader build() {
           clock,
           httpsTrustStore,
           verifyDownloadsOnly,
+          cachePolicy,
           makeHeaderJsonMapper,
           makePayloadJsonMapper);
     }
@@ -657,6 +662,40 @@ public FidoMetadataDownloaderBuilder verifyDownloadsOnly(final boolean verifyDow
       return this;
     }
 
+    /**
+     * Define a policy for how {@link #refreshBlob()} and {@link #loadCachedBlob()} should behave
+     * when a BLOB download fails.
+     *
+     * <p><code>cachePolicy</code> will be invoked when a cached BLOB is available and any attempt
+     * to download, parse and verify a new BLOB fails. Its argument will be the {@link Exception}
+     * that caused the failure. If <code>cachePolicy</code> returns {@link
+     * CachePolicyDecision#USE_CACHED}, then the {@link #refreshBlob()} or {@link #loadCachedBlob()}
+     * invocation will log a warning and return the cached BLOB as a successful result. If <code>
+     * cachePolicy</code> returns {@link CachePolicyDecision#THROW}, then the exception will be
+     * re-thrown and the {@link #refreshBlob()} or {@link #loadCachedBlob()} invocation will fail.
+     *
+     * <p><code>cachePolicy</code> MUST NOT return <code>null</code>.
+     *
+     * <p>When no cached BLOB is available, the exception is automatically re-thrown and <code>
+     * cachePolicy</code> is not invoked.
+     *
+     * <p>See the documentation of {@link #refreshBlob()} and {@link #loadCachedBlob()} for what
+     * kinds of exceptions may be thrown.
+     *
+     * <p>The default policy always returns {@link CachePolicyDecision#USE_CACHED}.
+     *
+     * @param cachePolicy the policy used to decide whether to throw or fall back to cache when a
+     *     BLOB download fails. MUST NOT return <code>null</code>.
+     * @see CachePolicyDecision
+     * @see #refreshBlob()
+     * @see #loadCachedBlob() ()
+     */
+    public FidoMetadataDownloaderBuilder cachePolicy(
+        final Function<Exception, CachePolicyDecision> cachePolicy) {
+      this.cachePolicy = cachePolicy;
+      return this;
+    }
+
     /** For internal testing use only. */
     FidoMetadataDownloaderBuilder headerJsonMapper(
         final Supplier<ObjectMapper> makeHeaderJsonMapper) {
@@ -929,15 +968,25 @@ private Optional<MetadataBLOB> refreshBlobInternal(
       }
     } catch (FidoMetadataDownloaderException e) {
       if (e.getReason() == Reason.BAD_SIGNATURE && cached.isPresent()) {
-        log.warn("New BLOB has bad signature - falling back to cached BLOB.");
-        return cached;
+        switch (cachePolicy.apply(e)) {
+          case USE_CACHED:
+            log.warn("New BLOB has bad signature - falling back to cached BLOB.");
+            return cached;
+          default:
+            throw e;
+        }
       } else {
         throw e;
       }
     } catch (Exception e) {
       if (cached.isPresent()) {
-        log.warn("Failed to download new BLOB - falling back to cached BLOB.", e);
-        return cached;
+        switch (cachePolicy.apply(e)) {
+          case USE_CACHED:
+            log.warn("Failed to download new BLOB - falling back to cached BLOB.", e);
+            return cached;
+          default:
+            throw e;
+        }
       } else {
         throw e;
       }
@@ -1411,4 +1460,17 @@ boolean isOk() {
       return content.isPresent();
     }
   }
+
+  /**
+   * Values for the {@link FidoMetadataDownloaderBuilder#cachePolicy(Function)} argument function to
+   * return to express how {@link #refreshBlob()} and {@link #loadCachedBlob()} should behave when a
+   * BLOB download fails.
+   */
+  public enum CachePolicyDecision {
+    /** Recover by returning the cached BLOB as a successful result. */
+    USE_CACHED,
+
+    /** Propagate the failure by re-throwing the exception. */
+    THROW;
+  }
 }

webauthn-server-attestation/src/test/scala/com/yubico/fido/metadata/FidoMetadataDownloaderSpec.scala

@@ -2,6 +2,8 @@ package com.yubico.fido.metadata
 
 import com.fasterxml.jackson.databind.node.IntNode
 import com.fasterxml.jackson.databind.node.ObjectNode
+import com.yubico.fido.metadata.FidoMetadataDownloader.CachePolicyDecision
+import com.yubico.fido.metadata.FidoMetadataDownloader.FidoMetadataDownloaderBuilder
 import com.yubico.fido.metadata.FidoMetadataDownloaderException.Reason
 import com.yubico.internal.util.BinaryUtil
 import com.yubico.internal.util.JacksonCodecs
@@ -30,6 +32,7 @@ import org.scalatestplus.junit.JUnitRunner
 import java.io.File
 import java.io.FileInputStream
 import java.io.FileOutputStream
+import java.io.IOException
 import java.net.URL
 import java.nio.charset.StandardCharsets
 import java.security.DigestException
@@ -52,6 +55,7 @@ import javax.servlet.http.HttpServletResponse
 import scala.jdk.CollectionConverters.ListHasAsScala
 import scala.jdk.CollectionConverters.SeqHasAsJava
 import scala.jdk.CollectionConverters.SetHasAsJava
+import scala.util.Failure
 import scala.util.Success
 import scala.util.Try
 
@@ -1090,7 +1094,7 @@ class FidoMetadataDownloaderSpec
           blob.getNo should equal(blobNo)
         }
 
-        it("The cache is used if the BLOB download fails.") {
+        describe("The cache is used if the BLOB download fails") {
           val oldBlobNo = 1
           val newBlobNo = 2
 
@@ -1120,22 +1124,25 @@ class FidoMetadataDownloaderSpec
             )
           )
 
-          val (server, serverUrl, httpsCert) =
-            makeHttpServer(
-              Map(
-                "/blob.jwt" -> (_ =>
-                  (
-                    HttpStatus.TOO_MANY_REQUESTS_429,
-                    newBlobJwt
-                      .getBytes(StandardCharsets.UTF_8),
+          def test[T](
+              configure: FidoMetadataDownloaderBuilder => T =
+                (_: FidoMetadataDownloaderBuilder) => {}
+          ): MetadataBLOBPayload = {
+            val (server, serverUrl, httpsCert) =
+              makeHttpServer(
+                Map(
+                  "/blob.jwt" -> (_ =>
+                    (
+                      HttpStatus.TOO_MANY_REQUESTS_429,
+                      newBlobJwt
+                        .getBytes(StandardCharsets.UTF_8),
+                    )
                   )
                 )
               )
-            )
-          startServer(server)
+            startServer(server)
 
-          val blob = load(
-            FidoMetadataDownloader
+            val builder = FidoMetadataDownloader
               .builder()
               .expectLegalHeader(
                 "Kom ihåg att du aldrig får snyta dig i mattan!"
@@ -1152,10 +1159,39 @@ class FidoMetadataDownloaderSpec
               .clock(Clock.fixed(CertValidFrom, ZoneOffset.UTC))
               .useCrls(crls.asJava)
               .trustHttpsCerts(httpsCert)
-              .build()
-          ).getPayload
-          blob should not be null
-          blob.getNo should equal(oldBlobNo)
+            configure(builder)
+            load(builder.build()).getPayload
+          }
+
+          it("with the default settings.") {
+            val blob = test()
+            blob should not be null
+            blob.getNo should equal(oldBlobNo)
+          }
+
+          it("and the cache policy allows it.") {
+            val blob = test(builder => {
+              builder.cachePolicy(_ => CachePolicyDecision.USE_CACHED)
+            })
+            blob should not be null
+            blob.getNo should equal(oldBlobNo)
+          }
+
+          it("unless the cache policy decides to re-throw.") {
+            val blob = Try(test(builder => {
+              builder.cachePolicy(_ => CachePolicyDecision.THROW)
+            }))
+            blob shouldBe a[Failure[_]]
+            blob.failed.get shouldBe an[IOException]
+          }
+
+          it("unless the cache policy returns null.") {
+            val blob = Try(test(builder => {
+              builder.cachePolicy(_ => null)
+            }))
+            blob shouldBe a[Failure[_]]
+            blob.failed.get shouldBe a[NullPointerException]
+          }
         }
       }
 
@@ -1806,7 +1842,7 @@ class FidoMetadataDownloaderSpec
           blob.getNo should equal(oldBlobNo)
         }
 
-        it("A newly downloaded BLOB is disregarded if it has an invalid signature but the cached one has a valid signature.") {
+        describe("A newly downloaded BLOB is disregarded if it has an invalid signature but the cached one has a valid signature") {
           val oldBlobNo = 1
           val newBlobNo = 2
 
@@ -1858,32 +1894,70 @@ class FidoMetadataDownloaderSpec
             )
             .mkString(".")
 
-          val (server, serverUrl, httpsCert) =
-            makeHttpServer("/blob.jwt", badNewBlobJwt)
-          startServer(server)
+          def test[T](
+              configure: FidoMetadataDownloaderBuilder => T =
+                (_: FidoMetadataDownloaderBuilder) => {}
+          ): MetadataBLOBPayload = {
+            val (server, serverUrl, httpsCert) =
+              makeHttpServer("/blob.jwt", badNewBlobJwt)
+            startServer(server)
 
-          val blob = load(
-            FidoMetadataDownloader
-              .builder()
-              .expectLegalHeader(
-                "Kom ihåg att du aldrig får snyta dig i mattan!"
-              )
-              .useTrustRoot(trustRootCert)
-              .downloadBlob(new URL(s"${serverUrl}/blob.jwt"))
-              .useBlobCache(
-                () =>
-                  Optional.of(
-                    new ByteArray(oldBlobJwt.getBytes(StandardCharsets.UTF_8))
-                  ),
-                _ => {},
+            val builder =
+              FidoMetadataDownloader
+                .builder()
+                .expectLegalHeader(
+                  "Kom ihåg att du aldrig får snyta dig i mattan!"
+                )
+                .useTrustRoot(trustRootCert)
+                .downloadBlob(new URL(s"${serverUrl}/blob.jwt"))
+                .useBlobCache(
+                  () =>
+                    Optional.of(
+                      new ByteArray(oldBlobJwt.getBytes(StandardCharsets.UTF_8))
+                    ),
+                  _ => {},
+                )
+                .clock(Clock.fixed(CertValidFrom, ZoneOffset.UTC))
+                .useCrls(crls.asJava)
+                .trustHttpsCerts(httpsCert)
+            configure(builder)
+            load(builder.build()).getPayload
+          }
+
+          it("under the default settings.") {
+            val blob = test()
+            blob should not be null
+            blob.getNo should equal(oldBlobNo)
+          }
+
+          it("if the cache policy allows falling back to the cache.") {
+            val blob = test(builder => {
+              builder.cachePolicy((_: Exception) =>
+                CachePolicyDecision.USE_CACHED
               )
-              .clock(Clock.fixed(CertValidFrom, ZoneOffset.UTC))
-              .useCrls(crls.asJava)
-              .trustHttpsCerts(httpsCert)
-              .build()
-          ).getPayload
-          blob should not be null
-          blob.getNo should equal(oldBlobNo)
+            })
+            blob should not be null
+            blob.getNo should equal(oldBlobNo)
+          }
+
+          it("unless the cache policy decides to re-throw.") {
+            val blob = Try(test(builder => {
+              builder.cachePolicy((_: Exception) => CachePolicyDecision.THROW)
+            }))
+            blob shouldBe a[Failure[_]]
+            blob.failed.get shouldBe a[FidoMetadataDownloaderException]
+            blob.failed.get
+              .asInstanceOf[FidoMetadataDownloaderException]
+              .getReason should be(Reason.BAD_SIGNATURE)
+          }
+
+          it("unless the cache policy returns null.") {
+            val blob = Try(test(builder => {
+              builder.cachePolicy((_: Exception) => null)
+            }))
+            blob shouldBe a[Failure[_]]
+            blob.failed.get shouldBe a[NullPointerException]
+          }
         }
 
         it("If verifyDownloadsOnly is not set, a cached BLOB may expire.") {