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

iss in Key Proof, conditionally required or generally optional? #349

Open
mickrau opened this issue Jun 14, 2024 · 5 comments
Open

iss in Key Proof, conditionally required or generally optional? #349

mickrau opened this issue Jun 14, 2024 · 5 comments

Comments

@mickrau
Copy link
Contributor

mickrau commented Jun 14, 2024

There were different interpretations during the LSP interop event whether the iss element in key proof (both jwt and cwt) is conditionally required or generally optional?

In Section 7.2.1.1. (and 7.2.1.3.) it says:

iss: OPTIONAL (string). The value of this claim MUST be the client_id of the Client making the Credential request. This claim MUST be omitted if the access token authorizing the issuance call was obtained from a Pre-Authorized Code Flow through anonymous access to the token endpoint.

Is there a reason to make it optional in the case the client_id is known to the issuer?

I would prefer to change it to REQUIRED when ..., MUST NOT ... (as has been done elsewhere)

@Sakurann
Copy link
Collaborator

the intention here was

  • when a Pre-Authorized Code Flow through anonymous access to the token endpoint was used, MUST omit
  • anything else, optional for wallet and issuer
    so, OPTIONAL is correct here, but if you have suggestions for the english text in the description, please feel free to do a PR :)

@bc-pi
Copy link
Member

bc-pi commented Jun 14, 2024

why have an iss at all in the proofs? honest question

@mickrau
Copy link
Contributor Author

mickrau commented Jun 18, 2024

@Sakurann If this is the intention, the current description fits.

Then i would agree to @bc-pi question.

Is the iss is security-relevant? If yes, it should be required or the issuer should have the possibility to inform the user that he requires it in my opinion.

With the current version we end up with a (worthless?) check on the issuer's side:

check(signedJwt.jwtClaimsSet.issuer == clientId || signedJwt.jwtClaimsSet.issuer == null) {
   "iss MUST be the clientId or null"
}

@jogu
Copy link
Contributor

jogu commented Jun 18, 2024

I think various security-ish issues end up being related and it's quite hard to reason about them all given the amount of optionality in the specification.

iss in the key proof is one way to bind the key proof to the client id (although in some cases there will be other mechanisms for binding the key proof to the client, e.g. the suggestion to include public keys in wallet attestations that was discussed by a small group at EIC, which I think was an iteration on #305 but that ticket hasn't been updated yet after that discussion).

#19 proposes an alternate mechanism for that binding that I think would make iss unnecessary.

For what it's worth I agree that iss is mostly worthless currently if it's completely optional for the client to include it, my question would be whether we try to fix that or remove it in favour of other mechanisms.

@babisRoutis
Copy link
Contributor

For what it's worth I agree that iss is mostly worthless currently if it's completely optional for the client to include it, my question would be whether we try to fix that or remove it in favour of other mechanisms.

Currently, VCI doesn't describe when this iss attribute must be provided.

This creates the following situation:
As a wallet better included, just in case issuer requires it (in the authorization code flow)
As an issuer better don't check it, or check it as per @mickrau comment.

IMHO, iss should be removed.

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

No branches or pull requests

5 participants