From 1eb33e1528ff6c6f6c22df17e940a93c2a37b5f5 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Thu, 26 Oct 2023 11:52:14 -0400 Subject: [PATCH] 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 --- .github/workflows/conformance.yml | 2 +- .../src/main/java/dev/sigstore/cli/Sign.java | 5 +- .../main/java/dev/sigstore/cli/Verify.java | 2 +- .../sigstore/KeylessVerificationRequest.java | 10 ++- .../java/dev/sigstore/KeylessVerifier.java | 63 ++----------------- .../test/java/dev/sigstore/KeylessTest.java | 1 - .../dev/sigstore/KeylessVerifierTest.java | 30 +++------ 7 files changed, 26 insertions(+), 87 deletions(-) diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 59e8d249..b4e6eea4 100644 --- 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 index 4a0f7658..ef30145b 100644 --- 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 { @Parameters(arity = "1", paramLabel = "", 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 index 8b25af9b..5f76355c 100644 --- 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 index 2e9aead5..15c2559e 100644 --- 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 getCertificateIdentities(); diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java index 16e4f3e9..a462e349 100644 --- 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 index 4fd66012..140a326e 100644 --- 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 index 6127a61d..82108100 100644 --- 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,