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

feat: initial proposal to use http headers #74

Merged
merged 23 commits into from
May 29, 2024

Conversation

tplooker
Copy link
Collaborator

@tplooker tplooker commented Mar 28, 2024

📑 Description

Based on the discussions had at IETF 119 in the OAuth WG, this PR attempts to make the following changes to the draft

  • Move away from usage of the assertion framework defined in RFC 7521 and RFC 7523 for conveying the client attestation and client attestation pop.
  • Instead use two newly defined HTTP headers OAuth-Client-Attestation and OAuth-Client-Attestation-PoP for conveyance in an HTTP request, akin to how DPoP works.
  • Explicitly mention this mechanism could be used in a variety of places including with a resource server.

Note, there are still many outstanding items I would like to add, however I felt this PR was already big enough as it is, these include

@c2bo
Copy link
Member

c2bo commented Mar 28, 2024

For some reason it seems to not create an html version?

This is the rendered text:
https://vcstuff.github.io/draft-ietf-oauth-attestation-based-client-auth/tl/header-syntax-proposal/draft-ietf-oauth-attestation-based-client-auth.txt

@jogu
Copy link

jogu commented Mar 28, 2024

I think there were potential objections mentioned about moving to headers as to whether we'd run into size limits (I think 8 KB was mentioned as a common restriction) and people felt jwts with embedded x5c headers etc may get close to these limits?

@peppelinux
Copy link

@jogu indeed. We may have a client attestation with an huge x509 certificate chain or an openid federation trust chain with several metadata, trust marks and jwks.

I don't see the benefit of moving from what it is actually more efficient (wa~wa-pop)

more over, technically we can have multiple client_assertions that embedes their pop (using ~), while having multiple http headers that divide attestation from pop forces the implementations to loop all the assertions and all the pop to find the matching ones


1. A Client Attestation JWT - typically produced by the client backend.
2. A Client Attestation Proof of Possession (PoP) - produced by the client instance.
Note that the protocol for steps (2) and (4) and how the Client Instance authenticates to the Client Backend is out of scope of this specification. Note also that this specification can be utilized without the client having a backend server at all; in this case, each Client Instance will perform the functions described as being done by the backend for itself.

Copy link
Collaborator

@paulbastian paulbastian Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add Motivation, public clients need to authenticate etc..
this spec for authenticating client instances of public clients

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 3, 2024

Added client instance definition and updated to use US based spelling of authorization instead of authorisation.

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 3, 2024

@jogu @peppelinux I can understand the desire to want to use X.509 certificates to validate the client assertion (e.g as an alternative key resolution mechanism to resolving a JWKS document from a domain), however I don't see how the certificate chain would be more then 1 certificate (maybe 2) in length (bearing in mind the root certificate(s) would be communicated to the issuer out of band and pre-trusted for the client). W.r.t other aspects of the client attestation or client attestation pop being too big for an HTTP header could you elaborate on what you see as the contents of your attestation @peppelinux? Because IIUC you perhaps want to use the client attestation to convey all client metadata? If that is the case IMO the client attestation isn't for this purpose, client metadata should continue to be communicated outside of these attestations with the pre-existing mechanisms we already have defined.

Finally even if we did begin to encroach on header payload limits with a client attestation, we aren't talking about hard limits here, rather just the default maximums that common web server platforms have configured, which can always be adjusted.

@peppelinux
Copy link

peppelinux commented Apr 4, 2024

there some point for clarification

  1. assertion weight: a client assertion may have a x5c or an openid federation trust chain within its JWT header. This is an issuer's choice, allowed by RFC 7515 that might be not supported by httpd maximum header body size, that's a deployment/configuration choice that might be different within different domains.
  2. chain length: it could be assumed that a chain may not exceed 2 or 3 certificate breaks. This depends on the number of intermediates, an aspect that falls outside the technical specification and is determined by the federation's topology and composition.
  3. client metadata: we have some required claims in the wallet attestations and the possibility to include any other custom claim or any other claims pertaining the wallet capabilities or anything else related to the trust framework. My comment is not related to assertion payload, but to the previous point 1 and 2. However, the payload also impacts in the assertion weight, as pointed out at point 1.

The missing point concerns the processing of multiple DPoP tokens within the same request. Handling multiple DPoP tokens necessitates a loop to identify the one that matches a specific assertion. This introduces additional computational and development efforts not present in the current approach (WA~WA-POP), which directly associates an assertion with its proof of possession (PoP), facilitating a straightforward and efficient identification of matching pairs.

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 5, 2024

assertion weight: a client assertion may have a x5c or an openid federation trust chain within its JWT header. This is an issuer's choice, allowed by RFC 7515 that might be not supported by httpd maximum header body size, that's a deployment/configuration choice that might be different within different domains.

Ok great so do we agree that this isn't an issue because servers are freely able to re-configure the max header size based on their application? Because it isn't a hard limit.

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 5, 2024

chain length: it could be assumed that a chain may not exceed 2 or 3 certificate breaks. This depends on the number of intermediates, an aspect that falls outside the technical specification and is determined by the federation's topology and composition.

Understood but based on the above point is this actually an issue if the limit on header size isn't a hard limit?

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 5, 2024

The missing point concerns the processing of multiple DPoP tokens within the same request. Handling multiple DPoP tokens necessitates a loop to identify the one that matches a specific assertion. This introduces additional computational and development efforts not present in the current approach (WA~WA-POP), which directly associates an assertion with its proof of possession (PoP), facilitating a straightforward and efficient identification of matching pairs.

Right but to be clear, the current draft does not handle multiple wallet attestations either though so I think this a new requirement that we should discuss separately, would you mind opening a seperate issue?

@peppelinux
Copy link

The issue extends beyond the realm of multiple wallet attestations, as there could also be multiple DPoP tokens involved.

An additional consideration, which we have not yet fully addressed but may emerge as a future requirement, is the scenario where a wallet instance operates under more than one trust framework, necessitating specialized assertions for each. While we have consciously chosen to limit the scope to a single wallet attestation to simplify implementation, it's important to acknowledge that this decision does not preclude the possibility of requiring multiple assertions in the future. Implementers may need to adapt or extend the specification to accommodate such needs. However, to avoid digressing further, let's focus on the matter of handling multiple DPoP headers for now.

The approach currently documented, which aims to streamline the process by limiting it to a single attestation, is designed to offer the same benefits while minimizing computational overhead.

@c2bo
Copy link
Member

c2bo commented Apr 5, 2024

assertion weight: a client assertion may have a x5c or an openid federation trust chain within its JWT header. This is an issuer's choice, allowed by RFC 7515 that might be not supported by httpd maximum header body size, that's a deployment/configuration choice that might be different within different domains.

Ok great so do we agree that this isn't an issue because servers are freely able to re-configure the max header size based on their application? Because it isn't a hard limit.

While it is possible to reconfigure these options, it is important to keep default values of the wide-spread implementations in mind to allow an easy path for adoption.

I took the time to look at some of the frameworks I would say are good indicators:

  • nginx: link --> 8KB for 1 header max
  • envoy (used in istio): link --> 60 KB for headers
  • node.js: link --> 16KB for headers
  • traefik: link --> re-uses go/http: 1MB for headers

--> After looking at this, I would say we can go with headers

@peppelinux
Copy link

peppelinux commented Apr 5, 2024

An OpenID Federation trust chain with two intermediates, the trust anchor entity configuration with a long list of trust mark issuers and the leaf entity configuration with a very verbose configuration, such as an AS + Openid4VCI + OpenID4VP (considering an EAA provider), with policies and a consistent number of trust mark might be 20KB.

A serialized X.509 certificate chain, with 4 intermediates, custom extensions and QWACs ... Considering each certificate up to 2.2KB, might it be 8.8KB?

(please, consider that openid federation brings policy languages, trust marks and multiple protocol specific metadata)

I'm worried about these aspects:

  1. loop required for parse/matching the correct DPoP when multiple DPoP are present, while WIA~WIA-POP doesn't bring this problem. What's the benefit of having this breaking change that reduce the flexibility while increasing the complexity?
  2. size limitations: Do we have to imagine to standardize something upon the assumption that a federation topology should not have more than 2 or 4 or 8 intermediates, since this decision is out of scope on this specification.

@c2bo
Copy link
Member

c2bo commented Apr 5, 2024

A bit off-topic for the current discussion:
We need to fix the examples - they are way too long which breaks the rendering/conversion and will cause problems with the datatracker. I do believe that this is also the reason why the html file was not created.

@bc-pi
Copy link
Contributor

bc-pi commented Apr 5, 2024

Note that rfc9449 does not allow for multiple DPoP headers. It could maybe have been stated more clearly but is kinda implied and was absolutely the intent of the RFC overall and specific text like https://www.rfc-editor.org/rfc/rfc9449.html#section-4.3-2.2

@tplooker
Copy link
Collaborator Author

tplooker commented Apr 7, 2024

loop required for parse/matching the correct DPoP when multiple DPoP are present, while WIA~WIA-POP doesn't bring this problem. What's the benefit of having this breaking change that reduce the flexibility while increasing the complexity?

@peppelinux as I implied with my earlier comment, the current draft text doesn't support multiple attestations so I don't view this proposal to shift to headers as removing functionality, it appears this is a new usecase and I think we should discuss it in a seperate issue.

size limitations: Do we have to imagine to standardize something upon the assumption that a federation topology should not have more than 2 or 4 or 8 intermediates, since this decision is out of scope on this specification.

No I dont think we do, what was identified above is that there is no size limit on headers in HTTP rather defaults that servers apply which can be adjusted, in reality web servers also apply limits to the body of an HTTP request (although often much larger then what is allocated for headers).

@tplooker
Copy link
Collaborator Author

Discussed with @paulbastian, I will add an implementation consideration around the default size limits for HTTP headers.

@peppelinux
Copy link

@tplooker what about using compressed values?

unfortunately this sounds like giving a solution that may bring other problems, at the same time we can consider zlib (RFC 1950) for doing this.

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should hopefully fix the rendering

draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
draft-ietf-oauth-attestation-based-client-auth.md Outdated Show resolved Hide resolved
Co-authored-by: Christian Bormann <[email protected]>
Co-authored-by: Tobias Looker <[email protected]>
@tplooker tplooker requested a review from tlodderstedt May 16, 2024 06:57
@c2bo
Copy link
Member

c2bo commented May 16, 2024

Copy link
Member

@c2bo c2bo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do believe there should be a discussion about terminology (attestation vs assertion) as it seems to create quite a bit of confusion, but I do think a different issue/PR is better suited for that.

Minor nit: Client Attestation seems to be used in lower and upper case somewhat inconsistently. Should every usage be upper case (e.g., also client attestation mechanism)?

@bc-pi
Copy link
Contributor

bc-pi commented May 21, 2024

It just (re)occurred to me that specs defining new HTTP headers (which the cool kids call fields nowadays) are supposed to request their registration in the Hypertext Transfer Protocol (HTTP) Field Name Registry. I don't want to hold up this PR for this but will create an issue from this comment so as not to lose track of it.

@paulbastian paulbastian requested a review from tlodderstedt May 22, 2024 05:50
@paulbastian paulbastian merged commit 2b711a0 into main May 29, 2024
2 checks passed
@paulbastian paulbastian deleted the tl/header-syntax-proposal branch May 29, 2024 06:45
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 this pull request may close these issues.

7 participants