"client secret jwt" and "private key jwt" auth methods

[forgo]

This PR is to address the #6881 issue of supporting OAuth 2.0 Client authentication methods, namely "client_secret_jwt" and "private_key_jwt" methods.

My understanding comes from reading the spec, but also from useful resources such as Authlete:

• OAuth 2.0 Client Authentication - Takahiko Kawasaki
  
• private key jwt
• client secret jwt

My use case has been to integrate with Login.gov, so my starting approach has been to add logic in order to simplify their private_key_jwt method, according to login.gov docs

The major difficulty and difference when comparing the private_key_jwt method to other examples is the overhead of:

1. generating a JWT from scratch, using information derived from the OAuth client registry information
2. configuring and managing a keystore which can be used to sign the client_assertion JWT in the token request.
3. finding a handle on the right place to sign the JWT before issuing the token request

Further complications exist downstream on the resource server side, primarily:

1. Making a clear connection that private_key_jwt with Login.gov returns something that Spring refers to as an "opaque" token, meaning, it is not in the form of a JWT as you might expect, and the configuration on the resource side is slightly different.
2. Once you make the connection that the following should be used:

.oauth2ResourceServer()
          .opaqueToken();  // as opposed to `.jwt()`

Then it also must be recognized that a custom @Bean OpaqueTokenIntrospector must be setup to make another trip to the authorization server and extract user info. This implementation of course, could vary greatly between authorization servers.

This last point I feel could be improved by adding concrete use-cases to the documentation that spell out the connection between the "opaque" token and "private_key_jwt" authentication method specifically.

What I've done so far:

• Added new client registration properties to configure a key store for signing.
• Modified AbstractWebClientReactiveOAuth2AccessTokenResponseClient to handle the two new authentication methods that add client_assertion and client_assertion_type to the body of the token request. This includes some logic to create and sign the JWT using the key store configuration added above.
• Create a client registrations utility to return a java KeyPair directly from the builder or from config, later simplifying the ability to sign with JWSSigner
• Create new constants for ClientAuthenticationMethod:
  • JWT (client secret)
  • PRIVATE_KEY_JWT
• Create new constants for OAuth param keys:
  • CLIENT_ASSERTION = 'client_assertion'
  • CLIENT_ASSERTION_TYPE = 'client_assertion_type'

Where I could use some guidance:

• General feedback on if I've been putting logic in the right places or if I should be creating new classes and what their naming conventions should be.
• I've been dealing only with the reactive token response client, so I haven't begun to realize how this might impact non-reactive.
• In the case where I am creating a gateway to retrieve the opaque token, it is unclear to me whether or not a session can be managed at the gateway to prevent this 2nd round trip by forwarding the JWT available at the gateway before proxying with the opaque token, and I could use some guidance.
• I had initially implemented the key store and JWT signing logic using the com.auth0:java-jwt dependency; however, for this PR directly with spring-security, I have re-written some of that logic using com.nimbusds.jose available on this project's classpath -- and that re-write may require some proper testing.
• In general, I haven't yet added any testing, and could use help on how you might approach this.

[krajcsovszkig-ms]

Hi,

As I mentioned in the original ticket, I'm interested in helping out with this PR. We are library developers and our goal is to enable JVM developers in our organization to use spring-security with OAuth 2.0. Our Authorization Server is going to be PingFederate and our security engineers decided to require private_key_jwt in all cases when it's possible.

Because we don't know how our users are going to want to use it, we're looking for a fairly complete implementation.

This is the initial design I came up with (some of it is already addressed in this PR, some of it is new or slightly different, I don't insist on the structure and naming, only the functionality):

• Create Encoder counterparts to Decoders in org.springframework.security.oauth2.jwt (in the spring-security-oauth2-jose project)
  • NimbusJwtDecoder currently only supports RSA, we should add support for EC as well
• Add JWT or a CLIENT_SECRET_JWT and a PRIVATE_KEY_JWT field in org.springframework.security.oauth2.core.ClientAuthenticationMethod
• Add any necessary pieces of information for creating and signing JWTs with either a client secret or private key to org.springframework.security.oauth2.client.registration.ClientRegistration, specifically:
  • the clientSecret field might be usable as a string representation of the private key as well, otherwise a separate one is needed and the unused one of these 2 properties must be null, which can be used to determine if the actual method is client_secret_jwt or private_key_jwt
  • org.springframework.security.oauth2.jose.jws.JwsAlgorithm for the encoding, which can be validated to be one of HS* for client_secret_jwt and one of RS* or ES* for private_key_jwt
  • Optionally a org.springframework.security.oauth2.jwt.JwtClaimAccessor (could be a org.springframework.security.oauth2.jwt.Jwt, although it needs some modifications to be convenient for this purpose*) which can define optional overrides for the default claims to include in the transmitted JWTs
    • *: currently the org.springframework.security.oauth2.jwt.Jwt class contains both the final token generated from the claims and the headers, so it more accurately represents a JWS rather than just a JWT (although it doesn’t contain the signature, only the full JWS in its serialized form). This is OK for what it’s used now (decoding JWS-es) but not convenient for storing user-provided JWT claims. It implements org.springframework.security.oauth2.jwt.JwtClaimAccessor which is a perfect interface for this purpose, but currently doesn’t have any other implementor. Consider renaming org.springframework.security.oauth2.jwt.Jwt to Jws or something more accurate and creating a separate Jwt class, or alternatively create a JwtClaimsSet or similar.
• Fix verifications in org.springframework.security.oauth2.client.registration.ClientRegistration.Builder and its create() method to account for the new configurations
  • In the create() method, if the client secret is missing, the authentication method is set to NONE, but it’ll be missing for private key authentication as well if we use a separate field for it
  • More verification might be necessary for the new additions
• Add conversions for new method(s) in org.springframework.security.oauth2.core.ClientAuthenticationMethod to
  • org.springframework.security.oauth2.client.registration.ClientRegistrations#getClientAuthenticationMethod
  • org.springframework.security.oauth2.client.jackson2.StdConverters.ClientAuthenticationMethodConverter#convert
• Add support for the new properties of org.springframework.security.oauth2.client.registration.ClientRegistration to
  • org.springframework.security.config.oauth2.client.ClientRegistrationsBeanDefinitionParser
  • org.springframework.security.oauth2.client.jackson2.ClientRegistrationDeserializer
  • org.springframework.security.oauth2.client.registration.ClientRegistrations
• Implement creating client_secret_jwt and private_key_jwt requests from org.springframework.security.oauth2.client.registration.ClientRegistrations at the OAuth2 Client Endpoints in org.springframework.security.oauth2.client.endpoint, specifically:
  • Create a org.springframework.security.oauth2.jwt.JwtEncoderFactory<org.springframework.security.oauth2.client.registration.ClientRegistration> (new interface) that creates a org.springframework.security.oauth2.jwt.NimbusJwtEncoder configured with a com.nimbusds.jwt.proc.JWTProcessor from a org.springframework.security.oauth2.client.registration.ClientRegistration
  • Create a utility that creates a JWT claims set from a org.springframework.security.oauth2.client.registration.ClientRegistration, using the default claims specified in it and falling back to:
    • The clientId for iss and sub
    • The token URI of the Authorization Server for aud
    • A random UUID for jti
    • The current timestamp for iat and that plus a configured amount of time (e.g. 5 minutes) for exp - this might need a configuration property as well for the default lifetime
  • Add the urn:ietf:params:oauth:client-assertion-type:jwt-bearer client_assertion_type and the client_assertion generated by the JWT encoder built by the above factory and JWT claims factory from the ClientRegistration in the form body at the following locations, if the authentication method is one of the new JWT-related ones:
    • org.springframework.security.oauth2.client.endpoint.AbstractWebClientReactiveOAuth2AccessTokenResponseClient#populateTokenRequestBody
    • org.springframework.security.oauth2.client.endpoint.NimbusAuthorizationCodeTokenResponseClient#getTokenResponse
    • org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequestEntityConverter#buildFormParameters
    • org.springframework.security.oauth2.client.endpoint.OAuth2ClientCredentialsGrantRequestEntityConverter#buildFormParameters
    • org.springframework.security.oauth2.client.endpoint.OAuth2PasswordGrantRequestEntityConverter#buildFormParameters
    • org.springframework.security.oauth2.client.endpoint.OAuth2RefreshTokenGrantRequestEntityConverter#buildFormParameters
• Testing:
  • Unit tests for the new fields and properties in org.springframework.security.oauth2.core.ClientAuthenticationMethod, org.springframework.security.oauth2.client.registration.ClientRegistration and the new converters/factories/deserializers/etc. where other similar tests currently exist
  • Copy of tests for decoders in org.springframework.security.oauth2.jwt (in the spring-security-oauth2-jose project) for the new encoders
  • Adjust existing tests in spring-security-oauth2-jose project for any changes to org.springframework.security.oauth2.jwt.Jwt and the decoders for EC support
  • Unit and integration tests for the new utilities in org.springframework.security.oauth2.client.endpoint
  • Copy of existing tests for the POST authentication method in org.springframework.security.oauth2.client.endpoint for client_secret_jwt and private_key_jwt for each class in the package that implements them

Open questions:

1. Should we have a single JWT or a separate CLIENT_SECRET_JWT and a PRIVATE_KEY_JWT field in org.springframework.security.oauth2.core.ClientAuthenticationMethod? The implementation could use the fields in org.springframework.security.oauth2.client.registration.ClientRegistration to determine which method to use, but it might not even be necessary to distinguish between them, since the only change is how to sign the JWT, and a generic signer that takes a signing algorithm and a String key could deal with that.
2. Should we store the private key as a String in the clientSecret field of the ClientRegistration, or have a separate typed field for it? For full disclosure, I haven't looked into the full implications of this (i.e. is there a convenient String serialization of all private key types we plan to support and how should we get the key from the keystore to the ClientRegistration).
3. org.springframework.security.oauth2.jwt.Jwt doesn't represent a JWT and we probably need something that does. (It requires to contain the JOSE headers and the entire JWS token (headers+JWT+signature) in its serialized form.) Could/should we rename the current class to something more accurate and create a new Jwt class which is just a JWT, or leave it as is and create something like a JwtClaimsSet? If we choose to rename (e.g. to Jws), "Jws" could extend the new Jwt class which could keep the existing extra contents as deprecated for a while, and then the change would be backwards-compatible besides the deprecation warning.

@jgrandja could you please have a look at this and say if you agree / give your opinion on the above "open questions"?
@forgo let me know if/how I can help once we agree on a design

Thanks a lot to both of you for working on this!

[forgo]

@krajcsovszkig-ms My suggested changes are definitely incomplete, but I tried to capture the major pieces I minimally needed for my use case. I anticipated there would be lots of changes and of course tests to account for everything. I tried to make my implementation as generic as possible for now, but I think we're mostly on the same page.

The one thing I don't necessarily agree with is storing the secret directly in a config. It sort of eliminates the enhanced security benefit of "private_key_jwt" that allows the secret to be locked behind a keystore in the first place. Especially if that configuration may move around in different environments and be exposed -- similarly to other auth methods.

I did attempt to maintain the "client-secret" attribute for the "client secret jwt" use case, but not sure if I did everything necessary there since I was focused on the "private key jwt" use case.

I have comments in the code where the JWT is signed that explain how one would create a keystore and extract the public key from it to be registered with an authorization server. This would be useful documentation on Spring's part; however, I'm open to having multiple ways of accomplishing signing that can be up to the user.

Personally, I like the idea of the key store. I can pass mine around as a Kubernetes secret and mount it in my gateway API. If there's a cleaner way to add key store configs for this purpose, but my first pass was closely emulating the way a key store would be set up for SSL -- but nested in the oauth specific config.

It would definitely be nice to get more Spring Security eyes on this, since I am not as familiar with the Spring codebase conventions, etc.

[forgo]

@krajcsovszkig-ms

Just circling back to your question 1 of three:

I had the same thought, but AFAIK there is a subtle distinction between the way those secrets are shared. For example, Google generating a secret when I register with them versus me signing my own JWT with that secret and sharing it ahead of time with the authorization provider. A slight difference in responsibility. I could be mistaken about this, and it may be fundamentally the same problem for "client secret jwt".

For "private key jwt", the secret isn't what I have to register with Login.gov explicitly ahead of time -- it is the corresponding public key of what I used to sign the JWT.

See my signing functions for both auth methods in AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java

• String signClientSecretJwt(ClientRegistration clientRegistration)
• String signPrivateKeyJwt(ClientRegistration clientRegistration)

[krajcsovszkig-ms]

@forgo

Good point about the key security, it has occurred to me as well, how safe is it to keep these in Strings, but I haven't looked into the alternatives yet. If Spring offers a standard way for it, I believe we should use that.

I'm not sure about including reading the key store in the ClientRegistration - wouldn't it be enough to have a java.security.PrivateKey field there and letting the user, or at least a separate specialized factory handle obtaining it in your opinion? Looking at the PrivateKey interface, it's essentially a data source, so it should even be possible (perhaps already done somewhere) to implement it as a proxy to a key store, and read the key on demand.

[forgo]

@krajcsovszkig-ms

If Spring offers a standard way for it, I believe we should use that.

Agreed. I wasn't really sure about my ClientRegistration approach for configuring a key store for this specific use case. I will gladly defer to the Spring team for the best approach there to make it most flexible, but I would hope it's not putting too much burden back on the user.

This was a feature that I really wanted to be built into Spring to make the auth method easier to understand and integrate quickly. Not having this built-in was the major point of confusion is getting my custom code with Login.gov working (apart from documentation).

If a user wants the flexibility to sign themselves in a custom way, they should be able to. Authorization services can have widely varying requirements. However, I believe both these new auth methods require signing with an "HMAC SHA algorithm", and that is mostly orthogonal to other requirements that you might run into.

It seems reasonable, therefore, to isolate the signing as I did and allow to point to a JKS key and algorithm -- but to default to "SHA-256", for instance.

I would hope whatever solution is made, it allows me to point to a keystore/key and algorithm in a config, somehow, some way.

[krajcsovszkig-ms]

@forgo

This is the RFC on the supported algorithms: https://tools.ietf.org/html/rfc7518#section-3.1

HMAC-SHA requires a shared secret (so it's only usable for client_secret_jwt), while the others require a private-public key pair (private_key_jwt). The PS* algorithms are not supported in PingFederate which we'll be using, not sure about others.

I agree it should be as simple as possible to inject the keys from a keystore, I'm just not sure it should be the responsibility of the ClientRegistration class to handle it, to me that class feels like a plain Java bean and shouldn't have any complicated logic.

Parsing a ClientRegistration from an app.yml seems to be done by org.springframework.security.config.oauth2.client.ClientRegistrationsBeanDefinitionParser, perhaps reading the key store could go there too.

[jgrandja]

@forgo @krajcsovszkig-ms Thank you for all the details! Sorry for the delay in my response but I've been juggling quite a few tasks :)

I took a look at the PR and here is my initial feedback:

• ClientRegistration will likely NOT hold all the PKI specific configuration so let's keep the changes out of ClientRegistration. Please remove the changes in ClientRegistrationsBeanDefinitionParser as well.
• The logic for client_secret_jwt and private_key_jwt resides in AbstractWebClientReactiveOAuth2AccessTokenResponseClient, which doesn't allow for reuse in the Servlet stack. We should encapsulate this logic in a "client authentication strategy" and design it in such a way that it can be reused in both stacks, if possible.

I'd like to propose the following next steps to move this feature forward.

First off, let's start simple and build it using an iterative approach.

I'm going to propose for the initial iteration we implement client_secret_jwt as it will be easier than private_key_jwt. As well, let's focus on the Servlet stack only - we can add the Reactive implementation in another PR.

Here is my thought for initial design:

• We need to define JwtEncoder, which is the other end of the existing JwtDecoder. The JwtEncoder is responsible for creating Jwt. This will be needed in the new Spring Authorization Server project and can also be used for this feature.
• Take a look at OidcIdTokenDecoderFactory, which creates a JwtDecoder for a specific ClientRegistration. I think we could apply the same pattern here and introduce a JwtEncoderFactory with an implementation of JwtEncoderFactory<ClientRegistration>.
• Instead of ClientRegistration containing the PKI specific configuration, the JwtEncoderFactory<ClientRegistration> would contain the configuration for client_secret_jwt and private_key_jwt.
• The implementation of JwtEncoderFactory<ClientRegistration> would be used by the OAuth2AccessTokenResponseClient implementations. For example, the DefaultAuthorizationCodeTokenResponseClient would be configured to use JwtEncoderFactory<ClientRegistration> possibly indirectly via the requestEntityConverter.

Let me know your thoughts on this design. Before you proceed with the proposed updates, let's first align on design.

[forgo]

@jgrandja I like your suggestions so far, and it's a good idea to start with the simple case first.

I'm going to be a bit busier this week, but I am open to @krajcsovszkig-ms starting to make changes on this branch if they are more eager to get started on your suggestions.

[krajcsovszkig-ms]

Thanks @jgrandja for having a look.

Your iterative approach for the implementation sounds good. I think the work on the encoders in spring-security-oauth2-jose is fairly straightforward and independent from the rest, so it could be spun out in a separate PR and worked on right now, WDYT? Part of this though is rethinking the org.springframework.security.oauth2.jwt.Jwt class as I mentioned in my design proposal (3rd open question), what are your thoughts about that?

About putting all the PKI-stuff in the encoder factory, how exactly would the users use that? For the decoders it's quite straightforward because everything either comes through the wire (JWS algorithm, JWT claims) or is already in the ClientRegistration (client secret), but for the encoder the user will have to provide these.

Since the client secret is already in the ClientRegistration, wouldn't it be logical to put the JWS algorithm and any custom JWT claims there as well? That way things like the org.springframework.security.oauth2.client.registration.ClientRegistrations and the org.springframework.security.config.oauth2.client.CommonOAuth2Provider could provide some of these as well, and manual configuration would be straightforward too.

I'm not sure about private keys, but if we are starting with client_secret_jwt only, we could put that off for later.

One more thing is the org.springframework.security.oauth2.core.ClientAuthenticationMethod (see my 1st point in the open questions in my initial design proposal) - should we have separate ones for the 2 signed-JWT-based authentication methods, or just one (since they are the same except for the signing algorithm used)?

[jgrandja]

@krajcsovszkig-ms

I think the work on the encoders in spring-security-oauth2-jose is fairly straightforward and independent from the rest, so it could be spun out in a separate PR and worked on right now, WDYT?

Agreed, this can be submitted in a separate PR. Feel free to take this on.

Part of this though is rethinking the org.springframework.security.oauth2.jwt.Jwt class as I mentioned in my design proposal (3rd open question), what are your thoughts about that?

org.springframework.security.oauth2.jwt.Jwt doesn't represent a JWT and we probably need something that does. (It requires to contain the JOSE headers and the entire JWS token (headers+JWT+signature) in its serialized form.)

The JOSE headers are in Jwt.headers, the claims are in Jwt.claims and the serialized form is in Jwt.tokenValue so I think we are good here? Or am I missing something?

Since the client secret is already in the ClientRegistration, wouldn't it be logical to put the JWS algorithm and any custom JWT claims there as well?

ClientRegistration is meant to be a simple POJO. I don't want to start adding top-level attributes that a lot of ClientRegistration would not use. I think it might make sense to add these type of attributes as second-level optional attributes, similar to how OIDC Discovery attributes are stored in ClientRegistration.ProviderDetails.configurationMetadata.

We may add support for OpenID Connect Dynamic Client Registration 1.0 at a later point so it might be useful to read that spec to see how ClientRegistration may contain this metadata that would work for this feature and work well for dynamic client registration later on. Maybe we have something like ClientRegistration.metadata as a Map (not a great name but this is the idea I have at the moment).

One more thing is the org.springframework.security.oauth2.core.ClientAuthenticationMethod (see my 1st point in the open questions in my initial design proposal) - should we have separate ones for the 2 signed-JWT-based authentication methods

Yes, let's define both auth methods as I like to align the implementation with the spec language.

[krajcsovszkig-ms]

@jgrandja

The JOSE headers are in Jwt.headers, the claims are in Jwt.claims and the serialized form is in Jwt.tokenValue so I think we are good here? Or am I missing something?

All 3 are currently mandatory (must be non-empty), but when we are constructing a JWT for later signing (or just to store some overrides for some of the default claims) we won't have the tokenValue, and probably not even any headers. Its builder can also only be constructed if you provide a tokenValue for it (although at that point it can be null, it'll only throw when you build it in that case).

I think it might make sense to add these type of attributes as second-level optional attributes

Sounds good

Yes, let's define both auth methods as I like to align the implementation with the spec language.

OK, so then they should be called the same as in the spec, right? (CLIENT_SECRET_JWT and PRIVATE_KEY_JWT)

[jgrandja]

@krajcsovszkig-ms

All 3 are currently mandatory (must be non-empty), but when we are constructing a JWT for later signing (or just to store some overrides for some of the default claims) we won't have the tokenValue, and probably not even any headers.

The Jwt structure has to have the 3 parts or it's not a Jwt. So the existing class Jwt is designed to spec. I guess I'm a little confused on why you would create a Jwt without a tokenValue to simply provides overrides for claims? It sounds like you are thinking the JwtEncoder would look like:

Jwt encode(Jwt token) throws JwtException

If that's what you're thinking then that is where the confusion is coming from. The return value is obvious Jwt but the input will NOT be Jwt. This is what we need to iron out first before the implementation is started.

As far as the way I see it, the JwtEncoder.encode would take in client and resource owner constructs and additionally be configured with the target JWS alg based on the client input. Take a look at OidcIdTokenDecoderFactory and the configuration it allows for supplying a JwtDecoder, eg. jwtValidatorFactory, jwsAlgorithmResolver...we might need something like jwtClaimSetConsumer (that can do overrides)

I would suggest that you open up a Draft PR so we can decide on the API for JwtEncoder and keep the discussion going on there.

OK, so then they should be called the same as in the spec, right? (CLIENT_SECRET_JWT and PRIVATE_KEY_JWT)

Yes

[krajcsovszkig-ms]

@jgrandja I've opened another PR (#8583), please have a look.

[jgrandja]

@forgo @krajcsovszkig-ms This feature is now available via gh-9520. Closing this as duplicate.