-
Notifications
You must be signed in to change notification settings - Fork 78
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
Verification method using EIP-712 #107
Comments
cc @awoie |
Yes! |
@clehner when you go down this route, please ensure you register the LD-Proof in the LD-Proof Registry at https://w3c-ccg.github.io/ld-cryptosuite-registry/ and write a LD-Proof spec for that. You would also need to update the In general, my comment would be how do you ensure that did:ethr didn't rotate away from the ETH-key in MM before the VC was issued? I guess, you don't need to but the VC would not be valid. For the same reason, onchain verification is an issue, because for onchain verification you would need to do onchain DID resolution to ensure your DID Doc still contains the ETH key. Then, the question is what would be the benefit of having this type of proof if onchain verification cannot be done? If all you want to do is using MM or ETH-wallets for VC issuance and VP presentation, then you could use custom RPC messages for this. Further, why is it |
yes, on-chain verification would not work for all situations, but it is still fully possible when using delegates. Those are visible on-chain, and they appear in the off-chain did-document as Note that I'm not advocating for general on-chain verification. This has some nasty privacy implications, but using a read-only on-chain method would give you the benefits without the privacy leak. |
Thanks for the feedback, @awoie, @mirceanis. I'm glad the on-chain idea is useful, since that helps constrain the design.
The verifier should check that the signing key was valid for the DID at the time of issuance. This could be done by resolving the DID document with DID Parameter
I assume you mean using existing proof types such as With a linked data proof type / verification method based on EIP-712, we should be able to ask the user to sign something where they can see what it is and what it means, ideally.
A linked data proof separately hashes and canonicalizes the document (VC or VP) and proof options. We will have to override this behavior to instead pass the document and proof options to the EIP-712 |
Isn't that what https://eips.ethereum.org/EIPS/eip-1812 aims to achieve? |
@jaredweinfurtner I believe the proposed method in EIP-1812 does not leverage Linked Data Proofs nor JWT, as per:
In contrast, I don't believe that this verification method is intended for on-chain use cases as a primary goal, but instead uses LD-Proofs. cc @pelle in case there is anything more to add here! |
Hi folks, I've implemented a working version of this verification method, in Rust as part of ssi/DIDKit: spruceid/ssi#99. It is integrated into a demo application: spruceid/degen-issuer#7. ChangesSince the original proposal, it is now more linked-data-based rather than JSON-based. There are two main reasons for this.
EncodingA linked data proof signing request is encoded for EIP-712 as a struct, This encoding aims to be similar to the usual way signing input for a linked data proof is created, while enabling the user to view and inspect the data they would sign. Encoding with 2D string arrays is supposed to help with readability. More descriptive encoding could be used, such as having each RDF statement be Until recently (MetaMask/metamask-extension#10485), strings in MetaMask's signing request window would overflow with an ellipsis rather than wrapping, as you can see in the screenshot at the top of this thread compared to the ones below. Now that that has been fixed, it might make sense to changing the encoding of the RDF statements to use a single string per statement (N-Quads line), rather than an array of strings per line, so that it would be closer to standard N-Quads and require less custom encoding. But it probably should still be to be one string per line rather than a single multi-line string, since I don't think the UI would respect embedded newlines. Lastly, note that there appears to be an inconsistency between eth-sig-util as used from MetaMask, and the EIP-712 specification, in hashing arrays: MetaMask/eth-sig-util#106. The encoding I am proposing is affected by this, since it relies on arrays in the EIP-712 data. The implementation in DIDKit now follows EIP-712 typed data for a linked data proof signing request{
"types": {
"EIP712Domain": [
{
"type": "string",
"name": "name"
}
],
"LDPSigningRequest": [
{
"type": "string[][]",
"name": "document"
},
{
"type": "string[][]",
"name": "proof"
}
]
},
"primaryType": "LDPSigningRequest",
"domain": {
"name": "Eip712Method2021"
},
"message": {
"document": [
...
],
"proof": [
...
]
}
} ExampleHere is the proposed EIP-712 encoding for a signing request for an example verifiable credential: The signing request in MetaMask looks like this: The resulting verifiable credential: On-chain considerationsAs @wyc noted, our use cases for this verification method are more for Ethereum wallet users to be able to use verifiable credentials and presentations, and I didn't have on-chain use cases in mind when opening this issue. But I think it would be great to support on-chain use cases to the extent possible. I think it would be difficult to implement linked data proofs on-chain in general, because of the computation needed to canonicalize, encode and hash the document and proof data, but I think it would be possible to do within some constraints. I also don't personally have experience in developing smart contracts, so my view here is limited. Minimize blank nodesOne way to reduce the cost of linked data proofs could be to limit use of blank nodes. Canonicalizing a RDF dataset using URDNA2015 involves assigning canonical names to blank node identifiers in the dataset (e.g. Optimize sortingURDNA2015 requires sorting RDF statements lexicographically. Deserializing JSON-LD to RDF also involves encoding data in a canonical lexical form which for JSON literals involves sorting keys lexicographically. There could be optimizations regarding this sorting that could help make on-chain issuance and/or verification more feasible. Issuance could work by constructing signing input with a template, rather than in a fully general way, while taking care that the input is in the canonical form. It could help with verification to require input to be pre-sorted and pre-canonicalized. Consider disallowing data types that are expensive to check if they are in canonical form, such as perhaps HashingThis thread suggests the cost of SHA-256 may be about 700-1300 gas: https://ethereum.stackexchange.com/questions/76110/gas-cost-of-a-sha256-hash. I think this hashing would only be needed for blank node canonicalization in URDNA2015. String processingEncoding as N-Quads or N-Quads-like strings requires some string processing, which might not be otherwise needed if terms were instead encoded as native EIP-712 structs. But fully using EIP-712 structs might be more inconvenient in the UI, and require more custom processing compared to N-Quads. Is this an acceptable trade-off? EIP-1812I didn't know about this, so thanks @jaredweinfurtner for pointing it out. It looks great for Ethereum-based uses. But I think it would be valuable to integrate with the W3C VC Data Model, to potentially interoperate with many issuers, holders and verifiers. A linked data proof type based on EIP-712 could enable existing wallet users to make use of these standards while remaining authoritative over what they are signing with their keys. |
@clehner The problem with this approach is that the EIP712 input that gets displayed in MM (output from RDF canonicalization) is essentially a blob of data to the user and basically not better than
For verification you would need to build the You could store the You could use either an Update: Some improvement might be that you would even able to encode the EIP712 types in the JSON schema, or use "A" EIP712 schema (if this exists) directly in the |
Also tagging @wyc |
I would also suggest naming the proof type |
The data as RDF/N-Quads should be able to be copy-pasted into other applications for further rendering or analysis before signing. Better rendering of signing input will probably require support from the wallet. Or an additional extension to make the wallet render better, e.g. meta-metamask ;) I attempt to describe the possible approaches for further discussion here: |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I'm closing this issue, as the idea has been implemented, and a specification is being drafted as a work item of the W3C Credentials Community Group: w3c-ccg/community#194.
We also still have the suite using arrays of RDF statements ( We've also been exploring using While working out the usability implications of signing linked data, we are exploring use of "signing input explainer" tools, to help users make sense of what they are being asked to sign, e.g. https://github.com/spruceid/tzvm2021-explainer
|
EIP-712 is a way for an Ethereum identity (secp256k1 keypair) to sign structured data. This is supposed to be good for security and user autonomy since rather than asking the user to sign arbitrary bytes, instead the user is prompted with some structured data where it can be more obvious what the meaning of it is. To use EIP-712 for linked data proofs on verifiable credentials and presentations, existing proof types such EcdsaSecp256k1Signature2019 cannot be used, because they rely on the user being able to sign arbitrary bytes; instead, a new verification method / proof type would be needed. Would this be a potentially useful addition to
did:ethr
DID Documents?Here is a screenshot of MetaMask showing a EIP-712 signing request of a hypothetical verifiable credential (not actually working yet, this is just to show what it could look like):
Source: demo.html.txt
More info about EIP-712
The text was updated successfully, but these errors were encountered: