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

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

Conversation

yob
Copy link

@yob yob commented Aug 6, 2023

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 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 #1074

/cc @sj26

Release Note

NONE

Documentation

Uncertain

…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]>
@yob yob marked this pull request as draft August 6, 2023 14:23

// 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.

@haydentherapper
Copy link
Contributor

cc @feelepxyz @marshall007 @wlynch for more thoughts

Thank you for starting this!

Issuer: p.issuer,
RunInvocationURI: jobUrl.String(),
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

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.)

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.

@haydentherapper
Copy link
Contributor

Hey all, just wanted to check in, has there been any progress on this?

@richardfan1126
Copy link
Contributor

richardfan1126 commented Sep 6, 2024

I believe this PR is obsolete due to the change introduced in


Instead, we need to update

*buildkite-type:
default-template-values:
url: "https://buildkite.com"
subject-alternative-name-template: "{{.url}}/{{.organization_slug}}/{{.pipeline_slug}}"

@haydentherapper
Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Buildkite issuer to include new extensions
6 participants