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

[OpenID4VP] direct post: nonce and state are required in the response #39

Closed
OIDF-automation opened this issue Aug 30, 2023 · 13 comments · Fixed by #139
Closed

[OpenID4VP] direct post: nonce and state are required in the response #39

OIDF-automation opened this issue Aug 30, 2023 · 13 comments · Fixed by #139

Comments

@OIDF-automation
Copy link

Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/2052

Original Reporter: peppelinux

In “6.3.1. Response Mode "direct_post.jwt"“ we should have state as optional and nonceas required because the response is related to a specific request.

even if one or more vp token contained within the claim vp_token has the nonce parameter within it, the entire response, that envelopes them, must be related to a specific request and then containing at least the nonce.

the implementers that are forced to get the content of the single vp tokens to link them to a previous request are raising concerns about how to deal with a response that may contain multiple VP with different nonces, this raises security and implementation concerns.

This issue wants to raise the requirement to have both state and nonce as parameters of the direct post response JWT.

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: josephheenan

state is already required in the direct_post.jwt claims if the verifier passed state in the request; there was a previous ticket (which I can’t find for some reason) and it’s mentioned in https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#name-state-management

Do we need to change something about state, and if so what?

I think you are suggesting that ‘nonce’ be returned as a claim in the response JWT (as well as in the holder binding part of the credential). I am not sure I see I strong reason for that - can you explain more please?

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

Thx Joseph!

I didn’t find where in 6.3.1. Response Mode "direct_post.jwt" we have the normative definition of the state in the response, we just have a non-normative example in 6.2. Response Mode "direct_post" where both state and nonce are represented

If I miss them please give me the right pointers.

Regarding the requirement to have the nonce in the JWT response, and then also in the embedded vp_token, this is relevant for giving the way to the implementers to know to which authentication session request the response is referred, first of all and before parsing and verifiing each vp token. IF the response is malformed and contains hunderds of vp tokens with fake signagures and things, this might produce a resrouce consumption attack based on the signature verification, while by filtering the nonce/state within the jwt response represent an efficient filter againts the abuse and also for made the implementation easier

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: josephheenan

I didn’t find where in 6.3.1. Response Mode "direct_post.jwt" we have the normative definition of the state in the response, we just have a non-normative example in 6.2. Response Mode "direct_post" where both state and nonce are represented

The normative required is from RFC6749 :-) from https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#section-5 and https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#section-6.1 and then JARM says all authorization response parameters must be inside the signed JWT. The certification tests already check this.

Regarding the requirement to have the nonce in the JWT response, and then also in the embedded vp_token, this is relevant for giving the way to the implementers to know to which authentication session request the response is referred, first of all and before parsing and verifiing each vp token. IF the response is malformed and contains hunderds of vp tokens with fake signagures and things, this might produce a resrouce consumption attack based on the signature verification, while by filtering the nonce/state within the jwt response represent an efficient filter againts the abuse and also for made the implementation easier

I think state is sufficient for that though? I’m struggling to see a situation where an attacker could obtain state but not nonce. I think it’s cleaner / there’s less potential for implementers to misunderstand the purpose if we keep nonce as purely for the holding binding challenge / response.

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

Ok on the paramenters according to JARM, even if it is not easy to be read because is a reference without any remark but I’m ok to leave it as it is.

My proof reading told me that:

The normative required is from RFC6749 :-) from https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#section-5 and https://openid.bitbucket.io/connect/openid-4-verifiable-presentations-1_0.html#section-6.1 and then JARM says all authorization response parameters must be inside the signed JWT. The certification tests already check this.

Section 5 is related to the request only. While Section 6.1 says Other parameters, such as state or code (from [RFC6749]), or id_token (from [OpenID.Core]), and iss (from [RFC9207]) MAY be included in the response as defined in the respective specifications.

I think that’s too weak for the reader than might need more guidance about how the final implementation should be. We should say that if the request has a state parameter the response MUST then give the same parameter back.

regarding the second point, the nonce in the signed JWT response would be intended for binding the response without parsing each vp within it, in the cases where the state is not present. We assume that all the vp tokens within a single response belongs to the same state/nonce, then why we do not have the nonce in the signed response?

I suggest to require the nonce in the signed response, by explicitly saying this in the text and that this MUST be the same of all the nonce within the single vp tokens.
Please note that there are soem case where the vp Token is a credential not enveloped, then it might be of a different format than JWT (MDOC CBOR). In this case the nonce parameter won’t be available

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: tlodderstedt

The text in the security consideration requires any implementation to link any presentation in the vp token to the nonce

The Wallet MUST link every Verifiable Presentation returned to the Verifier in the VP Token to the client_id and the nonce values of the respective Authentication Request.

How that is achieved might vary across credential formats. That does not necessarily mean the nonce must be a claim in the presentation. For example with AnonCreds the nonce will be incorporated into the proof.

Are you saying it is impossible with mdoc?

re signed requests: in the general case, I would not assume the wallet has a key to sign the response that is really meaningful to the verifier. I therefore would not rely on signed responses in general. If you have that key in your case, that might be an option ( I hope it won’t result in issues re privacy or availability of the wallet).

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

In mdoc cbor we may have some nonce equivalences like DeviceAuthentication.SessionTranscript.DeviceEngagementBytes and/or DeviceAuthentication.SessionTranscript.OpenID4VPHandover.nonce

I would like to improve the current text by giving more guidance to the implementers that, I repeat, have found concern about what would then be the correct approach and interpretation for a good security of the implementation.

And I include myself in those who have read more than a single time the specs :-)

So let’s Say that this Is an editorial issue, if you prefer

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

ISO 23220-4/18013-7 has defined a way how to put nonce in mdocs, so it is possible to bind mdocs to a specific transaction using nonce. are you suggesting to copy that text from ISO documents into OID4VC?

do we agree no additional clarification is needed re state parameter?

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: tlodderstedt

As far as I understand, we need more details on nonce handling.

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

I’d suggest to list all the required claims in the response. I found the text about the state a bit confusing for the implementers.
While for the nonce: in this issue I propose to have it also in the response to allow fast lookups for the implementers, without starting the processing of the vp tokens to know know the nonce. IF any of the contained vp token might have a different nonce the entire response should be invalidated

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: josephheenan

I’d suggest to list all the required claims in the response. I found the text about the state a bit confusing for the implementers.

Agreed, the text can be improved.

While for the nonce: in this issue I propose to have it also in the response to allow fast lookups for the implementers, without starting the processing of the vp tokens to know know the nonce. IF any of the contained vp token might have a different nonce the entire response should be invalidated

I can’t see any advantage to including nonce in the top level response. state is sufficient for everything. If state has leaked to an attacker, nonce has too. The implementer should know the expected nonce by looking it up from state.

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: peppelinux

I buy that @joseph, since the state is good enough for the scope of the lookup, then if provided in the request it MUST be returned in the response. Just by remarking this in the text I think that this issue would then be closed with success

@OIDF-automation
Copy link
Author

Imported from AB/Connect bitbucket - Original Commenter: josephheenan

Agreed. This is the behaviour defined in RFC6749, I think we just need to make it clearer.

Of the 3 wallets that have tried the VP conformance tests so far, all 3 failed to implement state :-( (but at least one does now implement it)

@Sakurann
Copy link
Collaborator

raising concerns about how to deal with a response that may contain multiple VP with different nonces, this raises security and implementation concerns.

@peppelinux would like to discuss in the PR "If the response contains multiple Verifiable Presentations which do not contain the same nonce value, the response is rejected." is the way to go...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants