search

spruceid / ssi

Core library for decentralized identity.
https://spruceid.dev
Apache License 2.0
193 stars 61 forks source link

Some clarification of Ed25519Signature2018 relative to Linked Data Proofs doc #358

Open vdods opened 2 years ago

vdods commented 2 years ago

I'm attempting but failing to verify some VCs that use Ed25519Signature2018, and found some confusing notes in the linked data proofs doc, step 5. It calls out Ed25519Proof2018 algorithm specifically: "The Ed25519Proof2018 algorithm also does not perform this last step", presumably referring to the construction of concat(sha256(signing_opts_canonical), sha256(doc_canonical)). The ssi implementation does do this sha256-using concatenation, and I'm just wondering if the discrepancy relative to the LDP doc is known.

clehner commented 2 years ago

I think the Ed25519Signature2018 implementation here is in alignment with ld-proofs. This implementation can verify test vectors (i.e. VCs/VPs generated by other implementations) in VC API Test Suite, as tested in DIDKit.

I also find that note confusing. Maybe Ed25519Proof2018 refers to something else. It reads to me like the "last step" is an additional hashing step that is no longer there. Maybe it used to say to hash output again, and then was pointing out that that was not done since JWS algorithms RS256 and EdDSA already perform hashing. I don't think it is saying that the 64 byte concatenation of hashes should be constructed differently.

Can you share what VCs you are trying to verify, and/or what implementations generated them, in case we may be able to diagnose this verification?

vdods commented 2 years ago

Yeah, that would be helpful. Might I email you the VC? It's just for a test network, but I'd rather not post it publicly on github. I see an email address for you under your github account. I appreciate the help!

clehner commented 2 years ago

@vdods sure

clehner commented 2 years ago

Received VC over email. Its credentialStatus object contains an id property which is not a valid URI/IRI. This is probably contributing to the verification failure. ssi/json-ld drops (ignores) the credentialStatus object during canonicalization/hashing due to the invalid id. We think this is correct behavior according to JSON-LD Processing, but not ideal obviously - In the future this implementation may produce a warning or error in this situation. The issuer may be able to fix this by percent-encoding special characters in the credentialStatus id.

cc @bumblefudge

clehner commented 2 years ago

@vdods do you think Ed25519Signature2018 proofs are clear now, or we do need more changes?