Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Non-compulsory BuilderID for BYOB Builders #674

Merged
merged 42 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
646a3ad
Squash commits from byob patch
enteraga6 Jul 31, 2023
fa2148a
improve error message and comments
enteraga6 Jul 31, 2023
ebf4ffc
Improvements
enteraga6 Jul 31, 2023
67c29de
Fixed logic and comments from review feedback
enteraga6 Aug 1, 2023
38ce418
changed scope of comment
enteraga6 Aug 1, 2023
03b26a6
Improved function description
enteraga6 Aug 1, 2023
1095060
condense comment
enteraga6 Aug 1, 2023
acac0b8
space nit
enteraga6 Aug 1, 2023
fd17c7d
nit: end sentence with period
enteraga6 Aug 1, 2023
eb104b8
Fix function and improve comment on conditional logic
enteraga6 Aug 1, 2023
c974544
Use builder.go vars for trusted builder string
enteraga6 Aug 1, 2023
9318f88
comment fix
enteraga6 Aug 1, 2023
ae63dee
moved comment to caller for better understanding
enteraga6 Aug 1, 2023
f00ae0d
nit: start doc comments with the name of the function as if it was pu…
enteraga6 Aug 2, 2023
42d475e
nit: builderIDPath --> builderIDPathPrefix
enteraga6 Aug 2, 2023
90aa32a
nit: pass in only expectedID instead of entire builderOpts
enteraga6 Aug 2, 2023
2301ade
nit: only pass in builderOpts.ExpectedID
enteraga6 Aug 2, 2023
996892d
nit: fix off comment
enteraga6 Aug 2, 2023
217f027
nit: inputted
enteraga6 Aug 2, 2023
78e971e
nit: inputted
enteraga6 Aug 2, 2023
c214062
nit: note --> NOTE
enteraga6 Aug 2, 2023
dc80287
style: remove else block
enteraga6 Aug 2, 2023
5245b53
nit: un-needed usage case for --builder-id
enteraga6 Aug 2, 2023
5c944e6
Merge branch 'slsa-framework:main' into byob-builder-feat
enteraga6 Aug 7, 2023
d03a11e
trusted builder test case added
enteraga6 Aug 7, 2023
0118fea
Added test case for untrusted builder
enteraga6 Aug 7, 2023
4209ffe
lint
enteraga6 Aug 7, 2023
6cd1257
explicity set id to nil
enteraga6 Aug 7, 2023
4fcc8b1
set id to nil explicitly
enteraga6 Aug 7, 2023
1a17e0c
nit: capitalization
enteraga6 Aug 7, 2023
c98f650
nit: remove empty lines
enteraga6 Aug 7, 2023
ca9e7ae
nit: remove empty lines
enteraga6 Aug 7, 2023
032b098
remove confusing comment
enteraga6 Aug 7, 2023
e999450
Merge remote-tracking branch 'origin/byob-builder-feat' into byob-bui…
enteraga6 Aug 7, 2023
1ff4e9b
nit: remove empty space
enteraga6 Aug 7, 2023
e6d22d4
Restructure Tests
enteraga6 Aug 11, 2023
b6e36b7
rename for clarity
enteraga6 Aug 11, 2023
3ba89d9
use t.Errorf
enteraga6 Aug 11, 2023
7ada2ad
Go Lint
enteraga6 Aug 11, 2023
a630e1f
Merge branch 'main' into byob-builder-feat
laurentsimon Aug 11, 2023
82d0676
Merged tests and added more
enteraga6 Aug 11, 2023
ba333a5
Merge remote-tracking branch 'origin/byob-builder-feat' into byob-bui…
enteraga6 Aug 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type ProvenanceOpts struct {
// ExpectedSourceURI is the expected source URI in the provenance.
ExpectedSourceURI string

// ExpectedBuilderID is the expected builder ID.
// ExpectedBuilderID is the expected builder ID that is passed from user and verified
ExpectedBuilderID string

// ExpectedWorkflowInputs is a map of key=value inputs.
Expand All @@ -31,6 +31,6 @@ type ProvenanceOpts struct {

// BuildOpts are the options for checking the builder.
type BuilderOpts struct {
// ExpectedID is the expected builder ID.
// ExpectedBuilderID is the builderID passed in from the user to be verified
ExpectedID *string
}
43 changes: 27 additions & 16 deletions verifiers/internal/gha/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,34 +109,45 @@ func verifyTrustedBuilderID(certBuilderID, certTag string, expectedBuilderID *st
if _, ok := defaultTrustedBuilders[certBuilderID]; !ok {
return nil, false, fmt.Errorf("%w: %s with builderID provided: %t", serrors.ErrorUntrustedReusableWorkflow, certBuilderID, expectedBuilderID != nil)
}

// Construct the builderID using the certificate's builder's name and tag.
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true)
if err != nil {
return nil, false, err
}
} else {
// Verify the builderID.
// We only accept IDs on github.com.
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true)
if err != nil {
return nil, false, err
}

// Check if:
// - the builder in the cert is a BYOB builder
// - the caller trusts the BYOB builder
// If both are true, we don't match the user-provided builder ID
// against the certificate. Instead that will be done by the caller.
if isTrustedDelegatorBuilder(trustedBuilderID, defaultTrustedBuilders) {
return trustedBuilderID, true, nil
}
//
// This return of the delegator builderID enables non-compulsory
// builderID feature for BYOB builders by setting byob flag to true.
return trustedBuilderID, isTrustedDelegatorBuilder(trustedBuilderID, defaultTrustedBuilders), nil
}

// Not a BYOB builder. BuilderID provided by user should match the certificate.
// Note: the certificate builderID has the form `name@refs/tags/v1.2.3`,
// so we pass `allowRef = true`.
if err := trustedBuilderID.MatchesLoose(*expectedBuilderID, true); err != nil {
return nil, false, fmt.Errorf("%w: %v", serrors.ErrorUntrustedReusableWorkflow, err)
}
// Verify the builderID.
// We only accept IDs on github.com.
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true)
if err != nil {
return nil, false, err
}

// Check if:
// - the builder in the cert is a BYOB builder
// - the caller trusts the BYOB builder
// If both are true, we don't match the user-provided builder ID
// against the certificate. Instead that will be done by the caller.
if isTrustedDelegatorBuilder(trustedBuilderID, defaultTrustedBuilders) {
return trustedBuilderID, true, nil
}

// Not a BYOB builder. BuilderID provided by user should match the certificate.
// Note: the certificate builderID has the form `name@refs/tags/v1.2.3`,
// so we pass `allowRef = true`.
if err := trustedBuilderID.MatchesLoose(*expectedBuilderID, true); err != nil {
return nil, false, fmt.Errorf("%w: %v", serrors.ErrorUntrustedReusableWorkflow, err)
}

return trustedBuilderID, false, nil
Expand Down
16 changes: 16 additions & 0 deletions verifiers/internal/gha/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,17 @@
defaults: defaultBYOBReusableWorkflows,
byob: true,
},
{
// This is a BYOB workflow without an id that tests non-compulsory builder-id
// feature of slsa-verifier and expects byob to be true
name: "generic delegator workflow no id",

Check failure on line 483 in verifiers/internal/gha/builder_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

File is not `gofmt`-ed with `-s` (gofmt)
path: trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml",
// NOTE: id is nil.
id: nil,
tag: "refs/tags/v1.2.3",
defaults: defaultBYOBReusableWorkflows,
byob: true,
},
{
name: "low perms delegator workflow short tag",
path: trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml",
Expand All @@ -486,10 +497,15 @@
byob: true,
},
{
// This is a BYOB workflow without an id that tests non-compulsory builder-id
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
// feature of slsa-verifier and expects byob to be true
name: "low perms delegator workflow no ID provided",
path: trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml",
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
// NOTE: id is nil.
id: nil,
tag: "v1.2.3",
defaults: defaultBYOBReusableWorkflows,
byob: true,
},
{
name: "default mismatch against container defaults long tag",
Expand Down
44 changes: 42 additions & 2 deletions verifiers/internal/gha/provenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,27 @@ func verifyBuilderIDExactMatch(prov iface.Provenance, expectedBuilderID string)
return nil
}

// verifyBuilderIDPathPrefix verifies that the builder ID in provenance matches the provided expectedBuilderIDPathPrefix.
// Returns provenance builderID if verified against provided expected Builder ID path prefix.
func verifyBuilderIDPathPrefix(prov iface.Provenance, expectedBuilderIDPathPrefix string) (string, error) {
id, err := prov.BuilderID()
if err != nil {
return "", err
}

provBuilderID, err := utils.TrustedBuilderIDNew(id, false)
if err != nil {
return "", err
}

// Compare actual BuilderID with the expected BuilderID Path Prefix.
if !strings.HasPrefix(provBuilderID.Name(), expectedBuilderIDPathPrefix) {
return "", fmt.Errorf("%w: BuilderID Path Mismatch. Got: %q. Expected BuilderID Path Prefix: %q", serrors.ErrorInvalidBuilderID, provBuilderID.Name(), expectedBuilderIDPathPrefix)
}

return provBuilderID.Name(), nil
}

// Verify Builder ID in provenance statement.
// This function verifies the names match. If the expected builder ID contains a version,
// it also verifies the versions match.
Expand All @@ -70,6 +91,7 @@ func verifyBuilderIDLooseMatch(prov iface.Provenance, expectedBuilderID string)
if err != nil {
return err
}

if err := provBuilderID.MatchesLoose(expectedBuilderID, true); err != nil {
return err
}
Expand Down Expand Up @@ -304,7 +326,8 @@ func builderID(env *dsselib.Envelope, certTrustedBuilderID *utils.TrustedBuilder
}

// VerifyProvenance verifies the provenance for the given DSSE envelope.
func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceOpts, trustedBuilderID *utils.TrustedBuilderID, byob bool) error {
func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceOpts, trustedBuilderID *utils.TrustedBuilderID, byob bool,
expectedID *string) error {
prov, err := slsaprovenance.ProvenanceFromEnvelope(trustedBuilderID.Name(), env)
if err != nil {
return err
Expand All @@ -315,7 +338,24 @@ func VerifyProvenance(env *dsselib.Envelope, provenanceOpts *options.ProvenanceO
if err := isValidDelegatorBuilderID(prov); err != nil {
return err
}
// Note: `provenanceOpts.ExpectedBuilderID` is provided by the user.

// If expectedID is not provided, check to see if it is a trusted builder.
// If not provided, then a trusted builder is expected, to populate provenanceOpts.ExpectedBuilderID
// with that builder, otherwise, populate from user input.
//
// This can verify the actual BYOB builderIDPath against the trusted builderIDPath provided.
// Currently slsa-framework path is the only one supported for ExpectedBuilderPath.
if expectedID == nil {
var trustedBuilderRepositoryPath = httpsGithubCom + trustedBuilderRepository + "/.github/workflows/"
if provenanceOpts.ExpectedBuilderID, err = verifyBuilderIDPathPrefix(prov, trustedBuilderRepositoryPath); err != nil {
return err
}
} else {
provenanceOpts.ExpectedBuilderID = *expectedID
}

// NOTE: `provenanceOpts.ExpectedBuilderID` is provided by the user
// or from return of verifyBuilderIDPath.
if err := verifyBuilderIDLooseMatch(prov, provenanceOpts.ExpectedBuilderID); err != nil {
return err
}
Expand Down
115 changes: 115 additions & 0 deletions verifiers/internal/gha/provenance_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package gha

import (
"fmt"
"os"
"path/filepath"
"testing"
"time"

Expand All @@ -9,6 +12,8 @@ import (
slsacommon "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/common"
slsa02 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v0.2"
slsa1 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v1"
"github.com/slsa-framework/slsa-verifier/v2/options"
"github.com/slsa-framework/slsa-verifier/v2/verifiers/utils"

serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common"
Expand Down Expand Up @@ -1182,3 +1187,113 @@ func Test_VerifyVersionedTag(t *testing.T) {
})
}
}

func Test_VerifyTrustedProvenance(t *testing.T) {
t.Parallel()
tests := []struct {
name string
envelopePath string
provenanceOpts *options.ProvenanceOpts
trustedBuilderIDName string
byob bool
expectedID *string
expected error
}{
{
name: "Verify Trusted (slsa-github-generator) Bazel Builder (v1.8.0",
envelopePath: "bazel-trusted-dsseEnvelope.build.slsa",
provenanceOpts: &options.ProvenanceOpts{
ExpectedBranch: nil,
ExpectedTag: nil,
ExpectedVersionedTag: nil,
ExpectedDigest: "caaadba2846905ac477c777e96a636e1c2e067fdf6fed90ec9eeca4df18d6ed9",
ExpectedSourceURI: "github.com/enteraga6/slsa-lvl3-generic-provenance-with-bazel-example",
ExpectedBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.8.0",
ExpectedWorkflowInputs: map[string]string{},
},
byob: true,
trustedBuilderIDName: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.8.0",
expectedID: nil,
},
}
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()
trustedBuilderID, tErr := utils.TrustedBuilderIDNew(tt.trustedBuilderIDName, true)
if tErr != nil {
t.Errorf("Provenance Verification FAILED. Error: %v", tErr)
}

envelopeBytes, err := os.ReadFile(filepath.Join("testdata", tt.envelopePath))
if err != nil {
panic(fmt.Errorf("os.ReadFile: %w", err))
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
}

env, err := EnvelopeFromBytes(envelopeBytes)
if err != nil {
panic(fmt.Errorf("unexpected error parsing envelope %s", err))
}

vErr := VerifyProvenance(env, tt.provenanceOpts, trustedBuilderID, tt.byob, tt.expectedID)
if vErr != nil {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
t.Errorf("Provenance Verification FAILED. Error: %v", vErr)
}
})
}
}

func Test_VerifyUntrustedProvenance(t *testing.T) {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
t.Parallel()
tests := []struct {
name string
envelopePath string
provenanceOpts *options.ProvenanceOpts
trustedBuilderIDName string
byob bool
expectedID *string
expected error
}{
{
name: "Verify Un-Trusted (slsa-github-generator) Bazel Builder (from enteraga6/slsa-github-generator)",
envelopePath: "bazel-untrusted-dsseEnvelope.sigstore",
provenanceOpts: &options.ProvenanceOpts{
ExpectedBranch: nil,
ExpectedTag: nil,
ExpectedVersionedTag: nil,
ExpectedDigest: "caaadba2846905ac477c777e96a636e1c2e067fdf6fed90ec9eeca4df18d6ed9",
ExpectedSourceURI: "github.com/enteraga6/slsa-lvl3-generic-provenance-with-bazel-example",
ExpectedBuilderID: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.7.0",
ExpectedWorkflowInputs: map[string]string{},
},
byob: true,
trustedBuilderIDName: "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/delegator_lowperms-generic_slsa3.yml@refs/tags/v1.7.0",
expectedID: nil,
},
}
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()
trustedBuilderID, tErr := utils.TrustedBuilderIDNew(tt.trustedBuilderIDName, true)
if tErr != nil {
t.Errorf("Provenance Verification FAILED. Error: %v", tErr)
}

envelopeBytes, err := os.ReadFile(filepath.Join("testdata", tt.envelopePath))
if err != nil {
panic(fmt.Errorf("os.ReadFile: %w", err))
}

env, err := EnvelopeFromBytes(envelopeBytes)
if err != nil {
panic(fmt.Errorf("unexpected error parsing envelope %s", err))
}

vErr := VerifyProvenance(env, tt.provenanceOpts, trustedBuilderID, tt.byob, tt.expectedID)
if vErr == nil {
t.Errorf("Provenance Verification should have failed but DID NOT. Error: untrusted builder is trusted")
}
})
}
}
Loading
Loading