consider changing iss/exp/aud to should omit or must omit when only encrypted

[Joseph Heenan]

As per https://bitbucket.org/openid/connect/pull-requests/478#comment-376512110 VP spec currently says:

If the JWT is only a JWE, the following processing rules MUST be followed:
- `iss`, `exp` and `aud` MAY be omitted in the JWT Claims Set of the JWE, and if omitted the processing rules
as per [JARM] Section 4.3 related to these claims do not apply.

‌

If the JWE just contains json (i.e. the encrypted but not signed case) then iss/aud/exp are not integrity protected and should probably not ever be trusted, so I’m not sure there’s a case where they’re ever useful and hence we could just say they shouldn’t/mustn’t be used.

‌

[Oliver Terbu]

They are kind of integrity protected through the JWE because the JWE is encrypted to the RP’s key. That is why I found it not completely useless and used MAY.

[Oliver Terbu]

But I can also be convinced to make them MUST/SHOULD be omitted.

[Joseph Heenan]

This is similar to the recent Microsoft security flaw I think? ( https://securityboulevard.com/2023/02/technical-advisory-azure-b2c-crypto-misuse-and-account-compromise/ )

The RP’s public key (which is what is used to encrypt) isn’t a secret, so anyone can produce a JWE with whatever iss/exp/aud they want.

[Oliver Terbu]

The response JWE contains the verifiable presentation. An attacker would not be able to change iss/aud/exp without having the verifiable credentials and probably the holder key. I agree that iss cannot be trusted since it is not authenticated by the JWE. We should add language that iss cannot be trusted since it is not authenticated and add something to the security considerations section. But I’m also ok with MUST not be used.

[Oliver Terbu]

I created the following PRs:

• Introduces non-normative language in the security considerations section that aud, exp and iss are not trusted: https://bitbucket.org/openid/connect/pull-requests/482

• Changes MAY to MUST omit: https://bitbucket.org/openid/connect/pull-requests/483

[Joseph Heenan]

The related issue that I’m a bit less sure about is that I’m not sure if we need language to warn about other fields too.

The VC itself obviously has it’s own signature, so I think that’s probably not a concern.

I’m not sure if we need a warning about presentation_submission though.

[Oliver Terbu]

That is a good point.

But note that response_mode = fragment does send presentation_submission without integrity protection to the client. In that case, we don’t have a section in the security considerations section either.

While I believe that an attacker can potentially modify presentation_submission it would be detectable and I don’t know whether it would cause any damage since vp_token kind of protects presentation_submission. What do you think?

[Joseph Heenan]

Yes, the warning would apply to both the plain text and encrypted-only responses.

I find it hard to see what you could accomplish - although it has JSON Path stuff in it, which may open some interesting vectors. I think mostly it’s making it clear to people that “encrypted only” should basically be treated as “plain text” from a security point of view, i.e. that you can’t make an assumption the encrypted object is less likely to be from an attacker. (There must be a better way to word that!)

‌

[Oliver Terbu]

I agree that we should provide more info in the security considerations section that speaks to that more broadly. I updated the PR https://bitbucket.org/openid/connect/pull-requests/482

[Kristina Yasuda]

both PRs merged