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

Verifying credentials in presentations #111

Closed
clehner opened this issue Feb 11, 2021 · 7 comments
Closed

Verifying credentials in presentations #111

clehner opened this issue Feb 11, 2021 · 7 comments

Comments

@clehner
Copy link
Member

clehner commented Feb 11, 2021

Should verifyPresentation verify VCs in a presentation, as well as the presentation itself (unless ProoflessVerifyPresentationRequest is used as proposed in #95)? I had assumed that verifying a VP means verifying the presentation proof(s) against the presentation's holder property or the verificationMethod property provided in VerifyOptions (formerly LinkedDataProofOptions), and that a client must separately verify the embedded credentials. But #95 suggests to me that it might be otherwise. If it is the case that verifyPresentation must verify embedded credentials, what VerifyOptions are used for verifying the credentials? I would assume an empty set, since it probably doesn't make sense to pass on options like challenge; this would be consistent with #95 which does not have VerifyOptions in the proposed ProoflessVerifyPresentationRequest.

@mprorock
Copy link
Contributor

That is my understanding, effectively "defaults" get used, where they apply, as noted in the update to VerifyOptions that calls out what happens if an item is not passed. I personally feel that this is an area that can be made more robust over time with a future cleanup on the api spec

@David-Chadwick
Copy link
Contributor

I would expect verifyPresentation to verify the embedded VCs by default, and maybe have an option to say "only the VP" if this is really wanted. But it is inefficient to have to call verifyPresentation and then verifyCredential n times.

@David-Chadwick
Copy link
Contributor

wrt Challenge I would expect verifyPresentation to return the embedded challenge in the response, again by default. I cannot imagine why you would want the challenge to be thrown away.

@clehner
Copy link
Member Author

clehner commented Feb 12, 2021

Thanks for the clarification, @mprorock, @David-Chadwick.

By not passing on the challenge option, I just meant that verifyPresentation would use the challenge property of the VerifyOptions for verifying the presentation proof, but not for verifying the embedded credential proofs (the "defaults" as being used for verifying those, like @mprorock said).

@David-Chadwick I'm not sure what you mean about returning the embedded challenge in the response. The response is a VerifyPresentationResponse (VerificationResult) which currently just contains arrays of strings for "checks", "warnings" and "errors". Are you suggesting that VerifyPresentationResponse should include the proofs that were verified?

@David-Chadwick
Copy link
Contributor

@clehner. I think we are talking about different challenges. Sorry. In my mental model the RP asks the Holder for a VP and presents a challenge. The RP then asks the Verifier to verify the VP and wants this challenge to be returned in the response so that it can ensure the VP was freshly made in response to its original request.

@clehner
Copy link
Member Author

clehner commented Feb 12, 2021

With vc-http-api currently, the RP should pass the challenge to the vc-http-api server (Verifier) in the VerifyOptions for the verifyPresentation call. The server then checks that the provided challenge equals the challenge in the VP proof. So the challenge doesn't need to be returned and the client shouldn't need to check it, because the server already has.

I'm closing this issue as I think my questions are answered, but it could be reopened if there should be text added about this in the API documentation.

@clehner clehner closed this as completed Feb 12, 2021
@David-Chadwick
Copy link
Contributor

Thanks. That answers my question perfectly

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

3 participants