Add signer-repo and signer-workflow flags to gh attestation verify (#9137)

* add signer-repo and signer-workflow flags Signed-off-by: Meredith Lancaster <malancas@github.com> * add check for SignerRepo option Signed-off-by: Meredith Lancaster <malancas@github.com> * add helper function and comment for clarity Signed-off-by: Meredith Lancaster <malancas@github.com> * update flag comment Signed-off-by: Meredith Lancaster <malancas@github.com> * reference correct field Signed-off-by: Meredith Lancaster <malancas@github.com> * move function to more relevant file Signed-off-by: Meredith Lancaster <malancas@github.com> * Update pkg/cmd/attestation/verify/verify.go Co-authored-by: Zach Steindler <steiza@github.com> * Update pkg/cmd/attestation/verify/verify.go Co-authored-by: Zach Steindler <steiza@github.com> * make all reusable workflow flags mutually exclusive Signed-off-by: Meredith Lancaster <malancas@github.com> * accept signer workflow without host Signed-off-by: Meredith Lancaster <malancas@github.com> * support client optionally providing host with signer workflow flag Signed-off-by: Meredith Lancaster <malancas@github.com> * comment Signed-off-by: Meredith Lancaster <malancas@github.com> * add tests for parsing signer workflow Signed-off-by: Meredith Lancaster <malancas@github.com> --------- Signed-off-by: Meredith Lancaster <malancas@github.com> Co-authored-by: Zach Steindler <steiza@github.com>
2024-05-30 07:40:55 -06:00 · 2024-05-30 07:40:55 -06:00 · cd5562f5ac
commit cd5562f5ac
parent 22991ab6be
5 changed files with 252 additions and 21 deletions
--- a/pkg/cmd/attestation/verify/options.go
+++ b/pkg/cmd/attestation/verify/options.go
@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"strings"

+	"github.com/cli/cli/v2/internal/gh"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/api"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/io"
@ -16,6 +17,7 @@ import (
 type Options struct {
 	ArtifactPath         string
 	BundlePath           string
+	Config               func() (gh.Config, error)
 	CustomTrustedRoot    string
 	DenySelfHostedRunner bool
 	DigestAlgorithm      string
@ -27,6 +29,8 @@ type Options struct {
 	Repo                 string
 	SAN                  string
 	SANRegex             string
+	SignerRepo           string
+	SignerWorkflow       string
 	APIClient            api.Client
 	Logger               *io.Handler
 	OCIClient            oci.Client
@ -51,12 +55,12 @@ func (opts *Options) SetPolicyFlags() {
 		// to Owner
 		opts.Owner = splitRepo[0]

-		if opts.SAN == "" && opts.SANRegex == "" {
+		if !isSignerIdentityProvided(opts) {
 			opts.SANRegex = expandToGitHubURL(opts.Repo)
 		}
 		return
 	}
-	if opts.SAN == "" && opts.SANRegex == "" {
+	if !isSignerIdentityProvided(opts) {
 		opts.SANRegex = expandToGitHubURL(opts.Owner)
 	}
 }
@ -64,13 +68,14 @@ func (opts *Options) SetPolicyFlags() {
 // AreFlagsValid checks that the provided flag combination is valid
 // and returns an error otherwise
 func (opts *Options) AreFlagsValid() error {
-	// check that Repo is in the expected format if provided
-	if opts.Repo != "" {
-		// we expect the repo argument to be in the format <OWNER>/<REPO>
-		splitRepo := strings.Split(opts.Repo, "/")
-		if len(splitRepo) != 2 {
-			return fmt.Errorf("invalid value provided for repo: %s", opts.Repo)
-		}
+	// If provided, check that the Repo option is in the expected format <OWNER>/<REPO>
+	if opts.Repo != "" && !isProvidedRepoValid(opts.Repo) {
+		return fmt.Errorf("invalid value provided for repo: %s", opts.Repo)
+	}
+
+	// If provided, check that the SignerRepo option is in the expected format <OWNER>/<REPO>
+	if opts.SignerRepo != "" && !isProvidedRepoValid(opts.SignerRepo) {
+		return fmt.Errorf("invalid value provided for signer-repo: %s", opts.SignerRepo)
 	}

 	// Check that limit is between 1 and 1000
@ -81,6 +86,13 @@ func (opts *Options) AreFlagsValid() error {
 	return nil
 }

-func expandToGitHubURL(ownerOrRepo string) string {
-	return fmt.Sprintf("^https://github.com/%s/", ownerOrRepo)
+// check if any of the signer identity flags have been provided
+func isSignerIdentityProvided(opts *Options) bool {
+	return opts.SAN != "" || opts.SANRegex != "" || opts.SignerRepo != "" || opts.SignerWorkflow != ""
+}
+
+func isProvidedRepoValid(repo string) bool {
+	// we expect a provided repository argument be in the format <OWNER>/<REPO>
+	splitRepo := strings.Split(repo, "/")
+	return len(splitRepo) == 2
 }
--- a/pkg/cmd/attestation/verify/policy.go
+++ b/pkg/cmd/attestation/verify/policy.go
@ -2,6 +2,8 @@ package verify

 import (
 	"fmt"
+	"os"
+	"regexp"

 	"github.com/sigstore/sigstore-go/pkg/fulcio/certificate"
 	"github.com/sigstore/sigstore-go/pkg/verify"
@ -14,18 +16,30 @@ const (
 	GitHubOIDCIssuer = "https://token.actions.githubusercontent.com"
 	// represents the GitHub hosted runner in the certificate RunnerEnvironment extension
 	GitHubRunner = "github-hosted"
+	githubHost   = "github.com"
+	hostRegex    = `^[a-zA-Z0-9-]+\.[a-zA-Z0-9-]+.*$`
 )

-func buildSANMatcher(san, sanRegex string) (verify.SubjectAlternativeNameMatcher, error) {
-	if san == "" && sanRegex == "" {
-		return verify.SubjectAlternativeNameMatcher{}, nil
+func expandToGitHubURL(ownerOrRepo string) string {
+	return fmt.Sprintf("^https://github.com/%s/", ownerOrRepo)
+}
+
+func buildSANMatcher(opts *Options) (verify.SubjectAlternativeNameMatcher, error) {
+	if opts.SignerRepo != "" {
+		signedRepoRegex := expandToGitHubURL(opts.SignerRepo)
+		return verify.NewSANMatcher("", "", signedRepoRegex)
+	} else if opts.SignerWorkflow != "" {
+		validatedWorkflowRegex, err := validateSignerWorkflow(opts)
+		if err != nil {
+			return verify.SubjectAlternativeNameMatcher{}, err
+		}
+
+		return verify.NewSANMatcher("", "", validatedWorkflowRegex)
+	} else if opts.SAN != "" || opts.SANRegex != "" {
+		return verify.NewSANMatcher(opts.SAN, "", opts.SANRegex)
 	}

-	sanMatcher, err := verify.NewSANMatcher(san, "", sanRegex)
-	if err != nil {
-		return verify.SubjectAlternativeNameMatcher{}, err
-	}
-	return sanMatcher, nil
+	return verify.SubjectAlternativeNameMatcher{}, nil
 }

 func buildCertExtensions(opts *Options, runnerEnv string) certificate.Extensions {
@ -43,7 +57,7 @@ func buildCertExtensions(opts *Options, runnerEnv string) certificate.Extensions
 }

 func buildCertificateIdentityOption(opts *Options, runnerEnv string) (verify.PolicyOption, error) {
-	sanMatcher, err := buildSANMatcher(opts.SAN, opts.SANRegex)
+	sanMatcher, err := buildSANMatcher(opts)
 	if err != nil {
 		return nil, err
 	}
@ -93,3 +107,54 @@ func buildVerifyPolicy(opts *Options, a artifact.DigestedArtifact) (verify.Polic
 	policy := verify.NewPolicy(artifactDigestPolicyOption, certIdOption)
 	return policy, nil
 }
+
+func addSchemeToRegex(s string) string {
+	return fmt.Sprintf("^https://%s", s)
+}
+
+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
+	match, err := regexp.MatchString(hostRegex, opts.SignerWorkflow)
+	if err != nil {
+		return "", err
+	}
+
+	if match {
+		return addSchemeToRegex(opts.SignerWorkflow), nil
+	}
+
+	// if the provided workflow does not contain a host, check for a host
+	// and prepend it to the workflow
+	host, err := chooseHost(opts)
+	if err != nil {
+		return "", err
+	}
+
+	return addSchemeToRegex(fmt.Sprintf("%s/%s", host, opts.SignerWorkflow)), nil
+}
+
+// if a host was not provided as part of a flag argument choose a host based
+// on gh cli configuration
+func chooseHost(opts *Options) (string, error) {
+	// check if GH_HOST is set and use that host if it is
+	host := os.Getenv("GH_HOST")
+	if host != "" {
+		return host, nil
+	}
+
+	// check if the CLI is authenticated with any hosts
+	cfg, err := opts.Config()
+	if err != nil {
+		return "", err
+	}
+
+	// if authenticated, return the authenticated host
+	authCfg := cfg.Authentication()
+	if host, _ := authCfg.DefaultHost(); host != "" {
+		return host, nil
+	}
+
+	// if not authenticated, return the default host github.com
+	return githubHost, nil
+}
--- a/pkg/cmd/attestation/verify/policy_test.go
+++ b/pkg/cmd/attestation/verify/policy_test.go
@ -1,10 +1,12 @@
 package verify

 import (
+	"os"
 	"testing"

 	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
+	"github.com/cli/cli/v2/pkg/cmd/factory"

 	"github.com/stretchr/testify/require"
 )
@ -29,3 +31,65 @@ func TestBuildPolicy(t *testing.T) {
 	_, err = buildVerifyPolicy(opts, *artifact)
 	require.NoError(t, err)
 }
+
+func ValidateSignerWorkflow(t *testing.T) {
+	type testcase struct {
+		name                   string
+		providedSignerWorkflow string
+		expectedWorkflowRegex  string
+		ghHost                 string
+		authHost               string
+	}
+
+	testcases := []testcase{
+		{
+			name:                   "workflow with no host specified",
+			providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectedWorkflowRegex:  "^https://github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+		},
+		{
+			name:                   "workflow with host specified",
+			providedSignerWorkflow: "github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectedWorkflowRegex:  "^https://github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+		},
+		{
+			name:                   "workflow with GH_HOST set",
+			providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectedWorkflowRegex:  "^https://myhost.github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			ghHost:                 "myhost.github.com",
+		},
+		{
+			name:                   "workflow with authenticated host",
+			providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectedWorkflowRegex:  "^https://authedhost.github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			authHost:               "authedhost.github.com",
+		},
+	}
+
+	for _, tc := range testcases {
+		cmdFactory := factory.New("test")
+
+		opts := &Options{
+			Config:         cmdFactory.Config,
+			SignerWorkflow: tc.providedSignerWorkflow,
+		}
+
+		if tc.ghHost != "" {
+			err := os.Setenv("GH_HOST", tc.ghHost)
+			require.NoError(t, err)
+		}
+
+		if tc.authHost != "" {
+			cfg, err := opts.Config()
+			require.NoError(t, err)
+
+			// if authenticated, return the authenticated host
+			authCfg := cfg.Authentication()
+			authCfg.SetDefaultHost(tc.authHost, "")
+		}
+
+		workflowRegex, err := validateSignerWorkflow(opts)
+		require.NoError(t, err)
+		require.Equal(t, tc.expectedWorkflowRegex, workflowRegex)
+	}
+}
--- a/pkg/cmd/attestation/verify/verify.go
+++ b/pkg/cmd/attestation/verify/verify.go
@ -123,6 +123,7 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command
 			}

 			opts.SigstoreVerifier = verification.NewLiveSigstoreVerifier(config)
+			opts.Config = f.Config

 			if err := runVerify(opts); err != nil {
 				return fmt.Errorf("\nError: %v", err)
@ -148,7 +149,9 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command
 	verifyCmd.Flags().BoolVarP(&opts.DenySelfHostedRunner, "deny-self-hosted-runners", "", false, "Fail verification for attestations generated on self-hosted runners")
 	verifyCmd.Flags().StringVarP(&opts.SAN, "cert-identity", "", "", "Enforce that the certificate's subject alternative name matches the provided value exactly")
 	verifyCmd.Flags().StringVarP(&opts.SANRegex, "cert-identity-regex", "i", "", "Enforce that the certificate's subject alternative name matches the provided regex")
-	verifyCmd.MarkFlagsMutuallyExclusive("cert-identity", "cert-identity-regex")
+	verifyCmd.Flags().StringVarP(&opts.SignerRepo, "signer-repo", "", "", "Repository of reusable workflow that signed attestation in the format <owner>/<repo>")
+	verifyCmd.Flags().StringVarP(&opts.SignerWorkflow, "signer-workflow", "", "", "Workflow that signed attestation in the format [host/]<owner>/<repo>/<path>/<to>/<workflow>")
+	verifyCmd.MarkFlagsMutuallyExclusive("cert-identity", "cert-identity-regex", "signer-repo", "signer-workflow")
 	verifyCmd.Flags().StringVarP(&opts.OIDCIssuer, "cert-oidc-issuer", "", GitHubOIDCIssuer, "Issuer of the OIDC token")

 	return verifyCmd
--- a/pkg/cmd/attestation/verify/verify_integration_test.go
+++ b/pkg/cmd/attestation/verify/verify_integration_test.go
@ -128,6 +128,15 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) {
 		require.NoError(t, err)
 	})

+	t.Run("with owner and valid reusable signer repo", func(t *testing.T) {
+		opts := baseOpts
+		opts.Owner = "malancas"
+		opts.SignerRepo = "github/artifact-attestations-workflows"
+
+		err := runVerify(&opts)
+		require.NoError(t, err)
+	})
+
 	t.Run("with repo and valid reusable workflow SAN", func(t *testing.T) {
 		opts := baseOpts
 		opts.Owner = "malancas"
@ -147,4 +156,82 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) {
 		err := runVerify(&opts)
 		require.NoError(t, err)
 	})
+
+	t.Run("with repo and valid reusable signer repo", func(t *testing.T) {
+		opts := baseOpts
+		opts.Owner = "malancas"
+		opts.Repo = "malancas/attest-demo"
+		opts.SignerRepo = "github/artifact-attestations-workflows"
+
+		err := runVerify(&opts)
+		require.NoError(t, err)
+	})
+}
+
+func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) {
+	artifactPath := test.NormalizeRelativePath("../test/data/reusable-workflow-artifact")
+	bundlePath := test.NormalizeRelativePath("../test/data/reusable-workflow-attestation.sigstore.json")
+
+	logger := io.NewTestHandler()
+
+	sigstoreConfig := verification.SigstoreConfig{
+		Logger: logger,
+	}
+
+	cmdFactory := factory.New("test")
+
+	hc, err := cmdFactory.HttpClient()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	baseOpts := Options{
+		APIClient:        api.NewLiveClient(hc, logger),
+		ArtifactPath:     artifactPath,
+		BundlePath:       bundlePath,
+		Config:           cmdFactory.Config,
+		DigestAlgorithm:  "sha256",
+		Logger:           logger,
+		OCIClient:        oci.NewLiveClient(),
+		OIDCIssuer:       GitHubOIDCIssuer,
+		Owner:            "malancas",
+		Repo:             "malancas/attest-demo",
+		SigstoreVerifier: verification.NewLiveSigstoreVerifier(sigstoreConfig),
+	}
+
+	type testcase struct {
+		name           string
+		signerWorkflow string
+		expectErr      bool
+	}
+
+	testcases := []testcase{
+		{
+			name:           "with invalid signer workflow",
+			signerWorkflow: "foo/bar/.github/workflows/attest.yml",
+			expectErr:      true,
+		},
+		{
+			name:           "valid signer workflow with host",
+			signerWorkflow: "github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectErr:      false,
+		},
+		{
+			name:           "valid signer workflow without host (defaults to github.com)",
+			signerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
+			expectErr:      false,
+		},
+	}
+
+	for _, tc := range testcases {
+		opts := baseOpts
+		opts.SignerWorkflow = tc.signerWorkflow
+
+		err := runVerify(&opts)
+		if tc.expectErr {
+			require.Error(t, err, "expected error for '%s'", tc.name)
+		} else {
+			require.NoError(t, err, "unexpected error for '%s'", tc.name)
+		}
+	}
 }