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

fix: Fulcio client id flag interpreted as OIDC issuer #270

Closed
wants to merge 2 commits into from

Conversation

alexashley
Copy link

@alexashley alexashley commented Jun 27, 2023

I attempted to use witness with the Sigstore staging Fulcio and ran into an issue where the client id flag was being used as the OIDC issuer:

$ ./bin/witness run -a git -o att.json --fulcio https://fulcio.sigstage.dev --fulcio-oidc-issuer https://oauth2.sigstage.dev/auth --fulcio-oidc-client-id sigstore -s example

ERROR   failed to create signer from Fulcio: Get "sigstore/.well-known/openid-configuration": unsupported protocol scheme ""
ERROR   failed to load signers

The type signature of the function in go-witness has the issuer after the Fulcio URL, but witness is passing the client id instead.

func Signer(ctx context.Context, fulcioURL string, oidcIssuer string, oidcClientID string, tokenOption string) (cryptoutil.Signer, error)

After swapping the order, I was able to sign an attestation with that same command.

This example references the out of order flags.

Signed-off-by: Alex Ashley <alexa@liatrio.com>
Copy link
Member

@colek42 colek42 left a comment

Choose a reason for hiding this comment

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

Thank you!

@colek42
Copy link
Member

colek42 commented Jul 5, 2023

@alexashley I am going to update this with some fixes for the CI. Thank you for the contribution!

@mikhailswift
Copy link
Collaborator

@alexashley thanks so much for this contribution. We've recently reworked how the CLI flags are created and settings are passed in, which fixed this inadvertently.

We really appreciate you contributing and finding this!

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.

None yet

3 participants