From 03433f6d02cde8d42306bb5d7bafe64d431fe070 Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Wed, 27 Mar 2024 13:48:24 -0400 Subject: [PATCH] Rework signing initialization Make the signing clients configured with URIs instead of objects from the trusted_root.json. This code still uses the URIs from the trusted_root.json to populate those values, but this is a setup towards using the SigningConfiguration type that is coming in the future. Signed-off-by: Appu Goundan --- .../main/java/dev/sigstore/KeylessSigner.java | 15 ++++--- .../java/dev/sigstore/KeylessVerifier.java | 43 +++++++++++++------ .../sigstore/fulcio/client/FulcioClient.java | 37 +++++----------- .../fulcio/client/FulcioVerifier.java | 10 +++++ .../sigstore/rekor/client/RekorClient.java | 34 ++++++--------- .../fulcio/client/FulcioClientTest.java | 23 +--------- .../rekor/client/RekorClientTest.java | 15 +------ 7 files changed, 77 insertions(+), 100 deletions(-) diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java b/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java index 5f000241..132ce2c7 100644 --- a/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java +++ b/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java @@ -191,9 +191,11 @@ public KeylessSigner build() Preconditions.checkNotNull(sigstoreTufClient, "sigstoreTufClient"); sigstoreTufClient.update(); var trustedRoot = sigstoreTufClient.getSigstoreTrustedRoot(); - var fulcioClient = FulcioClient.builder().setCertificateAuthority(trustedRoot).build(); + var fulcioClient = + FulcioClient.builder().setUri(trustedRoot.getCAs().current().getUri()).build(); var fulcioVerifier = FulcioVerifier.newFulcioVerifier(trustedRoot); - var rekorClient = RekorClient.builder().setTransparencyLog(trustedRoot).build(); + var rekorClient = + RekorClient.builder().setUri(trustedRoot.getTLogs().current().getBaseUrl()).build(); var rekorVerifier = RekorVerifier.newRekorVerifier(trustedRoot); return new KeylessSigner( fulcioClient, @@ -354,7 +356,7 @@ private void renewSigningCertificate() } } - CertPath signingCert = + CertPath renewedSigningCert = fulcioClient.signingCertificate( CertificateRequest.newCertificateRequest( signer.getPublicKey(), @@ -363,8 +365,11 @@ private void renewSigningCertificate() tokenInfo.getSubjectAlternativeName().getBytes(StandardCharsets.UTF_8)))); // TODO: this signing workflow mandates SCTs, but fulcio itself doesn't, figure out a way to // allow that to be known - fulcioVerifier.verifySigningCertificate(signingCert); - this.signingCert = signingCert; + + var trimmed = fulcioVerifier.trimTrustedParent(renewedSigningCert); + + fulcioVerifier.verifySigningCertificate(trimmed); + this.signingCert = trimmed; 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 index 6aa2f874..211bf927 100644 --- a/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java +++ b/sigstore-java/src/main/java/dev/sigstore/KeylessVerifier.java @@ -29,6 +29,7 @@ import dev.sigstore.rekor.client.RekorParseException; import dev.sigstore.rekor.client.RekorVerificationException; import dev.sigstore.rekor.client.RekorVerifier; +import dev.sigstore.trustroot.TransparencyLog; import dev.sigstore.tuf.SigstoreTufClient; import java.io.IOException; import java.nio.file.Path; @@ -36,27 +37,31 @@ import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; import java.security.SignatureException; -import java.security.cert.Certificate; 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.Optional; +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; - private final RekorClient rekorClient; + + // a client per remote trusted log + private final List rekorClients; private KeylessVerifier( - FulcioVerifier fulcioVerifier, RekorClient rekorClient, RekorVerifier rekorVerifier) { + FulcioVerifier fulcioVerifier, List rekorClients, RekorVerifier rekorVerifier) { this.fulcioVerifier = fulcioVerifier; - this.rekorClient = rekorClient; this.rekorVerifier = rekorVerifier; + this.rekorClients = rekorClients; } public static KeylessVerifier.Builder builder() { @@ -72,9 +77,14 @@ public KeylessVerifier build() Preconditions.checkNotNull(trustedRootProvider); var trustedRoot = trustedRootProvider.get(); var fulcioVerifier = FulcioVerifier.newFulcioVerifier(trustedRoot); - var rekorClient = RekorClient.builder().setTransparencyLog(trustedRoot).build(); var rekorVerifier = RekorVerifier.newRekorVerifier(trustedRoot); - return new KeylessVerifier(fulcioVerifier, rekorClient, rekorVerifier); + var rekorClients = + trustedRoot.getTLogs().getTransparencyLogs().stream() + .map(TransparencyLog::getBaseUrl) + .distinct() + .map(uri -> RekorClient.builder().setUri(uri).build()) + .collect(Collectors.toList()); + return new KeylessVerifier(fulcioVerifier, rekorClients, rekorVerifier); } public Builder sigstorePublicDefaults() throws IOException { @@ -207,7 +217,7 @@ public void verify(byte[] artifactDigest, KeylessVerificationRequest request) } private RekorEntry getEntryFromRekor( - byte[] artifactDigest, Certificate leafCert, byte[] signature) + byte[] artifactDigest, X509Certificate leafCert, byte[] signature) throws KeylessVerificationException { // rebuild the hashedRekord so we can query the log for it HashedRekordRequest hashedRekordRequest = null; @@ -221,15 +231,24 @@ private RekorEntry getEntryFromRekor( } Optional rekorEntry; - // attempt to grab the rekord from the rekor instance + // attempt to grab a valid rekord from all known rekor instances try { - rekorEntry = rekorClient.getEntry(hashedRekordRequest); - if (rekorEntry.isEmpty()) { - throw new KeylessVerificationException("Rekor entry was not found"); + for (var rekorClient : rekorClients) { + rekorEntry = rekorClient.getEntry(hashedRekordRequest); + if (rekorEntry.isPresent()) { + var entryTime = Date.from(rekorEntry.get().getIntegratedTimeInstant()); + try { + // only return this entry if it's valid for the certificate + leafCert.checkValidity(entryTime); + } catch (CertificateExpiredException | CertificateNotYetValidException ex) { + continue; + } + return rekorEntry.get(); + } } } catch (IOException | RekorParseException e) { throw new KeylessVerificationException("Could not retrieve rekor entry", e); } - return rekorEntry.get(); + throw new KeylessVerificationException("No valid rekor entry was not found in any known logs"); } } 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 index db74f6f3..ad3e5239 100644 --- a/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioClient.java +++ b/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioClient.java @@ -19,7 +19,6 @@ 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; @@ -29,9 +28,8 @@ import dev.sigstore.http.GrpcChannels; import dev.sigstore.http.HttpParams; import dev.sigstore.http.ImmutableHttpParams; -import dev.sigstore.trustroot.CertificateAuthority; -import dev.sigstore.trustroot.SigstoreTrustedRoot; import java.io.ByteArrayInputStream; +import java.net.URI; import java.security.cert.CertPath; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; @@ -45,19 +43,19 @@ public class FulcioClient { private final HttpParams httpParams; - private final CertificateAuthority certificateAuthority; + private final URI uri; public static Builder builder() { return new Builder(); } - private FulcioClient(HttpParams httpParams, CertificateAuthority certificateAuthority) { - this.certificateAuthority = certificateAuthority; + private FulcioClient(HttpParams httpParams, URI uri) { + this.uri = uri; this.httpParams = httpParams; } public static class Builder { - private CertificateAuthority certificateAuthority; + private URI uri = URI.create("https://fulcio.sigstore.dev"); private HttpParams httpParams = ImmutableHttpParams.builder().build(); private Builder() {} @@ -68,20 +66,14 @@ public Builder setHttpParams(HttpParams httpParams) { return this; } - /** The remote fulcio instance. */ - public Builder setCertificateAuthority(CertificateAuthority certificateAuthority) { - this.certificateAuthority = certificateAuthority; - return this; - } - - /** The remote fulcio instance inferred from a trustedRoot. */ - public Builder setCertificateAuthority(SigstoreTrustedRoot trustedRoot) { - this.certificateAuthority = trustedRoot.getCAs().current(); + /** Base url of the remote fulcio instance. */ + public Builder setUri(URI uri) { + this.uri = uri; return this; } public FulcioClient build() { - return new FulcioClient(httpParams, certificateAuthority); + return new FulcioClient(httpParams, uri); } } @@ -93,16 +85,11 @@ public FulcioClient build() { */ public CertPath signingCertificate(CertificateRequest request) throws InterruptedException, CertificateException { - if (!certificateAuthority.isCurrent()) { - throw new RuntimeException( - "Certificate Authority '" + certificateAuthority.getUri() + "' is not current"); - } // TODO: 1. If we want to reduce the cost of creating channels/connections, we could try // to make a new connection once per batch of fulcio requests, but we're not really // at that point yet. // TODO: 2. getUri().getAuthority() is potentially prone to error if we don't get a good URI - var channel = - GrpcChannels.newManagedChannel(certificateAuthority.getUri().getAuthority(), httpParams); + var channel = GrpcChannels.newManagedChannel(uri.getAuthority(), httpParams); try { var client = CAGrpc.newBlockingStub(channel); @@ -135,9 +122,7 @@ public CertPath signingCertificate(CertificateRequest request) if (certs.getCertificateCase() == SIGNED_CERTIFICATE_DETACHED_SCT) { throw new CertificateException("Detached SCTs are not supported"); } - return Certificates.trimParent( - decodeCerts(certs.getSignedCertificateEmbeddedSct().getChain()), - certificateAuthority.getCertPath()); + return decodeCerts(certs.getSignedCertificateEmbeddedSct().getChain()); } finally { channel.shutdownNow().awaitTermination(5, TimeUnit.SECONDS); } 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 index 4fbdac72..f41b0a24 100644 --- a/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioVerifier.java +++ b/sigstore-java/src/main/java/dev/sigstore/fulcio/client/FulcioVerifier.java @@ -150,6 +150,16 @@ public void verifySigningCertificate(CertPath signingCertificate) verifySct(fullCertPath); } + public CertPath trimTrustedParent(CertPath signingCertificate) + throws FulcioVerificationException, CertificateException { + for (var ca : cas) { + if (Certificates.containsParent(signingCertificate, ca.getCertPath())) { + return Certificates.trimParent(signingCertificate, ca.getCertPath()); + } + } + throw new FulcioVerificationException("Certificate does not chain to trusted roots"); + } + /** * 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 diff --git a/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorClient.java b/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorClient.java index aad151c9..44dee202 100644 --- a/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorClient.java +++ b/sigstore-java/src/main/java/dev/sigstore/rekor/client/RekorClient.java @@ -22,12 +22,9 @@ import com.google.api.client.http.HttpRequest; import com.google.api.client.http.HttpResponse; import com.google.api.client.http.HttpResponseException; -import com.google.common.base.Preconditions; import dev.sigstore.http.HttpClients; import dev.sigstore.http.HttpParams; import dev.sigstore.http.ImmutableHttpParams; -import dev.sigstore.trustroot.SigstoreTrustedRoot; -import dev.sigstore.trustroot.TransparencyLog; import java.io.IOException; import java.net.URI; import java.util.Arrays; @@ -42,20 +39,20 @@ public class RekorClient { public static final String REKOR_INDEX_SEARCH_PATH = "/api/v1/index/retrieve"; private final HttpParams httpParams; - private final TransparencyLog tlog; + private final URI uri; public static RekorClient.Builder builder() { return new RekorClient.Builder(); } - private RekorClient(HttpParams httpParams, TransparencyLog tlog) { - this.tlog = tlog; + private RekorClient(HttpParams httpParams, URI uri) { + this.uri = uri; this.httpParams = httpParams; } public static class Builder { private HttpParams httpParams = ImmutableHttpParams.builder().build(); - private TransparencyLog tlog; + private URI uri = URI.create("https://rekor.sigstore.dev"); private Builder() {} @@ -65,21 +62,14 @@ public Builder setHttpParams(HttpParams httpParams) { return this; } - /** Configure the remote rekor instance to communicate with. */ - public Builder setTransparencyLog(TransparencyLog tlog) { - this.tlog = tlog; - return this; - } - - /** Configure the remote rekor instance to communicate with, inferred from a trusted root. */ - public Builder setTransparencyLog(SigstoreTrustedRoot trustedRoot) { - this.tlog = trustedRoot.getTLogs().current(); + /** Base url of the remote rekor instance. */ + public Builder setUri(URI uri) { + this.uri = uri; return this; } public RekorClient build() { - Preconditions.checkNotNull(tlog); - return new RekorClient(httpParams, tlog); + return new RekorClient(httpParams, uri); } } @@ -91,7 +81,7 @@ public RekorClient build() { */ public RekorResponse putEntry(HashedRekordRequest hashedRekordRequest) throws IOException, RekorParseException { - URI rekorPutEndpoint = tlog.getBaseUrl().resolve(REKOR_ENTRIES_PATH); + URI rekorPutEndpoint = uri.resolve(REKOR_ENTRIES_PATH); HttpRequest req = HttpClients.newRequestFactory(httpParams) @@ -112,7 +102,7 @@ public RekorResponse putEntry(HashedRekordRequest hashedRekordRequest) resp.parseAsString())); } - URI rekorEntryUri = tlog.getBaseUrl().resolve(resp.getHeaders().getLocation()); + URI rekorEntryUri = uri.resolve(resp.getHeaders().getLocation()); String entry = resp.parseAsString(); return RekorResponse.newRekorResponse(rekorEntryUri, entry); } @@ -123,7 +113,7 @@ public Optional getEntry(HashedRekordRequest hashedRekordRequest) } public Optional getEntry(String UUID) throws IOException, RekorParseException { - URI getEntryURI = tlog.getBaseUrl().resolve(REKOR_ENTRIES_PATH + "/" + UUID); + URI getEntryURI = uri.resolve(REKOR_ENTRIES_PATH + "/" + UUID); HttpRequest req = HttpClients.newRequestFactory(httpParams).buildGetRequest(new GenericUrl(getEntryURI)); req.getHeaders().set("Accept", "application/json"); @@ -149,7 +139,7 @@ public Optional getEntry(String UUID) throws IOException, RekorParse public List searchEntry( String email, String hash, String publicKeyFormat, String publicKeyContent) throws IOException { - URI rekorSearchEndpoint = tlog.getBaseUrl().resolve(REKOR_INDEX_SEARCH_PATH); + URI rekorSearchEndpoint = uri.resolve(REKOR_INDEX_SEARCH_PATH); HashMap publicKeyParams = null; if (publicKeyContent != null) { diff --git a/sigstore-java/src/test/java/dev/sigstore/fulcio/client/FulcioClientTest.java b/sigstore-java/src/test/java/dev/sigstore/fulcio/client/FulcioClientTest.java index c2a39cde..984ad808 100644 --- a/sigstore-java/src/test/java/dev/sigstore/fulcio/client/FulcioClientTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/fulcio/client/FulcioClientTest.java @@ -23,19 +23,11 @@ import dev.sigstore.testing.FulcioWrapper; import dev.sigstore.testing.MockOAuth2ServerExtension; import dev.sigstore.testing.grpc.GrpcTypes; -import dev.sigstore.trustroot.CertificateAuthority; -import dev.sigstore.trustroot.ImmutableCertificateAuthority; -import dev.sigstore.trustroot.ImmutableValidFor; -import dev.sigstore.trustroot.Subject; -import java.net.URI; import java.nio.charset.StandardCharsets; -import java.security.cert.CertPath; import java.security.cert.CertificateException; -import java.time.Instant; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mockito; public class FulcioClientTest { @@ -58,8 +50,7 @@ public void testSigningCert( var client = FulcioClient.builder() .setHttpParams(ImmutableHttpParams.builder().allowInsecureConnections(true).build()) - .setCertificateAuthority( - createCA(fulcioWrapper.getGrpcURI2(), fulcioWrapper.getTrustBundle())) + .setUri(fulcioWrapper.getGrpcURI2()) .build(); var sc = client.signingCertificate(cReq); @@ -89,23 +80,13 @@ public void testSigningCert_NoSct( var client = FulcioClient.builder() .setHttpParams(ImmutableHttpParams.builder().allowInsecureConnections(true).build()) - .setCertificateAuthority( - createCA(fulcioWrapper.getGrpcURI2(), fulcioWrapper.getTrustBundle())) + .setUri(fulcioWrapper.getGrpcURI2()) .build(); var ex = Assertions.assertThrows(CertificateException.class, () -> client.signingCertificate(cReq)); Assertions.assertEquals(ex.getMessage(), "Detached SCTs are not supported"); } - private CertificateAuthority createCA(URI uri, CertPath trustBundle) { - return ImmutableCertificateAuthority.builder() - .uri(uri) - .certPath(trustBundle) - .subject(Mockito.mock(Subject.class)) - .validFor(ImmutableValidFor.builder().start(Instant.EPOCH).build()) - .build(); - } - @Test public void testDecode_embeddedGrpc() throws Exception { var certs = 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 index 32ccc3f4..ae6d5b5f 100644 --- a/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/rekor/client/RekorClientTest.java @@ -23,9 +23,6 @@ import dev.sigstore.encryption.certificates.Certificates; import dev.sigstore.encryption.signers.Signers; import dev.sigstore.testing.CertGenerator; -import dev.sigstore.trustroot.ImmutableTransparencyLog; -import dev.sigstore.trustroot.LogId; -import dev.sigstore.trustroot.PublicKey; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; @@ -44,7 +41,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; public class RekorClientTest { @@ -55,16 +51,7 @@ public class RekorClientTest { public void setupClient() throws URISyntaxException { // this tests directly against rekor in staging, it's a bit hard to bring up a rekor instance // without docker compose. - client = - RekorClient.builder() - .setTransparencyLog( - ImmutableTransparencyLog.builder() - .baseUrl(URI.create(REKOR_URL)) - .hashAlgorithm("ignored") - .publicKey(Mockito.mock(PublicKey.class)) - .logId(Mockito.mock(LogId.class)) - .build()) - .build(); + client = RekorClient.builder().setUri(URI.create(REKOR_URL)).build(); } @Test