-
Notifications
You must be signed in to change notification settings - Fork 49
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
Initial implementation of a DPoP proofing mechanism #277
base: main
Are you sure you want to change the base?
Conversation
Resolves smallrye#165 This adds the initial structure for handling proof of possession semantics at the application layer.
@acoburn Thanks, let me add a few initial comments |
implementation/src/main/java/io/smallrye/jwt/auth/AbstractDpopTokenValidator.java
Outdated
Show resolved
Hide resolved
implementation/src/main/java/io/smallrye/jwt/auth/AbstractDpopTokenValidator.java
Outdated
Show resolved
Hide resolved
final JwtConsumer parser = new JwtConsumerBuilder() | ||
.setRequireJwtId() | ||
.setExpectedType(true, DPOP_JWT_TYPE) | ||
.setJwsAlgorithmConstraints(new AlgorithmConstraints(PERMIT, |
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.
This algorithm is used for decrypting the claims or inner signed tokens when dealing with the access token. I believe you meant getSignatureAlgorithm
.
But you have a good point about ES256
/etc - I propose that we have a new property added, smallrye.jwt.verify.dpop.signature-algorithm
(and we doc it is for the DPoP proof JWT verification), its default value is the same as that of smallrye.jwt.verify.signature-algorithm
which is RS256
but the users can choose ES256
.
What do you think ?
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.
Yes, I did mean getSignatureAlgorithm
. I like your suggestion for adding a smallrye.jwt.verify.dpop.signature-algorithm
config property
If the IDP providers will start supporting the DPoP proof verification too then they'd deal with Overall, a great start, thanks for spending the time on it. Let me also ping Keycloak colleagues :-) |
Hey @acoburn How are you, by the way I've been thinking of having an |
@sberyozkin that sounds great. I know this PR has gone largely dormant, but I would be happy to help move it forward |
Keycloak now supports DPoP as an experimental feature so it is a good time to complete this PR, I'll try to have a look |
Hey @acoburn @sberyozkin , I hope you are well. I was wondering if there have been any updates regarding the PR? Additionally, are there any significant hurdles or blockers that need to be addressed? Your insights would be greatly appreciated. I believe this is the correct link to the RFC : https://datatracker.ietf.org/doc/html/rfc9449 |
@VinodAnandan Hi, preparation for testing it is in #772, but I won't have time to fix it in the very short term in smallrye-jwt. I'll play with it at the |
Resolves #165
This adds the initial structure for handling proof of possession semantics at the application layer.
This PR does not contain any tests, as I would like some initial feedback on this implementation. The basic design assumes that the
AbstractDpopTokenValidator
class would be extended and the following methods implemented:::getDpopHeaderValue
-- this returns the raw HTTP DPoP header value::getRequestUri
-- this returns the HTTP request URI::getRequestMethod
-- this returns the HTTP request methodThen, given a
@JsonWebToken
object, the::verify
method checks that the thumbprint of any access token-bound key:matches the public key in the request-specific DPoP token (i.e. that the DPoP token is scoped to the request URI and method). This method will throw a
ParseException
in any of the following cases:If this seems like a reasonable approach, I will add accompanying tests.
Note: this does not implement
jti
constraints (which are optional according to DPoP and difficult to support across a cluster of stateless resource servers)Also note: this makes use of the algorithm defined in
authContextInfo.getKeyEncryptionAlgorithm().getAlgorithm()
, though it isn't strictly necessary to align those (an identity provider may sign with RSA and a browser may use ECDSA for DPoP proofing -- ephemeral ECDSA keys for use with DPoP are considerably faster to generate in a browser)DPoP Specification Reference: https://tools.ietf.org/html/draft-fett-oauth-dpop-04