Skip to content

Commit 1778495

Browse files
author
Ian Lewis
authored
refactor: Use full builder id (#648)
Internally use full builder IDs including server url rather than worflow ref as a path. This should hopefully avoid confusion between dealing with builder IDs and `GITHUB_WORKFLOW_REF` which only contains the path portion. `GITHUB_WORKFLOW_REF` is the only thing that doesn't include the domain/server url part of the workflow/builder ID. The Fulcio OID claims include the full url. Code extracted from #641 --------- Signed-off-by: Ian Lewis <[email protected]>
1 parent 965f578 commit 1778495

File tree

8 files changed

+359
-236
lines changed

8 files changed

+359
-236
lines changed

verifiers/internal/gha/builder.go

+57-34
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ import (
44
"crypto/x509"
55
"encoding/asn1"
66
"fmt"
7+
"net/url"
78
"strings"
89

910
fulcio "github.com/sigstore/fulcio/pkg/certificate"
1011
serrors "github.com/slsa-framework/slsa-verifier/v2/errors"
1112
"github.com/slsa-framework/slsa-verifier/v2/options"
13+
"github.com/slsa-framework/slsa-verifier/v2/verifiers/internal/gha/slsaprovenance/common"
1214
"github.com/slsa-framework/slsa-verifier/v2/verifiers/utils"
1315
)
1416

@@ -24,23 +26,18 @@ var (
2426
)
2527

2628
var defaultArtifactTrustedReusableWorkflows = map[string]bool{
27-
trustedBuilderRepository + "/.github/workflows/generator_generic_slsa3.yml": true,
28-
trustedBuilderRepository + "/.github/workflows/builder_go_slsa3.yml": true,
29-
trustedBuilderRepository + "/.github/workflows/builder_container-based_slsa3.yml": true,
29+
common.GenericGeneratorBuilderID: true,
30+
common.GoBuilderID: true,
31+
common.ContainerBasedBuilderID: true,
3032
}
3133

3234
var defaultContainerTrustedReusableWorkflows = map[string]bool{
33-
trustedBuilderRepository + "/.github/workflows/generator_container_slsa3.yml": true,
35+
common.ContainerGeneratorBuilderID: true,
3436
}
3537

36-
var (
37-
delegatorGenericReusableWorkflow = trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml"
38-
delegatorLowPermsGenericReusableWorkflow = trustedBuilderRepository + "/.github/workflows/delegator_lowperms-generic_slsa3.yml"
39-
)
40-
4138
var defaultBYOBReusableWorkflows = map[string]bool{
42-
delegatorGenericReusableWorkflow: true,
43-
delegatorLowPermsGenericReusableWorkflow: true,
39+
common.GenericDelegatorBuilderID: true,
40+
common.GenericLowPermsDelegatorBuilderID: true,
4441
}
4542

4643
// VerifyCertficateSourceRepository verifies the source repository.
@@ -69,26 +66,27 @@ func VerifyBuilderIdentity(id *WorkflowIdentity,
6966
// Issuer verification.
7067
// NOTE: this is necessary before we do any further verification.
7168
if id.Issuer != certOidcIssuer {
72-
return nil, false, fmt.Errorf("%w: %s", serrors.ErrorInvalidOIDCIssuer, id.Issuer)
69+
return nil, false, fmt.Errorf("%w: %q", serrors.ErrorInvalidOIDCIssuer, id.Issuer)
7370
}
7471

75-
// cert URI path is /org/repo/path/to/workflow@ref
76-
workflowPath := strings.SplitN(id.SubjectWorkflowRef, "@", 2)
77-
if len(workflowPath) < 2 {
78-
return nil, false, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.SubjectWorkflowRef)
72+
// cert URI is https://github.com/org/repo/path/to/workflow@ref
73+
// Remove '@' from Path
74+
workflowID := id.SubjectWorkflowName()
75+
workflowTag := id.SubjectWorkflowRef()
76+
77+
if workflowID == "" || workflowTag == "" {
78+
return nil, false, fmt.Errorf("%w: workflow uri: %q", serrors.ErrorMalformedURI, id.SubjectWorkflow.String())
7979
}
8080

8181
// Verify trusted workflow.
82-
reusableWorkflowPath := strings.Trim(workflowPath[0], "/")
83-
reusableWorkflowTag := strings.Trim(workflowPath[1], "/")
84-
builderID, byob, err := verifyTrustedBuilderID(reusableWorkflowPath, reusableWorkflowTag,
82+
builderID, byob, err := verifyTrustedBuilderID(workflowID, workflowTag,
8583
builderOpts.ExpectedID, defaultBuilders)
8684
if err != nil {
8785
return nil, byob, err
8886
}
8987

9088
// Verify the ref is a full semantic version tag.
91-
if err := verifyTrustedBuilderRef(id, reusableWorkflowTag); err != nil {
89+
if err := verifyTrustedBuilderRef(id, workflowTag); err != nil {
9290
return nil, byob, err
9391
}
9492

@@ -97,26 +95,25 @@ func VerifyBuilderIdentity(id *WorkflowIdentity,
9795

9896
// Verifies the builder ID at path against an expected builderID.
9997
// If an expected builderID is not provided, uses the defaultBuilders.
100-
func verifyTrustedBuilderID(certPath, certTag string, expectedBuilderID *string, defaultTrustedBuilders map[string]bool) (*utils.TrustedBuilderID, bool, error) {
98+
func verifyTrustedBuilderID(certBuilderID, certTag string, expectedBuilderID *string, defaultTrustedBuilders map[string]bool) (*utils.TrustedBuilderID, bool, error) {
10199
var trustedBuilderID *utils.TrustedBuilderID
102100
var err error
103-
certBuilderName := httpsGithubCom + certPath
104101
// WARNING: we don't validate the tag here, because we need to allow
105102
// refs/heads/main for e2e tests. See verifyTrustedBuilderRef().
106103
// No builder ID provided by user: use the default trusted workflows.
107104
if expectedBuilderID == nil || *expectedBuilderID == "" {
108-
if _, ok := defaultTrustedBuilders[certPath]; !ok {
109-
return nil, false, fmt.Errorf("%w: %s with builderID provided: %t", serrors.ErrorUntrustedReusableWorkflow, certPath, expectedBuilderID != nil)
105+
if _, ok := defaultTrustedBuilders[certBuilderID]; !ok {
106+
return nil, false, fmt.Errorf("%w: %s with builderID provided: %t", serrors.ErrorUntrustedReusableWorkflow, certBuilderID, expectedBuilderID != nil)
110107
}
111108
// Construct the builderID using the certificate's builder's name and tag.
112-
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName+"@"+certTag, true)
109+
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true)
113110
if err != nil {
114111
return nil, false, err
115112
}
116113
} else {
117114
// Verify the builderID.
118115
// We only accept IDs on github.com.
119-
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderName+"@"+certTag, true)
116+
trustedBuilderID, err = utils.TrustedBuilderIDNew(certBuilderID+"@"+certTag, true)
120117
if err != nil {
121118
return nil, false, err
122119
}
@@ -144,7 +141,7 @@ func verifyTrustedBuilderID(certPath, certTag string, expectedBuilderID *string,
144141
func isTrustedDelegatorBuilder(certBuilder *utils.TrustedBuilderID, trustedBuilders map[string]bool) bool {
145142
for byobBuilder := range defaultBYOBReusableWorkflows {
146143
// Check that the certificate builder is a BYOB workflow.
147-
if err := certBuilder.MatchesLoose(httpsGithubCom+byobBuilder, true); err == nil {
144+
if err := certBuilder.MatchesLoose(byobBuilder, true); err == nil {
148145
// We found a delegator workflow that matches the certificate identity.
149146
// Check that the BYOB builder is trusted by the caller.
150147
if _, ok := trustedBuilders[byobBuilder]; !ok {
@@ -204,6 +201,7 @@ const (
204201
HostedGitHub
205202
)
206203

204+
// WorkflowIdentity is a identity captured from a Fulcio certificate.
207205
// See https://github.com/sigstore/fulcio/blob/main/docs/oid-info.md.
208206
type WorkflowIdentity struct {
209207
// The source repository
@@ -218,7 +216,7 @@ type WorkflowIdentity struct {
218216
SourceOwnerID *string
219217

220218
// Workflow path OIDC subject - ref of reuseable workflow or trigger workflow.
221-
SubjectWorkflowRef string
219+
SubjectWorkflow *url.URL
222220
// Subject commit sha1.
223221
SubjectSha1 *string
224222
// Hosted status of the subject.
@@ -235,6 +233,33 @@ type WorkflowIdentity struct {
235233
Issuer string
236234
}
237235

236+
// SubjectWorkflowName returns the subject workflow without the git ref.
237+
func (id *WorkflowIdentity) SubjectWorkflowName() string {
238+
// NOTE: You should be able to copy a net.URL struct safely.
239+
// See: https://github.com/golang/go/issues/38351
240+
withoutRef := *id.SubjectWorkflow
241+
withoutRef.Path = id.SubjectWorkflowPath()
242+
return withoutRef.String()
243+
}
244+
245+
// SubjectWorkflowPath returns the subject workflow without the server url.
246+
func (id *WorkflowIdentity) SubjectWorkflowPath() string {
247+
i := strings.LastIndex(id.SubjectWorkflow.Path, "@")
248+
if i == -1 {
249+
return id.SubjectWorkflow.Path
250+
}
251+
return id.SubjectWorkflow.Path[:i]
252+
}
253+
254+
// SubjectWorkflowRef returns the ref for the subject workflow.
255+
func (id *WorkflowIdentity) SubjectWorkflowRef() string {
256+
i := strings.LastIndex(id.SubjectWorkflow.Path, "@")
257+
if i == -1 {
258+
return ""
259+
}
260+
return id.SubjectWorkflow.Path[i+1:]
261+
}
262+
238263
func getHosted(cert *x509.Certificate) (*Hosted, error) {
239264
runnerEnv, err := getExtension(cert, fulcio.OIDRunnerEnvironment, true)
240265
if err != nil {
@@ -423,9 +448,7 @@ func GetWorkflowInfoFromCertificate(cert *x509.Certificate) (*WorkflowIdentity,
423448
if !strings.HasPrefix(cert.URIs[0].Path, "/") {
424449
return nil, fmt.Errorf("%w: %s", serrors.ErrorInvalidFormat, cert.URIs[0].Path)
425450
}
426-
// Remove the starting '/'.
427-
// NOTE: The Path has the following structure: repo/name/path/to/workflow.yml@ref.
428-
subjectWorkflowRef := cert.URIs[0].Path[1:]
451+
subjectWorkflow := cert.URIs[0]
429452

430453
var pSubjectSha1, pSourceID, pSourceRef, pSourceOwnerID, pBuildConfigPath, pRunID *string
431454
if subjectSha1 != "" {
@@ -451,9 +474,9 @@ func GetWorkflowInfoFromCertificate(cert *x509.Certificate) (*WorkflowIdentity,
451474
// Issuer.
452475
Issuer: issuer,
453476
// Subject
454-
SubjectWorkflowRef: subjectWorkflowRef,
455-
SubjectSha1: pSubjectSha1,
456-
SubjectHosted: subjectHosted,
477+
SubjectWorkflow: subjectWorkflow,
478+
SubjectSha1: pSubjectSha1,
479+
SubjectHosted: subjectHosted,
457480
// Source.
458481
SourceRepository: sourceRepository,
459482
SourceSha1: sourceSha1,

0 commit comments

Comments
 (0)