Security best practices for verifying AnonCreds W3C VPs

[TimoGlastra]

It might be good to set up some best practices for verifying AnonCreds W3C VPs, especially in relation to usage with for example PEX.

Here's some security things I've ran into that maybe allow for some unepxected behaviour:

• AnonCreds W3C VCs and VPs only allow the credentialSubject to be integrity integrity protected. If you for example add a field hello on the top-level of the w3c credential, AnonCreds will just strip it (as it only looks at the credential Subject). This is different from other W3C Signature Suite implementation I've seen, which always throw an error if there's a field that's not integrity protected. If you now take the JSON of a W3C cred, pass it to AnonCreds RS to verify and it's OK 👍 , you think you can trust the W3C JSON, but rather you should then get the W3C credential from AnonCreds RS as that will have the non-integrity-protected values stripped. I think AnonCreds RS should default to throwing an error if a value is not integrity protected. This means that after creating the presentation I can include some properties top-level that may be needed for the PEX submission. In our case the PEX library checks if the presentation / credential has the right attributes and values, but it's up to the credential/presentation verification logic (in this case AnonCresd) to make sure the values are actually integrity protected.
• I was able to change the issuer on a VC in a VP and the verification went succesfull. I think a check may need to be added to make sure the issuer claim in the W3C cred matches with the issuer_id in the credential definition id
• I was able to change the verificationMethod on a VC in VP and the verification went successfull. Also here a check would be useful as you cannot trust this value at all.

I think some concrete things we can do to make bypassing validation harder is:

• only allow integrity protected fields to be present on AnonCreds VCs and VPs. If the AnonCreds RS notices any field that is not integrity protected it should throw an error (like a normal JWT verification would also fail if you add a field to the JWT payload)
• Verify some parameters not native in AnonCreds VC, but present in W3c AnonCreds VCs (like issuer and proof.verificationMethod)

I think especially the verificationMethod on the proof is what we currently used to fetch the credential definitions before passing it to the verify method, however it could be that the actual credential is using different credential definitions. (a smart constructed request could exploit this)

There's probably more things to consider but these are few I encountered when tinkering with AnonCreds RS

[TimoGlastra]

I think we should address these in the AnonCreds RS lib. We could also address these in Credo / ACA-Py but it'd mean duplication of work, and also that other frameworks need to be very careful in adopting AnonCreds W3C

[swcurran]

@andrewwhitehead is working on this now. Let’s hold off on 0.2.0 for this. Reasonable?