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

Add API for fetching Fulcio configuration #608

Merged
merged 4 commits into from
Jun 1, 2022

Conversation

haydentherapper
Copy link
Contributor

@haydentherapper haydentherapper commented May 24, 2022

This API provides the following:

  • All OIDC issuers, including the meta/wildcard issuers
  • The expected audience of the token
  • The claim that must be signed for a proof of possession
  • The SPIFFE trust domain, when the issuer is of type SPIFFE

This has only been added to the V2 API, as we are no longer updating the V1 API.

Signed-off-by: Hayden Blauzvern [email protected]

Summary

Ticket Link

Fixes #607

Release Note

Added API for fetching the Fulcio OIDC issuer configuration

@haydentherapper
Copy link
Contributor Author

cc @var-sdk

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2022

Codecov Report

Merging #608 (feb67a2) into main (fa41bd9) will increase coverage by 1.06%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             main     #608      +/-   ##
==========================================
+ Coverage   59.86%   60.92%   +1.06%     
==========================================
  Files          26       26              
  Lines        1582     1638      +56     
==========================================
+ Hits          947      998      +51     
- Misses        558      562       +4     
- Partials       77       78       +1     
Impacted Files Coverage Δ
pkg/server/grpc_server.go 54.54% <58.33%> (+0.24%) ⬆️
pkg/config/config.go 69.25% <93.18%> (+4.65%) ⬆️
pkg/ca/fileca/load.go 68.96% <0.00%> (+10.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa41bd9...feb67a2. Read the comment docs.

@haydentherapper
Copy link
Contributor Author

@bobcallaway Any idea why the version of protoc isn't updated when running CI? I've rebuilt the protos locally and only can generate an updated version.

@bobcallaway
Copy link
Member

@bobcallaway Any idea why the version of protoc isn't updated when running CI? I've rebuilt the protos locally and only can generate an updated version.

The version installed in the CI env doesn't match what you have locally.

run: sudo apt update -y && sudo apt install -y -q protobuf-compiler
is where it gets installed, and I thought I had written a regex that would protect against this in
run: git update-index --refresh && git diff-index --quiet -I"^\/\/\s+protoc(-gen-go)?\s+v[0-9]+\.[0-9]+\.[0-9]+$" HEAD -- || git diff -I"^\/\/\s+protoc(-gen-go)?\s+v[0-9]+\.[0-9]+\.[0-9]+$" --exit-code
but I'm guessing it is not working.

@cpanato
Copy link
Member

cpanato commented May 24, 2022

@haydentherapper @bobcallaway what is the version we need to run?

@haydentherapper
Copy link
Contributor Author

@cpanato I have 3.12.4 installed, but the tooling is installing 3.6.1. I'll take a look at the regex

@cpanato
Copy link
Member

cpanato commented May 24, 2022

@cpanato I have 3.12.4 installed, but the tooling is installing 3.6.1. I'll take a look at the regex

ok, otherwise i can make a pr to install a version that we want

@haydentherapper
Copy link
Contributor Author

@cpanato A PR would be great - Should we automatically pick up the latest version?

@haydentherapper
Copy link
Contributor Author

Hm, not sure why the regex isn't working, ^\/\/\s+protoc(-gen-go)?\s+v[0-9]+\.[0-9]+\.[0-9]+$ correctly matches // protoc v3.12.4

@haydentherapper
Copy link
Contributor Author

haydentherapper commented May 24, 2022

If I try to replicate this locally, by modifying one of the generated build files to have an older version, running git update-index --refresh && git diff-index --quiet -I"^\/\/\s+protoc(-gen-go)?\s+v[0-9]+\.[0-9]+\.[0-9]+$" HEAD -- || git diff -I"^\/\/\s+protoc(-gen-go)?\s+v[0-9]+\.[0-9]+\.[0-9]+$" --exit-code, I only get pkg/generated/protobuf/fulcio.pb.go: needs update

Unable to reproduce the issue

This API provides the following:
* All OIDC issuers, including the meta/wildcard issuers
* The expected audience of the token
* The claim that must be signed for a proof of possession
* The SPIFFE trust domain, when the issuer is of type SPIFFE

Signed-off-by: Hayden Blauzvern <[email protected]>
Signed-off-by: Hayden Blauzvern <[email protected]>
Signed-off-by: Hayden Blauzvern <[email protected]>
@haydentherapper
Copy link
Contributor Author

Issue with workflow has been fixed, we had to also match on // - protoc v1.2.3, in addition to // protoc v.1.2.3

Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for fixing the regex!

string challenge_claim = 4;
// The expected SPIFFE trust domain. Only present when the OIDC issuer issues tokens for SPIFFE identities.
string spiffe_trust_domain = 5;
}
Copy link
Member

Choose a reason for hiding this comment

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

nit: new line

@haydentherapper haydentherapper merged commit 509c789 into sigstore:main Jun 1, 2022
@haydentherapper haydentherapper deleted the config-api branch June 1, 2022 16:09
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.

Add API for fetching Fulcio configuration
5 participants