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

[WIP] proposal for new oidc and fulcio abstractions #1394

Closed
wants to merge 1 commit into from

Conversation

dekkagaijin
Copy link
Member

@dekkagaijin dekkagaijin commented Feb 2, 2022

Re: #844
PTAL @mattmoor

Signed-off-by: Jake Sanders <jsand@google.com>
@dekkagaijin dekkagaijin requested a review from mattmoor February 2, 2022 20:43
@dekkagaijin dekkagaijin changed the title proposal for new oidc and fulcio abstractions [WIP] proposal for new oidc and fulcio abstractions Feb 2, 2022

// NewDSSEAttestor returns a `cosign.DSSEAttestor`
func NewDSSEAttestor(payloadType string,
s signature.Signer, oidp oidc.Provider) cosign.DSSEAttestor {
Copy link
Member

Choose a reason for hiding this comment

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

Why does the non-keyless take oidp?

Copy link
Member Author

Choose a reason for hiding this comment

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

so that one can use a non-ephemeral key, or an ephemeral key generated elsewhere, and associate it with an OID

Comment on lines +29 to +36
// *NOTE* This is a higher-level client than exists in `sigstore/fulcio`, should probably live there

// Client is a Fulcio client.
type Client struct {
APIClient fulcioapi.Client
}

// GetCert retrieves a Fulcio certificate which associates the `Signer`'s public key with the ID provided
Copy link
Member

Choose a reason for hiding this comment

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

It would be tremendously helpful if the comments were actually useful in describing what you were trying to accomplish with the higher level client.

It seems like your goal is to capture some of the challenge semantics (e.g. proving we have the private key by signing the subject of the IDToken), but TBQH the objectives of this as a higher level client aren't clear to me at all, which makes it sort of hard to judge good or bad.

Copy link
Member Author

Choose a reason for hiding this comment

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

fair enough. The higher level client I was thinking of uses "real" types, e.g. x509.Certificate instead of serialized []bytes, and performs the boilerplate operations to get the ID token, sign the subject, and retrieve a signing cert from Fulcio.

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

This PR was closed because it has been stalled for 10 days with no activity.

@github-actions github-actions bot closed this Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants