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

Conflicting requirement on request object signature #392

Open
frederikrothenberger opened this issue Jan 22, 2025 · 6 comments · May be fixed by #403
Open

Conflicting requirement on request object signature #392

frederikrothenberger opened this issue Jan 22, 2025 · 6 comments · May be fixed by #403
Labels
has-PR ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API
Milestone

Comments

@frederikrothenberger
Copy link

Hi

The standard says in section 5.10.4:

  • redirect_uri: This value indicates that the Client Identifier (without the prefix redirect_uri:) is the Verifier's Redirect URI (or Response URI when Response Mode direct_post is used). The Authorization Request MUST NOT be signed.

The requirement on the absence of a signature seems to be in conflict with the requirement from section 5.11.1 if combined with the request_uri parameter:

The Request URI response MUST be an HTTP response with the content type "application/oauth-authz-req+jwt" and the body being a signed, optionally encrypted, request object as defined in [RFC9101].

So, is there a signature required on the request object returned from the request_uri resource if the client_id_scheme is redirect_uri?

@Sakurann
Copy link
Collaborator

I think the intention is that client identifier scheme redirect_uri cannot be used with RFC9101 since rfc9101 does not allow alg = none. clarified in #403

@Sakurann Sakurann added has-PR ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API labels Jan 30, 2025
@Sakurann Sakurann added this to the Final 1.0 milestone Jan 30, 2025
@c2bo
Copy link
Member

c2bo commented Jan 31, 2025

I agree this needs to be formulated a bit clearer:

  • I am under the impression that JAR allows alg=none
  • we say the object is signed and optionally encrypted -> this would imply alg=none is not allowed - but the statement is not very clear

If we want alg=none to not be allowed for the Request Object, then we should be clearer

If we want to allow alg=none (which I do believe at least some implementations do or did), then we should change the sentence to something like

The Request URI response MUST be an HTTP response with the content type "application/oauth-authz-req+jwt" and the body being a Request Object as defined in [RFC9101] that can be signed and encrypted.

relevant issue #292

@tlodderstedt
Copy link
Collaborator

JAR does require implementations to refuse processing of request objects if alg in none.

Image

We should keep it that way.

@c2bo
Copy link
Member

c2bo commented Feb 4, 2025

Isn't that only true in the context of the metadata parameter require_signed_request_object=true? alg=none would be allowed if require_signed_request_object is omitted or false. Perhaps I am misunderstanding the RFC, but I would currently understand it as if require_signed_request_object is false or omitted, then there could be a request object with alg=none.

For the problem at hand (a request with redirect_uri (=unsigned) & request_uri) that would allow alg=none. I am also pretty sure I have seen implementations in the past that worked that way with redirect_uri.
I agree that we should probably not allow/encourage the use of alg=none, but I don't think it is clear enough with the current wording.

@jogu
Copy link
Collaborator

jogu commented Feb 4, 2025

Agree with Christian here, JAR allows alg none by default and there is no text in VP that disallows it. As I said on #403 the conformance tests support alg none for this and a number of people do seem to be using it when testing. We shouldn't rush to make a breaking change here.

I'm pretty sure HAIP already disallows alg none (by requiring x509_san_dns) which seems right/sufficient.

@awoie
Copy link
Contributor

awoie commented Feb 5, 2025

I also agree with @jogu and @c2bo. I don't like alg=none but we should not make a breaking change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-PR ISO_VirtualMeeting relevant for ISO OID4VP mdoc profile over DC API
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants