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.

Sign up for GitHub

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

Update Buildkite issuer to include some of the new certificate extensions #1307

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Update Buildkite issuer to include some of the new certificate extensions #1307

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 61 additions & 12 deletions pkg/identity/buildkite/principal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ package buildkite
import (
"context"
"crypto/x509"
"encoding/json"
"errors"
"fmt"
"net/url"
"strconv"

"github.com/coreos/go-oidc/v3/oidc"
"github.com/sigstore/fulcio/pkg/certificate"
Expand All @@ -35,16 +36,38 @@ type jobPrincipal struct {
// https://agent.buildkite.com/.well-known/openid-configuration
issuer string

// The URL of the pipeline, the container of many builds. This will be
// set as a human-friendly SubjectAlternativeName URI in the certificate.
// Buildkite's domain
url string

// Unique identifier for a Buildkite customer
organizationSlug string

// Unique identifier (within the scope of an OrganizationSlug) for a container of many builds.
pipelineSlug string

// Incrementing number within each pipeline
buildNumber int64

// The commit sha being tested by a build
buildCommit string

// UUID that identifies a single unique execution within a build
jobId string

// Did the job run in a cloud hosted environment or self hosted by the customer. All
// Buildkite jobs execute on self hosted agents, so this will always be `self-hosted`
runnerEnvironment string
}

func JobPrincipalFromIDToken(_ context.Context, token *oidc.IDToken) (identity.Principal, error) {
var claims struct {
OrganizationSlug string `json:"organization_slug"`
PipelineSlug string `json:"pipeline_slug"`
OrganizationSlug string `json:"organization_slug"`
PipelineSlug string `json:"pipeline_slug"`
BuildNumber json.Number `json:"build_number"`
BuildCommit string `json:"build_commit"`
JobId string `json:"job_id"`
}

if err := token.Claims(&claims); err != nil {
return nil, err
}
Expand All @@ -57,10 +80,29 @@ func JobPrincipalFromIDToken(_ context.Context, token *oidc.IDToken) (identity.P
return nil, errors.New("missing pipeline_slug claim in ID token")
}

buildNumber, err := claims.BuildNumber.Int64()
if err != nil {
return nil, errors.New("error parsing build_number claim in ID token")
}

if claims.BuildCommit == "" {
return nil, errors.New("missing build_commit claim in ID token")
}

if claims.JobId == "" {
return nil, errors.New("missing job_id claim in ID token")
}

return &jobPrincipal{
subject: token.Subject,
issuer: token.Issuer,
url: fmt.Sprintf("https://buildkite.com/%s/%s", claims.OrganizationSlug, claims.PipelineSlug),
subject: token.Subject,
issuer: token.Issuer,
url: "https://buildkite.com",
organizationSlug: claims.OrganizationSlug,
pipelineSlug: claims.PipelineSlug,
buildNumber: buildNumber,
buildCommit: claims.BuildCommit,
jobId: claims.JobId,
runnerEnvironment: "self-hosted",
}, nil
}

Expand All @@ -69,16 +111,23 @@ func (p jobPrincipal) Name(_ context.Context) string {
}

func (p jobPrincipal) Embed(_ context.Context, cert *x509.Certificate) error {
// Set SubjectAlternativeName to the pipeline URL on the certificate
parsed, err := url.Parse(p.url)
baseURL, err := url.Parse(p.url)
if err != nil {
return err
}
cert.URIs = []*url.URL{parsed}

pipelineUrl := baseURL.JoinPath(p.organizationSlug, p.pipelineSlug)
jobUrl := baseURL.JoinPath(p.organizationSlug, p.pipelineSlug, "builds", strconv.FormatInt(p.buildNumber, 10)+"#"+p.jobId)

// Set SubjectAlternativeName to the pipeline URL on the certificate
cert.URIs = []*url.URL{pipelineUrl}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this support a use case to write a verification policy to only allow builds from a particular buildkite project/pipeline/thing. Looks like this identifier would be stable across jobs but unique to the parent resource responsible for running the job. Could you have different build's from different repos running with the same org and pipeline slug?

I would also use this value to populate BuildSignerURI as these are currently mirrored for github and gitlab. If a pipeline slug is always configured from a pipeline.yml and we know the sha of the pipleline file we could also use this to populate BuildConfigDigest and BuildSignerDigest.


// Embed additional information into custom extensions
cert.ExtraExtensions, err = certificate.Extensions{
Issuer: p.issuer,
Issuer: p.issuer,
RunInvocationURI: jobUrl.String(),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the one I'm most interested in - having the specific job URL in the certificate to provide provenance would be excellent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yob I think it makes sense, we went job-specific for the GitLab provider: https://github.com/sigstore/fulcio/blob/04e4ac9b2d867d0f94e58a858f373a831d87653d/pkg/identity/gitlabcom/principal.go#L233C63-L233C63

Some related discussion in #1182. I found it confusing that this extension was named RunInvocationURI (instead of BuildInvocationURI), sort of implying it ought to be pipeline-level.

GitHub does not have job_id available in their ID tokens, so they don't have the option to align with us on that presently.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 job url makes sense to me. "run invocation" terminology was cribbed from the slsa spec.

RunnerEnvironment: p.runnerEnvironment,
SourceRepositoryDigest: p.buildCommit,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to see support for a few additional extensions so we could get richer provenance info:

  • Would it not be possible to also forward the repo url and ref from the originating source host to populate SourceRepositoryURI and SourceRepositoryRef? What about the repo and owner IDs?
  • Could you point the BuildConfigURI to pipeline.yml file that configures the build? Is there something else that configures the build? Is the sha of this always the same as p.buildCommit?
  • Is there not some concept of the event trigger that kicks of the job in buildkite that could be used to populate BuildTrigger?

Copy link
Contributor

@sj26 sj26 Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buildkite has no access to the source repository.

Because of this, SourceRepositoryURI and SourceRepositoryRef are resolved within the runner environment, and are not always known on the Buildkite side which generates the OIDC tokens. We have a repository url which is provided as an input to the runner, but it is not always a fully-qualified URI. We have a build branch and commit, but these often start as values like HEAD and are resolved to a concrete ref by the runner which sends back the resolve commit sha.

Similarly for BuildConfigURI. The initial steps are defined as part of the pipeline on Buildkite. But those initial steps are usually just a single "upload" step. That step runs within the runner environment, to access the source repository. It conventionally looks at buildkite.yml or .buildkite/pipeline.yml, but can choose any file in the source repository. It may not even be a file, the runner may generate content, like with a script, which is piped into an upload command to use as build config. And the build is not limited to one upload - many uploads may happen over the course of a build, from any step, sometimes in a chain.

"build config" beyond those initial steps is not an immutable input to a build for us, but an output. (There are ways to make it immutable, but it's not the default, and not something we can currently provide at platform level.)

BuildTrigger is probably something we could add. But I'd lean towards another PR for that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'd love to see the contents of this PR land as an increment, and consider additional extensions later.)

}.Render()
if err != nil {
return err
Expand Down