From 929a0f6fb85c1c21b98ef095c2bdce7529c1f52c Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Wed, 18 Oct 2023 14:03:24 -0400 Subject: [PATCH] Allow signers to specify allow list of oidc ids - Also signer now throws KeylessSignerException Signed-off-by: Appu Goundan --- .../sigstore/cli/TokenStringOidcClient.java | 1 + .../main/java/dev/sigstore/KeylessSigner.java | 99 ++++++++++++++----- .../dev/sigstore/KeylessSignerException.java | 30 ++++++ .../main/java/dev/sigstore/OidcIdentity.java | 40 ++++++++ .../dev/sigstore/oidc/client/OidcToken.java | 3 + .../sigstore/oidc/client/WebOidcClient.java | 1 + .../test/java/dev/sigstore/KeylessTest.java | 30 ++++++ 7 files changed, 178 insertions(+), 26 deletions(-) create mode 100644 sigstore-java/src/main/java/dev/sigstore/KeylessSignerException.java create mode 100644 sigstore-java/src/main/java/dev/sigstore/OidcIdentity.java diff --git a/sigstore-cli/src/main/java/dev/sigstore/cli/TokenStringOidcClient.java b/sigstore-cli/src/main/java/dev/sigstore/cli/TokenStringOidcClient.java index 98259dc1..ac5629f0 100644 --- a/sigstore-cli/src/main/java/dev/sigstore/cli/TokenStringOidcClient.java +++ b/sigstore-cli/src/main/java/dev/sigstore/cli/TokenStringOidcClient.java @@ -42,6 +42,7 @@ public OidcToken getIDToken() throws OidcException { var jws = JsonWebSignature.parse(new GsonFactory(), idToken); return ImmutableOidcToken.builder() .idToken(idToken) + .issuer(jws.getPayload().getIssuer()) .subjectAlternativeName(jws.getPayload().getSubject()) .build(); } catch (IOException e) { diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java b/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java index e7d7e2c2..fc627a15 100644 --- a/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java +++ b/sigstore-java/src/main/java/dev/sigstore/KeylessSigner.java @@ -36,6 +36,7 @@ import dev.sigstore.rekor.client.HashedRekordRequest; import dev.sigstore.rekor.client.RekorClient; import dev.sigstore.rekor.client.RekorParseException; +import dev.sigstore.rekor.client.RekorResponse; import dev.sigstore.rekor.client.RekorVerificationException; import dev.sigstore.rekor.client.RekorVerifier; import dev.sigstore.tuf.SigstoreTufClient; @@ -51,6 +52,7 @@ import java.security.spec.InvalidKeySpecException; import java.time.Duration; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -77,6 +79,7 @@ public class KeylessSigner implements AutoCloseable { private final RekorClient rekorClient; private final RekorVerifier rekorVerifier; private final OidcClients oidcClients; + private final List oidcIdentities; private final Signer signer; private final Duration minSigningCertificateLifetime; @@ -99,6 +102,7 @@ private KeylessSigner( RekorClient rekorClient, RekorVerifier rekorVerifier, OidcClients oidcClients, + List oidcIdentities, Signer signer, Duration minSigningCertificateLifetime) { this.fulcioClient = fulcioClient; @@ -106,6 +110,7 @@ private KeylessSigner( this.rekorClient = rekorClient; this.rekorVerifier = rekorVerifier; this.oidcClients = oidcClients; + this.oidcIdentities = oidcIdentities; this.signer = signer; this.minSigningCertificateLifetime = minSigningCertificateLifetime; } @@ -129,6 +134,7 @@ public static Builder builder() { public static class Builder { private SigstoreTufClient sigstoreTufClient; private OidcClients oidcClients; + private List oidcIdentities = Collections.emptyList(); private Signer signer; private Duration minSigningCertificateLifetime = DEFAULT_MIN_SIGNING_CERTIFICATE_LIFETIME; @@ -144,6 +150,16 @@ public Builder oidcClients(OidcClients oidcClients) { return this; } + /** + * An allow list OIDC identities to be used during signing. If the OidcClients are misconfigured + * or pick up unexpected credentials, this should prevent signing from proceeding + */ + @CanIgnoreReturnValue + public Builder allowedOidcIdentities(List oidcIdentities) { + this.oidcIdentities = ImmutableList.copyOf(oidcIdentities); + return this; + } + @CanIgnoreReturnValue public Builder signer(Signer signer) { this.signer = signer; @@ -185,6 +201,7 @@ public KeylessSigner build() rekorClient, rekorVerifier, oidcClients, + oidcIdentities, signer, minSigningCertificateLifetime); } @@ -225,11 +242,7 @@ public Builder sigstoreStagingDefaults() throws IOException, NoSuchAlgorithmExce * @return a list of keyless singing results. */ @CheckReturnValue - public List sign(List artifactDigests) - throws OidcException, NoSuchAlgorithmException, SignatureException, InvalidKeyException, - UnsupportedAlgorithmException, CertificateException, IOException, - FulcioVerificationException, RekorVerificationException, InterruptedException, - RekorParseException, InvalidKeySpecException { + public List sign(List artifactDigests) throws KeylessSignerException { if (artifactDigests.size() == 0) { throw new IllegalArgumentException("Require one or more digests"); @@ -238,12 +251,31 @@ public List sign(List artifactDigests) var result = ImmutableList.builder(); for (var artifactDigest : artifactDigests) { - var signature = signer.signDigest(artifactDigest); + byte[] signature; + try { + signature = signer.signDigest(artifactDigest); + } catch (NoSuchAlgorithmException | SignatureException | InvalidKeyException ex) { + throw new KeylessSignerException("Failed to sign artifact", ex); + } // Technically speaking, it is unlikely the certificate will expire between signing artifacts // However, files might be large, and it might take time to talk to Rekor // so we check the certificate expiration here. - renewSigningCertificate(); + try { + renewSigningCertificate(); + } catch (KeylessSignerException + | FulcioVerificationException + | UnsupportedAlgorithmException + | OidcException + | IOException + | InterruptedException + | InvalidKeyException + | NoSuchAlgorithmException + | SignatureException + | CertificateException ex) { + throw new KeylessSignerException("Failed to obtain signing certificate", ex); + } + CertPath signingCert; byte[] signingCertPemBytes; lock.readLock().lock(); @@ -260,8 +292,19 @@ public List sign(List artifactDigests) var rekorRequest = HashedRekordRequest.newHashedRekordRequest( artifactDigest, signingCertPemBytes, signature); - var rekorResponse = rekorClient.putEntry(rekorRequest); - rekorVerifier.verifyEntry(rekorResponse.getEntry()); + + RekorResponse rekorResponse; + try { + rekorResponse = rekorClient.putEntry(rekorRequest); + } catch (RekorParseException | IOException ex) { + throw new KeylessSignerException("Failed to put entry in rekor", ex); + } + + try { + rekorVerifier.verifyEntry(rekorResponse.getEntry()); + } catch (RekorVerificationException ex) { + throw new KeylessSignerException("Failed to validate rekor response after signing", ex); + } result.add( KeylessSignature.builder() @@ -277,7 +320,7 @@ public List sign(List artifactDigests) private void renewSigningCertificate() throws InterruptedException, CertificateException, IOException, UnsupportedAlgorithmException, NoSuchAlgorithmException, InvalidKeyException, SignatureException, - FulcioVerificationException, OidcException { + FulcioVerificationException, OidcException, KeylessSignerException { // Check if the certificate is still valid lock.readLock().lock(); try { @@ -300,6 +343,18 @@ private void renewSigningCertificate() signingCert = null; signingCertPemBytes = null; OidcToken tokenInfo = oidcClients.getIDToken(); + + // check if we have an allow list and if so, ensure the provided token is in there + if (!oidcIdentities.isEmpty()) { + var obtainedToken = OidcIdentity.from(tokenInfo); + if (!oidcIdentities.contains(OidcIdentity.from(tokenInfo))) { + throw new KeylessSignerException( + "Obtained Oidc Token " + + obtainedToken + + " does not match any identities in allow list"); + } + } + CertPath signingCert = fulcioClient.signingCertificate( CertificateRequest.newCertificateRequest( @@ -324,11 +379,7 @@ private void renewSigningCertificate() * @return a keyless singing results. */ @CheckReturnValue - public KeylessSignature sign(byte[] artifactDigest) - throws FulcioVerificationException, RekorVerificationException, UnsupportedAlgorithmException, - CertificateException, NoSuchAlgorithmException, SignatureException, IOException, - OidcException, InvalidKeyException, InterruptedException, RekorParseException, - InvalidKeySpecException { + public KeylessSignature sign(byte[] artifactDigest) throws KeylessSignerException { return sign(List.of(artifactDigest)).get(0); } @@ -339,18 +390,18 @@ public KeylessSignature sign(byte[] artifactDigest) * @return a map of artifacts and their keyless singing results. */ @CheckReturnValue - public Map signFiles(List artifacts) - throws FulcioVerificationException, RekorVerificationException, UnsupportedAlgorithmException, - CertificateException, NoSuchAlgorithmException, SignatureException, IOException, - OidcException, InvalidKeyException, InterruptedException, RekorParseException, - InvalidKeySpecException { + public Map signFiles(List artifacts) throws KeylessSignerException { if (artifacts.size() == 0) { throw new IllegalArgumentException("Require one or more paths"); } var digests = new ArrayList(artifacts.size()); for (var artifact : artifacts) { var artifactByteSource = com.google.common.io.Files.asByteSource(artifact.toFile()); - digests.add(artifactByteSource.hash(Hashing.sha256()).asBytes()); + try { + digests.add(artifactByteSource.hash(Hashing.sha256()).asBytes()); + } catch (IOException ex) { + throw new KeylessSignerException("Failed to hash artifact " + artifact); + } } var signingResult = sign(digests); var result = ImmutableMap.builder(); @@ -367,11 +418,7 @@ public Map signFiles(List artifacts) * @return a keyless singing results. */ @CheckReturnValue - public KeylessSignature signFile(Path artifact) - throws FulcioVerificationException, RekorVerificationException, UnsupportedAlgorithmException, - CertificateException, NoSuchAlgorithmException, SignatureException, IOException, - OidcException, InvalidKeyException, InterruptedException, RekorParseException, - InvalidKeySpecException { + public KeylessSignature signFile(Path artifact) throws KeylessSignerException { return signFiles(List.of(artifact)).get(artifact); } } diff --git a/sigstore-java/src/main/java/dev/sigstore/KeylessSignerException.java b/sigstore-java/src/main/java/dev/sigstore/KeylessSignerException.java new file mode 100644 index 00000000..6c01a8eb --- /dev/null +++ b/sigstore-java/src/main/java/dev/sigstore/KeylessSignerException.java @@ -0,0 +1,30 @@ +/* + * Copyright 2022 The Sigstore Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package dev.sigstore; + +public class KeylessSignerException extends Exception { + public KeylessSignerException(String message) { + super(message); + } + + public KeylessSignerException(String message, Throwable cause) { + super(message, cause); + } + + public KeylessSignerException(Throwable cause) { + super(cause); + } +} diff --git a/sigstore-java/src/main/java/dev/sigstore/OidcIdentity.java b/sigstore-java/src/main/java/dev/sigstore/OidcIdentity.java new file mode 100644 index 00000000..5e6996bc --- /dev/null +++ b/sigstore-java/src/main/java/dev/sigstore/OidcIdentity.java @@ -0,0 +1,40 @@ +/* + * Copyright 2023 The Sigstore Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package dev.sigstore; + +import dev.sigstore.oidc.client.OidcToken; +import org.immutables.value.Value.Immutable; + +@Immutable +public interface OidcIdentity { + + static OidcIdentity of(String identity, String issuer) { + return ImmutableOidcIdentity.builder().identity(identity).issuer(issuer).build(); + } + + static OidcIdentity from(OidcToken oidcToken) { + return ImmutableOidcIdentity.builder() + .identity(oidcToken.getSubjectAlternativeName()) + .issuer(oidcToken.getIssuer()) + .build(); + } + + /** The user or machineId. */ + String getIdentity(); + + /** The oidc issuing authority */ + String getIssuer(); +} diff --git a/sigstore-java/src/main/java/dev/sigstore/oidc/client/OidcToken.java b/sigstore-java/src/main/java/dev/sigstore/oidc/client/OidcToken.java index 01b2c431..e82fe323 100644 --- a/sigstore-java/src/main/java/dev/sigstore/oidc/client/OidcToken.java +++ b/sigstore-java/src/main/java/dev/sigstore/oidc/client/OidcToken.java @@ -23,6 +23,9 @@ public interface OidcToken { /** The subject or email claim from the token to include in the SAN on the certificate. */ String getSubjectAlternativeName(); + /** The issuer of the id token. */ + String getIssuer(); + /** The full oidc token obtained from the provider. */ String getIdToken(); } diff --git a/sigstore-java/src/main/java/dev/sigstore/oidc/client/WebOidcClient.java b/sigstore-java/src/main/java/dev/sigstore/oidc/client/WebOidcClient.java index 8d30a386..067312aa 100644 --- a/sigstore-java/src/main/java/dev/sigstore/oidc/client/WebOidcClient.java +++ b/sigstore-java/src/main/java/dev/sigstore/oidc/client/WebOidcClient.java @@ -193,6 +193,7 @@ public OidcToken getIDToken() throws OidcException { return ImmutableOidcToken.builder() .subjectAlternativeName(emailFromIDToken) .idToken(idTokenString) + .issuer(issuer) .build(); } diff --git a/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java b/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java index 4fd66012..57462162 100644 --- a/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/KeylessTest.java @@ -76,6 +76,36 @@ public void sign_production() throws Exception { } } + @Test + @EnabledIfOidcExists(provider = OidcProviderType.GITHUB) + // this test will only pass on the github.com/sigstore/sigstore-java repository + public void sign_failGithubOidcCheck() throws Exception { + var signer = + KeylessSigner.builder() + .sigstorePublicDefaults() + .allowedOidcIdentities(List.of(OidcIdentity.of("goose@goose.com", "goose.com"))) + .build(); + var ex = + Assertions.assertThrows(KeylessSignerException.class, () -> signer.sign(artifactDigests)); + Assertions.assertEquals( + "Obtained Oidc Token " + + OidcIdentity.of("github machine", "github.com") + + " does not match any identities in allow list", + ex.getMessage()); + } + + @Test + @EnabledIfOidcExists(provider = OidcProviderType.GITHUB) + // this test will only pass on the github.com/sigstore/sigstore-java repository + public void sign_passGithubOidcCheck() throws Exception { + var signer = + KeylessSigner.builder() + .sigstorePublicDefaults() + .allowedOidcIdentities(List.of(OidcIdentity.of("goose@goose.com", "goog"))) + .build(); + signer.sign(artifactDigests); + } + @Test @EnabledIfOidcExists(provider = OidcProviderType.ANY) public void sign_staging() throws Exception {