siwe
siwe copied to clipboard
spruceid
Add support for ERC-6492
Thank you so much for this proposed PR and checking in on it. Apologies on the delay in response.
This PR makes significant changes to the security model, namely relying the following library to perform signature validation instead of ethers, which seems to be a wrapper around ethers which would require another security evaluation:
https://github.com/AmbireTech/signature-validator
Though the library does mention that a formal audit is pending, one is not available yet, and though it suggests that "~80 lines of code in index.js" are readily reviewed easily, it is clear that it creates a dependency on the smart contract expressed here for evaluating the validity across three different signing mechanisms.
There was a formal security audit of this library (siwe), and this change is likely to warrant another audit requirement. So at this point, it doesn't seem like a good idea to move forward on these changes because:
- It switches the critical path of the security model to an unaudited smart contract without a track record of use nor production testing.
- The spec doesn't mention EIP-6492 and this specific repository aims to follow the spec without surprises to implementers.
However, I think there are many ways forward to support EIP-6492 along with SIWE, such as evaluating a SIWE message signature against EIP-6492, or even accepting an EOA signature with additional authorizations. You can see how Gnosis Safe support was implemented in SSX as an extension here, on top of SIWE. In general this approach may be better in the short term, but if you feel strongly about inclusion of EIP-6492 then the best path may be to consider a making a case to add it to EIP-4361 to add that as an option alongside EIP-1271, and if those changes are accepted then we could update this repository to follow the spec as not to surprise implementors.
Hope this was helpful, and thanks again for your interest and patience in awaiting a response. I will leave this open for discussion, but close this in 2 weeks if there is no response. Looking forward to identifying ways to collaborate and move SIWE forward while maintaining security and stability for people who use it.
hey @wyc @derekchiang, since this was initially proposed, ERC-6492 has been made final and stable, and it has undergone multiple reviews. There's no formal audit yet, but it is something we're working on.
A few clarifications:
- 6492 is not a replacement for 1271, it's a superset of it and layer on top of it. It depends on 1271
- 6492 can broadly be divided into two components
- the wrap specification: in case a contract is not deployed, the standard defines a wrap specification which wraps the signature in a format that starts with recognizable magic bytes and packs deployment data in it, such as to allow verification
- the helper contract which allows verification of 6492 (wrapped 1271 signatures), 1271 (pure unwrapped 1271 signatures) and ecrecover signatures - this is meant to make implementation easier but it's not mandatory to use this contract
The helper contract has another "clever" feature, which is that you can use it with eth_call to verify signatures off-chain.
If you already implement 1271 and EOA signatures, adding the support for the wrapping format could be easy. But you'll still need a helper contract to simulate deploying the account for the purposes of the verification, so you might as well rewrite or just use the contract provided with 6492 which also has branches for 1271 and EOA.
A very easy way to minimize both risk and effort on your end is to simply detect the magic bytes and only use the originally provided contract (in 6492) only if the magic bytes are detected as a prefix to the signature, otherwise use your usual branches for pure 1271 and EOA. Once again, given the simplicity of the contract that shouldn't be needed and you can actually drop a lot of complexity from your codebase by going "full 6492".
