✨ DPoP Middleware extensions #118
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Unify on a middleware method, `withCreds` - Previously, the OIDC middleware only could return one value, Authorization header, which means we can't also set the DPoP header - The appID middleware updates HttpRequest in place, but that isn't best practice; instead, return a copy-and-modified version - Related fixes to use the middeware where needed - This pre-work simplifies adding DPoP headers in the middleware
Closed
- Register dpop with the AccessToken by adding a DPoP to the request. - Add DPoP headers to the OIDC middleware - Unfortunately since we have odd parameters on our public keys with TDF3, we can't use the existing signing keys and have to generate new ones for those. NanoTDF works with the existing keys, however Be more strict add 'ath' claim Cleanups and assertions - Adds ability to introspect auth providers - Adds spy on the cli auth provider - Uses spy to make sure dpop token is present - Adds `--dpop` parameter to cli tool to enforce DPoP fixup
dmihalcik-virtru
force-pushed
the
feature/4-dpop
branch
from
7f50e99 to
b9cefd4
Compare
pflynn-virtru
previously approved these changes
Dec 16, 2022
- Unify on a middleware method, `withCreds` - Previously, the OIDC middleware only could return one value, Authorization header, which means we can't also set the DPoP header - The appID middleware updates HttpRequest in place, but that isn't best practice; instead, return a copy-and-modified version - Related fixes to use the middeware where needed - This pre-work simplifies adding DPoP headers in the middleware
- Register dpop with the AccessToken by adding a DPoP to the request. - Add DPoP headers to the OIDC middleware - Unfortunately since we have odd parameters on our public keys with TDF3, we can't use the existing signing keys and have to generate new ones for those. NanoTDF works with the existing keys, however Be more strict add 'ath' claim Cleanups and assertions - Adds ability to introspect auth providers - Adds spy on the cli auth provider - Uses spy to make sure dpop token is present - Adds `--dpop` parameter to cli tool to enforce DPoP fixup
…client-web into feature/4-dpop
|
Kudos, SonarCloud Quality Gate passed!
|
pflynn-virtru
approved these changes
Dec 16, 2022
11 tasks
dmihalcik-virtru
added a commit
to opentdf/spec
that referenced
this pull request
Jan 4, 2023
### MINOR Update: Explicitly supporting [IETF DPoP Draft Proposal](https://datatracker.ietf.org/doc/html/draft-ietf-oauth-dpop) * This proposal aligns our Proof of Possession of the access token with the current IETF DPoP draft proposal. This includes: * Deprecating storing the entire key in the `tdf_claims.client_public_signing_key` subclaim * Store a SHA-256 hash of the key in the `cnf.jkt` field, per the current draft proposal * Submit a DPoP in the `DPoP` header, which will include the complete key and request identifying information * Not included: * To get complete support for the missing functionality we may desire to add a body hash to the DPoP claims list, which will provides an additional check for partial message integrity. A reference implementation of this change is now included with opentdf/backend and opentdf/client-web. If accepted we will port this to the remaining opentdf/client-*. #### Reference Implementation Details ##### Client Implementation For the client, we use [an existing DPoP implementation](https://www.npmjs.com/package/dpop) to generate proofs. To use, we added a parameter (dpopEnabled) to the clients, and if present make this required. Implementation: opentdf/web-sdk#118 ##### Identity Provider Implementation (Access Token Generation) Our reference IdP, Keycloak, provides a java plugin environment for adding claims to access tokens. Our current reference implementation extended our existing plugin, which adds the `tdf_claims` claim, to include the `cnf.jkt` claim when there is a DPoP header in the code exchange. Reference: https://github.com/opentdf/backend/blob/9603538287e23240c6d40ea596cf42f8b26bfcc4/containers/keycloak-protocol-mapper/custom-mapper/src/main/java/com/virtru/keycloak/TdfClaimsMapper.java#L292 ##### Access Point Validation (KAS) KAS currently uses application level validation of the OIDC header. We extended this to include the DPoP check iff the access token includes the `jkt` claim. https://github.com/opentdf/backend/blob/9603538287e23240c6d40ea596cf42f8b26bfcc4/containers/kas/kas_core/tdf3_kas_core/dpop.py#L55 Co-authored-by: Ben Leggett <854255+bleggett@users.noreply.github.com> Co-authored-by: b-long <b-long@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--dpopparameter to cli to optionally enable it (for now)