-
Notifications
You must be signed in to change notification settings - Fork 556
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
Conversation
Signed-off-by: Jake Sanders <jsand@google.com>
|
||
// NewDSSEAttestor returns a `cosign.DSSEAttestor` | ||
func NewDSSEAttestor(payloadType string, | ||
s signature.Signer, oidp oidc.Provider) cosign.DSSEAttestor { |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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
// *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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 []byte
s, and performs the boilerplate operations to get the ID token, sign the subject, and retrieve a signing cert from Fulcio.
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. |
This PR was closed because it has been stalled for 10 days with no activity. |
Re: #844
PTAL @mattmoor