w3c / vc-data-integrity

W3C Data Integrity Specification
https://w3c.github.io/vc-data-integrity/
Other
42 stars 19 forks source link

Cleanly separate cryptosuite verification and purpose verification algorithms #66

Closed msporny closed 1 year ago

msporny commented 1 year ago

@dlongley wrote:

I think we need a cleaner separation of cryptosuite verification and purpose verification. We should:

  1. Find a proof that matches the expected purpose; if none is found, raise an error.
  2. Verify the proof using the cryptosuite. This returns true or false or throws if the operation cannot be executed.
  3. If cryptosuite verification failed, return that the proof is not verified, otherwise...
  4. Verify the proof against the expected purpose's specific checks (checking 'domain' and 'challenge' and so on are run here for authentication proofs, allowing for other purposes to be slotted in, i.e., "authentication" isn't the only known proof purpose).
  5. Finally, return whether the proof is verified or not.

_Originally posted by @dlongley in https://github.com/w3c/vc-data-integrity/pull/64#discussion_r997527870_

TallTed commented 1 year ago
  1. If cryptosuite verification failed, return that the proof is not verified, otherwise...

For clarity in working through this, I think that should be reworded to

  1. If cryptosuite verification does not return true (in other words, if cryptosuite verification returns false or throws an error), return that the proof is not verified, otherwise...

At this point, there will be some debate about the return (i.e., just "not verified", or some detail like "not verified due to error xyz"). I think the level of returned detail should be left to the implementer and/or deployer (better the latter), and that any errors thrown should be recorded/returned to the server-admin, whether via logs, configurable messaging, or otherwise.

mprorock commented 1 year ago

should we consider using RFC 7807 for error responses?

msporny commented 1 year ago

should we consider using RFC 7807 for error responses?

It feels like it would be a good idea to consider RFC7807 for use in VC API, and should figure out a way to map what's happening in the low-level libraries that implement Data Integrity to something that can be exposed through that mechanism.

The thing I'm concerned about are things like C/C++ libraries that only have integer return values -- those implementers have traditionally railed against not being able to pick their own error codes (as integers).

msporny commented 1 year ago

@dlongley The specification now contains a section on verification method retrieval and validation here:

https://www.w3.org/TR/vc-data-integrity/#retrieve-verification-method

It also has a section on proof purpose validation here:

https://www.w3.org/TR/vc-data-integrity/#verify-proof

Are these thing enough to address your initial issue raised?

dlongley commented 1 year ago

I think the only thing missing is a bit more text around options.expectedProofPurpose. The proof verification algorithm mentions this option and requires that it be checked (and an error thrown if there is no match), but we say nothing about what the option is -- leaving it to the name of the option to provide any explanation. It might just be missing an entry in Terminology.

At this point, I do think we can let implementations decide however they want to structure their APIs to find proofs with matching purposes, but we should say a little more about the option itself -- and perhaps mention that implementations may look for proofs with matching expected purposes before even attempting verification as a shortcut.

msporny commented 1 year ago

PR #134 has been raised to address this issue. This issue will be closed once that PR is merged.

msporny commented 1 year ago

PR #134 has been merged, closing.