openid / OpenID4VCI

62 stars 18 forks source link

Security Issue with untrusted Issuer Metadata #202

Open paulbastian opened 8 months ago

paulbastian commented 8 months ago

I see a potential security issue as some parameters in the Credential Issuer Metadata may be self-asserted, in particular: display.name, credentials_supported.display.name and credentials_supported.display.logo. These values are today taken by Wallets and shown to the user. This might give a false sense of security, as attackers easily can create a fake issuer and fake their identity with this metadata.

The security issue originates in my opinion to the fact that OpenID4vc does not mandate specific trust mechanisms similar as being open to any credential format. While this is a strength in general, it is not clear enough to me from the security and privacy considerations that this gap MUST be filled by ecosystems relying on OpenID4vc.

Proposal: Add a section about the significance of an underlying trust framework ans what resources of the OpenID4VCI protocol must rely on it, i.e. which things must be protected by that trust mechanisms.

Same applies to OpenID4VP probably, but I didn't cross check the considerations section there.

peppelinux commented 8 months ago

I agree.

From my perspective OpenID Federation represents a solution for this kind of issue, providing metadata overrides and policies within the superior's entity statements issued for their subordinates (the VCI in this case, that's a subordinate of the accreditation body, be this the TA or its Intermediate).

Other trust frameworks might employ different strategies to address this issue. Currently, I'm not aware of any technologies or standards that can encapsulate metadata and policies into a verifiable artifact. OpenID Federation uses the Trust Chain as the result of the chained signed artifacts, representing the relationships and permissions established between various independent organizations and entities.

David-Chadwick commented 8 months ago

I also agree. In our implementation, using TRAIN as the trust infrastructure, the URL of the verifier's metadata was returned to the wallet, allowing it to download the metadata without trusting anything that the verifier told it (other than its identity). Of course if it lied about its identity, then the wallet would send the VPs to the asserted identity and not to the verifier who lied about its identity.

Sakurann commented 7 months ago

any parameter including the ones you are pointing out can be put inside signed_metadata parameter introduced in PR #140. having said that I am not against a security considerations section that talks about establishing trust in the issuer metadata

awoie commented 7 months ago

IMO, it boils down to trusting the issuer. If this trust is broken, you have no chance to prevent fake credentials. The same applies to the issuer metadata. If you don't trust the issuer to provide a trustworthy issuer metadata, then a wallet/verifier should not accept those credentials.

I can issue mDLs of Utopia and if somebody trusts me as an issuer of such, then I can do that. Essentially, this relates to accreditation of issuers and whatever form of trust framework is in place. Same applies to metadata.

tlodderstedt commented 7 months ago

I would not say it boils down on whether you trust the issuer or not. I think there are subtle difference between different kinds of metadata. Especially the name (display.name) of an entity is something, a party might only trust in when asserted by a trusted 3rd party. I suggest to add a note (perhaps security consideration) stating that the wallet must be aware of the fact all metadata in the file (if not signed by a trusted 3rd party) are self asserted by the issuer. The wallet must consider that when using this data (including presenting it to the user).

awoie commented 7 months ago

I would not say it boils down on whether you trust the issuer or not. I think there are subtle difference between different kinds of metadata. Especially the name (display.name) of an entity is something, a party might only trust in when asserted by a trusted 3rd party. I suggest to add a note (perhaps security consideration) stating that the wallet must be aware of the fact all metadata in the file (if not signed by a trusted 3rd party) are self asserted by the issuer. The wallet must consider that when using this data (including presenting it to the user).

Yes, they are self-asserted by the issuer, yes, so the wallet/verifier has to trust the issuer for that information to be true. I doubt that signed_metadata would change that assumption. IMO, in addition to enabling hosting the metadata by a 3rd party service, signed_metadata protects against certain attacks where an attacker might replace the JWT with their own JWT (signed by the attacker's key) without the knowledge of the issuer. I don't think signed_metadata should suggest that the information in the signed_metadata is more accredited than the information in the unsigned metadata.

paulbastian commented 7 months ago

This is why I said the motivation of signed_metadata remains unclear to me. You can use it for different purposes and the fact that you had different ideas is a bad signal to me.

Don't get me wrong, I think that signed_metadata may be useful, thats why I opened this issue, but how exactly it is going to be used and what for is not sufficiently clear.