update to conformance 0.8.0 and rework online mode

If no entry is provided during verification, one will always be requested from remote rekor If an entry is provided, one will only be requested from rekor if explicitly instructed to do so. And if a remote entry is requested, the provided entry is completely ignored. Signed-off-by: Appu Goundan <[email protected]>
sigstore · Oct 26, 2023 · 1eb33e1 · 1eb33e1
1 parent 93681d9
commit 1eb33e1
Show file tree

Hide file tree

Showing 7 changed files with 26 additions and 87 deletions.
diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml
@@ -31,6 +31,6 @@ jobs:
       - name: Unpack sigstore-java distribution
         run: tar -xvf ${{ github.workspace }}/sigstore-cli/build/distributions/sigstore-cli-*.tar --strip-components 1
 
-      - uses: sigstore/sigstore-conformance@1abc82cdefe80bd907855d8447f903ba8b4918e0 # v0.0.6
+      - uses: sigstore/sigstore-conformance@00922385de455be5ec46288a947044aa44fb0981 # v0.0.8
         with:
           entrypoint: ${{ github.workspace }}/bin/sigstore-cli
diff --git a/sigstore-cli/src/main/java/dev/sigstore/cli/Sign.java b/sigstore-cli/src/main/java/dev/sigstore/cli/Sign.java
@@ -28,7 +28,10 @@
 import picocli.CommandLine.Option;
 import picocli.CommandLine.Parameters;
 
-@Command(name = "sign", description = "sign an artifacts")
+@Command(
+    name = "sign",
+    aliases = {"sign-bundle"},
+    description = "sign an artifacts")
 public class Sign implements Callable<Integer> {
 
   @Parameters(arity = "1", paramLabel = "<artifact>", description = "artifact to sign")

diff --git a/sigstore-cli/src/main/java/dev/sigstore/cli/Verify.java b/sigstore-cli/src/main/java/dev/sigstore/cli/Verify.java
@@ -89,7 +89,7 @@ public Integer call() throws Exception {
               .subjectAlternativeName(policy.certificateSan)
               .build());
     }
-    var verificationOptions = verificationOptionsBuilder.isOnline(true).build();
+    var verificationOptions = verificationOptionsBuilder.alwaysUseRemoteRekorEntry(false).build();
 
     var verifier = new KeylessVerifier.Builder().sigstorePublicDefaults().build();
     verifier.verify(

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerificationRequest.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerificationRequest.java
@@ -26,12 +26,18 @@ public interface KeylessVerificationRequest {
 
   @Default
   default VerificationOptions getVerificationOptions() {
-    return VerificationOptions.builder().isOnline(true).build();
+    return VerificationOptions.builder().alwaysUseRemoteRekorEntry(false).build();
   }
 
   @Immutable
   interface VerificationOptions {
-    boolean isOnline();
+
+    /**
+     * Connect to rekor to obtain a rekor entry for verification. This will ignore the provided
+     * rekor entry in a {@link KeylessSignature}. Verifier may still connect to Rekor to obtain an
+     * entry if no {@link KeylessSignature#getEntry()} is empty.
+     */
+    boolean alwaysUseRemoteRekorEntry();
 
     List<CertificateIdentity> getCertificateIdentities();
 

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java
@@ -18,7 +18,6 @@
 import com.google.api.client.util.Preconditions;
 import com.google.common.hash.Hashing;
 import com.google.common.io.Files;
-import dev.sigstore.KeylessVerificationRequest.VerificationOptions;
 import dev.sigstore.encryption.certificates.Certificates;
 import dev.sigstore.encryption.signers.Verifiers;
 import dev.sigstore.fulcio.client.FulcioCertificateVerifier;
@@ -90,35 +89,6 @@ public Builder sigstoreStagingDefaults() throws IOException {
     }
   }
 
-  /**
-   * Verify that the inputs can attest to the validity of a signature using sigstore's keyless
-   * infrastructure. If no exception is thrown, it should be assumed verification has passed.
-   *
-   * @param artifactDigest the sha256 digest of the artifact that was signed
-   * @param certChain the certificate chain obtained from a fulcio instance
-   * @param signature the signature on the artifact
-   * @throws KeylessVerificationException if the signing information could not be verified
-   */
-  @Deprecated
-  public void verifyOnline(byte[] artifactDigest, byte[] certChain, byte[] signature)
-      throws KeylessVerificationException {
-    try {
-      verify(
-          artifactDigest,
-          KeylessVerificationRequest.builder()
-              .keylessSignature(
-                  KeylessSignature.builder()
-                      .signature(signature)
-                      .certPath(Certificates.fromPemChain(certChain))
-                      .digest(artifactDigest)
-                      .build())
-              .verificationOptions(VerificationOptions.builder().isOnline(true).build())
-              .build());
-    } catch (CertificateException ex) {
-      throw new KeylessVerificationException("Certificate was not valid: " + ex.getMessage(), ex);
-    }
-  }
-
   /** Convenience wrapper around {@link #verify(byte[], KeylessVerificationRequest)}. */
   public void verify(Path artifact, KeylessVerificationRequest request)
       throws KeylessVerificationException {
@@ -181,34 +151,11 @@ public void verify(byte[] artifactDigest, KeylessVerificationRequest request)
 
     var signature = request.getKeylessSignature().getSignature();
 
-    // Logic is a bit convoluted for obtaining rekor entry for further processing
-    // 1. if we're in "online mode":
-    //   a. grab the entry from rekor remote to use for verification
-    //   b. if an entry was also provided directly to this library, verify it is valid and the
-    //      same signable content as the one we obtained from rekor. SETs will be different
-    //      because rekor can generate those using a non-idempotent signer, but all signatures
-    //      should still be valid
-    // 2. if we're in offline mode, ensure an entry was provided
-
     RekorEntry rekorEntry;
-
-    if (request.getVerificationOptions().isOnline()) {
+    if (request.getVerificationOptions().alwaysUseRemoteRekorEntry()
+        || request.getKeylessSignature().getEntry().isEmpty()) {
+      // this setting means we ignore any provided entry
       rekorEntry = getEntryFromRekor(artifactDigest, leafCert, signature);
-      if (request.getKeylessSignature().getEntry().isPresent()) {
-        var provided = request.getKeylessSignature().getEntry().get();
-        if (!Arrays.equals(
-            rekorEntry.getSignableContent(),
-            request.getKeylessSignature().getEntry().get().getSignableContent())) {
-          throw new KeylessVerificationException(
-              "Entry obtained from rekor does not match provided entry");
-        }
-        // verify the provided rekor entry is valid even if we are in online mode
-        try {
-          rekorVerifier.verifyEntry(provided);
-        } catch (RekorVerificationException ex) {
-          throw new KeylessVerificationException("Rekor entry signature was not valid");
-        }
-      }
     } else {
       rekorEntry =
           request
@@ -234,8 +181,8 @@ public void verify(byte[] artifactDigest, KeylessVerificationRequest request)
       } catch (RekorVerificationException ex) {
         throw new KeylessVerificationException("Rekor entry inclusion proof was not valid");
       }
-    } else if (request.getVerificationOptions().isOnline()) {
-      throw new KeylessVerificationException("Fetched rekor entry did not contain inclusion proof");
+    } 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

diff --git a/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java b/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java
@@ -67,7 +67,6 @@ public void sign_production() throws Exception {
     verifySigningResult(results);
 
     var verifier = KeylessVerifier.builder().sigstorePublicDefaults().build();
-
     for (int i = 0; i < results.size(); i++) {
       verifier.verify(
           artifactDigests.get(i),

diff --git a/sigstore-java/src/test/java/dev/sigstore/KeylessVerifierTest.java b/sigstore-java/src/test/java/dev/sigstore/KeylessVerifierTest.java
@@ -22,29 +22,12 @@
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
 public class KeylessVerifierTest {
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  public void testVerify(boolean isOnline) throws Exception {
-    var bundleFile =
-        Resources.toString(
-            Resources.getResource("dev/sigstore/samples/bundles/bundle.sigstore"),
-            StandardCharsets.UTF_8);
-    var artifact = Resources.getResource("dev/sigstore/samples/bundles/artifact.txt").getPath();
-
-    var verifier = KeylessVerifier.builder().sigstorePublicDefaults().build();
-    var verificationReq =
-        KeylessVerificationRequest.builder()
-            .keylessSignature(BundleFactory.readBundle(new StringReader(bundleFile)))
-            .verificationOptions(VerificationOptions.builder().isOnline(isOnline).build())
-            .build();
-    verifier.verify(Path.of(artifact), verificationReq);
-  }
-
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   public void testVerify_noDigestInBundle(boolean isOnline) throws Exception {
@@ -58,14 +41,14 @@ public void testVerify_noDigestInBundle(boolean isOnline) throws Exception {
     var verificationReq =
         KeylessVerificationRequest.builder()
             .keylessSignature(BundleFactory.readBundle(new StringReader(bundleFile)))
-            .verificationOptions(VerificationOptions.builder().isOnline(isOnline).build())
+            .verificationOptions(
+                VerificationOptions.builder().alwaysUseRemoteRekorEntry(isOnline).build())
             .build();
     verifier.verify(Path.of(artifact), verificationReq);
   }
 
-  @ParameterizedTest
-  @ValueSource(booleans = {true, false})
-  public void testVerify_mismatchedSet(boolean isOnline) throws Exception {
+  @Test
+  public void testVerify_mismatchedSet() throws Exception {
     // a bundle file where the SET is replaced with a valid SET for another artifact
     var bundleFile =
         Resources.toString(
@@ -78,7 +61,8 @@ public void testVerify_mismatchedSet(boolean isOnline) throws Exception {
     var verificationReq =
         KeylessVerificationRequest.builder()
             .keylessSignature(BundleFactory.readBundle(new StringReader(bundleFile)))
-            .verificationOptions(VerificationOptions.builder().isOnline(isOnline).build())
+            .verificationOptions(
+                VerificationOptions.builder().alwaysUseRemoteRekorEntry(false).build())
             .build();
     Assertions.assertThrows(
         KeylessVerificationException.class,