Merge pull request #523 from sigstore/handle-leaf-certs

Update signing result to store leaf certs only
sigstore · Sep 19, 2023 · 2a3ca6b · 2a3ca6b
2 parents fcd554b + 1f94cff
commit 2a3ca6b
Show file tree

Hide file tree

Showing 15 changed files with 164 additions and 439 deletions.
diff --git a/fuzzing/src/main/java/fuzzing/FulcioVerifierFuzzer.java b/fuzzing/src/main/java/fuzzing/FulcioVerifierFuzzer.java
@@ -18,11 +18,11 @@
 import com.code_intelligence.jazzer.api.FuzzedDataProvider;
 import dev.sigstore.fulcio.client.FulcioVerificationException;
 import dev.sigstore.fulcio.client.FulcioVerifier;
-import dev.sigstore.fulcio.client.SigningCertificate;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.security.InvalidAlgorithmParameterException;
 import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertPath;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
@@ -34,33 +34,19 @@
 public class FulcioVerifierFuzzer {
   public static void fuzzerTestOneInput(FuzzedDataProvider data) {
     try {
-      int[] intArray = data.consumeInts(data.consumeInt(1, 10));
-
       var cas = Tuf.certificateAuthoritiesFrom(data);
       var ctLogs = Tuf.transparencyLogsFrom(data);
 
-      byte[] byteArray = data.consumeRemainingAsBytes();
-      List<Certificate> certList = new ArrayList<Certificate>();
+      List<Certificate> certList = new ArrayList<>();
       CertificateFactory cf = CertificateFactory.getInstance("X.509");
-      certList.add(cf.generateCertificate(new ByteArrayInputStream(byteArray)));
-      certList.add(cf.generateCertificate(new ByteArrayInputStream(byteArray)));
+      certList.add(cf.generateCertificate(new ByteArrayInputStream(data.consumeBytes(10240))));
+      certList.add(
+          cf.generateCertificate(new ByteArrayInputStream(data.consumeRemainingAsBytes())));
 
-      SigningCertificate sc = SigningCertificate.from(cf.generateCertPath(certList));
+      CertPath sc = cf.generateCertPath(certList);
       FulcioVerifier fv = FulcioVerifier.newFulcioVerifier(cas, ctLogs);
 
-      for (int choice : intArray) {
-        switch (choice % 4) {
-          case 0:
-            sc.getCertificates();
-            break;
-          case 1:
-            sc.getLeafCertificate();
-            break;
-          case 3:
-            fv.verifySigningCertificate(sc);
-            break;
-        }
-      }
+      fv.verifySigningCertificate(sc);
     } catch (CertificateException
         | FulcioVerificationException
         | InvalidKeySpecException

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessSignature.java b/sigstore-java/src/main/java/dev/sigstore/KeylessSignature.java
@@ -25,7 +25,11 @@ public interface KeylessSignature {
   /** The sha256 hash digest of the artifact */
   byte[] getDigest();
 
-  /** The full certificate chain provided by fulcio for the public key used to sign the artifact */
+  /**
+   * The partial certificate chain provided by fulcio for the public key and identity used to sign
+   * the artifact, this should NOT contain the trusted root or any trusted intermediates. But users
+   * of this object should understand that older signatures may include the full chain.
+   */
   CertPath getCertPath();
 
   /** The signature over the artifact */

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java b/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java
@@ -29,7 +29,6 @@
 import dev.sigstore.fulcio.client.FulcioClient;
 import dev.sigstore.fulcio.client.FulcioVerificationException;
 import dev.sigstore.fulcio.client.FulcioVerifier;
-import dev.sigstore.fulcio.client.SigningCertificate;
 import dev.sigstore.fulcio.client.UnsupportedAlgorithmException;
 import dev.sigstore.oidc.client.OidcClients;
 import dev.sigstore.oidc.client.OidcException;
@@ -47,6 +46,7 @@
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.security.SignatureException;
+import java.security.cert.CertPath;
 import java.security.cert.CertificateException;
 import java.security.spec.InvalidKeySpecException;
 import java.time.Duration;
@@ -82,7 +82,7 @@ public class KeylessSigner implements AutoCloseable {
 
   /** The code signing certificate from Fulcio. */
   @GuardedBy("lock")
-  private @Nullable SigningCertificate signingCert;
+  private @Nullable CertPath signingCert;
 
   /**
    * Representation {@link #signingCert} in PEM bytes format. This is used to avoid serializing the
@@ -244,7 +244,7 @@ public List<KeylessSignature> sign(List<byte[]> artifactDigests)
       // However, files might be large, and it might take time to talk to Rekor
       // so we check the certificate expiration here.
       renewSigningCertificate();
-      SigningCertificate signingCert;
+      CertPath signingCert;
       byte[] signingCertPemBytes;
       lock.readLock().lock();
       try {
@@ -266,7 +266,7 @@ public List<KeylessSignature> sign(List<byte[]> artifactDigests)
       result.add(
           KeylessSignature.builder()
               .digest(artifactDigest)
-              .certPath(signingCert.getCertPath())
+              .certPath(signingCert)
               .signature(signature)
               .entry(rekorResponse.getEntry())
               .build());
@@ -284,7 +284,7 @@ private void renewSigningCertificate()
       if (signingCert != null) {
         @SuppressWarnings("JavaUtilDate")
         long lifetimeLeft =
-            signingCert.getLeafCertificate().getNotAfter().getTime() - System.currentTimeMillis();
+            Certificates.getLeaf(signingCert).getNotAfter().getTime() - System.currentTimeMillis();
         if (lifetimeLeft > minSigningCertificateLifetime.toMillis()) {
           // The current certificate is fine, reuse it
           return;
@@ -300,7 +300,7 @@ private void renewSigningCertificate()
       signingCert = null;
       signingCertPemBytes = null;
       OidcToken tokenInfo = oidcClients.getIDToken();
-      SigningCertificate signingCert =
+      CertPath signingCert =
           fulcioClient.signingCertificate(
               CertificateRequest.newCertificateRequest(
                   signer.getPublicKey(),
@@ -311,7 +311,7 @@ private void renewSigningCertificate()
       // allow that to be known
       fulcioVerifier.verifySigningCertificate(signingCert);
       this.signingCert = signingCert;
-      signingCertPemBytes = Certificates.toPemBytes(signingCert.getLeafCertificate());
+      signingCertPemBytes = Certificates.toPemBytes(signingCert);
     } finally {
       lock.writeLock().unlock();
     }

diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java
@@ -24,7 +24,6 @@
 import dev.sigstore.fulcio.client.FulcioCertificateVerifier;
 import dev.sigstore.fulcio.client.FulcioVerificationException;
 import dev.sigstore.fulcio.client.FulcioVerifier;
-import dev.sigstore.fulcio.client.SigningCertificate;
 import dev.sigstore.rekor.client.HashedRekordRequest;
 import dev.sigstore.rekor.client.RekorClient;
 import dev.sigstore.rekor.client.RekorEntry;
@@ -142,8 +141,8 @@ public void verify(Path artifact, KeylessVerificationRequest request)
    */
   public void verify(byte[] artifactDigest, KeylessVerificationRequest request)
       throws KeylessVerificationException {
-    var signingCert = SigningCertificate.from(request.getKeylessSignature().getCertPath());
-    var leafCert = signingCert.getLeafCertificate();
+    var signingCert = request.getKeylessSignature().getCertPath();
+    var leafCert = Certificates.getLeaf(signingCert);
 
     // this ensures the provided artifact digest matches what may have come from a bundle (in
     // keyless signature)

diff --git a/sigstore-java/src/main/java/dev/sigstore/encryption/certificates/Certificates.java b/sigstore-java/src/main/java/dev/sigstore/encryption/certificates/Certificates.java
@@ -140,12 +140,14 @@ public static CertPath toCertPath(Certificate certificate) throws CertificateExc
     return cf.generateCertPath(Collections.singletonList(certificate));
   }
 
-  /** Appends a CertPath to another {@link CertPath} as children. */
-  public static CertPath appendCertPath(CertPath parent, Certificate child)
-      throws CertificateException {
+  /** Appends an CertPath to another {@link CertPath} as children. */
+  public static CertPath append(CertPath parent, CertPath child) throws CertificateException {
     CertificateFactory cf = CertificateFactory.getInstance("X.509");
     List<Certificate> certs =
-        ImmutableList.<Certificate>builder().add(child).addAll(parent.getCertificates()).build();
+        ImmutableList.<Certificate>builder()
+            .addAll(child.getCertificates())
+            .addAll(parent.getCertificates())
+            .build();
     return cf.generateCertPath(certs);
   }
 

diff --git a/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioClient.java b/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioClient.java
@@ -17,8 +17,11 @@
 
 import static dev.sigstore.fulcio.v2.SigningCertificate.CertificateCase.SIGNED_CERTIFICATE_DETACHED_SCT;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.protobuf.ByteString;
+import dev.sigstore.encryption.certificates.Certificates;
 import dev.sigstore.fulcio.v2.CAGrpc;
+import dev.sigstore.fulcio.v2.CertificateChain;
 import dev.sigstore.fulcio.v2.CreateSigningCertificateRequest;
 import dev.sigstore.fulcio.v2.Credentials;
 import dev.sigstore.fulcio.v2.PublicKey;
@@ -28,7 +31,13 @@
 import dev.sigstore.http.ImmutableHttpParams;
 import dev.sigstore.trustroot.CertificateAuthority;
 import dev.sigstore.trustroot.SigstoreTrustedRoot;
+import java.io.ByteArrayInputStream;
+import java.security.cert.CertPath;
 import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
+import java.security.cert.CertificateParsingException;
+import java.security.cert.X509Certificate;
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.concurrent.TimeUnit;
 
@@ -80,9 +89,9 @@ public FulcioClient build() {
    * Request a signing certificate from fulcio.
    *
    * @param request certificate request parameters
-   * @return a {@link SigningCertificate} from fulcio
+   * @return a {@link CertPath} from fulcio
    */
-  public SigningCertificate signingCertificate(CertificateRequest request)
+  public CertPath signingCertificate(CertificateRequest request)
       throws InterruptedException, CertificateException {
     if (!certificateAuthority.isCurrent()) {
       throw new RuntimeException(
@@ -126,9 +135,27 @@ public SigningCertificate signingCertificate(CertificateRequest request)
       if (certs.getCertificateCase() == SIGNED_CERTIFICATE_DETACHED_SCT) {
         throw new CertificateException("Detached SCTs are not supported");
       }
-      return SigningCertificate.newSigningCertificate(certs.getSignedCertificateEmbeddedSct());
+      return Certificates.trimParent(
+          decodeCerts(certs.getSignedCertificateEmbeddedSct().getChain()),
+          certificateAuthority.getCertPath());
     } finally {
       channel.shutdownNow().awaitTermination(5, TimeUnit.SECONDS);
     }
   }
+
+  @VisibleForTesting
+  CertPath decodeCerts(CertificateChain certChain) throws CertificateException {
+    var certificateFactory = CertificateFactory.getInstance("X.509");
+    var certs = new ArrayList<X509Certificate>();
+    if (certChain.getCertificatesCount() == 0) {
+      throw new CertificateParsingException(
+          "no valid PEM certificates were found in response from Fulcio");
+    }
+    for (var cert : certChain.getCertificatesList().asByteStringList()) {
+      certs.add(
+          (X509Certificate)
+              certificateFactory.generateCertificate(new ByteArrayInputStream(cert.toByteArray())));
+    }
+    return certificateFactory.generateCertPath(certs);
+  }
 }
diff --git a/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioVerifier.java b/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioVerifier.java
@@ -44,7 +44,7 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-/** Verifier for fulcio {@link SigningCertificate}. */
+/** Verifier for fulcio generated signing cerificates */
 public class FulcioVerifier {
   private final CertificateAuthorities cas;
   private final TransparencyLogs ctLogs;
@@ -90,25 +90,21 @@ private FulcioVerifier(
   }
 
   @VisibleForTesting
-  void verifySct(SigningCertificate signingCertificate, CertPath rebuiltCert)
-      throws FulcioVerificationException {
+  void verifySct(CertPath fullCertPath) throws FulcioVerificationException {
     if (ctLogs.size() == 0) {
       throw new FulcioVerificationException("No ct logs were provided to verifier");
     }
 
-    if (signingCertificate.getDetachedSct().isPresent()) {
-      throw new FulcioVerificationException(
-          "Detached SCTs are not supported for validating certificates");
-    } else if (signingCertificate.getEmbeddedSct().isPresent()) {
-      verifyEmbeddedScts(rebuiltCert);
+    if (Certificates.getEmbeddedSCTs(Certificates.getLeaf(fullCertPath)).isPresent()) {
+      verifyEmbeddedScts(fullCertPath);
     } else {
       throw new FulcioVerificationException("No valid SCTs were found during verification");
     }
   }
 
-  private void verifyEmbeddedScts(CertPath rebuiltCert) throws FulcioVerificationException {
+  private void verifyEmbeddedScts(CertPath certPath) throws FulcioVerificationException {
     @SuppressWarnings("unchecked")
-    var certs = (List<X509Certificate>) rebuiltCert.getCertificates();
+    var certs = (List<X509Certificate>) certPath.getCertificates();
     CTVerificationResult result;
     try {
       result = ctVerifier.verifySignedCertificateTimestamps(certs, null, null);
@@ -144,22 +140,23 @@ private void verifyEmbeddedScts(CertPath rebuiltCert) throws FulcioVerificationE
    * configured in this validator. Also verify that the leaf certificate contains at least one valid
    * SCT
    *
-   * @param signingCertificate containing the certificate chain
+   * @param signingCertificate containing a certificate chain, this chain should not contain any
+   *     trusted root or trusted intermediates
    * @throws FulcioVerificationException if verification fails for any reason
    */
-  public void verifySigningCertificate(SigningCertificate signingCertificate)
+  public void verifySigningCertificate(CertPath signingCertificate)
       throws FulcioVerificationException, IOException {
-    CertPath reconstructedCert = reconstructValidCertPath(signingCertificate);
-    verifySct(signingCertificate, reconstructedCert);
+    CertPath fullCertPath = validateCertPath(signingCertificate);
+    verifySct(fullCertPath);
   }
 
   /**
    * Find a valid cert path that chains back up to the trusted root certs and reconstruct a
    * certificate path combining the provided un-trusted certs and a known set of trusted and
-   * intermediate certs.
+   * intermediate certs. If a full certificate is provided with a self signed root, this should
+   * attempt to match the root/intermediate with a trusted chain.
    */
-  CertPath reconstructValidCertPath(SigningCertificate signingCertificate)
-      throws FulcioVerificationException {
+  CertPath validateCertPath(CertPath signingCertificate) throws FulcioVerificationException {
     CertPathValidator cpv;
     try {
       cpv = CertPathValidator.getInstance("PKIX");
@@ -170,7 +167,7 @@ CertPath reconstructValidCertPath(SigningCertificate signingCertificate)
           e);
     }
 
-    var leaf = signingCertificate.getLeafCertificate();
+    var leaf = Certificates.getLeaf(signingCertificate);
     var validCAs = cas.find(leaf.getNotBefore().toInstant());
 
     if (validCAs.size() == 0) {
@@ -194,25 +191,35 @@ CertPath reconstructValidCertPath(SigningCertificate signingCertificate)
       // these certs are only valid for 15 minutes, so find a time in the validity period
       @SuppressWarnings("JavaUtilDate")
       Date dateInValidityPeriod =
-          new Date(signingCertificate.getLeafCertificate().getNotBefore().getTime());
+          new Date(Certificates.getLeaf(signingCertificate).getNotBefore().getTime());
       pkixParams.setDate(dateInValidityPeriod);
 
-      CertPath rebuiltCert;
+      CertPath fullCertPath;
       try {
-        // build a cert chain with the root-chain in question and the provided leaf
-        rebuiltCert =
-            Certificates.appendCertPath(ca.getCertPath(), signingCertificate.getLeafCertificate());
+        if (Certificates.isSelfSigned(signingCertificate)) {
+          if (Certificates.containsParent(signingCertificate, ca.getCertPath())) {
+            fullCertPath = signingCertificate;
+          } else {
+            // verification failed because we didn't match to a trusted root
+            caVerificationFailure.put(
+                ca.getUri().toString(), "Trusted root in chain does not match");
+            continue;
+          }
+        } else {
+          // build a cert chain with the root-chain in question and the provided signing certificate
+          fullCertPath = Certificates.append(ca.getCertPath(), signingCertificate);
+        }
 
         // a result is returned here, but we ignore it
-        cpv.validate(rebuiltCert, pkixParams);
+        cpv.validate(fullCertPath, pkixParams);
       } catch (CertPathValidatorException
           | InvalidAlgorithmParameterException
           | CertificateException ve) {
         caVerificationFailure.put(ca.getUri().toString(), ve.getMessage());
         // verification failed
         continue;
       }
-      return rebuiltCert;
+      return fullCertPath;
       // verification passed so just end this method
     }
     String errors =