From f96d91bdd227493972ea7c8661131ad062bdf97a Mon Sep 17 00:00:00 2001 From: Ian Lewis Date: Tue, 11 Apr 2023 08:54:33 +0900 Subject: [PATCH] fix: Support pre-releases on trusted repos (#552) Support pre-releases on trusted repos --------- Signed-off-by: Ian Lewis --- .github/workflows/pre-submit.e2e.yml | 3 +- .github/workflows/scripts/e2e-cli.sh | 20 +++---- cli/slsa-verifier/main.go | 8 +++ cli/slsa-verifier/main_regression_test.go | 1 + options/env.go | 21 ++++++- verifiers/internal/gha/builder.go | 22 +++++++- verifiers/internal/gha/builder_test.go | 67 ++++++++++++++++++++--- verifiers/internal/gha/provenance.go | 4 +- verifiers/internal/gha/provenance_test.go | 17 ++++++ 9 files changed, 138 insertions(+), 25 deletions(-) diff --git a/.github/workflows/pre-submit.e2e.yml b/.github/workflows/pre-submit.e2e.yml index 6efa3f0..cef0394 100644 --- a/.github/workflows/pre-submit.e2e.yml +++ b/.github/workflows/pre-submit.e2e.yml @@ -33,6 +33,7 @@ jobs: repository: slsa-framework/example-package - name: Run verification script with testdata and slsa-verifier HEAD - run: ./__THIS_REPO__/.github/workflows/scripts/e2e-cli.sh env: + SLSA_VERIFIER_TESTING: "true" GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Necessary to use the gh CLI. + run: ./__THIS_REPO__/.github/workflows/scripts/e2e-cli.sh diff --git a/.github/workflows/scripts/e2e-cli.sh b/.github/workflows/scripts/e2e-cli.sh index 6ff8c95..2eca968 100755 --- a/.github/workflows/scripts/e2e-cli.sh +++ b/.github/workflows/scripts/e2e-cli.sh @@ -4,15 +4,15 @@ repo="slsa-framework/example-package" api_version="X-GitHub-Api-Version: 2022-11-28" # Verify provenance authenticity with slsa-verifier at HEAD -download_artifact(){ - local run_id="$1" - local artifact_name="$2" - # Get the artifact ID for 'artifact1' - artifact_id=$(gh api -H "Accept: application/vnd.github+json" -H "$api_version" "/repos/$repo/actions/runs/$run_id/artifacts" | jq ".artifacts[] | select(.name == \"$artifact_name\") | .id") - echo "artifact_id:$artifact_id" +download_artifact() { + local run_id="$1" + local artifact_name="$2" + # Get the artifact ID for 'artifact1' + artifact_id=$(gh api -H "Accept: application/vnd.github+json" -H "$api_version" "/repos/$repo/actions/runs/$run_id/artifacts" | jq ".artifacts[] | select(.name == \"$artifact_name\") | .id") + echo "artifact_id:$artifact_id" - gh api -H "Accept: application/vnd.github+json" -H "$api_version" "/repos/$repo/actions/artifacts/$artifact_id/zip" > "$artifact_name.zip" - unzip "$artifact_name".zip + gh api -H "Accept: application/vnd.github+json" -H "$api_version" "/repos/$repo/actions/artifacts/$artifact_id/zip" >"$artifact_name.zip" + unzip "$artifact_name".zip } # Get workflow ID. @@ -26,7 +26,7 @@ echo "run_id:$run_id" download_artifact "$run_id" "artifacts1" download_artifact "$run_id" "attestation1.intoto.jsonl" -cd __EXAMPLE_PACKAGE__ +cd __EXAMPLE_PACKAGE__ || exit 1 # shellcheck source=/dev/null source "./.github/workflows/scripts/e2e-verify.common.sh" @@ -35,7 +35,7 @@ export THIS_FILE=e2e.generic.schedule.main.multi-uses.slsa3.yml export BRANCH=main # Set BINARY and PROVENANCE -cd - +cd - || exit 1 export BINARY=artifact1 export PROVENANCE=attestation1.intoto.jsonl diff --git a/cli/slsa-verifier/main.go b/cli/slsa-verifier/main.go index f41694a..fefd9f3 100644 --- a/cli/slsa-verifier/main.go +++ b/cli/slsa-verifier/main.go @@ -5,6 +5,7 @@ import ( "fmt" "os" + "github.com/slsa-framework/slsa-verifier/v2/options" "github.com/spf13/cobra" "sigs.k8s.io/release-utils/version" ) @@ -16,6 +17,12 @@ func check(err error) { } } +func envWarnings() { + if options.TestingEnabled() { + fmt.Fprintf(os.Stderr, "WARNING: Insecure SLSA_VERIFIER_TESTING is enabled.\n") + } +} + func rootCmd() *cobra.Command { c := &cobra.Command{ Use: "slsa-verifier", @@ -36,5 +43,6 @@ For more information on SLSA, visit https://slsa.dev`, } func main() { + envWarnings() check(rootCmd().Execute()) } diff --git a/cli/slsa-verifier/main_regression_test.go b/cli/slsa-verifier/main_regression_test.go index 67b43ae..d62c205 100644 --- a/cli/slsa-verifier/main_regression_test.go +++ b/cli/slsa-verifier/main_regression_test.go @@ -1303,6 +1303,7 @@ func Test_runVerifyGCBArtifactImage(t *testing.T) { func Test_runVerifyGHADockerBased(t *testing.T) { // We cannot use t.Setenv due to parallelized tests. os.Setenv("SLSA_VERIFIER_EXPERIMENTAL", "1") + os.Setenv("SLSA_VERIFIER_TESTING", "1") t.Parallel() diff --git a/options/env.go b/options/env.go index 5b52787..ef22d63 100644 --- a/options/env.go +++ b/options/env.go @@ -1,7 +1,24 @@ package options -import "os" +import ( + "os" + "strconv" +) +// ExperimentalEnabled returns true if experimental features are currently +// enabled. func ExperimentalEnabled() bool { - return os.Getenv("SLSA_VERIFIER_EXPERIMENTAL") == "1" + if b, err := strconv.ParseBool(os.Getenv("SLSA_VERIFIER_EXPERIMENTAL")); err == nil { + return b + } + return false +} + +// TestingEnabled returns true if the SLSA_VERIFIER_TESTING environment +// variable is set. +func TestingEnabled() bool { + if b, err := strconv.ParseBool(os.Getenv("SLSA_VERIFIER_TESTING")); err == nil { + return b + } + return false } diff --git a/verifiers/internal/gha/builder.go b/verifiers/internal/gha/builder.go index 01193c7..422cd74 100644 --- a/verifiers/internal/gha/builder.go +++ b/verifiers/internal/gha/builder.go @@ -131,7 +131,27 @@ func verifyTrustedBuilderID(certPath, certTag string, expectedBuilderID *string, func verifyTrustedBuilderRef(id *WorkflowIdentity, ref string) error { if (id.CallerRepository == trustedBuilderRepository || id.CallerRepository == e2eTestRepository) && - strings.EqualFold("refs/heads/main", ref) { + options.TestingEnabled() { + // Allow verification on the main branch to support e2e tests. + if ref == "refs/heads/main" { + return nil + } + + // Extract the tag. + pin, err := utils.TagFromGitHubRef(ref) + if err != nil { + return err + } + + // Tags on trusted repositories should be a valid semver with version + // core including all three parts and no build identifier. + versionCore := strings.Split(pin, "-")[0] + if !semver.IsValid(pin) || + len(strings.Split(versionCore, ".")) != 3 || + semver.Build(pin) != "" { + return fmt.Errorf("%w: %s: version tag not valid", serrors.ErrorInvalidRef, pin) + } + return nil } diff --git a/verifiers/internal/gha/builder_test.go b/verifiers/internal/gha/builder_test.go index cf83bc6..aa99ee4 100644 --- a/verifiers/internal/gha/builder_test.go +++ b/verifiers/internal/gha/builder_test.go @@ -488,18 +488,25 @@ func Test_verifyTrustedBuilderID(t *testing.T) { } func Test_verifyTrustedBuilderRef(t *testing.T) { - t.Parallel() tests := []struct { - name string - callerRepo string - builderRef string - expected error + name string + callerRepo string + builderRef string + expected error + testingEnabled bool }{ // Trusted repo. { - name: "main allowed for builder", + name: "main not allowed for builder", callerRepo: trustedBuilderRepository, builderRef: "refs/heads/main", + expected: serrors.ErrorInvalidRef, + }, + { + name: "main allowed for builder w/ testing enabled", + callerRepo: trustedBuilderRepository, + builderRef: "refs/heads/main", + testingEnabled: true, }, { name: "full semver for builder", @@ -538,10 +545,18 @@ func Test_verifyTrustedBuilderRef(t *testing.T) { }, // E2e tests repo. { - name: "main allowed for test repo", + name: "main not allowed for test repo", callerRepo: e2eTestRepository, builderRef: "refs/heads/main", + expected: serrors.ErrorInvalidRef, }, + { + name: "main allowed for test repo w/ testing enabled", + callerRepo: e2eTestRepository, + builderRef: "refs/heads/main", + testingEnabled: true, + }, + { name: "full semver for test repo", callerRepo: e2eTestRepository, @@ -585,6 +600,14 @@ func Test_verifyTrustedBuilderRef(t *testing.T) { expected: serrors.ErrorInvalidRef, }, { + name: "main not allowed for other repos w/ testing enabled", + callerRepo: "some/repo", + builderRef: "refs/heads/main", + testingEnabled: true, + expected: serrors.ErrorInvalidRef, + }, + { + name: "full semver for other repos", callerRepo: "some/repo", builderRef: "refs/tags/v1.2.3", @@ -607,28 +630,54 @@ func Test_verifyTrustedBuilderRef(t *testing.T) { builderRef: "refs/tags/v1.2.3-alpha", expected: serrors.ErrorInvalidRef, }, + { + name: "full semver with prerelease for other repos w/ testing enabled", + callerRepo: "some/repo", + builderRef: "refs/tags/v1.2.3-alpha", + testingEnabled: true, + expected: serrors.ErrorInvalidRef, + }, { name: "full semver with build for other repos", callerRepo: "some/repo", builderRef: "refs/tags/v1.2.3+123", expected: serrors.ErrorInvalidRef, }, + { + name: "full semver with build for other repos w/ testing enabled", + callerRepo: "some/repo", + builderRef: "refs/tags/v1.2.3+123", + testingEnabled: true, + expected: serrors.ErrorInvalidRef, + }, { name: "full semver with build/prerelease for other repos", callerRepo: "some/repo", builderRef: "refs/tags/v1.2.3-alpha+123", expected: serrors.ErrorInvalidRef, }, + { + name: "full semver with build/prerelease for other repos w/ testing enabled", + callerRepo: "some/repo", + builderRef: "refs/tags/v1.2.3-alpha+123", + testingEnabled: true, + expected: serrors.ErrorInvalidRef, + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below t.Run(tt.name, func(t *testing.T) { - t.Parallel() - wf := WorkflowIdentity{ CallerRepository: tt.callerRepo, } + if tt.testingEnabled { + t.Setenv("SLSA_VERIFIER_TESTING", "1") + } else { + // Ensure that the variable is not set. + t.Setenv("SLSA_VERIFIER_TESTING", "") + } + err := verifyTrustedBuilderRef(&wf, tt.builderRef) if !errCmp(err, tt.expected) { t.Errorf(cmp.Diff(err, tt.expected, cmpopts.EquateErrors())) diff --git a/verifiers/internal/gha/provenance.go b/verifiers/internal/gha/provenance.go index f8240cf..752ff50 100644 --- a/verifiers/internal/gha/provenance.go +++ b/verifiers/internal/gha/provenance.go @@ -317,7 +317,7 @@ func VerifyBranch(prov slsaprovenance.Provenance, expectedBranch string) error { } expectedBranch = "refs/heads/" + expectedBranch - if !strings.EqualFold(branch, expectedBranch) { + if branch != expectedBranch { return fmt.Errorf("expected branch '%s', got '%s': %w", expectedBranch, branch, serrors.ErrorMismatchBranch) } @@ -331,7 +331,7 @@ func VerifyTag(prov slsaprovenance.Provenance, expectedTag string) error { } expectedTag = "refs/tags/" + expectedTag - if !strings.EqualFold(tag, expectedTag) { + if tag != expectedTag { return fmt.Errorf("expected tag '%s', got '%s': %w", expectedTag, tag, serrors.ErrorMismatchTag) } diff --git a/verifiers/internal/gha/provenance_test.go b/verifiers/internal/gha/provenance_test.go index a2d73b4..bfa2ffa 100644 --- a/verifiers/internal/gha/provenance_test.go +++ b/verifiers/internal/gha/provenance_test.go @@ -638,6 +638,12 @@ func Test_VerifyBranch(t *testing.T) { path: "./testdata/dsse-branch3-ref-v1.intoto.jsonl", branch: "branch3", }, + { + name: "ref main case-sensitive", + path: "./testdata/dsse-main-ref-v1.intoto.jsonl", + branch: "Main", + expected: serrors.ErrorMismatchBranch, + }, { name: "invalid ref type", @@ -868,11 +874,22 @@ func Test_VerifyTag(t *testing.T) { path: "./testdata/dsse-invalid-ref-type-v1.intoto.jsonl", expected: serrors.ErrorInvalidDssePayload, }, + { + name: "tag vSLSA1 case-sensitive", + path: "./testdata/dsse-vslsa1-tag.intoto.jsonl", + tag: "vSLSA1", + expected: serrors.ErrorMismatchTag, + }, { name: "tag vslsa1", path: "./testdata/dsse-vslsa1-tag-v1.intoto.jsonl", tag: "vslsa1", }, + { + name: "case sensitive", + path: "./testdata/dsse-vslsa1-tag-v1.intoto.jsonl", + tag: "vslsa1", + }, } for _, tt := range tests { tt := tt // Re-initializing variable so it is not changed while executing the closure below