-
Notifications
You must be signed in to change notification settings - Fork 139
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
Update Buildkite issuer to include some of the new certificate extensions #1307
base: main
Are you sure you want to change the base?
Conversation
…ions The Buildkite Issuer was added in sigstore#890, prior to the efforts to standardise certificate extensions for CI providers, and sigstore#1074 calls for the Buildkite issuer to be updated to use the new extensions (where applicable). This is an early attempt to make those changes. I've added the extensions that make the most sense in a Buildkite context, like RunInvocationURI, RunnerEnvironment and SourceRepositoryDiget. Many of the other extensions don't apply because we're not a code host as well, or need further discussion. I have not added tests yet. This is my first contribution to fulcio and I'm keen to confirm I'm heading in the right direction before adding tests. However, I have tested this locally with a Buildkite agent and OIDC token, and the certificate was issued as expected. Using `git cat-file commit HEAD` and piping it through `openssl pkcs7 -print -print_certs -text`, the extensions section looks like this: X509v3 extensions: X509v3 Key Usage: critical Digital Signature X509v3 Extended Key Usage: Code Signing X509v3 Subject Key Identifier: 19:9E:E7:53:4D:F6:65:D4:23:9D:60:21:B8:F3:12:80:FD:11:30:7F X509v3 Authority Key Identifier: 8A:3E:9E:34:19:F7:32:18:3D:2A:1B:F9:5F:60:29:24:0F:70:0B:B4 X509v3 Subject Alternative Name: critical URI:https://buildkite.com/yob-opensource/oidc-signing-experiment 1.3.6.1.4.1.57264.1.1: https://agent.buildkite.com 1.3.6.1.4.1.57264.1.8: ..https://agent.buildkite.com 1.3.6.1.4.1.57264.1.11: ..self-hosted 1.3.6.1.4.1.57264.1.13: .(5242de9e5c2ca164cb3dfc500fb605f0b8b86763 1.3.6.1.4.1.57264.1.21: .mhttps://buildkite.com/yob-opensource/oidc-signing-experiment/builds/35%230189cb29-62fa-41af-8641-62e1d6c5edfd Fixes sigstore#1074 Signed-off-by: James Healy <[email protected]>
|
||
// Embed additional information into custom extensions | ||
cert.ExtraExtensions, err = certificate.Extensions{ | ||
Issuer: p.issuer, | ||
Issuer: p.issuer, | ||
RunInvocationURI: jobUrl.String(), |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
cc @feelepxyz @marshall007 @wlynch for more thoughts Thank you for starting this! |
Issuer: p.issuer, | ||
RunInvocationURI: jobUrl.String(), | ||
RunnerEnvironment: p.runnerEnvironment, | ||
SourceRepositoryDigest: p.buildCommit, |
There was a problem hiding this comment.
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
andSourceRepositoryRef
? What about the repo and owner IDs? - Could you point the
BuildConfigURI
topipeline.yml
file that configures the build? Is there something else that configures the build? Is the sha of this always the same asp.buildCommit
? - Is there not some concept of the event trigger that kicks of the job in buildkite that could be used to populate
BuildTrigger
?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
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} |
There was a problem hiding this comment.
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
.
Hey all, just wanted to check in, has there been any progress on this? |
I believe this PR is obsolete due to the change introduced in
Instead, we need to update fulcio/config/identity/config.yaml Lines 230 to 233 in 5237979
|
I agree no more work is needed on the PR - If someone could convert the changes over to configuration in the above linked file, that would be appreciated! |
Summary
The Buildkite Issuer was added in #890, prior to the efforts to standardise certificate extensions for CI providers, and #1074 calls for the Buildkite issuer to be updated to use the new extensions (where applicable).
This is an early attempt to make those changes.
I've added the extensions that make the most sense in a Buildkite context, like RunInvocationURI, RunnerEnvironment and SourceRepositoryDiget. Many of the other extensions don't apply because we're not a code host as well, or need further discussion.
I have not added tests yet. This is my first contribution to fulcio and I'm keen to confirm I'm heading in the right direction before adding tests. However, I have tested this locally with a Buildkite agent and OIDC token, and the certificate was issued as expected.
Using
git cat-file commit HEAD
and piping it throughopenssl pkcs7 -print -print_certs -text
, the extensions section looks like this:Fixes #1074
/cc @sj26
Release Note
NONE
Documentation
Uncertain