-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: initial proposal to use dpop #67
Conversation
d4f98e7
to
b0c5f05
Compare
b0c5f05
to
d963646
Compare
A similar vision was brought by me here generally I agree with this, however we still have an issue with DPoP, as explained below. There are discussions about the requirement to provide more than a single wallet attestation in eIDAS. at the current stage, we can provide multiple client_assertion in a single url (see this) providing their PoP. Using DPoP it is possible for HTTP to carry multiple headers with the same name. This is defined in the HTTP/1.1 specification (RFC 2616). According to the specification, multiple message-header fields with the same field-name may be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list. It must be possible to combine the multiple header fields into one "field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma. however this approach doesn't give the binding of which PoP is related to which client attestation, requiring to the entity that evaluates assertions+pop to parse and validate them in a loop. This loop has computational costs that can be avoided by design, as the current specs allows. another issue could be that HTTP Header maximum size configured in the httpd services could represent a limit where multiple DPoP are provided. Anyway I feel optimistic about each the DPoP payload size and the number of http headers parameters included in a single http request. just to be clear: I prefer DPoP and I love to reuse things without redefining new one. |
Thanks for doing the work on this @tplooker! I am very supportive of this direction. I do wonder if it'd be worthwhile to include some explanatory text with this change about the fact that use of a DPoP proof does not mandate bound access tokens? It was a point of confusion/contention in prior discussions (eg #64 (comment), #64 (comment), etc) which might benefit from a brief mention in this document. |
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.
I am pretty strongly against this mechanism. I think it is ok to have this as an optimization but mandating using the same PoP for client attestation and DPoP overloads both mechanisms and limits the use-cases.
If I understand DPoP correctly, this proposals leads to a situation where authentication with an client attestation always leads to an access token bound to the public key in the 'cnf' claim of the attestation. This means, clients must always bind their access tokens and cannot use different keys for client authentication and the key binding of their access tokens. Is that correct? |
No that is not entirely correct, as @bc-pi pointed out in this comment just because you are using a DPoP proof in the token request to convey the client attestation PoP doesn't mean a DPoP enabled access token has to be issued as a response. You are correct though that in the event a DPoP access token is issued, the access token will be bound to the one in the |
Please see my comment above to ensure you understand what we are proposing here, I don't think this significantly limits use cases and on the flip side it significantly reduces what needs to be defined in this draft. If you still feel the same can you please elaborate on which usecases you think this would limit? |
Is there any reaction about the potential requirement to provide multiple client assertions in a request and my previous comment, shown below:
|
Co-authored-by: Brian Campbell <[email protected]>
Hi Giuseppe, |
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.
I have discussed this with my colleagues and we came to the following conclusions:
status quo:
preparation: potentially fetch 2x nonce
Token Request: client_attestation + client_attestation_pop + Dpop proof header
Token Response: Dpop-bound access token
proposed:
preparation: potentially fetch 1x nonce
Token Request: client_attestation + Dpop proof header (client_attestation.cnf == Dpop.header.jwk)
Token Response: Dpop-bound access token
con:
- concepts are tightly coupled, key must be the same if both concepts want to be used, may have unforeseeable disadvantages
- benefits in auth code flow are unclear if I would use client attestation at PAR endpoint
- jwk present in the client attestation JWT is duplicated to jwk header in Dpop proof
- refresh tokens are automatically dpop-bound?
pro:
- only fetch 1x nonce
- do one less key/PoP operation / generate one less key
- wallet attestation makes only sense when coupled with Dpop currently
- leverage better assurance of client_attestation key for Dpop (may be hw-bound)
- draft may leverage the Dpop nonce fetching (although its ugly ;) ) -> may avoid Adds server-provided nonces for Client Attestation PoP JWT freshness verification #64
For me, the advantages are bigger, so I'm supportive of this. But I would like to get more feedback and understand @Sakurann concerns. Also, some more editorial work may be needed
The text @bc-pi is referring to leaves it at the discretion of the AS to decide whether the access token is key bound. How would the client influence that decision? |
I think that is a general question for DPoP, but one that is important to answer. If by influence you mean the client is able to communicate to the AS that it supports dpop bound access tokens then I would agree, but at the end of the day the decision as to whether to issue a DPop enabled access token is still always at the discretion of the AS. With regard to how this would be achieved practically I would assume client metadata is the most effective way for the client to communicate this and Section 5.2 has the
Alternatively if we don't think this is sufficient perhaps we can consider registering a new client metadata element. Perhaps @bc-pi you have some further thoughts on this? |
Is there really a reason the client would need to influence that decision? I'd imagine the AS would issue the access token based on expectations/knowledge of the RS(s) it's intended for. |
With the current revision of the spec (before this PR), a client can use an attestation to just authenticate. If it wants a DPoP bound access token, it would add a DPoP header to the request. With the proposal in this PR, this is no longer possible. This effectively means every client using client attestation must also implement DPoP and use it if the AS decides the access token must be key bound. Also, the client does not have a choice to use different keys for attestation and DPoP. I could think of situations where the DPoP key is managed in a local security module to achieve better performance for subsequent API calls whereas the client attestation key lives in a remote HSM for higher security, which adds significant latency to every signing operation. With the new proposal, the application's performance would suffer simply because any API call might also require a remote signing operation for the DPoP proof. Also, I would like to understand whether the proposed approach would work with PAR. We will most likely need attestation based client authentication at the PAR endpoint. I assume the DPoP header would be used to perform the authentication only. Is that correct? |
True but ultimately the need to use key bound access token comes from the RS so the client would have to already be capable. I don't think it'd really be an issue in practice. But maybe I'm wrong. Some sort of signaling could be added to this - metadata as Tobias mentioned or a new claim in the DPoP proof or similar.
It is true that this PR doesn't allow the client to use of different keys for attestation and DPoP. I don't know if the kind of scenario you describe is likely enough to warrant the extra complexity of requiring two proofs in the other more common and simpler cases. I tend to aim for simplicity as much as possible/appropriate.
It would work with PAR the same as at the token endpoint as far as I understand it. The authz code would be key bound as a result too but that'd be invisible to the client who anyway needs to present the same attestation auth and proof when subsequently exchange that auth code. So it'd just work while adding a bit of extra security to the code. |
I think the most important point here is highlighted by @bc-pi. In the situation where the AS requires DPoP bound access tokens as per an RS policy, whether we use the DPoP proof syntax in this spec or not, it doesn't change the fact that the client won't be able to get an access token it can actually use with the RS. The only potential gap I see (which I think is highly unlikely) is in the event a client supports this client authentication method, but doesn't support DPoP access tokens AND the RS + AS supports both DPoP and bearer based access tokens, the situation could arise where the client might be sent back a DPoP access token (based on the DPoP header implying support in the token request) which it can't use, when it could have been sent a bearer token. Again as I said above, I find this situation highly unlikely, but I think it could be easily resolved through additional client metadata.
I'm also wary as to how common this use case is and whether the complexity it creates is justifiable. To play the devils advocate, this proposal as it currently stands creates an efficient way (when DPoP access tokens are used), to authenticate that the key the DPoP access token will be bound to, is clearly authenticated back to the client (through the client attestation). If instead we want to continue to support different keys for client authentication and DPoP token binding, the question then becomes how do we get the same assurance e.g that the DPoP key is authenticated for the client? The client attestation PoP, to use the terminology pre this PR would have to somehow reference and authenticate the seperate DPoP key.
This is my understanding too. |
Co-authored-by: Brian Campbell <[email protected]>
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.
I'm also supportive of this PR since it simplifies things since I'm unclear about the use cases for which the proposed approach won't work. It is actually a good higher assurance enforcing function.
@@ -117,31 +110,31 @@ To perform client authentication using this scheme, the client instance uses the | |||
|
|||
The value of the "client_assertion_type" parameter (as defined in {{RFC7521}}) set to "urn:ietf:params:oauth:client-assertion-type:jwt-client-attestation". | |||
|
|||
The value of the "client_assertion" parameter (as defined in {{RFC7521}}) set to a value containing two JWTs, separated by a '~' character. It MUST NOT contain more or less than precisely two JWTs separated by the '~' character. The first JWT MUST be the client attestation JWT defined in [](#client-attestation-jwt), the second JWT MUST be the client attestation PoP defined in [](#client-attestation-pop-jwt). | |||
The value of the "client_assertion" parameter contains a single JWT known as the client attestation JWT as defined defined in [](#client-attestation-jwt). It MUST NOT contain more than one JWT. |
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.
I don't think the second sentence is necessary since it is implied by the former sentence.
The value of the "client_assertion" parameter contains a single JWT known as the client attestation JWT as defined defined in [](#client-attestation-jwt). It MUST NOT contain more than one JWT. | |
The value of the "client_assertion" parameter contains a single JWT known as the client attestation JWT as defined defined in [](#client-attestation-jwt). |
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.
I think we would want this constraint wouldn't we? Why would you like to remove it?
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.
Didn't read your original comment just the suggestion, while I agree I think this makes the normative requirement a little more explicit. I took the language structure from RFC7523
The value of the "client_assertion" parameter contains a single JWT.
It MUST NOT contain more than one JWT.
I'm ok with removing this additional sentence though.
As a consequence of discussing some of the advantages this PR brings, a new usecase/requirement surfaced which has been captured in PR #69 |
Closing in favour of #74 |
DRAFT PR FOR DISCUSSION
📑 Description
Following discussion across multiple issues and PR's including #59 and #64. This PR represents a draft proposal to use the DPoP HTTP Header as defined in RFC9449 instead of the client attestation pop.
Preview Link
click here for rendered preview of PR