Add new OID processing

- Add regex to verification options - Correctly parse DER Encoded Strings (vs raw) - Allow arbitrary comparison of bytes Signed-off-by: Appu Goundan <[email protected]>
sigstore · Jun 20, 2024 · c0e7338 · c0e7338
1 parent 2745e36
commit c0e7338
Show file tree

Hide file tree

Showing 10 changed files with 588 additions and 331 deletions.
diff --git a/...zing/FulcioCertificateVerifierFuzzer.java → ...zzing/FulcioCertificateMatcherFuzzer.java b/...zing/FulcioCertificateVerifierFuzzer.java → ...zzing/FulcioCertificateMatcherFuzzer.java
@@ -16,19 +16,20 @@
 package fuzzing;
 
 import com.code_intelligence.jazzer.api.FuzzedDataProvider;
-import dev.sigstore.VerificationOptions.CertificateIdentity;
-import dev.sigstore.fulcio.client.FulcioCertificateVerifier;
-import dev.sigstore.fulcio.client.FulcioVerificationException;
+import dev.sigstore.VerificationOptions.UncheckedCertificateException;
+import dev.sigstore.fulcio.client.FulcioCertificateMatcher;
+import dev.sigstore.fulcio.client.ImmutableFulcioCertificateMatcher;
+import dev.sigstore.strings.StringMatcher;
 import java.io.ByteArrayInputStream;
 import java.nio.charset.Charset;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
-import java.util.List;
 
-public class FulcioCertificateVerifierFuzzer {
+public class FulcioCertificateMatcherFuzzer {
   public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     byte[] byteArray = data.consumeRemainingAsBytes();
-    String string = new String(byteArray, Charset.defaultCharset());
+    String san = new String(byteArray, Charset.defaultCharset());
+    String issuer = new String(byteArray, Charset.defaultCharset());
 
     X509Certificate certificate;
     try {
@@ -40,14 +41,14 @@ public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     }
 
     try {
-      FulcioCertificateVerifier verifier = new FulcioCertificateVerifier();
-      List<CertificateIdentity> list =
-          List.of(
-              CertificateIdentity.builder().subjectAlternativeName(string).issuer(string).build(),
-              CertificateIdentity.builder().subjectAlternativeName(string).issuer(string).build());
+      FulcioCertificateMatcher matcher =
+          ImmutableFulcioCertificateMatcher.builder()
+              .subjectAlternativeName(StringMatcher.string(san))
+              .issuer(StringMatcher.string(issuer))
+              .build();
 
-      verifier.verifyCertificateMatches(certificate, list);
-    } catch (FulcioVerificationException e) {
+      matcher.test(certificate);
+    } catch (UncheckedCertificateException e) {
       // Known exception
     }
   }

diff --git a/sigstore-cli/src/main/java/dev/sigstore/cli/Verify.java b/sigstore-cli/src/main/java/dev/sigstore/cli/Verify.java
@@ -21,13 +21,14 @@
 import dev.sigstore.KeylessVerifier;
 import dev.sigstore.TrustedRootProvider;
 import dev.sigstore.VerificationOptions;
-import dev.sigstore.VerificationOptions.CertificateIdentity;
+import dev.sigstore.VerificationOptions.CertificateMatcher;
 import dev.sigstore.bundle.Bundle;
 import dev.sigstore.bundle.Bundle.HashAlgorithm;
 import dev.sigstore.bundle.Bundle.MessageSignature;
 import dev.sigstore.bundle.ImmutableBundle;
 import dev.sigstore.encryption.certificates.Certificates;
 import dev.sigstore.rekor.client.RekorEntryFetcher;
+import dev.sigstore.strings.StringMatcher;
 import dev.sigstore.tuf.RootProvider;
 import dev.sigstore.tuf.SigstoreTufClient;
 import java.net.URL;
@@ -135,10 +136,10 @@ public Integer call() throws Exception {
 
     var verificationOptionsBuilder = VerificationOptions.builder();
     if (policy != null) {
-      verificationOptionsBuilder.addCertificateIdentities(
-          CertificateIdentity.builder()
-              .issuer(policy.certificateIssuer)
-              .subjectAlternativeName(policy.certificateSan)
+      verificationOptionsBuilder.addCertificateMatchers(
+          CertificateMatcher.fulcio()
+              .issuer(StringMatcher.string(policy.certificateIssuer))
+              .subjectAlternativeName(StringMatcher.string(policy.certificateSan))
               .build());
     }
     var verificationOptions = verificationOptionsBuilder.build();

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java
@@ -16,12 +16,14 @@
 package dev.sigstore;
 
 import com.google.api.client.util.Preconditions;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.hash.Hashing;
 import com.google.common.io.Files;
+import dev.sigstore.VerificationOptions.CertificateMatcher;
+import dev.sigstore.VerificationOptions.UncheckedCertificateException;
 import dev.sigstore.bundle.Bundle;
 import dev.sigstore.encryption.certificates.Certificates;
 import dev.sigstore.encryption.signers.Verifiers;
-import dev.sigstore.fulcio.client.FulcioCertificateVerifier;
 import dev.sigstore.fulcio.client.FulcioVerificationException;
 import dev.sigstore.fulcio.client.FulcioVerifier;
 import dev.sigstore.rekor.client.RekorEntry;
@@ -37,13 +39,17 @@
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateExpiredException;
 import java.security.cert.CertificateNotYetValidException;
+import java.security.cert.X509Certificate;
 import java.security.spec.InvalidKeySpecException;
 import java.sql.Date;
 import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
 import org.bouncycastle.util.encoders.Hex;
 
 /** Verify hashrekords from rekor signed using the keyless signing flow with fulcio certificates. */
 public class KeylessVerifier {
+
   private final FulcioVerifier fulcioVerifier;
   private final RekorVerifier rekorVerifier;
 
@@ -57,6 +63,7 @@ public static KeylessVerifier.Builder builder() {
   }
 
   public static class Builder {
+
     private TrustedRootProvider trustedRootProvider;
 
     public KeylessVerifier build()
@@ -162,15 +169,7 @@ public void verify(byte[] artifactDigest, Bundle bundle, VerificationOptions opt
     }
 
     // verify the certificate identity if options are present
-    if (options.getCertificateIdentities().size() > 0) {
-      try {
-        new FulcioCertificateVerifier()
-            .verifyCertificateMatches(leafCert, options.getCertificateIdentities());
-      } catch (FulcioVerificationException fve) {
-        throw new KeylessVerificationException(
-            "Could not verify certificate identities: " + fve.getMessage(), fve);
-      }
-    }
+    checkCertificateMatchers(leafCert, options.getCertificateMatchers());
 
     var signature = messageSignature.getSignature();
 
@@ -208,4 +207,20 @@ public void verify(byte[] artifactDigest, Bundle bundle, VerificationOptions opt
           "Signature could not be processed: " + ex.getMessage(), ex);
     }
   }
+
+  @VisibleForTesting
+  void checkCertificateMatchers(X509Certificate cert, List<CertificateMatcher> matchers)
+      throws KeylessVerificationException {
+    try {
+      if (matchers.size() > 0 && matchers.stream().noneMatch(matcher -> matcher.test(cert))) {
+        var matcherSpec =
+            matchers.stream().map(Object::toString).collect(Collectors.joining(",", "[", "]"));
+        throw new KeylessVerificationException(
+            "No provided certificate identities matched values in certificate: " + matcherSpec);
+      }
+    } catch (UncheckedCertificateException ce) {
+      throw new KeylessVerificationException(
+          "Could not verify certificate identities: " + ce.getMessage());
+    }
+  }
 }
diff --git a/sigstore-java/src/main/java/dev/sigstore/VerificationOptions.java b/sigstore-java/src/main/java/dev/sigstore/VerificationOptions.java
@@ -15,26 +15,38 @@
  */
 package dev.sigstore;
 
+import dev.sigstore.fulcio.client.FulcioCertificateMatcher;
+import dev.sigstore.fulcio.client.ImmutableFulcioCertificateMatcher;
+import java.security.cert.X509Certificate;
 import java.util.List;
-import java.util.Map;
+import java.util.function.Predicate;
 import org.immutables.value.Value.Immutable;
 
 @Immutable(singleton = true)
 public interface VerificationOptions {
 
   /** An allow list of certificate identities to match with. */
-  List<CertificateIdentity> getCertificateIdentities();
-
-  @Immutable
-  interface CertificateIdentity {
-    String getIssuer();
-
-    String getSubjectAlternativeName();
-
-    Map<String, String> getOther();
+  List<CertificateMatcher> getCertificateMatchers();
+
+  /**
+   * An interface for allowing matching of certificates. Use {@link #fulcio()} to instantiate the
+   * default {@link FulcioCertificateMatcher} implementation. Custom implementations may throw
+   * {@link UncheckedCertificateException} if an error occurs processing the certificate on calls to
+   * {@link #test(X509Certificate)}. Any other runtime exception will not be handled.
+   */
+  interface CertificateMatcher extends Predicate<X509Certificate> {
+    @Override
+    boolean test(X509Certificate certificate) throws UncheckedCertificateException;
+
+    static ImmutableFulcioCertificateMatcher.Builder fulcio() {
+      return ImmutableFulcioCertificateMatcher.builder();
+    }
+  }
 
-    static ImmutableCertificateIdentity.Builder builder() {
-      return ImmutableCertificateIdentity.builder();
+  /** Exceptions thrown by implementations of {@link CertificateMatcher#test(X509Certificate)} */
+  class UncheckedCertificateException extends RuntimeException {
+    public UncheckedCertificateException(String message, Throwable cause) {
+      super(message, cause);
     }
   }