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

Add DPoP proofing mechanism #165

Open
acoburn opened this issue Jan 16, 2020 · 6 comments · May be fixed by #277
Open

Add DPoP proofing mechanism #165

acoburn opened this issue Jan 16, 2020 · 6 comments · May be fixed by #277

Comments

@acoburn
Copy link
Contributor

acoburn commented Jan 16, 2020

Now that custom token schemes are supported, this issue is to discuss adding a DPoP verifier to the smallrye-jwt code. We could also discuss implementing this as a quarkus extension, but I'd like to propose implementing the verifier at this layer.

The approach that I'd like to propose would be implemented as a i.s.j.auth.jaxrs.DPoPAuthenticationFilter class. This filter would have a @Priority(AUTHENTICATION) and perform the following validation steps:

  1. Extract a JWT from the DPoP header, if one exists
  2. Parse that token and verify that the embedded public key can be used to verify its signature
  3. Verify that the htm (HTTP Method) claim matches the method from the JAX-RS request context
  4. Verify that the htu (HTTP URI) claim matches the URI from the JAX-RS request context
  5. Verify (this is the crucial step) that the thumbprint of the public key from the DPoP header matches the thumbprint in the cnf (confirmation) claim of the access token (the injected JsonWebToken)

The DPoP draft specification does not define how to handle error conditions, should validation fail, but I would propose returning a 401 Unauthorized, since that's what this library already returns when a token can't be validated.

The Proof-of-Possession Key Semantics specification defines several ways that an access token can represent confirmation claims, including:

  1. Thumprint: "cnf": { "jkt": "SHA-256 thumbprint" }
  2. Embedded public key: "cnf": { "jwk": { ... } }
  3. The location of a public key: "cnf": { "jku": "URL of JWKS", "kid": "key-identifier" }
  4. Key ID reference: "cnf": { "kid": "key-identifier" }

I would like to propose supporting only the first two options. One of the nice features of DPoP is that it is self-contained and makes use of ephemeral keys, and the third option conflicts with both of those features. The fourth option is complicated because it would require that resource servers have extra out-of-band knowledge and it has questionable practicality in this context.

One part of this that could use some discussion relates to how to prevent against downgrade attacks. Since it would be important to continue supporting regular Bearer tokens in addition to DPoP, one should consider the case where an Authorization: DPoP <token> header gets replayed as Authorization: Bearer <token> without the added DPoP header. Typically, one would want to reject this request, but this assumes that the cnf claim is not being used for any other purpose, and I'm not sure that's a safe assumption in the general case. I suspect the simplest way to address this is the following:

By default, DPoP proofing is entirely disabled, but it can be enabled via an MP-config value (e.g. smallrye.jwt.enable.dpop). If enabled and if the access token contains a cnf claim, then the DPoP filter becomes relevant. At that point, if the token scheme is Bearer, the request can be denied. But if the access token does not contain a cnf claim, then the token is treated as a simple bearer token and DPoP validation doesn't come into play at all.

If this seems like a reasonable approach, I can implement it.

@sberyozkin
Copy link
Contributor

@acoburn Hi, thanks for this proposal.
FYI, these smallrye-jwt JAX-RS filters are currently not used in Quarkus/Thorntail, ex, quarkus-smallrye-jwt does not have any JAX-RS dependencies. However, indeed lets talk here, as having a filter at this level would be good. In fact we can have a JAX-RS filter with the bulk of the DPoP code being in the abstract class, which the authentication mechanisms in Quarkus (in quarkus-smallrye-jwt and quarkus-oidc/BearerAuthenticationMechanism), WildFly, etc could extend independently.

Please give me some time before I will comment further. Cheers
CC @darranl @MikeEdgar

@sberyozkin
Copy link
Contributor

sberyozkin commented Jan 17, 2020

@acoburn I've read the description, it is very clearly typed, and makes sense IMHO.
The option 3 may also make sense because MP JWT allows to support HTTPS based JWK sets, but starting with the first 2 options makes sense.
My only suggestion at this stage is to implement most of this code in the abstract class, similarly to AbstractBearerTokenExtractor, where this abstract class would have the abstract HTTP method and request URI getters. The DPop JAX-RS filter extending it will indeed be located in this library while Quarkus, WildFly, etc extensions could use Vertx/Undertow API etc to get the method/URI
Please start experimenting with the PR :-)
Thanks

@acoburn
Copy link
Contributor Author

acoburn commented Jan 17, 2020

@sberyozkin thanks for the feedback. Using an abstract class, as you propose, makes a lot of sense, and that should be reasonably straight-forward to implement.

@sberyozkin
Copy link
Contributor

@acoburn Hi, have you had a chance to do some prototype ? thanks

@acoburn
Copy link
Contributor Author

acoburn commented Apr 1, 2020

@sberyozkin thanks for the ping. I have done some initial work on this. I'll try to wrap that up and submit as part of a PR

@sberyozkin
Copy link
Contributor

@acoburn Nice, would be good to see it, please take your time

acoburn added a commit to acoburn/smallrye-jwt that referenced this issue Jul 8, 2020
Resolves smallrye#165

This adds the initial structure for handling proof of possession
semantics at the application layer.
acoburn added a commit to acoburn/smallrye-jwt that referenced this issue Jul 8, 2020
Resolves smallrye#165

This adds the initial structure for handling proof of possession
semantics at the application layer.
@acoburn acoburn linked a pull request Jul 8, 2020 that will close this issue
@sberyozkin sberyozkin added this to the 4.5.0 milestone Nov 27, 2023
@sberyozkin sberyozkin removed this from the 4.5.0 milestone Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants