diff --git a/examples/4337-passkeys/src/logic/safe.ts b/examples/4337-passkeys/src/logic/safe.ts index 98cbb56a..c65b1927 100644 --- a/examples/4337-passkeys/src/logic/safe.ts +++ b/examples/4337-passkeys/src/logic/safe.ts @@ -61,7 +61,7 @@ function getInitHash(safeInitializer: SafeInitializer, chainId: ethers.BigNumber { type: 'address', name: 'signerFactory' }, { type: 'uint256', name: 'signerX' }, { type: 'uint256', name: 'signerY' }, - { type: 'uint192', name: 'signerVerifiers' }, + { type: 'uint176', name: 'signerVerifiers' }, { type: 'address', name: 'setupTo' }, { type: 'bytes', name: 'setupData' }, { type: 'address', name: 'fallbackHandler' }, diff --git a/modules/passkey/README.md b/modules/passkey/README.md index 722fcfe7..6fa1bd71 100644 --- a/modules/passkey/README.md +++ b/modules/passkey/README.md @@ -39,7 +39,7 @@ bytes memory signature = abi.encode(authenticatorData, clientDataFields, r, s); ### [P256](./contracts/libraries/P256.sol) -`P256` is a library for P256 signature verification with contracts that follows the EIP-7212 EC verify precompile interface. This library defines a custom type `Verifiers`, which encodes two addresses into a single `uint192`. The first address (4 bytes) is a precompile address dedicated to verification, and the second (20 bytes) is a fallback address. This setup allows the library to support networks where the precompile is not yet available, seamlessly transitioning to the precompile when it becomes active, while relying on a fallback contract address in the meantime. +`P256` is a library for P256 signature verification with contracts that follows the EIP-7212 EC verify precompile interface. This library defines a custom type `Verifiers`, which encodes two addresses into a single `uint176`. The first address (2 bytes) is a precompile address dedicated to verification, and the second (20 bytes) is a fallback address. This setup allows the library to support networks where the precompile is not yet available, seamlessly transitioning to the precompile when it becomes active, while relying on a fallback contract address in the meantime. ## Setup and Execution flow diff --git a/modules/passkey/contracts/4337/SafeSignerLaunchpad.sol b/modules/passkey/contracts/4337/SafeSignerLaunchpad.sol index b64beab2..0db15d28 100644 --- a/modules/passkey/contracts/4337/SafeSignerLaunchpad.sol +++ b/modules/passkey/contracts/4337/SafeSignerLaunchpad.sol @@ -41,13 +41,13 @@ contract SafeSignerLaunchpad is IAccount, SafeStorage { * {address} signerFactory - The custom ECDSA signer factory to use for creating an owner. * {uint256} signerX - The X coordinate of the public key of the custom ECDSA signing scheme. * {uint256} signerY - The Y coordinate of the public key of the custom ECDSA signing scheme. - * {uint256} signerVerifiers - The P-256 verifiers to use for signature validation. + * {uint176} signerVerifiers - The P-256 verifiers to use for signature validation. * {address} setupTo - The contract to `DELEGATECALL` during setup. * {bytes} setupData - The calldata for the setup `DELEGATECALL`. * {address} fallbackHandler - The fallback handler to initialize the Safe with. - * @custom:computed-as keccak256("SafeInit(address singleton,address signerFactory,uint256 signerX,uint256 signerY,uint192 signerVerifiers,address setupTo,bytes setupData,address fallbackHandler)") + * @custom:computed-as keccak256("SafeInit(address singleton,address signerFactory,uint256 signerX,uint256 signerY,uint176 signerVerifiers,address setupTo,bytes setupData,address fallbackHandler)") */ - bytes32 private constant SAFE_INIT_TYPEHASH = 0xb8b5d6678d8c3ed815330874b6c0a30142f64104b7f6d1361d6775a7dbc5318b; + bytes32 private constant SAFE_INIT_TYPEHASH = 0xb847e47c00fe4c75163e39c8673522943ada9c9b5a783bc876c85566138a8583; /** * @notice The keccak256 hash of the EIP-712 SafeInitOp struct, representing the user operation to execute alongside initialization. diff --git a/modules/passkey/contracts/SafeWebAuthnSignerFactory.sol b/modules/passkey/contracts/SafeWebAuthnSignerFactory.sol index 4f2b38da..237b03e8 100644 --- a/modules/passkey/contracts/SafeWebAuthnSignerFactory.sol +++ b/modules/passkey/contracts/SafeWebAuthnSignerFactory.sol @@ -11,8 +11,18 @@ import {P256} from "./libraries/P256.sol"; * @dev A factory contract for creating and managing WebAuthn proxy signers. */ contract SafeWebAuthnSignerFactory is ISafeSignerFactory { + /** + * @notice The {SafeWebAuthnSignerSingleton} implementation to that is used for signature + * verification by this contract and any proxies it deploys. + */ SafeWebAuthnSignerSingleton public immutable SINGLETON; + /** + * @notice Creates a new WebAuthn Safe signer factory contract. + * @dev The {SafeWebAuthnSignerSingleton} singleton implementation is created with as part of + * this constructor. This ensures that the singleton contract is known, and lets us make certain + * assumptions about how it works. + */ constructor() { SINGLETON = new SafeWebAuthnSignerSingleton(); } @@ -67,7 +77,8 @@ contract SafeWebAuthnSignerFactory is ISafeSignerFactory { assembly { let dataSize := mload(data) let dataLocation := add(data, 0x20) - // staticcall to the singleton contract with return size given as 32 bytes. The singleton contract is known and immutable so, it is safe to specify return size. + // staticcall to the singleton contract with return size given as 32 bytes. The + // singleton contract is known and immutable so it is safe to specify return size. if staticcall(gas(), singleton, dataLocation, dataSize, 0, 32) { magicValue := mload(0) } diff --git a/modules/passkey/contracts/SafeWebAuthnSignerProxy.sol b/modules/passkey/contracts/SafeWebAuthnSignerProxy.sol index 023c4bb4..310a457f 100644 --- a/modules/passkey/contracts/SafeWebAuthnSignerProxy.sol +++ b/modules/passkey/contracts/SafeWebAuthnSignerProxy.sol @@ -1,37 +1,42 @@ // SPDX-License-Identifier: LGPL-3.0-only +/* solhint-disable no-complex-fallback */ pragma solidity >=0.8.0; + import {P256} from "./libraries/WebAuthn.sol"; /** - * @title WebAuthn Safe Signature Validator - * @dev A proxy contracy that points to Safe signature validator implementation for a WebAuthn P-256 credential. + * @title Safe WebAuthn Signer Proxy + * @dev A proxy to a {SafeWebAuthnSignerSingleton} signature validator implementation for Safe + * accounts. Using a proxy pattern for the signature validator greatly reduces deployment gas costs. * @custom:security-contact bounty@safe.global */ contract SafeWebAuthnSignerProxy { + /** + * @notice The {SafeWebAuthnSignerSingleton} implementation to proxy to. + */ + address internal immutable _SINGLETON; + /** * @notice The x coordinate of the P-256 public key of the WebAuthn credential. */ uint256 internal immutable _X; + /** * @notice The y coordinate of the P-256 public key of the WebAuthn credential. */ uint256 internal immutable _Y; - /** - * @notice The P-256 verifiers used for ECDSA signature validation. - */ - P256.Verifiers internal immutable _VERIFIERS; /** - * @notice The contract address to which proxy contract forwards the call via delegatecall. + * @notice The P-256 verifiers used for ECDSA signature verification. */ - address internal immutable _SINGLETON; + P256.Verifiers internal immutable _VERIFIERS; /** * @notice Creates a new WebAuthn Safe Signer Proxy. - * @param singleton Address of the singleton contract to which the proxy forwards the call via delegatecall. + * @param singleton The {SafeWebAuthnSignerSingleton} implementation to proxy to. * @param x The x coordinate of the P-256 public key of the WebAuthn credential. * @param y The y coordinate of the P-256 public key of the WebAuthn credential. - * @param verifiers The P-256 verifiers used for ECDSA signature validation. + * @param verifiers The P-256 verifiers used for ECDSA signature verification. */ constructor(address singleton, uint256 x, uint256 y, P256.Verifiers verifiers) { _SINGLETON = singleton; @@ -43,7 +48,6 @@ contract SafeWebAuthnSignerProxy { /** * @dev Fallback function forwards all transactions and returns all received return data. */ - // solhint-disable-next-line no-complex-fallback fallback() external payable { bytes memory data = abi.encodePacked(msg.data, _X, _Y, _VERIFIERS); address singleton = _SINGLETON; diff --git a/modules/passkey/contracts/SafeWebAuthnSignerSingleton.sol b/modules/passkey/contracts/SafeWebAuthnSignerSingleton.sol index 098af02c..b7caafc0 100644 --- a/modules/passkey/contracts/SafeWebAuthnSignerSingleton.sol +++ b/modules/passkey/contracts/SafeWebAuthnSignerSingleton.sol @@ -3,9 +3,12 @@ pragma solidity >=0.8.0; import {SignatureValidator} from "./base/SignatureValidator.sol"; import {P256, WebAuthn} from "./libraries/WebAuthn.sol"; + /** * @title WebAuthn Safe Signature Validator Singleton - * @dev A contract that represents a WebAuthn signer. + * @dev A singleton contract that implements WebAuthn signature verification. This sigleton contract + * must be used with the {SafeWebAuthnSignerProxy}, as it encodes the credential configuration + * (public key coordinates and P-256 verifier to use) that is required by this implementation. * @custom:security-contact bounty@safe.global */ contract SafeWebAuthnSignerSingleton is SignatureValidator { @@ -19,7 +22,9 @@ contract SafeWebAuthnSignerSingleton is SignatureValidator { } /** - * @notice Returns the x coordinate, y coordinate, and P-256 verifiers used for ECDSA signature validation. The values are expected to be passed by the SafeWebAuthnSignerProxy contract in msg.data. + * @notice Returns the x coordinate, y coordinate, and P-256 verifiers used for ECDSA signature + * validation. The values are expected to appended to calldata by the caller. See the + * {SafeWebAuthnSignerProxy} contract implementation. * @return x The x coordinate of the P-256 public key. * @return y The y coordinate of the P-256 public key. * @return verifiers The P-256 verifiers. @@ -27,9 +32,9 @@ contract SafeWebAuthnSignerSingleton is SignatureValidator { function getConfiguration() public pure returns (uint256 x, uint256 y, P256.Verifiers verifiers) { // solhint-disable-next-line no-inline-assembly assembly ("memory-safe") { - x := calldataload(sub(calldatasize(), 88)) - y := calldataload(sub(calldatasize(), 56)) - verifiers := shr(64, calldataload(sub(calldatasize(), 24))) + x := calldataload(sub(calldatasize(), 86)) + y := calldataload(sub(calldatasize(), 54)) + verifiers := shr(80, calldataload(sub(calldatasize(), 22))) } } } diff --git a/modules/passkey/contracts/libraries/P256.sol b/modules/passkey/contracts/libraries/P256.sol index d5b4d766..0473e6c1 100644 --- a/modules/passkey/contracts/libraries/P256.sol +++ b/modules/passkey/contracts/libraries/P256.sol @@ -19,13 +19,13 @@ library P256 { /** * @notice P-256 precompile and fallback verifiers. - * @dev This is the packed `uint32(precompile) | uint160(fallback)` addresses to use for the + * @dev This is the packed `uint16(precompile) | uint160(fallback)` addresses to use for the * verifiers. This allows both a precompile and a fallback Solidity implementation of the P-256 * curve to be specified. For networks where the P-256 precompile is planned to be enabled but * not yet available, this allows for a verifier to seamlessly start using the precompile once * it becomes available. */ - type Verifiers is uint192; + type Verifiers is uint176; /** * @notice Verifies the signature of a message using the P256 elliptic curve with signature diff --git a/modules/passkey/test/4337/WebAuthn.spec.ts b/modules/passkey/test/4337/WebAuthn.spec.ts index ff3d4c9c..5aa10910 100644 --- a/modules/passkey/test/4337/WebAuthn.spec.ts +++ b/modules/passkey/test/4337/WebAuthn.spec.ts @@ -11,7 +11,6 @@ import { chainId } from '@safe-global/safe-4337/test/utils/encoding' import { Safe4337 } from '@safe-global/safe-4337/src/utils/safe' import { WebAuthnCredentials } from '../../test/utils/webauthnShim' import { decodePublicKey, encodeWebAuthnSignature } from '../../src/utils/webauthn' -import { setCode } from '@nomicfoundation/hardhat-toolbox/network-helpers' describe('Safe4337Module - WebAuthn Owner', () => { const setupTests = deployments.createFixture(async ({ deployments }) => { @@ -33,11 +32,8 @@ describe('Safe4337Module - WebAuthn Owner', () => { const safeModuleSetup = await ethers.getContractAt(SafeModuleSetup.abi, SafeModuleSetup.address) const signerLaunchpad = await ethers.getContractAt('SafeSignerLaunchpad', SafeSignerLaunchpad.address) const singleton = await ethers.getContractAt(SafeL2.abi, SafeL2.address) - const verifier = await ethers.getContractAt('IP256Verifier', FCLP256Verifier.address) const signerFactory = await ethers.getContractAt('SafeWebAuthnSignerFactory', SafeWebAuthnSignerFactory.address) - const precompileAddress = `0x${'00'.repeat(18)}0100` - await setCode(precompileAddress, await ethers.provider.getCode(verifier)) - const verifiers = BigInt(ethers.solidityPacked(['uint32', 'address'], [Number(precompileAddress), verifier.target])) + const verifiers = BigInt(FCLP256Verifier.address) const navigator = { credentials: new WebAuthnCredentials(), @@ -52,8 +48,8 @@ describe('Safe4337Module - WebAuthn Owner', () => { signerLaunchpad, singleton, signerFactory, - navigator, verifiers, + navigator, } }) @@ -98,7 +94,7 @@ describe('Safe4337Module - WebAuthn Owner', () => { { type: 'address', name: 'signerFactory' }, { type: 'uint256', name: 'signerX' }, { type: 'uint256', name: 'signerY' }, - { type: 'uint192', name: 'signerVerifiers' }, + { type: 'uint176', name: 'signerVerifiers' }, { type: 'address', name: 'setupTo' }, { type: 'bytes', name: 'setupData' }, { type: 'address', name: 'fallbackHandler' }, diff --git a/modules/passkey/test/4337/WebAuthnSigner.spec.ts b/modules/passkey/test/4337/WebAuthnSigner.spec.ts index 1d927757..0941efe4 100644 --- a/modules/passkey/test/4337/WebAuthnSigner.spec.ts +++ b/modules/passkey/test/4337/WebAuthnSigner.spec.ts @@ -110,7 +110,7 @@ describe('WebAuthn Signers [@4337]', () => { { type: 'address', name: 'signerFactory' }, { type: 'uint256', name: 'signerX' }, { type: 'uint256', name: 'signerY' }, - { type: 'uint192', name: 'signerVerifiers' }, + { type: 'uint176', name: 'signerVerifiers' }, { type: 'address', name: 'setupTo' }, { type: 'bytes', name: 'setupData' }, { type: 'address', name: 'fallbackHandler' }, diff --git a/modules/passkey/test/GasBenchmarkingProxy.spec.ts b/modules/passkey/test/GasBenchmarking.spec.ts similarity index 100% rename from modules/passkey/test/GasBenchmarkingProxy.spec.ts rename to modules/passkey/test/GasBenchmarking.spec.ts diff --git a/modules/passkey/test/SafeWebAuthnSignerProxyFactory.spec.ts b/modules/passkey/test/SafeWebAuthnSignerFactory.spec.ts similarity index 99% rename from modules/passkey/test/SafeWebAuthnSignerProxyFactory.spec.ts rename to modules/passkey/test/SafeWebAuthnSignerFactory.spec.ts index a53db536..82e79a85 100644 --- a/modules/passkey/test/SafeWebAuthnSignerProxyFactory.spec.ts +++ b/modules/passkey/test/SafeWebAuthnSignerFactory.spec.ts @@ -17,7 +17,7 @@ describe('SafeWebAuthnSignerFactory', () => { const precompileAddress = `0x${'00'.repeat(18)}0100` const mockPrecompile = await ethers.getContractAt('MockContract', precompileAddress) await setCode(precompileAddress, await ethers.provider.getCode(mockVerifier)) - const verifiers = BigInt(ethers.solidityPacked(['uint32', 'uint160'], [mockPrecompile.target, mockVerifier.target])) + const verifiers = BigInt(ethers.solidityPacked(['uint16', 'uint160'], [mockPrecompile.target, mockVerifier.target])) return { factory, mockPrecompile, mockVerifier, verifiers } }) diff --git a/modules/passkey/test/SafeWebAuthnSignerProxy.spec.ts b/modules/passkey/test/SafeWebAuthnSignerProxy.spec.ts index feff1d50..c2e3805c 100644 --- a/modules/passkey/test/SafeWebAuthnSignerProxy.spec.ts +++ b/modules/passkey/test/SafeWebAuthnSignerProxy.spec.ts @@ -15,7 +15,7 @@ describe('SafeWebAuthnSignerProxy', () => { const precompileAddress = `0x${'00'.repeat(18)}0100` const mockPrecompile = await ethers.getContractAt('MockContract', precompileAddress) await setCode(precompileAddress, await ethers.provider.getCode(mockVerifier)) - const verifiers = BigInt(ethers.solidityPacked(['uint32', 'address'], [Number(precompileAddress), mockVerifier.target])) + const verifiers = BigInt(ethers.solidityPacked(['uint16', 'address'], [Number(precompileAddress), mockVerifier.target])) const singleton = await (await ethers.getContractFactory('SafeWebAuthnSignerSingleton')).deploy() const SafeWebAuthnSignerProxy = await ethers.getContractFactory('SafeWebAuthnSignerProxy') const signer = await ethers.getContractAt( @@ -26,13 +26,13 @@ describe('SafeWebAuthnSignerProxy', () => { return { x, y, mockPrecompile, mockVerifier, verifiers, signer } }) - describe('forward call', function () { + describe('fallback', function () { it('Should forward call to singleton with additional information', async () => { const { x, y, mockVerifier } = await setupTests() const [sender] = await ethers.getSigners() const mockSingleton = await ethers.getContractAt('MockContract', await (await ethers.getContractFactory('MockContract')).deploy()) - const verifiers = ethers.solidityPacked(['uint32', 'address'], [0x0100, mockVerifier.target]) + const verifiers = ethers.solidityPacked(['uint16', 'address'], [0x0100, mockVerifier.target]) const signerProxy = await ethers.getContractAt( 'MockContract', @@ -44,7 +44,7 @@ describe('SafeWebAuthnSignerProxy', () => { await sender.sendTransaction({ to: signerProxy.target, value: 0, data: callData }) expect(await signerProxy.invocationCount()).to.equal(1) - const data = ethers.solidityPacked(['bytes', 'uint256', 'uint256', 'uint192'], [callData, x, y, verifiers]) + const data = ethers.solidityPacked(['bytes', 'uint256', 'uint256', 'uint176'], [callData, x, y, verifiers]) expect(await signerProxy.invocationCountForCalldata(data)).to.equal(1) }) }) @@ -129,7 +129,7 @@ describe('SafeWebAuthnSignerProxy', () => { }) it('Should return false when the verifier does not return true', async () => { - const { x, y, mockVerifier, signer } = await setupTests() + const { x, y, mockPrecompile, mockVerifier, signer } = await setupTests() const data = ethers.toUtf8Bytes('some data to sign') const dataHash = ethers.keccak256(data) @@ -150,6 +150,7 @@ describe('SafeWebAuthnSignerProxy', () => { s, }) + await mockPrecompile.givenAnyReturnBool(false) await mockVerifier.givenAnyReturnBool(true) await mockVerifier.givenCalldataReturn( ethers.solidityPacked( diff --git a/modules/passkey/test/libraries/P256.spec.ts b/modules/passkey/test/libraries/P256.spec.ts index e999c1a9..32493877 100644 --- a/modules/passkey/test/libraries/P256.spec.ts +++ b/modules/passkey/test/libraries/P256.spec.ts @@ -15,13 +15,13 @@ describe('P256', function () { await setCode(precompileAddress, await ethers.provider.getCode(verifier)) const precompile = await ethers.getContractAt('IP256Verifier', precompileAddress) - const verifiers = BigInt(ethers.solidityPacked(['uint32', 'address'], [precompileAddress, FCLP256Verifier.address])) + const verifiers = BigInt(ethers.solidityPacked(['uint16', 'address'], [precompileAddress, FCLP256Verifier.address])) const allVerifiers = [ [precompileAddress, FCLP256Verifier.address], [ethers.ZeroAddress, FCLP256Verifier.address], [precompileAddress, ethers.ZeroAddress], [ethers.ZeroAddress, precompileAddress], - ].map(([precompile, fallback]) => BigInt(ethers.solidityPacked(['uint32', 'address'], [precompile, fallback]))) + ].map(([precompile, fallback]) => BigInt(ethers.solidityPacked(['uint16', 'address'], [precompile, fallback]))) const P256Lib = await ethers.getContractFactory('TestP256Lib') const p256Lib = await P256Lib.deploy() @@ -160,7 +160,7 @@ describe('P256', function () { await setCode(await precompile.getAddress(), await ethers.provider.getCode(mockVerifier)) const mockPrecompile = await ethers.getContractAt('MockContract', precompile) - const verifiers = BigInt(ethers.solidityPacked(['uint32', 'address'], [mockPrecompile.target, mockVerifier.target])) + const verifiers = BigInt(ethers.solidityPacked(['uint16', 'address'], [mockPrecompile.target, mockVerifier.target])) const configurations = [ // wrong return data length