Open rdileep13 opened 1 year ago
what are the proposed API additions to support this feature?
CC @golang/security
Chiming in since I've maintained https://github.com/coreos/go-oidc for a while now, which implements OpenID Connect on top of golang.org/x/oauth2. OpenID Connect supports JWTs as an additional field returned as part of the OAuth 2.0 token response.
If I'm reading the spec right, you can do a lot of this today with an auth code option oauth2.OAuthCodeOption
. go-oidc does something similar for nonce's (oidc.Nonce
).
opts := []oauth2.AuthCodeOption{
oauth2. SetAuthURLParam("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"),
oauth2. SetAuthURLParam("client_assertion", myJWT),
}
config. Exchange(ctx, code, opts...)
I believe AuthCodeOption even lets you override the grant_type (don't know if that's intended).
@ericchiang thanks for the response! While AuthcodeOption lets you add custom name-value pairs in the API, here are some issues with that.
Please let me know your thoughts. Excuse me if I'm misinterpreting anything or missing something.
What providers support private_key_jwt? Do any require it?
@neild I can confirm that AzureAD, Okta, PingId, Kong support it. I'm sure many others that I'm not aware of support it.
@seankhliao Only one internal API is planned to support this. That API computes the jwt (https://datatracker.ietf.org/doc/html/rfc7523) to be sent in TokenAPI
@rdileep13 proposals that introduce new APIs generally include the new API.
You may want to look at https://golang.org/x/oauth2/jwt, which is documented to support an older draft of RFC 7523.
Supporting private_key_jwt seems reasonable, but the proposal needs to include the proposed new API.
How does this proposal relate to golang.org/x/oauth2/jwt
? Does it supersede that package? Should it be a feature of that package rather than of golang.org/x/oauth2
?
@rdileep13 , hey do you want any help moving this along? I'd also like to see this feature, and can help draft up the actual API changes for this team to review.
@neild , for your question on the golang.org/x/oauth2/jwt
package, I don't think that's the right place to make the change. To illustrate, here's a table of the grant types and client authentication methods we support across a few packages:
Grant Type | Supported Client Authentication | Package/Relevant Config |
---|---|---|
authorization_code | none?, client_secret_basic, client_secret_post | golang.org/x/oauth2 |
client_credentials | none?, client_secret_basic, client_secret_post | golang.org/x/oauth2/clientcredentials |
urn:ietf:params:oauth:grant-type:jwt-bearer | none (technically, "assertion" JWT doesn't count) | golang.org/x/oauth2/jwt |
What rdileep13 is proposing above is to add private_key_jwt to the supported client authn methods of the authorization_code grant type. Since that grant type is implemented in golang.org/x/oauth2
, I think it makes sense to add it there.
If I do contribute anything to this, I'd also request adding private_key_jwt to the client_credentials grant, implemented in golang.org/x/oauth2/clientcredentials
.
To give an example of a spec that requires private_key_jwt, it's the preferred client authn protocol for the SMART App Launch, which is a healthcare specific superset of OAuth 2.0 and OIDC. It is a strict requirement for the client_credentials grant in that workflow.
To reiterate, I understand someone needs to propose an API for this team to approve, just adding some context.
@madaster97 Please go ahead with the API proposal. That would be a great help!
Here's my proposal for the API changes, in line with the work rdileep13 has already done.
With respect to client auth, both these packages have the same API (with slight differences in where the values go): golang.org/x/oauth2 golang.org/x/oauth2/clientcredentials
The fields that matter for this change are:
My proposed are:
PrivateKey []byte
and KeyID string
as a new struct called PrivateKeyConfig
Having a dedicated struct will make it easier to make edits later on specific to the JWT signing, and is in line with some of the changes @neild has recommended.
What providers support private_key_jwt? Do any require it?
The login.gov OIDC provider requires use of private_key_jwt
or PKCE
, rather than client_secret
. See this site for details: https://developers.login.gov/oidc/ where they state why they don't allow client_secret:
Other implementations of OpenID Connect use the “implicit flow” or the client_secret param, but Login.gov does not support those methods. The implicit flow is not recommended by the OAuth group for security reasons. Similarly client_secret is not supported by Login.gov because of security reasons. It requires managing a shared secret in two places: the client and the server, where private_key_jwt flow involves only sharing public keys with the server and PKCE only has a one-time secret.
If you don't provide the client_secret today, then the AuthStyle setting shouldn't matter:
If this is exclusively to be used by OpenID Connect, then it might be reasonable for go-oidc (or other third-party packages) to provide this API (https://go.dev/doc/faq#x_in_std)? E.g. something like
package oidc
type JWTAuthConfig struct {
PrivateKey crypto.PrivateKey
KeyID string
ClientID string
Endpoint oauth2.Endpoint
Now func() time.Time
Validity time.Duration
}
func NewJWTAuth(rand io.Reader, config JWTChallengeConfig) ([]oauth2.AuthCodeOption, error) {
// ...
}
Anecdotally, it seems like the Go team has been hesitant to provide a full fledged JWT package (and I don't blame them).
Am I missing any blockers here? Or can this be done outside of golang.org/x/oauth2?
Hi @ericchiang , JWT client authentication is independent of OIDC. The OIDC group initially proposed it, but it's been registered as an OAuth parameter rather than as OIDC specific
I think it would be best to implement private_key_jwt here, given that client authentication is a core part of the OAuth 2.0 protocol.
The current proposal supplies a PrivateKey as a []byte
:
// PrivateKey contains the contents of an RSA private key or the
// contents of a PEM file that contains a private key. The provided
// private key is used to sign the Token request (https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint).
// PEM containers with a passphrase are not supported.
PrivateKey []byte
This matches jwt.Config.PrivateKey
, but it doesn't seem right.
Providing a serialized key means the oauth
package needs to parse it. Either the key is reparsed on each request, or the package needs to cache it. Caching introduces questions of cache size and lifetime. In addition, if the caller already has a deserialized key, they're going to need to serialize it again to provide it to the oauth
package.
I think the oauth.Config
should contain the serialized key, such as an *rsa.PrivateKey
. That avoids all these questions.
Is an *rsa.PrivateKey
right, though? RFC 7515 describes a number of signature algorithms, and RFC 7518 Section 8.1 specifically suggests that implementations use modular algorithms to allow for evolution over time. Should this be a crypto.Signer
instead? (Note that *rsa.PrivateKey
implements crypto.Signer
.)
If so, the API might look something like:
type Config struct {
Signer crypto.Signer
KeyID string
}
And perhaps the crypto.SignerOpts
needs to be in there as well?
@neild Spec allows various signature algorithms as mentioned at https://www.rfc-editor.org/rfc/rfc7518#section-3
As you mentioned, it does not make sense to pass *rsa.PrivateKey
as some ECDSA algorithms are allowed as well. crypto.Signer
might be a good option since *ecdsa.PrivateKey
also implements the crypto.Signer
interface.
@neild Looks like the issue is more involved.
Underlying jws
library only seems to support rsa
keys - https://github.com/golang/oauth2/blob/master/jws/jws.go#L156
func Encode(header *Header, c *ClaimSet, key *rsa.PrivateKey) (string, error) {
Underlying jws library only seems to support rsa keys
There's also jws.EncodeWithSigner
, which looks like it should be straightforward to use with a crypto.Signer
:
type Signer func(data []byte) (sig []byte, err error)
func EncodeWithSigner(header *Header, c *ClaimSet, sg Signer) (string, error)
This proposal is to support private_key_jwt client authentication method in x/oauth2 package. Details of this method are at https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
Details: This is applicable for both two-legged (client_credentials grant) and three-legged (authorization_code grant) OAuth flows. In the three-legged oauth flow, when the Relying Party (RP) tries to exchange the authorization code with the Authorization Server (AS) (aka IdP), it has a choice of various methods to authenticate itself. most common method is client_secret_post which sends the client_id and client_secret of the RP in the token API request as post body parameters along with the code to be exchanged and other details.
private_key_jwt is a more secure way of making the token API call where in instead of sending client_secret in the tokenAPI, the RP would compute a digitally-signed-token (aka jwt) to prove its identity at the AS. Details of this token computation are at https://datatracker.ietf.org/doc/html/rfc7523
This proposal is to enhance current token exchange API in x/oauth2 to offer private_key_jwt as one of the client authentication methods.