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

Implement client assertions (client_secret_jwt/private_key_jwt) support #1251

Closed
kevinchalet opened this issue Apr 26, 2021 · 10 comments · Fixed by #1893
Closed

Implement client assertions (client_secret_jwt/private_key_jwt) support #1251

kevinchalet opened this issue Apr 26, 2021 · 10 comments · Fixed by #1893

Comments

@kevinchalet
Copy link
Member

https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication

Note: this includes supporting assertions in the validation stack (for introspection).

@kevinchalet kevinchalet added this to the 4.0.0-preview1 milestone Apr 26, 2021
@kevinchalet kevinchalet self-assigned this Apr 26, 2021
@schmitch
Copy link

schmitch commented Apr 26, 2021

ah :-D ... I wanted to ask you today, if it might be a good idea to add client_assertion and I wanted to look into it, if I find enough time to make a PR.

I think the biggest trouble might be that openiddict only has a 1:1 mapping between client_id and client_secret, but it would be good if it would be possible to add a client_secret and assertions, since else it will be really hard for existing clients to update to assertions.

(btw. microsoft forces people to use assertions, because new client_secret's can only have a maximum lifetime of 2 years, while assertions can have a certificate over 100 years)

@kevinchalet
Copy link
Member Author

Given that client assertions are not really a common thing in most libs, I think it's safe to assume the good old client_secret_basic and client_secret_post will be used in 99% cases 😄

With that in mind, I'll likely end up opting for a new SigningKeys or SecurityKeys property on the Application entity, where we'll store the public keys used for client authentication.

What's still not clear is whether we'll want to support client_secret_jwt: since OpenIddict "hashes" the secret before storing it in the database (using the same key derivation format as Identity), using client_secret_jwt is not possible ATM. It's something we could work around by allowing to store secrets as symmetric keys (HMAC keys) in the new SigningKeys, but I'm not sure it's something I'm comfortable with, as it kinda sucks from a security perspective.

@schmitch
Copy link

schmitch commented Apr 26, 2021

well, to:

With that in mind, I'll likely end up opting for a new SigningKeys or SecurityKeys property on the Application entity, where we'll store the public keys used for client authentication.

I would rather not prefer it.

I mean it would be ok, if it would be going through an ClientSecretManager or so, because at the moment, the currently system means that keys will be hardcoded in the application and it's hard to use something like vault, azure key vault, etc..

@kevinchalet
Copy link
Member Author

kevinchalet commented Apr 26, 2021

Well, you'll still be able to override OpenIddictApplicationManager.GetSigningKeysAsync(...) to resolve them from wherever you want. That said, for client authentication, the OpenIddict server components will only need the public part of the key, so there's no real gain to expect by storing them in a "safer" place like AKV.

The only place where the private key will be needed will be in the validation options when using introspection. And for that, good old AddSigningCredentials methods will be exposed, just like in the server options.

@kevinchalet
Copy link
Member Author

I opened #1252 to track the ability to make things dynamic by resolving the JWKS from the API server.

@kevinchalet
Copy link
Member Author

For the record, client assertions support is part of the 5.0 preview1 release that shipped today: https://kevinchalet.com/2023/10/20/introducing-native-applications-per-client-token-lifetimes-and-client-assertions-support-in-openiddict-5-0-preview1/ 😃

@hisuwh
Copy link

hisuwh commented Feb 23, 2024

@kevinchalet any plans to support a Public keyset URL in the client registration rather than manually have to register all the keys?

@kevinchalet
Copy link
Member Author

kevinchalet commented Feb 23, 2024

@hisuwh not in the near future, I want to make sure client assertions are widely adopted before investing more time on that. If I determine it's worth it, it will likely be implemented alongside with #1252.

Would your company be interested in seeing this feature supported?

@JeffBarnard
Copy link

How would you recommend distributing the private keys to clients? Can they be read from a certificates? My industrial manufacturing IoT company may be interested in this as a safer alternative to distributing client_secrets to our services. If an attacker gains access to the private keys then they can still authenticate, so I don't see how this is safer than client_secret, but our customers can generate and distribute their own certs so it seems more convenient from that perspective.

@schmitch
Copy link

schmitch commented Apr 4, 2024

certificates do not solve the leakage of the key, but it solves mitm key leakages. Secrets can be farmed by introspection of the traffic between the server and the client, however with assertions the server has the pub key and the client generates an assertion and signs it, which the server than can use to verify the client.
Oh and btw. if you have a tpm or a crypto device that can generate or store a certificate securely you could make it hard to gain access to the key since the server only needs the public key, if that public key gets leaked mitm‘s can only verify your assertion, they can not change it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants