Refactor WebAuthn Implementation (#320)

This PR refactors the WebAuthn implementation into a library instead of split into multiple contracts. This removes the amount of `CALL`s required for verifying a signature. I did some analysis on the deployment vs. signature verification costs comparing this code, `main` and this change incorporating optimizations from #289 and found this: | setup | deployment | verification (dummy) | break even | | ------------------------------------ | ---------- | -------------------- | ---------- | | main (viaIR = false) | 373543 | 17789 | 0 | | main (viaIR = true) | 393494 | 16443 | 15 | | this branch (viaIR = false) | 612123 | 13835 | 61 | | this branch (viaIR = true) | 594058 | 12551 | 43 | | base64 optimizations (viaIR = false) | 442545 | 12965 | 15 | | base64 optimizations (viaIR = true) | 414941 | 11102 | 7 | Meaning that after 7 signatures, even with the slightly larger deployment costs, we would break even by avoiding the extra `CALL` (noting that ABI-encoding the call to the `WebAuthnVerifier` is not particularly efficient in the first place).
safe-global · Mar 18, 2024 · 7468b32 · 7468b32
1 parent 4ad1f0c
commit 7468b32
Show file tree

Hide file tree

Showing 17 changed files with 289 additions and 429 deletions.
diff --git a/...acts/4337/Safe256BitECSignerLaunchpad.sol → ...ntracts/4337/SafeECDSASignerLaunchpad.sol b/...acts/4337/Safe256BitECSignerLaunchpad.sol → ...ntracts/4337/SafeECDSASignerLaunchpad.sol
@@ -6,15 +6,17 @@ import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/Pac
 import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol";
 import {SafeStorage} from "@safe-global/safe-contracts/contracts/libraries/SafeStorage.sol";
 
-import {ICustom256BitECSignerFactory} from "../interfaces/ICustomSignerFactory.sol";
+import {ICustomECDSASignerFactory} from "../interfaces/ICustomECDSASignerFactory.sol";
 import {ISafe} from "../interfaces/ISafe.sol";
 import {ERC1271} from "../libraries/ERC1271.sol";
 
 /**
- * @title SafeOpLaunchpad - A contract for Safe initialization with custom unique signers that would violate ERC-4337 factory rules.
- * @dev The is intended to be set as a Safe proxy's implementation for ERC-4337 user operation that deploys the account.
+ * @title Safe Launchpad for Custom ECDSA Signing Schemes.
+ * @dev A launchpad account implementation that enables the creation of Safes that use custom ECDSA
+ * signing schemes that require additional contract deployments over ERC-4337.
+ * @custom:security-contact [email protected]
  */
-contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage {
+contract SafeECDSASignerLaunchpad is IAccount, SafeStorage {
     bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)");
 
     // keccak256("SafeSignerLaunchpad.initHash") - 1
@@ -189,13 +191,7 @@ contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage {
         bytes memory operationData = _getOperationData(userOpHash, validAfter, validUntil);
         bytes32 operationHash = keccak256(operationData);
         try
-            ICustom256BitECSignerFactory(signerFactory).isValidSignatureForSigner(
-                signerX,
-                signerY,
-                signerVerifier,
-                operationHash,
-                signature
-            )
+            ICustomECDSASignerFactory(signerFactory).isValidSignatureForSigner(operationHash, signature, signerX, signerY, signerVerifier)
         returns (bytes4 magicValue) {
             // The timestamps are validated by the entry point, therefore we will not check them again
             validationData = _packValidationData(magicValue != ERC1271.MAGIC_VALUE, validUntil, validAfter);
@@ -218,7 +214,7 @@ contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage {
         SafeStorage.singleton = singleton;
         {
             address[] memory owners = new address[](1);
-            owners[0] = ICustom256BitECSignerFactory(signerFactory).createSigner(signerX, signerY, signerVerifier);
+            owners[0] = ICustomECDSASignerFactory(signerFactory).createSigner(signerX, signerY, signerVerifier);
 
             ISafe(address(this)).setup(owners, 1, setupTo, setupData, fallbackHandler, address(0), 0, payable(address(0)));
         }

diff --git a/modules/passkey/contracts/WebAuthnSigner.sol b/modules/passkey/contracts/WebAuthnSigner.sol
@@ -2,9 +2,8 @@
 pragma solidity >=0.8.0;
 
 import {SignatureValidator} from "./base/SignatureValidator.sol";
-import {IWebAuthnVerifier} from "./interfaces/IWebAuthnVerifier.sol";
-import {WebAuthnFlags} from "./libraries/WebAuthnFlags.sol";
-import {WebAuthnSignature} from "./libraries/WebAuthnSignature.sol";
+import {IP256Verifier} from "./interfaces/IP256Verifier.sol";
+import {WebAuthn} from "./libraries/WebAuthn.sol";
 
 /**
  * @title WebAuthn Safe Signature Validator
@@ -14,36 +13,25 @@ import {WebAuthnSignature} from "./libraries/WebAuthnSignature.sol";
 contract WebAuthnSigner is SignatureValidator {
     uint256 public immutable X;
     uint256 public immutable Y;
-    IWebAuthnVerifier public immutable WEBAUTHN_SIG_VERIFIER;
+    IP256Verifier public immutable VERIFIER;
 
     /**
      * @dev Constructor function.
-     * @param qx The X coordinate of the signer's public key.
-     * @param qy The Y coordinate of the signer's public key.
-     * @param webAuthnVerifier The address of the P256Verifier contract.
+     * @param x The X coordinate of the signer's public key.
+     * @param y The Y coordinate of the signer's public key.
+     * @param verifier The P-256 verifier to use for signature validation. It MUST implement the
+     * same interface as the EIP-7212 precompile.
      */
-    constructor(uint256 qx, uint256 qy, address webAuthnVerifier) {
-        X = qx;
-        Y = qy;
-        WEBAUTHN_SIG_VERIFIER = IWebAuthnVerifier(webAuthnVerifier);
+    constructor(uint256 x, uint256 y, address verifier) {
+        X = x;
+        Y = y;
+        VERIFIER = IP256Verifier(verifier);
     }
 
     /**
      * @inheritdoc SignatureValidator
      */
-    function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual override returns (bool isValid) {
-        WebAuthnSignature.Data calldata data = WebAuthnSignature.cast(signature);
-
-        return
-            WEBAUTHN_SIG_VERIFIER.verifyWebAuthnSignatureAllowMalleability(
-                data.authenticatorData,
-                WebAuthnFlags.USER_VERIFICATION,
-                message,
-                data.clientDataFields,
-                data.r,
-                data.s,
-                X,
-                Y
-            );
+    function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual override returns (bool success) {
+        success = WebAuthn.verifySignature(message, signature, WebAuthn.USER_VERIFICATION, X, Y, VERIFIER);
     }
 }
diff --git a/modules/passkey/contracts/WebAuthnSignerFactory.sol b/modules/passkey/contracts/WebAuthnSignerFactory.sol
@@ -1,61 +1,48 @@
 // SPDX-License-Identifier: LGPL-3.0-only
 pragma solidity >=0.8.0;
 
-import {ICustom256BitECSignerFactory} from "./interfaces/ICustomSignerFactory.sol";
-import {IWebAuthnVerifier} from "./interfaces/IWebAuthnVerifier.sol";
+import {ICustomECDSASignerFactory} from "./interfaces/ICustomECDSASignerFactory.sol";
+import {IP256Verifier} from "./interfaces/IP256Verifier.sol";
 import {ERC1271} from "./libraries/ERC1271.sol";
-import {WebAuthnFlags} from "./libraries/WebAuthnFlags.sol";
-import {WebAuthnSignature} from "./libraries/WebAuthnSignature.sol";
+import {WebAuthn} from "./libraries/WebAuthn.sol";
 import {WebAuthnSigner} from "./WebAuthnSigner.sol";
 
 /**
  * @title WebAuthnSignerFactory
  * @dev A factory contract for creating and managing WebAuthn signers.
  */
-contract WebAuthnSignerFactory is ICustom256BitECSignerFactory {
-    // @inheritdoc ICustom256BitECSignerFactory
-    function getSigner(uint256 qx, uint256 qy, address verifier) public view override returns (address signer) {
-        bytes32 codeHash = keccak256(abi.encodePacked(type(WebAuthnSigner).creationCode, qx, qy, uint256(uint160(verifier))));
+contract WebAuthnSignerFactory is ICustomECDSASignerFactory {
+    /**
+     * @inheritdoc ICustomECDSASignerFactory
+     */
+    function getSigner(uint256 x, uint256 y, address verifier) public view override returns (address signer) {
+        bytes32 codeHash = keccak256(abi.encodePacked(type(WebAuthnSigner).creationCode, x, y, uint256(uint160(verifier))));
         signer = address(uint160(uint256(keccak256(abi.encodePacked(hex"ff", address(this), bytes32(0), codeHash)))));
     }
 
-    // @inheritdoc ICustom256BitECSignerFactory
-    function createSigner(uint256 qx, uint256 qy, address verifier) external returns (address signer) {
-        signer = getSigner(qx, qy, verifier);
+    /**
+     * @inheritdoc ICustomECDSASignerFactory
+     */
+    function createSigner(uint256 x, uint256 y, address verifier) external returns (address signer) {
+        signer = getSigner(x, y, verifier);
 
         if (_hasNoCode(signer) && _validVerifier(verifier)) {
-            WebAuthnSigner created = new WebAuthnSigner{salt: bytes32(0)}(qx, qy, verifier);
+            WebAuthnSigner created = new WebAuthnSigner{salt: bytes32(0)}(x, y, verifier);
             require(address(created) == signer);
         }
     }
 
-    // @inheritdoc ICustom256BitECSignerFactory
+    /**
+     * @inheritdoc ICustomECDSASignerFactory
+     */
     function isValidSignatureForSigner(
-        uint256 qx,
-        uint256 qy,
-        address verifier,
         bytes32 message,
-        bytes calldata signature
+        bytes calldata signature,
+        uint256 x,
+        uint256 y,
+        address verifier
     ) external view override returns (bytes4 magicValue) {
-        WebAuthnSignature.Data calldata data = WebAuthnSignature.cast(signature);
-
-        // Work around stack-too-deep issues by helping out the compiler figure out how to re-order
-        // the stack.
-        uint256 x = qx;
-        uint256 y = qy;
-
-        if (
-            IWebAuthnVerifier(verifier).verifyWebAuthnSignatureAllowMalleability(
-                data.authenticatorData,
-                WebAuthnFlags.USER_VERIFICATION,
-                message,
-                data.clientDataFields,
-                data.r,
-                data.s,
-                x,
-                y
-            )
-        ) {
+        if (WebAuthn.verifySignature(message, signature, WebAuthn.USER_VERIFICATION, x, y, IP256Verifier(verifier))) {
             magicValue = ERC1271.MAGIC_VALUE;
         }
     }

diff --git a/modules/passkey/contracts/base/SignatureValidator.sol b/modules/passkey/contracts/base/SignatureValidator.sol
@@ -37,7 +37,7 @@ abstract contract SignatureValidator {
      * @dev Verifies a signature.
      * @param message The signed message.
      * @param signature The signature to be validated.
-     * @return isValid Whether or not the signature is valid.
+     * @return success Whether the signature is valid.
      */
-    function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual returns (bool isValid);
+    function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual returns (bool success);
 }
diff --git a/modules/passkey/contracts/interfaces/ICustomECDSASignerFactory.sol b/modules/passkey/contracts/interfaces/ICustomECDSASignerFactory.sol
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.8.0 <0.9.0;
+
+/**
+ * @title Custom ECDSA Signer Factory
+ * @dev Interface for a factory contract that can create ERC-1271 compatible signers, and verify
+ * signatures for custom ECDSA schemes.
+ * @custom:security-contact [email protected]
+ */
+interface ICustomECDSASignerFactory {
+    /**
+     * @notice Gets the unique signer address for the specified data.
+     * @dev The unique signer address must be unique for some given data. The signer is not
+     * guaranteed to be created yet.
+     * @param x The x-coordinate of the public key.
+     * @param y The y-coordinate of the public key.
+     * @param verifier The address of the verifier.
+     * @return signer The signer address.
+     */
+    function getSigner(uint256 x, uint256 y, address verifier) external view returns (address signer);
+
+    /**
+     * @notice Create a new unique signer for the specified data.
+     * @dev The unique signer address must be unique for some given data. This must not revert if
+     * the unique owner already exists.
+     * @param x The x-coordinate of the public key.
+     * @param y The y-coordinate of the public key.
+     * @param verifier The address of the verifier.
+     * @return signer The signer address.
+     */
+    function createSigner(uint256 x, uint256 y, address verifier) external returns (address signer);
+
+    /**
+     * @notice Verifies a signature for the specified address without deploying it.
+     * @dev This must be equivalent to first deploying the signer with the factory, and then
+     * verifying the signature with it directly:
+     * `factory.createSigner(signerData).isValidSignature(message, signature)`
+     * @param message The signed message.
+     * @param signature The signature bytes.
+     * @param x The x-coordinate of the public key.
+     * @param y The y-coordinate of the public key.
+     * @param verifier The address of the verifier.
+     * @return magicValue Returns the ERC-1271 magic value when the signature is valid. Reverting or
+     * returning any other value implies an invalid signature.
+     */
+    function isValidSignatureForSigner(
+        bytes32 message,
+        bytes calldata signature,
+        uint256 x,
+        uint256 y,
+        address verifier
+    ) external view returns (bytes4 magicValue);
+}
diff --git a/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol b/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol
diff --git a/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol b/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol