undo policy method changes

Signed-off-by: Meredith Lancaster <malancas@github.com>

pkg/cmd/attestation/verification/extensions.go

@@ -11,14 +11,14 @@ var (
 	GitHubTenantOIDCIssuer = "https://token.actions.%s.ghe.com"
 )
 
-func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner, repo, issuer string) error {
+func VerifyCertExtensions(results []*AttestationProcessingResult, ec EnforcementCriteria) error {
 	if len(results) == 0 {
 		return errors.New("no attestations proccessing results")
 	}
 
 	var atLeastOneVerified bool
 	for _, attestation := range results {
-		if err := verifyCertExtensions(attestation, tenant, owner, repo, issuer); err != nil {
+		if err := verifyCertExtensions(attestation, ec); err != nil {
 			return err
 		}
 		atLeastOneVerified = true
@@ -31,51 +31,31 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, tenant, owner,
 	}
 }
 
-func verifyCertExtensions(attestation *AttestationProcessingResult, tenant, owner, repo, issuer string) error {
-	var want string
-
-	if tenant == "" {
-		want = fmt.Sprintf("https://github.com/%s", owner)
-	} else {
-		want = fmt.Sprintf("https://%s.ghe.com/%s", tenant, owner)
-	}
-	sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI
-	if !strings.EqualFold(want, sourceRepositoryOwnerURI) {
-		return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", want, sourceRepositoryOwnerURI)
+func verifyCertExtensions(attestation *AttestationProcessingResult, c EnforcementCriteria) error {
+	if c.Extensions.SourceRepositoryOwnerURI != "" {
+		sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI
+		if !strings.EqualFold(c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) {
+			return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", c.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI)
+		}
 	}
 
 	// if repo is set, check the SourceRepositoryURI field
-	if repo != "" {
-		if tenant == "" {
-			want = fmt.Sprintf("https://github.com/%s", repo)
-		} else {
-			want = fmt.Sprintf("https://%s.ghe.com/%s", tenant, repo)
-		}
-
+	if c.Extensions.SourceRepositoryURI != "" {
 		sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI
-		if !strings.EqualFold(want, sourceRepositoryURI) {
-			return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", want, sourceRepositoryURI)
+		if !strings.EqualFold(c.Extensions.SourceRepositoryURI, sourceRepositoryURI) {
+			return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", c.Extensions.SourceRepositoryURI, sourceRepositoryURI)
 		}
 	}
 
 	// if issuer is anything other than the default, use the user-provided value;
 	// otherwise, select the appropriate default based on the tenant
-	if issuer != GitHubOIDCIssuer {
-		want = issuer
-	} else {
-		if tenant != "" {
-			want = fmt.Sprintf(GitHubTenantOIDCIssuer, tenant)
-		} else {
-			want = GitHubOIDCIssuer
-		}
-	}
-
-	certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer
-	if !strings.EqualFold(want, certIssuer) {
-		if strings.Index(certIssuer, want+"/") == 0 {
-			return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", want, certIssuer)
-		} else {
-			return fmt.Errorf("expected Issuer to be %s, got %s", want, certIssuer)
+	if c.OIDCIssuer != "" {
+		certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer
+		if !strings.EqualFold(c.OIDCIssuer, certIssuer) {
+			if strings.Index(certIssuer, c.OIDCIssuer+"/") == 0 {
+				return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", c.OIDCIssuer, certIssuer)
+			}
+			return fmt.Errorf("expected Issuer to be %s, got %s", c.OIDCIssuer, certIssuer)
 		}
 	}
 

pkg/cmd/attestation/verification/policy.go

@@ -18,3 +18,20 @@ func BuildDigestPolicyOption(a artifact.DigestedArtifact) (verify.ArtifactPolicy
 	}
 	return verify.WithArtifactDigest(a.Algorithm(), decoded), nil
 }
+
+type Extensions struct {
+	RunnerEnvironment        string
+	SANRegex                 string
+	SAN                      string
+	BuildSourceRepoURI       string
+	SignerWorkflow           string
+	SourceRepositoryOwnerURI string
+	SourceRepositoryURI      string
+}
+
+type EnforcementCriteria struct {
+	Extensions    Extensions
+	PredicateType string
+	Artifact      artifact.DigestedArtifact
+	OIDCIssuer    string
+}

pkg/cmd/attestation/verify/policy.go

@@ -4,12 +4,10 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
-	"strings"
 
 	"github.com/sigstore/sigstore-go/pkg/fulcio/certificate"
 	"github.com/sigstore/sigstore-go/pkg/verify"
 
-	"github.com/cli/cli/v2/pkg/cmd/attestation/api"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
 )
@@ -20,82 +18,6 @@ const (
 	hostRegex    = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$`
 )
 
-type Extensions struct {
-	RunnerEnvironment        string
-	SANRegex                 string
-	SAN                      string
-	BuildSourceRepoURI       string
-	SignerWorkflow           string
-	SourceRepositoryOwnerURI string
-	SourceRepositoryURI      string
-}
-
-type Policy struct {
-	Extensions    Extensions
-	PredicateType string
-	Artifact      artifact.DigestedArtifact
-	OIDCIssuer    string
-}
-
-func newPolicy(opts *Options, a artifact.DigestedArtifact) (Policy, error) {
-	p := Policy{
-		Artifact: a,
-	}
-
-	if opts.SignerRepo != "" {
-		signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo)
-		p.Extensions.SANRegex = signedRepoRegex
-	} else if opts.SignerWorkflow != "" {
-		validatedWorkflowRegex, err := validateSignerWorkflow(opts)
-		if err != nil {
-			return Policy{}, err
-		}
-
-		p.Extensions.SANRegex = validatedWorkflowRegex
-	} else {
-		p.Extensions.SANRegex = opts.SANRegex
-		p.Extensions.SAN = opts.SAN
-	}
-
-	if opts.DenySelfHostedRunner {
-		p.Extensions.RunnerEnvironment = GitHubRunner
-	} else {
-		p.Extensions.RunnerEnvironment = "*"
-	}
-
-	if opts.Repo != "" {
-		if opts.Tenant != "" {
-			p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo)
-		}
-		p.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo)
-	}
-
-	if opts.Tenant != "" {
-		p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner)
-	} else {
-		p.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner)
-	}
-
-	// if issuer is anything other than the default, use the user-provided value;
-	// otherwise, select the appropriate default based on the tenant
-	if opts.Tenant != "" {
-		p.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant)
-	} else {
-		p.OIDCIssuer = opts.OIDCIssuer
-	}
-
-	return p, nil
-}
-
-func (p *Policy) Verify(a []*api.Attestation) (bool, string) {
-	filtered := verification.FilterAttestations(p.PredicateType, a)
-	if len(filtered) == 0 {
-		return false, fmt.Sprintf("✗ No attestations found with predicate type: %s\n", p.PredicateType)
-	}
-
-	return true, ""
-}
-
 func expandToGitHubURL(tenant, ownerOrRepo string) string {
 	if tenant == "" {
 		return fmt.Sprintf("(?i)^https://github.com/%s/", ownerOrRepo)
@@ -103,55 +25,6 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string {
 	return fmt.Sprintf("(?i)^https://%s.ghe.com/%s/", tenant, ownerOrRepo)
 }
 
-func (p *Policy) buildCertificateIdentityOption() (verify.PolicyOption, error) {
-	sanMatcher, err := verify.NewSANMatcher(p.Extensions.SAN, p.Extensions.SANRegex)
-	if err != nil {
-		return nil, err
-	}
-
-	// Accept any issuer, we will verify the issuer as part of the extension verification
-	issuerMatcher, err := verify.NewIssuerMatcher("", ".*")
-	if err != nil {
-		return nil, err
-	}
-
-	extensions := certificate.Extensions{
-		RunnerEnvironment: p.Extensions.RunnerEnvironment,
-	}
-
-	certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions)
-	if err != nil {
-		return nil, err
-	}
-
-	return verify.WithCertificateIdentity(certId), nil
-}
-
-func (p *Policy) VerifyPredicateType(a []*api.Attestation) ([]*api.Attestation, error) {
-	filteredAttestations := verification.FilterAttestations(p.PredicateType, a)
-
-	if len(filteredAttestations) == 0 {
-		return nil, fmt.Errorf("✗ No attestations found with predicate type: %s\n", p.PredicateType)
-	}
-
-	return filteredAttestations, nil
-}
-
-func (p *Policy) SigstorePolicy() (verify.PolicyBuilder, error) {
-	artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(p.Artifact)
-	if err != nil {
-		return verify.PolicyBuilder{}, err
-	}
-
-	certIdOption, err := p.buildCertificateIdentityOption()
-	if err != nil {
-		return verify.PolicyBuilder{}, err
-	}
-
-	policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption)
-	return policy, nil
-}
-
 func validateSignerWorkflow(opts *Options) (string, error) {
 	// we expect a provided workflow argument be in the format [HOST/]/<OWNER>/<REPO>/path/to/workflow.yml
 	// if the provided workflow does not contain a host, set the host
@@ -171,51 +44,91 @@ func validateSignerWorkflow(opts *Options) (string, error) {
 	return fmt.Sprintf("^https://%s/%s", opts.Hostname, opts.SignerWorkflow), nil
 }
 
-func (p *Policy) VerifyCertExtensions(results []*verification.AttestationProcessingResult) error {
-	if len(results) == 0 {
-		return errors.New("no attestations proccessing results")
+func newEnforcementCriteria(opts *Options, a artifact.DigestedArtifact) (verification.EnforcementCriteria, error) {
+	c := verification.EnforcementCriteria{
+		Artifact: a,
 	}
 
-	var atLeastOneVerified bool
-	for _, attestation := range results {
-		if err := p.verifyCertExtensions(attestation); err != nil {
-			return err
+	if opts.SignerRepo != "" {
+		signedRepoRegex := expandToGitHubURL(opts.Tenant, opts.SignerRepo)
+		c.Extensions.SANRegex = signedRepoRegex
+	} else if opts.SignerWorkflow != "" {
+		validatedWorkflowRegex, err := validateSignerWorkflow(opts)
+		if err != nil {
+			return verification.EnforcementCriteria{}, err
 		}
-		atLeastOneVerified = true
-	}
 
-	if atLeastOneVerified {
-		return nil
+		c.Extensions.SANRegex = validatedWorkflowRegex
 	} else {
-		return verification.ErrNoAttestationsVerified
+		c.Extensions.SANRegex = opts.SANRegex
+		c.Extensions.SAN = opts.SAN
 	}
+
+	if opts.DenySelfHostedRunner {
+		c.Extensions.RunnerEnvironment = GitHubRunner
+	} else {
+		c.Extensions.RunnerEnvironment = "*"
+	}
+
+	if opts.Repo != "" {
+		if opts.Tenant != "" {
+			c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Repo)
+		}
+		c.Extensions.BuildSourceRepoURI = fmt.Sprintf("https://github.com/%s", opts.Repo)
+	}
+
+	if opts.Tenant != "" {
+		c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://%s.ghe.com/%s", opts.Tenant, opts.Owner)
+	} else {
+		c.Extensions.SourceRepositoryOwnerURI = fmt.Sprintf("https://github.com/%s", opts.Owner)
+	}
+
+	// if issuer is anything other than the default, use the user-provided value;
+	// otherwise, select the appropriate default based on the tenant
+	if opts.Tenant != "" {
+		c.OIDCIssuer = fmt.Sprintf(verification.GitHubTenantOIDCIssuer, opts.Tenant)
+	} else {
+		c.OIDCIssuer = opts.OIDCIssuer
+	}
+
+	return c, nil
 }
 
-func (p *Policy) verifyCertExtensions(attestation *verification.AttestationProcessingResult) error {
-	if p.Extensions.SourceRepositoryOwnerURI != "" {
-		sourceRepositoryOwnerURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryOwnerURI
-		if !strings.EqualFold(p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) {
-			return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", p.Extensions.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI)
-		}
+func buildCertificateIdentityOption(c verification.EnforcementCriteria) (verify.PolicyOption, error) {
+	sanMatcher, err := verify.NewSANMatcher(c.Extensions.SAN, c.Extensions.SANRegex)
+	if err != nil {
+		return nil, err
 	}
 
-	// if repo is set, check the SourceRepositoryURI field
-	if p.Extensions.SourceRepositoryURI != "" {
-		sourceRepositoryURI := attestation.VerificationResult.Signature.Certificate.Extensions.SourceRepositoryURI
-		if !strings.EqualFold(p.Extensions.SourceRepositoryURI, sourceRepositoryURI) {
-			return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", p.Extensions.SourceRepositoryURI, sourceRepositoryURI)
-		}
+	// Accept any issuer, we will verify the issuer as part of the extension verification
+	issuerMatcher, err := verify.NewIssuerMatcher("", ".*")
+	if err != nil {
+		return nil, err
 	}
 
-	if p.OIDCIssuer != "" {
-		certIssuer := attestation.VerificationResult.Signature.Certificate.Extensions.Issuer
-		if !strings.EqualFold(p.OIDCIssuer, certIssuer) {
-			if strings.Index(certIssuer, p.OIDCIssuer+"/") == 0 {
-				return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", p.OIDCIssuer, certIssuer)
-			}
-			return fmt.Errorf("expected Issuer to be %s, got %s", p.OIDCIssuer, certIssuer)
-		}
+	extensions := certificate.Extensions{
+		RunnerEnvironment: c.Extensions.RunnerEnvironment,
 	}
 
-	return nil
+	certId, err := verify.NewCertificateIdentity(sanMatcher, issuerMatcher, extensions)
+	if err != nil {
+		return nil, err
+	}
+
+	return verify.WithCertificateIdentity(certId), nil
+}
+
+func SigstorePolicy(c verification.EnforcementCriteria) (verify.PolicyBuilder, error) {
+	artifactDigestPolicyOption, err := verification.BuildDigestPolicyOption(c.Artifact)
+	if err != nil {
+		return verify.PolicyBuilder{}, err
+	}
+
+	certIdOption, err := buildCertificateIdentityOption(c)
+	if err != nil {
+		return verify.PolicyBuilder{}, err
+	}
+
+	policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption)
+	return policy, nil
 }

pkg/cmd/attestation/verify/policy_test.go

@@ -26,7 +26,7 @@ func TestBuildPolicy(t *testing.T) {
 		SANRegex:     "^https://github.com/sigstore/",
 	}
 
-	_, err = newPolicy(opts, *artifact)
+	_, err = newEnforcementCriteria(opts, *artifact)
 	require.NoError(t, err)
 }
 

pkg/cmd/attestation/verify/verify.go

@@ -255,9 +255,27 @@ func runVerify(opts *Options) error {
 		attestations = filteredAttestations
 	}
 
-	sigstoreResults, err := verifyAll(opts, *artifact, attestations)
+	ec, err := newEnforcementCriteria(opts, *artifact)
 	if err != nil {
-		opts.Logger.Println(opts.Logger.ColorScheme.Red(err.Error()))
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build verification policy"))
+		return err
+	}
+
+	sp, err := SigstorePolicy(ec)
+	if err != nil {
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to build Sigstore verification policy"))
+		return err
+	}
+
+	sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp)
+	if sigstoreRes.Error != nil {
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Sigstore verification failed"))
+		return err
+	}
+
+	// Verify extensions
+	if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, ec); err != nil {
+		opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Policy verification failed"))
 		return err
 	}
 
@@ -266,7 +284,7 @@ func runVerify(opts *Options) error {
 	// If an exporter is provided with the --json flag, write the results to the terminal in JSON format
 	if opts.exporter != nil {
 		// print the results to the terminal as an array of JSON objects
-		if err = opts.exporter.Write(opts.Logger.IO, sigstoreResults.VerifyResults); err != nil {
+		if err = opts.exporter.Write(opts.Logger.IO, sigstoreRes.VerifyResults); err != nil {
 			opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Failed to write JSON output"))
 			return err
 		}
@@ -276,7 +294,7 @@ func runVerify(opts *Options) error {
 	opts.Logger.Printf("%s was attested by:\n", artifact.DigestWithAlg())
 
 	// Otherwise print the results to the terminal in a table
-	tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreResults.VerifyResults)
+	tableContent, err := buildTableVerifyContent(opts.Tenant, sigstoreRes.VerifyResults)
 	if err != nil {
 		opts.Logger.Println(opts.Logger.ColorScheme.Red("failed to parse results"))
 		return err
@@ -352,27 +370,3 @@ func buildTableVerifyContent(tenant string, results []*verification.AttestationP
 
 	return content, nil
 }
-
-func verifyAll(opts *Options, artifact artifact.DigestedArtifact, attestations []*api.Attestation) (*verification.SigstoreResults, error) {
-	policy, err := newPolicy(opts, artifact)
-	if err != nil {
-		return nil, fmt.Errorf("✗ Failed to build verification policy")
-	}
-
-	sp, err := policy.SigstorePolicy()
-	if err != nil {
-		return nil, fmt.Errorf("✗ Failed to build Sigstore verification policy")
-	}
-
-	sigstoreRes := opts.SigstoreVerifier.Verify(attestations, sp)
-	if sigstoreRes.Error != nil {
-		return nil, fmt.Errorf("✗ Sigstore verification failed")
-	}
-
-	// Verify extensions
-	if err := verification.VerifyCertExtensions(sigstoreRes.VerifyResults, opts.Tenant, opts.Owner, opts.Repo, opts.OIDCIssuer); err != nil {
-		return nil, fmt.Errorf("✗ Policy verification failed")
-	}
-
-	return sigstoreRes, nil
-}