Configure additional certificate extensions for Buildkite

[yob]

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 initially started these in #1307, however this is a new swing at it using the new CIProvider issuer (see #1729 and #1743).

I've added the extensions that make the most sense in a Buildkite context, like RunInvocationURI, RunnerEnvironment and SourceRepositoryDigest. Many of the other extensions don't apply because we're not a code host as well, or need further discussion.

I have tested this locally with a Buildkite agent and OIDC token, and the certificate was issued as expected. I started a local fulcio like this:

$ go run main.go serve --port 5555 --ca ephemeralca --ct-log-url="" --config-path config/identity/config.yaml

... and signed git commits with gitsign. The relevant bits of the certificates look like:

    git cat-file commit HEAD | sed -n '/-BEGIN/, /-END/p' | sed 's/^ //g' | sed 's/gpgsig //g' | sed 's/SIGNED MESSAGE/PKCS7/g' | openssl pkcs7 -print -print_certs -text
    ...
    X509v3 extensions:
        X509v3 Key Usage: critical
            Digital Signature
        X509v3 Extended Key Usage:
            Code Signing
        X509v3 Subject Key Identifier:
            36:D2:99:B9:BA:98:4B:3A:77:51:DC:08:05:83:12:9A:F4:EE:41:E5
        X509v3 Authority Key Identifier:
            D2:41:21:29:23:AD:E9:27:69:6F:DB:85:6D:1B:3F:7E:A9:55:F3:02
        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:
            .(078a6dd4a32fa40592c21a40aedaf27105503140
        1.3.6.1.4.1.57264.1.20:
            ..ui
        1.3.6.1.4.1.57264.1.21:
            .khttps://buildkite.com/yob-opensource/oidc-signing-experiment/builds/52#01943a38-f93e-4355-abe8-90a30369c270

Fixes #1074

cc @sj26

Release Note

None.

Documentation

None

[yob]

The build_number claim in Buildkite OIDC tokens is an integer, so this is generating URLs like:

https://buildkite.com/yob-opensource/oidc-signing-experiment/builds/%!s(float64=42)#01942921-7883-409b-81d0-3f6b20bcdabf

Is there an existing way to have 42 formatted as a string nicely? I had a poke at pkg/identity/ciprovider/principal.go, but can't see anything relevant

[haydentherapper]

Good find. We can fix this in https://github.com/sigstore/fulcio/blob/main/pkg/identity/ciprovider/principal.go#L33-L39. Either change the formatting string to %v, or do type reflection and use %d when the type is an integer.

[yob]

@haydentherapper @feelepxyz I've had another swing at this. It's not mergable yet, but I'd value your feedback!

A specific question I had while working on this: if Buildkite identity is now managed by the CIProvider type, can we rip out the Buildkite specific issuer - either in this PR or in a follow up? Or is it needed for backwards compatibility?

Also: would you like this PR to update the table in docs/oid-info.md ?

[haydentherapper]

No additional tests are necessary if this is working locally! We'll roll this out to our staging environment first, fulcio.sigstage.dev, for you to test against.

[haydentherapper]

Good find. We can fix this in https://github.com/sigstore/fulcio/blob/main/pkg/identity/ciprovider/principal.go#L33-L39. Either change the formatting string to %v, or do type reflection and use %d when the type is an integer.

[haydentherapper]

A specific question I had while working on this: if Buildkite identity is now managed by the CIProvider type, can we rip out the Buildkite specific issuer - either in this PR or in a follow up? Or is it needed for backwards compatibility?

The Buildkite specific issuer should no longer be necessary. I was planning to remove all specific CI issuers as part of a 2.x release, waiting for backwards compatibility just in case someone has deployed a local instance.

Also: would you like this PR to update the table in docs/oid-info.md ?

Yes please! Thank you.

[yob]

Brilliant, thanks! I'll look at getting those two new claims into our tokens this coming week, then update this and mark it ready for review

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.13%. Comparing base (cf238ac) to head (d3fc2b2).
Report is 267 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1903      +/-   ##
==========================================
- Coverage   57.93%   49.13%   -8.80%     
==========================================
  Files          50       70      +20     
  Lines        3119     5204    +2085     
==========================================
+ Hits         1807     2557     +750     
- Misses       1154     2413    +1259     
- Partials      158      234      +76     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yob]

@haydentherapper this is now as functional enough to be mergable from our perspective. The new extensions are working in my local testing (see updated PR description), and I've fixed the URL construction to handle the integer build_number claim.

I assume you might want a test for the changes in mapValuesToString()? I'm also a bit weirded out that our integer build_number is a float64 by the time it reaches mapValuesToString(). Maybe the real fix is to make that function just use %v with no reflection, and find a way to get the claim value passed around as the integer it is?

https://buildkite.com/docs/agent/v3/cli-oidc#claims-example-token-contents as an example token with our claims (not runner_environment or build_source yet), and you can see build_number is an int.