oauth-wg / draft-ietf-oauth-attestation-based-client-auth

Other
12 stars 7 forks source link

feat: initial proposal to use dpop #67

Closed tplooker closed 7 months ago

tplooker commented 9 months ago

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

peppelinux commented 9 months ago

A similar vision was brought by me here ~https://github.com/vcstuff/draft-ietf-oauth-attestation-based-client-auth/pull/67/files~ sorry, wrong url, see: https://github.com/vcstuff/draft-ietf-oauth-attestation-based-client-auth/issues/45

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 same time, for the purpose of a global standard, I see the requirement to provide a way to allow multiple client_assertion, where an ecosystem might require this, for future features.

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. At the same time the DPoP HTTP header doesn't give a proper mapping to the client assertion where multiple client assertion are provided. The real-world may require this and we should take the best decision in term of design

bc-pi commented 9 months ago

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 https://github.com/vcstuff/draft-ietf-oauth-attestation-based-client-auth/pull/64#issuecomment-1815362871, https://github.com/vcstuff/draft-ietf-oauth-attestation-based-client-auth/pull/64#issuecomment-1815668038, etc) which might benefit from a brief mention in this document.

tlodderstedt commented 9 months ago

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?

tplooker commented 9 months ago

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.

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 cnf claim of the client attestation which I personally find a useful simplification. What are the usecases for wanting a seperate key for client auth vs token binding (in the event it is used)?

tplooker commented 9 months ago

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.

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?

peppelinux commented 9 months ago

Is there any reaction about the potential requirement to provide multiple client assertions in a request and my previous comment, shown below:

however this approach doesn't give the binding of which PoP is related to which client attestation, requiring to the entity that evaluates assertions+(d)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.

paulbastian commented 9 months ago

A similar vision was brought by me here ~https://github.com/vcstuff/draft-ietf-oauth-attestation-based-client-auth/pull/67/files~ sorry, wrong url, see: #45

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 same time, for the purpose of a global standard, I see the requirement to provide a way to allow multiple client_assertion, where an ecosystem might require this, for future features.

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. At the same time the DPoP HTTP header doesn't give a proper mapping to the client assertion where multiple client assertion are provided. The real-world may require this and we should take the best decision in term of design

Hi Giuseppe, the proposed idea in the eIDAS discussion EPIC-09 is the wrong way. Currently client attestation supports one attestation and that's the way it should stay. I have outlined my position here: https://github.com/openid/OpenID4VCI/issues/215

tlodderstedt commented 9 months ago

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.

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?

tplooker commented 9 months ago

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 dpop_bound_access_tokens element registered. Although the definition for this parameter might not intuitively appear to fit, I think the intended normative requirements on the AS do? Specifically this statement which applies to this metadata elements value:

If true, the authorization server MUST reject token requests from this client that do not contain the DPoP header.

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?

bc-pi commented 9 months ago

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.

tlodderstedt commented 9 months ago

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?

bc-pi commented 9 months ago

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.

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.

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.

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.

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?

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.

tplooker commented 9 months ago

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.

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.

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.

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.

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.

This is my understanding too.

tplooker commented 9 months ago

As a consequence of discussing some of the advantages this PR brings, a new usecase/requirement surfaced which has been captured in PR #69

tplooker commented 7 months ago

Closing in favour of #74