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 11 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
6 changes: 4 additions & 2 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ 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 parsed from the envelope
// of the provenance.
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
ExpectedBuilderID string

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

// BuildOpts are the options for checking the builder.
type BuilderOpts struct {
// ExpectedID is the expected builder ID.
// ExpectedID is the expected builder ID that is inputted
// at the command line by the user to be verified.
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
ExpectedID *string
}
14 changes: 14 additions & 0 deletions verifiers/internal/gha/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,25 @@ 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
}

// 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.
//
// This return enables non-compulsory builderID feature for BYOB builders
// by setting byob flag to true.
if isTrustedDelegatorBuilder(trustedBuilderID, defaultTrustedBuilders) {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
return trustedBuilderID, true, nil
}

} else {
// Verify the builderID.
// We only accept IDs on github.com.
Expand Down
12 changes: 12 additions & 0 deletions verifiers/internal/gha/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,15 @@ func Test_verifyTrustedBuilderID(t *testing.T) {
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",
path: trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml",
tag: "refs/tags/v1.2.3",
defaults: defaultBYOBReusableWorkflows,
byob: true,
},
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
{
name: "low perms delegator workflow short tag",
path: trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml",
Expand All @@ -486,10 +495,13 @@ func Test_verifyTrustedBuilderID(t *testing.T) {
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
tag: "v1.2.3",
defaults: defaultBYOBReusableWorkflows,
byob: true,
},
{
name: "default mismatch against container defaults long tag",
Expand Down
45 changes: 43 additions & 2 deletions verifiers/internal/gha/provenance.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,31 @@ func verifyBuilderIDExactMatch(prov iface.Provenance, expectedBuilderID string)
return nil
}

// Verifies expectedBuilderIDPath against path in provenance, and returns the actual
// builderIDPath if verified against inputted expected path.
//
// Example: the expectedBuilderID will default to the delegator builder ID for BYOB.
// This can verify actual BYOB builderIDPath against the trusted builderIDPath inputted,
// Currently slsa-framework path is the only one supported for ExpectedBuilderPath.
func verifyBuilderIDPath(prov iface.Provenance, expectedBuilderIDPath string) (string, error) {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
id, err := prov.BuilderID()
if err != nil {
return "", err
}

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

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

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 +95,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 @@ -286,7 +312,8 @@ func isValidDelegatorBuilderID(prov iface.Provenance) error {
}

// 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,
builderOpts *options.BuilderOpts) error {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
prov, err := slsaprovenance.ProvenanceFromEnvelope(trustedBuilderID.Name(), env)
if err != nil {
return err
Expand All @@ -297,7 +324,21 @@ 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 empty, check to see if it is a trusted builder.
// If empty and trusted builder, populate with path from provenance,
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
// otherwise, populate from user input.
if builderOpts.ExpectedID == nil || *builderOpts.ExpectedID == "" {
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
var trustedBuilderRepositoryPath = httpsGithubCom + trustedBuilderRepository + "/.github/workflows/"
if provenanceOpts.ExpectedBuilderID, err = verifyBuilderIDPath(prov, trustedBuilderRepositoryPath); err != nil {
return err
}
} else {
provenanceOpts.ExpectedBuilderID = *builderOpts.ExpectedID
}

// Note: `provenanceOpts.ExpectedBuilderID` is provided by the user
enteraga6 marked this conversation as resolved.
Show resolved Hide resolved
// or from return of verifyBuilderIDPath.
if err := verifyBuilderIDLooseMatch(prov, provenanceOpts.ExpectedBuilderID); err != nil {
return err
}
Expand Down
11 changes: 2 additions & 9 deletions verifiers/internal/gha/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,8 @@ func verifyEnvAndCert(env *dsse.Envelope,
// There is a corner-case to handle: if the verified builder ID from the cert
// is a delegator builder, the user MUST provide an expected builder ID
// and we MUST match it against the content of the provenance.
if byob {
if builderOpts.ExpectedID == nil || *builderOpts.ExpectedID == "" {
// NOTE: we will need to update the logic here once our default trusted builders
// are migrated to using BYOB.
return nil, nil, fmt.Errorf("%w: empty ID", serrors.ErrorInvalidBuilderID)
}
provenanceOpts.ExpectedBuilderID = *builderOpts.ExpectedID
}
if err := VerifyProvenance(env, provenanceOpts, verifiedBuilderID, byob); err != nil {

if err := VerifyProvenance(env, provenanceOpts, verifiedBuilderID, byob, builderOpts); err != nil {
return nil, nil, err
}

Expand Down