Implement identites, fix bug in webhook validation.

Signed-off-by: Ville Aikas <vaikas@chainguard.dev>

.github/workflows/kind-cluster-image-policy.yaml

@@ -140,7 +140,7 @@ jobs:
         echo '::group:: test job success'
         # We signed this above, this should work
         if ! kubectl create -n demo-keyless-signing job demo --image=${{ env.demoimage }} ; then
-          echo Failed to create Job in namespace without label!
+          echo Failed to create Job in namespace with matching signature!
           exit 1
         else
           echo Succcessfully created Job with signed image
@@ -157,6 +157,44 @@ jobs:
         fi
         echo '::endgroup::'
 
+    - name: Add ClusterImagePolicy with identities that match issuer and subject
+      run: |
+        kubectl apply -f ./test/testdata/cosigned/e2e/cip-keyless-with-identities.yaml
+        # make sure the reconciler has enough time to update the configmap
+        sleep 5
+
+    - name: Verify the job still works with additional constraints
+        echo '::group:: test job success'
+        # This has correct issuer/subject, so should work
+        if ! kubectl create -n demo-keyless-signing job demo-identities-works --image=${{ env.demoimage }} ; then
+          echo Failed to create Job in namespace without label!
+          exit 1
+        else
+          echo Succcessfully created Job with signed image
+        fi
+        echo '::endgroup:: test job success'
+
+    - name: Add ClusterImagePolicy with identities that do not match issuer and subject
+      run: |
+        kubectl apply -f ./test/testdata/cosigned/e2e/cip-keyless-with-identities-mismatch.yaml
+        # make sure the reconciler has enough time to update the configmap
+        sleep 5
+
+    - name: Verify the job now fails because subject and issuer do not match
+        echo '::group:: test job block'
+        if kubectl create -n demo-keyless-signing job demo-identities-works --image=${{ env.demoimage }} ; then
+          echo Failed to block Job in namespace with non matching issuer and subject!
+          exit 1
+        else
+          echo Succcessfully blocked Job with mismatching issuer and subject
+        fi
+        echo '::endgroup:: test job block'
+
+    - name: Remove the mismatching cip
+      run: |
+        kubectl delete cip image-policy-keyless-with-identities-mismatch
+        sleep 5
+
     - name: Generate New Signing Key
       run: |
         COSIGN_PASSWORD="" ./cosign generate-key-pair

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

@@ -115,12 +115,9 @@ func (keyless *KeylessRef) Validate(ctx context.Context) *apis.FieldError {
 		errs = errs.Also(apis.ErrMissingOneOf("url", "identities", "ca-cert"))
 	}
 
-	if keyless.URL != nil {
-		if keyless.CACert != nil || keyless.Identities != nil {
-			errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-cert"))
-		}
-	} else if keyless.CACert != nil && keyless.Identities != nil {
-		errs = errs.Also(apis.ErrMultipleOneOf("url", "identities", "ca-cert"))
+	// TODO: Are these really mutually exclusive?
+	if keyless.URL != nil && keyless.CACert != nil {
+		errs = errs.Also(apis.ErrMultipleOneOf("url", "ca-cert"))
 	}
 
 	if keyless.Identities != nil && len(keyless.Identities) == 0 {

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go

@@ -295,7 +295,7 @@ func TestKeylessValidation(t *testing.T) {
 		{
 			name:        "Should fail when keyless has multiple properties",
 			expectErr:   true,
-			errorString: "expected exactly one, got both: spec.authorities[0].keyless.ca-cert, spec.authorities[0].keyless.identities, spec.authorities[0].keyless.url",
+			errorString: "expected exactly one, got both: spec.authorities[0].keyless.ca-cert, spec.authorities[0].keyless.url",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{

pkg/cosign/kubernetes/webhook/validation.go

@@ -29,6 +29,7 @@ import (
 	"knative.dev/pkg/logging"
 
 	"github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioroots"
+	v1alpha1 "github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1"
 	"github.com/sigstore/cosign/pkg/cosign"
 	"github.com/sigstore/cosign/pkg/oci"
 	ociremote "github.com/sigstore/cosign/pkg/oci/remote"
@@ -39,7 +40,7 @@ import (
 func valid(ctx context.Context, ref name.Reference, keys []*ecdsa.PublicKey, opts ...ociremote.Option) ([]oci.Signature, error) {
 	if len(keys) == 0 {
 		// If there are no keys, then verify against the fulcio root.
-		sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroots.Get(), nil /* rekor */, opts...)
+		sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroots.Get(), nil /* rekor */, nil /* no identities */, opts...)
 		if err != nil {
 			return nil, err
 		}
@@ -86,12 +87,13 @@ func validSignatures(ctx context.Context, ref name.Reference, verifier signature
 
 // validSignaturesWithFulcio expects a Fulcio Cert to verify against. An
 // optional rekorClient can also be given, if nil passed, default is assumed.
-func validSignaturesWithFulcio(ctx context.Context, ref name.Reference, fulcioRoots *x509.CertPool, rekorClient *client.Rekor, opts ...ociremote.Option) ([]oci.Signature, error) {
+func validSignaturesWithFulcio(ctx context.Context, ref name.Reference, fulcioRoots *x509.CertPool, rekorClient *client.Rekor, identities []v1alpha1.Identity, opts ...ociremote.Option) ([]oci.Signature, error) {
 	sigs, _, err := cosignVerifySignatures(ctx, ref, &cosign.CheckOpts{
 		RegistryClientOpts: opts,
 		RootCerts:          fulcioRoots,
 		RekorClient:        rekorClient,
 		ClaimVerifier:      cosign.SimpleClaimVerifier,
+		Identities:         identities,
 	})
 	return sigs, err
 }

pkg/cosign/kubernetes/webhook/validator.go

@@ -303,7 +303,7 @@ func ValidatePolicy(ctx context.Context, ref name.Reference, kc authn.Keychain,
 						continue
 					}
 				}
-				sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroot, rekorClient, remoteOpts...)
+				sps, err := validSignaturesWithFulcio(ctx, ref, fulcioroot, rekorClient, authority.Keyless.Identities, remoteOpts...)
 				if err != nil {
 					logging.FromContext(ctx).Errorf("failed validSignatures with fulcio for %s: %v", ref.Name(), err)
 					authorityErrors = append(authorityErrors, errors.Wrap(err, "validate signatures with fulcio"))

pkg/cosign/verify.go

@@ -27,9 +27,11 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"regexp"
 	"strings"
 	"time"
 
+	"github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1"
 	cbundle "github.com/sigstore/cosign/pkg/cosign/bundle"
 	"github.com/sigstore/cosign/pkg/cosign/tuf"
 
@@ -84,6 +86,11 @@ type CheckOpts struct {
 
 	// SignatureRef is the reference to the signature file
 	SignatureRef string
+
+	// Identities is an array of Identity (Subject, Issuer) matchers that have
+	// to be met for the signature to ve valid.
+	// Supercedes CertEmail / CertOidcIssuer
+	Identities []v1alpha1.Identity
 }
 
 func getSignedEntity(signedImgRef name.Reference, regClientOpts []ociremote.Option) (oci.SignedEntity, v1.Hash, error) {
@@ -143,7 +150,7 @@ func verifyOCIAttestation(_ context.Context, verifier signature.Verifier, att pa
 }
 
 // ValidateAndUnpackCert creates a Verifier from a certificate. Veries that the certificate
-// chains up to a trusted root. Optionally verifies the subject of the certificate.
+// chains up to a trusted root. Optionally verifies the subject and issuer of the certificate.
 func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Verifier, error) {
 	verifier, err := signature.LoadVerifier(cert.PublicKey, crypto.SHA256)
 	if err != nil {
@@ -167,23 +174,91 @@ func ValidateAndUnpackCert(cert *x509.Certificate, co *CheckOpts) (signature.Ver
 		}
 	}
 	if co.CertOidcIssuer != "" {
-		issuer := ""
-		for _, ext := range cert.Extensions {
-			if ext.Id.Equal(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1}) {
-				issuer = string(ext.Value)
-				break
-			}
-		}
-		if issuer != co.CertOidcIssuer {
+		if getIssuer(cert) != co.CertOidcIssuer {
 			return nil, errors.New("expected oidc issuer not found in certificate")
 		}
 	}
+	// If there are identities given, go through them and if one of them
+	// matches, call that good, otherwise, return an error.
+	if len(co.Identities) > 0 {
+		for _, identity := range co.Identities {
+			issuerMatches := false
+			// Check the issuer first
+			fmt.Fprintf(os.Stderr, "Checking identity: %+v", identity)
+			if identity.Issuer != "" {
+				issuer := getIssuer(cert)
+				if regex, err := regexp.Compile(identity.Issuer); err != nil {
+					return nil, fmt.Errorf("malformed issuer in identity: %s : %w", identity.Issuer, err)
+				} else if regex.MatchString(issuer) {
+					issuerMatches = true
+				}
+			} else {
+				// No issuer constraint on this identity, so checks out
+				issuerMatches = true
+			}
+
+			// Then the subject
+			subjectMatches := false
+			if identity.Subject != "" {
+				regex, err := regexp.Compile(identity.Subject)
+				if err != nil {
+					return nil, fmt.Errorf("malformed subject in identity: %s : %w", identity.Subject, err)
+				}
+				for _, san := range getSubjectAlternateNames(cert) {
+					if regex.MatchString(san) {
+						subjectMatches = true
+						break
+					}
+				}
+			} else {
+				// No subject constraint on this identity, so checks out
+				subjectMatches = true
+			}
+			if subjectMatches && issuerMatches {
+				// If both issuer / subject match, return verifier
+				return verifier, nil
+			}
+		}
+		return nil, errors.New("none of the expected identities matched what was in the certificate")
+	}
 	return verifier, nil
 }
 
-// ValidateAndUnpackCertWithChain creates a Verifier from a certificate. Veries that the certificate
+// getSubjectAlternateNames returns all of the following for a Certificate.
+// DNSNames
+// EmailAddresses
+// IPAddresses
+// URIs
+func getSubjectAlternateNames(cert *x509.Certificate) []string {
+	sans := []string{}
+	for _, dnsName := range cert.DNSNames {
+		sans = append(sans, dnsName)
+	}
+	for _, email := range cert.EmailAddresses {
+		sans = append(sans, email)
+	}
+	for _, ip := range cert.IPAddresses {
+		sans = append(sans, ip.String())
+	}
+	for _, uri := range cert.URIs {
+		sans = append(sans, uri.String())
+	}
+	return sans
+}
+
+// getIssuer returns the issuer for a Certificate
+func getIssuer(cert *x509.Certificate) string {
+	for _, ext := range cert.Extensions {
+		if ext.Id.Equal(asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 57264, 1, 1}) {
+			return string(ext.Value)
+		}
+	}
+	return ""
+}
+
+// ValidateAndUnpackCertWithChain creates a Verifier from a certificate. Verifies that the certificate
 // chains up to the provided root. Chain should start with the parent of the certificate and end with the root.
-// Optionally verifies the subject of the certificate.
+// Optionally verifies the subject and issuer of the certificate.
 func ValidateAndUnpackCertWithChain(cert *x509.Certificate, chain []*x509.Certificate, co *CheckOpts) (signature.Verifier, error) {
 	if len(chain) == 0 {
 		return nil, errors.New("no chain provided to validate certificate")

pkg/cosign/verify_test.go

@@ -24,13 +24,16 @@ import (
 	"encoding/json"
 	"encoding/pem"
 	"io"
+	"net"
+	"net/url"
 	"strings"
 	"testing"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/in-toto/in-toto-golang/in_toto"
 	"github.com/pkg/errors"
 	"github.com/secure-systems-lab/go-securesystemslib/dsse"
+	"github.com/sigstore/cosign/pkg/apis/cosigned/v1alpha1"
 	"github.com/sigstore/cosign/pkg/oci/static"
 	"github.com/sigstore/cosign/pkg/types"
 	"github.com/sigstore/cosign/test"
@@ -420,6 +423,95 @@ func TestValidateAndUnpackCertWithChainFailsWithInvalidChain(t *testing.T) {
 	}
 }
 
+func TestValidateAndUnpackCertWithIdentities(t *testing.T) {
+	u, err := url.Parse("http://url.example.com")
+	if err != nil {
+		t.Fatal("failed to parse url", err)
+	}
+	emailSubject := "email@example.com"
+	dnsSubjects := []string{"dnssubject.example.com"}
+	ipSubjects := []net.IP{net.ParseIP("1.2.3.4")}
+	uriSubjects := []*url.URL{u}
+	oidcIssuer := "https://accounts.google.com"
+
+	tests := []struct {
+		identities       []v1alpha1.Identity
+		wantErrSubstring string
+		dnsNames         []string
+		emailAddresses   []string
+		ipAddresses      []net.IP
+		uris             []*url.URL
+	}{
+		{identities: nil /* No matches required, checks out */},
+		{identities: []v1alpha1.Identity{ // Strict match on both
+			{Subject: emailSubject, Issuer: oidcIssuer}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // just issuer
+			{Issuer: oidcIssuer}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // just subject
+			{Subject: emailSubject}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // mis-match
+			{Subject: "wrongsubject", Issuer: oidcIssuer},
+			{Subject: emailSubject, Issuer: "wrongissuer"}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: "none of the expected identities matched"},
+		{identities: []v1alpha1.Identity{ // one good identity, other does not match
+			{Subject: "wrongsubject", Issuer: "wrongissuer"},
+			{Subject: emailSubject, Issuer: oidcIssuer}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // illegal regex for subject
+			{Subject: "****", Issuer: oidcIssuer}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: "invalid subject in identity"},
+		{identities: []v1alpha1.Identity{ // illegal regex for issuer
+			{Subject: emailSubject, Issuer: "****"}},
+			wantErrSubstring: "invalid issuer in identity"},
+		{identities: []v1alpha1.Identity{ // regex matches
+			{Subject: ".*example.com", Issuer: ".*accounts.google.*"}},
+			emailAddresses:   []string{emailSubject},
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // regex matches dnsNames
+			{Subject: ".*ubject.example.com", Issuer: ".*accounts.google.*"}},
+			dnsNames:         dnsSubjects,
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // regex matches ip
+			{Subject: "1.2.3.*", Issuer: ".*accounts.google.*"}},
+			ipAddresses:      ipSubjects,
+			wantErrSubstring: ""},
+		{identities: []v1alpha1.Identity{ // regex matches urls
+			{Subject: ".*url.examp.*", Issuer: ".*accounts.google.*"}},
+			uris:             uriSubjects,
+			wantErrSubstring: ""},
+	}
+	for _, tc := range tests {
+		rootCert, rootKey, _ := test.GenerateRootCa()
+		leafCert, _, _ := test.GenerateLeafCertWithSubjectAlternateNames(tc.dnsNames, tc.emailAddresses, tc.ipAddresses, tc.uris, oidcIssuer, rootCert, rootKey)
+
+		rootPool := x509.NewCertPool()
+		rootPool.AddCert(rootCert)
+
+		co := &CheckOpts{
+			RootCerts:  rootPool,
+			Identities: tc.identities,
+		}
+		_, err := ValidateAndUnpackCert(leafCert, co)
+		if err == nil && tc.wantErrSubstring != "" {
+			t.Errorf("Expected error %s got none", tc.wantErrSubstring)
+		} else if err != nil {
+			if tc.wantErrSubstring == "" {
+				t.Errorf("Did not expect an error, got err = %v", err)
+			} else if !strings.Contains(err.Error(), tc.wantErrSubstring) {
+				t.Errorf("Did not get the expected error %s, got err = %v", tc.wantErrSubstring, err)
+			}
+		}
+	}
+}
 func TestCompareSigs(t *testing.T) {
 	// TODO(nsmith5): Add test cases for invalid signature, missing signature etc
 	tests := []struct {