Inclusion proof is required

loosebazooka · loosebazooka · commit 40c04c2e26e6 · 2024-03-14T17:19:31.000-04:00
This makes inclusion proof on entry.verification not optional.

Verification of an entry implies the inclusion proof will also
be verified.

Rework the code a bit to reflect this and update tests.

Signed-off-by: Appu Goundan &lt;appu@google.com&gt;
diff --git a/fuzzing/src/main/java/fuzzing/RekorVerifierFuzzer.java b/fuzzing/src/main/java/fuzzing/RekorVerifierFuzzer.java
@@ -40,7 +40,6 @@ public static void fuzzerTestOneInput(FuzzedDataProvider data) {
       RekorVerifier verifier = RekorVerifier.newRekorVerifier(tLogs);
 
       verifier.verifyEntry(entry);
-      verifier.verifyInclusionProof(entry);
     } catch (URISyntaxException | RekorParseException | RekorVerificationException e) {
       // Known exception
     }
diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java
@@ -180,17 +180,6 @@ public void verify(byte[] artifactDigest, KeylessVerificationRequest request)
       throw new KeylessVerificationException("Rekor entry signature was not valid");
     }
 
-    // verify any inclusion proof
-    if (rekorEntry.getVerification().getInclusionProof().isPresent()) {
-      try {
-        rekorVerifier.verifyInclusionProof(rekorEntry);
-      } catch (RekorVerificationException ex) {
-        throw new KeylessVerificationException("Rekor entry inclusion proof was not valid");
-      }
-    } else if (request.getVerificationOptions().alwaysUseRemoteRekorEntry()) {
-      throw new KeylessVerificationException("Rekor entry did not contain inclusion proof");
-    }
-
     // check if the time of entry inclusion in the log (a stand-in for signing time) is within the
     // validity period for the certificate
     var entryTime = Date.from(rekorEntry.getIntegratedTimeInstant());
diff --git a/sigstore-java/src/main/java/dev/sigstore/bundle/BundleFactoryInternal.java b/sigstore-java/src/main/java/dev/sigstore/bundle/BundleFactoryInternal.java
@@ -43,7 +43,6 @@
 import java.security.cert.CertificateException;
 import java.util.Base64;
 import java.util.List;
-import java.util.Optional;
 import java.util.stream.Collectors;
 import org.bouncycastle.util.encoders.Hex;
 
@@ -127,14 +126,7 @@ private static TransparencyLogEntry.Builder buildTlogEntries(RekorEntry entry) {
 
   private static void addInclusionProof(
       TransparencyLogEntry.Builder transparencyLogEntry, RekorEntry entry) {
-    RekorEntry.InclusionProof inclusionProof =
-        entry
-            .getVerification()
-            .getInclusionProof()
-            .orElseThrow(
-                () ->
-                    new IllegalArgumentException(
-                        "An inclusion proof must be present in the log entry in the signing result"));
+    RekorEntry.InclusionProof inclusionProof = entry.getVerification().getInclusionProof();
     transparencyLogEntry.setInclusionProof(
         InclusionProof.newBuilder()
             .setLogIndex(inclusionProof.getLogIndex())
@@ -166,9 +158,8 @@ static KeylessSignature readBundle(Reader jsonReader) throws BundleParseExceptio
     var bundleEntry = bundle.getVerificationMaterial().getTlogEntries(0);
     RekorEntry.InclusionProof inclusionProof = null;
     if (!bundleEntry.hasInclusionProof()) {
-      if (!bundle.getMediaType().equals(BUNDLE_V_0_1)) {
-        throw new BundleParseException("Could not find an inclusion proof");
-      }
+      // all consumed bundles must have an inclusion proof
+      throw new BundleParseException("Could not find an inclusion proof");
     } else {
       var bundleInclusionProof = bundleEntry.getInclusionProof();
 
@@ -192,7 +183,7 @@ static KeylessSignature readBundle(Reader jsonReader) throws BundleParseExceptio
                 Base64.getEncoder()
                     .encodeToString(
                         bundleEntry.getInclusionPromise().getSignedEntryTimestamp().toByteArray()))
-            .inclusionProof(Optional.ofNullable(inclusionProof))
+            .inclusionProof(inclusionProof)
             .build();
 
     var rekorEntry =
diff --git a/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorEntry.java b/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorEntry.java
@@ -37,7 +37,8 @@ interface Verification {
     /** Return the signed entry timestamp. */
     String getSignedEntryTimestamp();
 
-    Optional<InclusionProof> getInclusionProof();
+    /** Return the inclusion proof. */
+    InclusionProof getInclusionProof();
   }
 
   /**
diff --git a/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorVerifier.java b/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorVerifier.java
@@ -81,24 +81,15 @@ public void verifyEntry(RekorEntry entry) throws RekorVerificationException {
     } catch (NoSuchAlgorithmException nsae) {
       throw new AssertionError("Required verification algorithm 'SHA256withECDSA' not found.");
     }
+
+    // verify inclusion proof
+    verifyInclusionProof(entry);
   }
 
-  /**
-   * Verify that a Rekor Entry is in the log by checking inclusion proof.
-   *
-   * @param entry the entry to verify
-   * @throws RekorVerificationException if the entry cannot be verified
-   */
-  public void verifyInclusionProof(RekorEntry entry) throws RekorVerificationException {
+  /** Verify that a Rekor Entry is in the log by checking inclusion proof. */
+  private void verifyInclusionProof(RekorEntry entry) throws RekorVerificationException {
 
-    var inclusionProof =
-        entry
-            .getVerification()
-            .getInclusionProof()
-            .orElseThrow(
-                () ->
-                    new RekorVerificationException(
-                        "No inclusion proof was found in the rekor entry"));
+    var inclusionProof = entry.getVerification().getInclusionProof();
 
     var leafHash =
         Hashing.sha256()
diff --git a/sigstore-java/src/test/java/dev/sigstore/KeylessVerifierTest.java b/sigstore-java/src/test/java/dev/sigstore/KeylessVerifierTest.java
@@ -69,6 +69,7 @@ public void testVerify_mismatchedSet() throws Exception {
 
   @Test
   public void testVerify_canVerifyV01Bundle() throws Exception {
+    // note that this v1 bundle contains an inclusion proof
     verifyBundle(
         "dev/sigstore/samples/bundles/artifact.txt",
         "dev/sigstore/samples/bundles/bundle.v1.sigstore");
diff --git a/sigstore-java/src/test/java/dev/sigstore/bundle/BundleFactoryTest.java b/sigstore-java/src/test/java/dev/sigstore/bundle/BundleFactoryTest.java
@@ -29,6 +29,15 @@ public void readV1Bundle() throws Exception {
     readBundle("dev/sigstore/samples/bundles/bundle.v1.sigstore");
   }
 
+  @Test
+  public void readV1Bundle_noInclusion() {
+    var ex =
+        Assertions.assertThrows(
+            BundleParseException.class,
+            () -> readBundle("dev/sigstore/samples/bundles/bundle.v1.no.inclusion.sigstore"));
+    Assertions.assertEquals("Could not find an inclusion proof", ex.getMessage());
+  }
+
   @Test
   public void readV2Bundle() throws Exception {
     readBundle("dev/sigstore/samples/bundles/bundle.v2.sigstore");
diff --git a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java b/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java
@@ -170,11 +170,10 @@ public void getEntry_hashedRekordRequest_byCalculatedUuid() throws Exception {
   private void assertEntry(RekorResponse resp, Optional<RekorEntry> entry) {
     assertTrue(entry.isPresent());
     assertEquals(resp.getEntry().getLogID(), entry.get().getLogID());
-    assertTrue(entry.get().getVerification().getInclusionProof().isPresent());
-    assertNotNull(entry.get().getVerification().getInclusionProof().get().getTreeSize());
-    assertNotNull(entry.get().getVerification().getInclusionProof().get().getRootHash());
-    assertNotNull(entry.get().getVerification().getInclusionProof().get().getLogIndex());
-    assertTrue(entry.get().getVerification().getInclusionProof().get().getHashes().size() > 0);
+    assertNotNull(entry.get().getVerification().getInclusionProof().getTreeSize());
+    assertNotNull(entry.get().getVerification().getInclusionProof().getRootHash());
+    assertNotNull(entry.get().getVerification().getInclusionProof().getLogIndex());
+    assertTrue(entry.get().getVerification().getInclusionProof().getHashes().size() > 0);
   }
 
   @Test
diff --git a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorTypesTest.java b/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorTypesTest.java
@@ -32,15 +32,15 @@ private RekorEntry fromResource(String path) throws Exception {
 
   @Test
   public void getHashedRekord_pass() throws Exception {
-    var entry = fromResource("dev/sigstore/samples/rekor-response/valid/response.json");
+    var entry = fromResource("dev/sigstore/samples/rekor-response/valid/entry.json");
 
     var hashedRekord = RekorTypes.getHashedRekord(entry);
     Assertions.assertNotNull(hashedRekord);
   }
 
   @Test
   public void getHashedRekord_badType() throws Exception {
-    var entry = fromResource("dev/sigstore/samples/rekor-response/valid/jar-response.json");
+    var entry = fromResource("dev/sigstore/samples/rekor-response/valid/jar-entry.json");
 
     var exception =
         Assertions.assertThrows(RekorTypeException.class, () -> RekorTypes.getHashedRekord(entry));
diff --git a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorVerifierTest.java b/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorVerifierTest.java
@@ -41,7 +41,6 @@
 
 public class RekorVerifierTest {
   public String rekorResponse;
-  public String rekorQueryResponse;
   public byte[] rekorPub;
 
   public static SigstoreTrustedRoot trustRoot;
@@ -50,15 +49,11 @@ public class RekorVerifierTest {
   public void loadResources() throws IOException {
     rekorResponse =
         Resources.toString(
-            Resources.getResource("dev/sigstore/samples/rekor-response/valid/response.json"),
+            Resources.getResource("dev/sigstore/samples/rekor-response/valid/entry.json"),
             StandardCharsets.UTF_8);
     rekorPub =
         Resources.toByteArray(
             Resources.getResource("dev/sigstore/samples/rekor-response/valid/rekor.pub"));
-    rekorQueryResponse =
-        Resources.toString(
-            Resources.getResource("dev/sigstore/samples/rekor-response/valid/query-response.json"),
-            StandardCharsets.UTF_8);
   }
 
   @BeforeAll
@@ -73,18 +68,10 @@ public static void initTrustRoot() throws IOException, CertificateException {
     trustRoot = SigstoreTrustedRoot.from(builder.build());
   }
 
-  @Test
-  public void verifyEntry_valid() throws Exception {
-    var response = RekorResponse.newRekorResponse(new URI("https://somewhere"), rekorResponse);
-    var verifier = RekorVerifier.newRekorVerifier(trustRoot);
-
-    verifier.verifyEntry(response.getEntry());
-  }
-
   @Test
   public void verifyEntry_invalid() throws Exception {
     // change the logindex
-    var invalidResponse = rekorResponse.replace("79", "80");
+    var invalidResponse = rekorResponse.replace("1688", "1700");
     var response = RekorResponse.newRekorResponse(new URI("https://somewhere"), invalidResponse);
     var verifier = RekorVerifier.newRekorVerifier(trustRoot);
 
@@ -95,29 +82,27 @@ public void verifyEntry_invalid() throws Exception {
   }
 
   @Test
-  public void verifyEntry_withInclusionProof() throws Exception {
-    var response = RekorResponse.newRekorResponse(new URI("https://somewhere"), rekorQueryResponse);
+  public void verifyEntry() throws Exception {
+    var response = RekorResponse.newRekorResponse(new URI("https://somewhere"), rekorResponse);
     var verifier = RekorVerifier.newRekorVerifier(trustRoot);
 
     var entry = response.getEntry();
     verifier.verifyEntry(entry);
-    verifier.verifyInclusionProof(entry);
   }
 
   @Test
   public void verifyEntry_withInvalidInclusionProof() throws Exception {
     // replace a hash in the inclusion proof to make it bad
-    var invalidResponse = rekorQueryResponse.replace("b4439e", "aaaaaa");
+    var invalidResponse = rekorResponse.replace("b4439e", "aaaaaa");
 
     var response = RekorResponse.newRekorResponse(new URI("https://somewhere"), invalidResponse);
     var verifier = RekorVerifier.newRekorVerifier(trustRoot);
 
     var entry = response.getEntry();
-    verifier.verifyEntry(entry);
-
     var thrown =
         Assertions.assertThrows(
-            RekorVerificationException.class, () -> verifier.verifyInclusionProof(entry));
+            RekorVerificationException.class, () -> verifier.verifyEntry(entry));
+
     MatcherAssert.assertThat(
         thrown.getMessage(),
         CoreMatchers.startsWith(
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/bundles/bundle.v1.no.inclusion.sigstore b/sigstore-java/src/test/resources/dev/sigstore/samples/bundles/bundle.v1.no.inclusion.sigstore
@@ -0,0 +1 @@
+{"mediaType":"application/vnd.dev.sigstore.bundle+json;version=0.1","verificationMaterial":{"x509CertificateChain":{"certificates":[{"rawBytes":"MIICyDCCAk6gAwIBAgIUenpCAKVU2BcGqhnsCe0doys6ibwwCgYIKoZIzj0EAwMwNzEVMBMGA1UEChMMc2lnc3RvcmUuZGV2MR4wHAYDVQQDExVzaWdzdG9yZS1pbnRlcm1lZGlhdGUwHhcNMjMxMDExMTQyODAzWhcNMjMxMDExMTQzODAzWjAAMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEC0NNg9GUBzDJz2+i3Ffl1RiCwBekqVQvaYdjXJu6O7zd0WOofuc9zaBQ3WhE8o3EXH0Y5prD6bGajd2XEaMW6KOCAW0wggFpMA4GA1UdDwEB/wQEAwIHgDATBgNVHSUEDDAKBggrBgEFBQcDAzAdBgNVHQ4EFgQULM+gqZePC3pfVoEclUSrtVPwGwgwHwYDVR0jBBgwFoAU39Ppz1YkEZb5qNjpKFWixi4YZD8wHQYDVR0RAQH/BBMwEYEPYXBwdUBnb29nbGUuY29tMCkGCisGAQQBg78wAQEEG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTArBgorBgEEAYO/MAEIBB0MG2h0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbTCBigYKKwYBBAHWeQIEAgR8BHoAeAB2AN09MGrGxxEyYxkeHJlnNwKiSl643jyt/4eKcoAvKe6OAAABix8jOOQAAAQDAEcwRQIhANeveOTEE7bNqX0UyaSYlSoOXtOntGNsEAO9FUaa/p/XAiBaMNu8aUovaFnoY+e3VIwsLfKoQls17tZbbH6sY+RZQjAKBggqhkjOPQQDAwNoADBlAjB63/BTZjqtZFoKpilCsvEzz4a6eBGf0dbnqhhDS/9ELuKjj2H7jQuNICxqED5p9vECMQDxUugt9pitEpNkOR4gnHdEAS726LUKhCic2QUhwbNJt/+Th+O7MyvyfoDjdlgGGMI="}]},"tlogEntries":[{"logIndex":"41958702","logId":{"keyId":"wNI9atQGlz+VWfO6LRygH4QUfY/8W4RFwiT5i5WRgB0="},"kindVersion":{"kind":"hashedrekord","version":"0.0.1"},"integratedTime":"1697034484","inclusionPromise":{"signedEntryTimestamp":"MEUCIFJST52F75FnhiyWApcgiQWgszyb/rf6J5wIRHEFb6LiAiEAgK/el1WsJLdpoRn0Pp9np9LPbY4ebQyyX03j8325Q48="},"canonicalizedBody":"eyJhcGlWZXJzaW9uIjoiMC4wLjEiLCJraW5kIjoiaGFzaGVkcmVrb3JkIiwic3BlYyI6eyJkYXRhIjp7Imhhc2giOnsiYWxnb3JpdGhtIjoic2hhMjU2IiwidmFsdWUiOiJhMGNmYzcxMjcxZDZlMjc4ZTU3Y2QzMzJmZjk1N2MzZjcwNDNmZGRhMzU0YzRjYmIxOTBhMzBkNTZlZmEwMWJmIn19LCJzaWduYXR1cmUiOnsiY29udGVudCI6Ik1FVUNJRG9xaGp4NWJ6Z1EwS3dNRDFtNzJDTGR4bTZMRHNhWU9oWE94L1NkS0NiRkFpRUExalY0V3kxUkhSN0pWS1FHUmZ5UGpLQTVzRUhWb0M0VUJnRHhpZjJ2Nmx3PSIsInB1YmxpY0tleSI6eyJjb250ZW50IjoiTFMwdExTMUNSVWRKVGlCRFJWSlVTVVpKUTBGVVJTMHRMUzB0Q2sxSlNVTjVSRU5EUVdzMlowRjNTVUpCWjBsVlpXNXdRMEZMVmxVeVFtTkhjV2h1YzBObE1HUnZlWE0yYVdKM2QwTm5XVWxMYjFwSmVtb3dSVUYzVFhjS1RucEZWazFDVFVkQk1WVkZRMmhOVFdNeWJHNWpNMUoyWTIxVmRWcEhWakpOVWpSM1NFRlpSRlpSVVVSRmVGWjZZVmRrZW1SSE9YbGFVekZ3WW01U2JBcGpiVEZzV2tkc2FHUkhWWGRJYUdOT1RXcE5lRTFFUlhoTlZGRjVUMFJCZWxkb1kwNU5hazE0VFVSRmVFMVVVWHBQUkVGNlYycEJRVTFHYTNkRmQxbElDa3R2V2tsNmFqQkRRVkZaU1V0dldrbDZhakJFUVZGalJGRm5RVVZETUU1T1p6bEhWVUo2UkVwNk1pdHBNMFptYkRGU2FVTjNRbVZyY1ZaUmRtRlpaR29LV0VwMU5rODNlbVF3VjA5dlpuVmpPWHBoUWxFelYyaEZPRzh6UlZoSU1GazFjSEpFTm1KSFlXcGtNbGhGWVUxWE5rdFBRMEZYTUhkblowWndUVUUwUndwQk1WVmtSSGRGUWk5M1VVVkJkMGxJWjBSQlZFSm5UbFpJVTFWRlJFUkJTMEpuWjNKQ1owVkdRbEZqUkVGNlFXUkNaMDVXU0ZFMFJVWm5VVlZNVFN0bkNuRmFaVkJETTNCbVZtOUZZMnhWVTNKMFZsQjNSM2RuZDBoM1dVUldVakJxUWtKbmQwWnZRVlV6T1ZCd2VqRlphMFZhWWpWeFRtcHdTMFpYYVhocE5Ga0tXa1E0ZDBoUldVUldVakJTUVZGSUwwSkNUWGRGV1VWUVdWaENkMlJWUW01aU1qbHVZa2RWZFZreU9YUk5RMnRIUTJselIwRlJVVUpuTnpoM1FWRkZSUXBITW1nd1pFaENlazlwT0haWlYwNXFZak5XZFdSSVRYVmFNamwyV2pKNGJFeHRUblppVkVGeVFtZHZja0puUlVWQldVOHZUVUZGU1VKQ01FMUhNbWd3Q21SSVFucFBhVGgyV1ZkT2FtSXpWblZrU0UxMVdqSTVkbG95ZUd4TWJVNTJZbFJEUW1sbldVdExkMWxDUWtGSVYyVlJTVVZCWjFJNFFraHZRV1ZCUWpJS1FVNHdPVTFIY2tkNGVFVjVXWGhyWlVoS2JHNU9kMHRwVTJ3Mk5ETnFlWFF2TkdWTFkyOUJka3RsTms5QlFVRkNhWGc0YWs5UFVVRkJRVkZFUVVWamR3cFNVVWxvUVU1bGRtVlBWRVZGTjJKT2NWZ3dWWGxoVTFsc1UyOVBXSFJQYm5SSFRuTkZRVTg1UmxWaFlTOXdMMWhCYVVKaFRVNTFPR0ZWYjNaaFJtNXZDbGtyWlROV1NYZHpUR1pMYjFGc2N6RTNkRnBpWWtnMmMxa3JVbHBSYWtGTFFtZG5jV2hyYWs5UVVWRkVRWGRPYjBGRVFteEJha0kyTXk5Q1ZGcHFjWFFLV2tadlMzQnBiRU56ZGtWNmVqUmhObVZDUjJZd1pHSnVjV2hvUkZNdk9VVk1kVXRxYWpKSU4ycFJkVTVKUTNoeFJVUTFjRGwyUlVOTlVVUjRWWFZuZEFvNWNHbDBSWEJPYTA5U05HZHVTR1JGUVZNM01qWk1WVXRvUTJsak1sRlZhSGRpVGtwMEx5dFVhQ3RQTjAxNWRubG1iMFJxWkd4blIwZE5TVDBLTFMwdExTMUZUa1FnUTBWU1ZFbEdTVU5CVkVVdExTMHRMUW89In19fX0="}]},"messageSignature":{"messageDigest":{"algorithm":"SHA2_256","digest":"oM/HEnHW4njlfNMy/5V8P3BD/do1TEy7GQow1W76Ab8="},"signature":"MEUCIDoqhjx5bzgQ0KwMD1m72CLdxm6LDsaYOhXOx/SdKCbFAiEA1jV4Wy1RHR7JVKQGRfyPjKA5sEHVoC4UBgDxif2v6lw="}}
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/bundles/bundle.v1.sigstore b/sigstore-java/src/test/resources/dev/sigstore/samples/bundles/bundle.v1.sigstore
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/entry-no-inclusion-proof.json b/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/entry-no-inclusion-proof.json
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/entry.json b/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/entry.json
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/jar-entry.json b/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/jar-entry.json
diff --git a/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/jar-response.json b/sigstore-java/src/test/resources/dev/sigstore/samples/rekor-response/valid/jar-response.json

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,6 @@ public static void fuzzerTestOneInput(FuzzedDataProvider data) {`
`40`	`40`	`RekorVerifier verifier = RekorVerifier.newRekorVerifier(tLogs);`
`41`	`41`
`42`	`42`	`verifier.verifyEntry(entry);`
`43`		`- verifier.verifyInclusionProof(entry);`
`44`	`43`	`} catch (URISyntaxException \| RekorParseException \| RekorVerificationException e) {`
`45`	`44`	`// Known exception`
`46`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,8 @@ interface Verification {`
`37`	`37`	`/** Return the signed entry timestamp. */`
`38`	`38`	`String getSignedEntryTimestamp();`
`39`	`39`
`40`		`- Optional<InclusionProof> getInclusionProof();`
	`40`	`+ /** Return the inclusion proof. */`
	`41`	`+ InclusionProof getInclusionProof();`
`41`	`42`	`}`
`42`	`43`
`43`	`44`	`/**`