From 73b5d259835f3ff800873168603b9408681b341a Mon Sep 17 00:00:00 2001 From: Nicholas Rodrigues Lordello Date: Tue, 12 Mar 2024 15:29:34 +0100 Subject: [PATCH] Refactor WebAuthn Implementation --- ...chpad.sol => SafeECDSASignerLaunchpad.sol} | 20 +- modules/passkey/contracts/WebAuthnSigner.sol | 38 ++-- .../contracts/WebAuthnSignerFactory.sol | 59 +++--- .../contracts/base/SignatureValidator.sol | 4 +- .../interfaces/ICustomECDSASignerFactory.sol | 53 ++++++ .../interfaces/ICustomSignerFactory.sol | 87 --------- .../interfaces/IWebAuthnVerifier.sol | 56 ------ .../passkey/contracts/libraries/WebAuthn.sol | 178 ++++++++++++++++++ .../contracts/libraries/WebAuthnFlags.sol | 42 ----- .../contracts/libraries/WebAuthnSignature.sol | 44 ----- ...tractsImports.sol => 4337Dependencies.sol} | 0 .../passkey/contracts/test/TestP256Lib.sol | 3 +- .../test/TestWebAuthnSignerFactory.sol | 3 +- .../contracts/verifiers/WebAuthnVerifier.sol | 104 ---------- modules/passkey/src/deploy/launchpad.ts | 2 +- modules/passkey/src/deploy/verifiers.ts | 9 +- .../passkey/test/4337/WebAuthnSigner.spec.ts | 16 +- 17 files changed, 289 insertions(+), 429 deletions(-) rename modules/passkey/contracts/4337/{Safe256BitECSignerLaunchpad.sol => SafeECDSASignerLaunchpad.sol} (93%) create mode 100644 modules/passkey/contracts/interfaces/ICustomECDSASignerFactory.sol delete mode 100644 modules/passkey/contracts/interfaces/ICustomSignerFactory.sol delete mode 100644 modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol create mode 100644 modules/passkey/contracts/libraries/WebAuthn.sol delete mode 100644 modules/passkey/contracts/libraries/WebAuthnFlags.sol delete mode 100644 modules/passkey/contracts/libraries/WebAuthnSignature.sol rename modules/passkey/contracts/test/{4337ContractsImports.sol => 4337Dependencies.sol} (100%) delete mode 100644 modules/passkey/contracts/verifiers/WebAuthnVerifier.sol diff --git a/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol b/modules/passkey/contracts/4337/SafeECDSASignerLaunchpad.sol similarity index 93% rename from modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol rename to modules/passkey/contracts/4337/SafeECDSASignerLaunchpad.sol index 5e286e496..064991b70 100644 --- a/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol +++ b/modules/passkey/contracts/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 bounty@safe.global */ -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 index 1ccfd5a63..661f7cccc 100644 --- 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 index a8ac73a7d..0f2c72911 100644 --- 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 index d2816abc8..800282b2d 100644 --- 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 new file mode 100644 index 000000000..0df5acab1 --- /dev/null +++ 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 bounty@safe.global + */ +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 deleted file mode 100644 index b68f8c920..000000000 --- a/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol +++ /dev/null @@ -1,87 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -pragma solidity >=0.8.0 <0.9.0; - -/** - * @title ICustomECSignerFactory - * @dev Interface for creating and verifying ECDSA signers. This is a generalized interface that should be - * compatible with curves of any order size. Currently not used in the project and exists here for reference. - * @custom:security-contact bounty@safe.global - */ -interface ICustomECSignerFactory { - /** - * @notice Gets the unique signer address for the specified data. - * @dev The signer address should be unique for the given data. The signer does not need to be created yet. - * @param data The signer-specific data. - * @return signer The signer address. - */ - function getSigner(bytes memory data) external view returns (address signer); - - /** - * @notice Creates a new unique signer for the specified data. - * @dev The signer address must be unique for the given data. This should not revert if the signer already exists. - * @param data The signer-specific data. - * @return signer The signer address. - */ - function createSigner(bytes memory data) external returns (address signer); - - /** - * @notice Verifies a signature for the specified address without deploying it. - * @dev This should 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 signerData The signer data to verify the signature for. - * @return magicValue Returns the legacy EIP-1271 magic value (`bytes4(keccak256("isValidSignature(bytes,bytes)")`) when the signature is valid. Reverting or returning any other value implies an invalid signature. - */ - function isValidSignatureForSigner( - bytes32 message, - bytes calldata signature, - bytes calldata signerData - ) external view returns (bytes4 magicValue); -} - -/** - * @title ICustom256BitECSignerFactory - * @dev Interface for creating and verifying ECDSA signers using 256-bit elliptic curves. - * @custom:security-contact bounty@safe.global - */ -interface ICustom256BitECSignerFactory { - /** - * @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 qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @param verifier The address of the verifier. - * @return signer The signer address. - */ - function getSigner(uint256 qx, uint256 qy, 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 qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @param verifier The address of the verifier. - * @return signer The signer address. - */ - function createSigner(uint256 qx, uint256 qy, 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 qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @param verifier The address of the verifier. - * @param message The signed message. - * @param signature The signature bytes. - * @return magicValue Returns a legacy EIP-1271 magic value (`bytes4(keccak256(isValidSignature(bytes,bytes))`) when the signature is valid. Reverting or returning any other value implies an invalid signature. - */ - function isValidSignatureForSigner( - uint256 qx, - uint256 qy, - address verifier, - bytes32 message, - bytes calldata signature - ) external view returns (bytes4 magicValue); -} diff --git a/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol b/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol deleted file mode 100644 index 2ba86b4f6..000000000 --- a/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol +++ /dev/null @@ -1,56 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable payable-fallback */ -pragma solidity ^0.8.0; - -/** - * @title WebAuthn Verifier Interface - * @dev Interface for verifying WebAuthn signatures. - * @custom:security-contact bounty@safe.global - */ -interface IWebAuthnVerifier { - /** - * @dev Verifies a WebAuthn signature. - * @param authenticatorData The authenticator data. - * @param authenticatorFlags The authenticator flags. - * @param challenge The challenge. - * @param clientDataFields The client data fields. - * @param r The ECDSA signature's R component. - * @param s The ECDSA signature's S component. - * @param qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @return success Whether the signature is valid. - */ - function verifyWebAuthnSignature( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256 r, - uint256 s, - uint256 qx, - uint256 qy - ) external view returns (bool success); - - /** - * @dev Verifies a WebAuthn signature allowing malleability. - * @param authenticatorData The authenticator data. - * @param authenticatorFlags The authenticator flags. - * @param challenge The challenge. - * @param clientDataFields The client data fields. - * @param r The ECDSA signature's R component. - * @param s The ECDSA signature's S component. - * @param qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @return success Whether the signature is valid. - */ - function verifyWebAuthnSignatureAllowMalleability( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256 r, - uint256 s, - uint256 qx, - uint256 qy - ) external view returns (bool success); -} diff --git a/modules/passkey/contracts/libraries/WebAuthn.sol b/modules/passkey/contracts/libraries/WebAuthn.sol new file mode 100644 index 000000000..687618f0f --- /dev/null +++ b/modules/passkey/contracts/libraries/WebAuthn.sol @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity ^0.8.0; + +import {Base64Url} from "../vendor/FCL/utils/Base64Url.sol"; +import {IP256Verifier, P256} from "./P256.sol"; + +/** + * @title WebAuthn Signature Verification + * @dev Library for verifying WebAuthn signatures for public key credentials using the ES256 + * algorithm. + * @custom:security-contact bounty@safe.global + */ +library WebAuthn { + using P256 for IP256Verifier; + + /** + * @notice The WebAuthn signature data format. + * @dev WebAuthn signatures are expected to be the ABI-encoded bytes of the following structure. + * @param authenticatorData The authenticator data from the WebAuthn credential assertion. + * @param clientDataFields The additional fields from the client data JSON. This is the comma + * separated fields as they appear in the client data JSON from the WebAuthn credential + * assertion after the leading {type} and {challenge} fields. + * @param r The ECDSA signature's R component. + * @param s The ECDSA signature's S component. + */ + struct Signature { + bytes authenticatorData; + string clientDataFields; + uint256 r; + uint256 s; + } + + /** + * @notice A WebAuthn authenticator bit-flags + * @dev Represents flags that are included in a WebAuthn assertion's authenticator data and can + * be used to check on-chain how the user was authorized by the device when signing. + */ + type AuthenticatorFlags is bytes1; + + /** + * @notice Authenticator data flag indicating user presence (UP). + * @dev A test of user presence is a simple form of authorization gesture and technical process + * where a user interacts with an authenticator by (typically) simply touching it (other + * modalities may also exist), yielding a Boolean result. Note that this does not constitute + * user verification because a user presence test, by definition, is not capable of biometric + * recognition, nor does it involve the presentation of a shared secret such as a password or + * PIN. + * + * See . + */ + AuthenticatorFlags internal constant USER_PRESENCE = AuthenticatorFlags.wrap(0x01); + + /** + * @notice Authenticator data flag indicating user verification (UV). + * @dev The technical process by which an authenticator locally authorizes the invocation of the + * authenticatorMakeCredential and authenticatorGetAssertion operations. User verification MAY + * be instigated through various authorization gesture modalities; for example, through a touch + * plus pin code, password entry, or biometric recognition (e.g., presenting a fingerprint). The + * intent is to distinguish individual users. + * + * Note that user verification does not give the Relying Party a concrete identification of the + * user, but when 2 or more ceremonies with user verification have been done with that + * credential it expresses that it was the same user that performed all of them. The same user + * might not always be the same natural person, however, if multiple natural persons share + * access to the same authenticator. + * + * See . + */ + AuthenticatorFlags internal constant USER_VERIFICATION = AuthenticatorFlags.wrap(0x04); + + /** + * @notice Casts calldata bytes to a WebAuthn signature data structure. + * @param signature The calldata bytes of the WebAuthn signature. + * @return data A pointer to the signature data in calldata. + * @dev This method casts the dynamic bytes array to a signature calldata pointer without + * additional verification. This is not a security issue for the WebAuthn implementation, as any + * signature data that would be represented from an invalid `signature` value, could also be + * encoded by a valid one. It does, however, mean that callers into the WebAuthn signature + * verification implementation might not validate as much of the data that they pass in as they + * would expect. With that in mind, callers should not rely on the encoding being verified. + */ + function castSignature(bytes calldata signature) internal pure returns (Signature calldata data) { + // solhint-disable-next-line no-inline-assembly + assembly ("memory-safe") { + data := signature.offset + } + } + + /** + * @notice Generate a signing message based on the authenticator data, challenge, and client + * data fields. + * @dev The signing message are the 32-bytes that are actually signed by the P-256 private key + * when doing a WebAuthn credential assertion. Note that we verify that the challenge is indeed + * signed by using its value to compute the signing message on-chain. + * @param challenge The WebAuthn challenge used for the credential assertion. + * @param authenticatorData Authenticator data. + * @param clientDataFields Client data fields. + * @return message Signing message. + */ + function signingMessage( + bytes32 challenge, + bytes calldata authenticatorData, + string calldata clientDataFields + ) internal pure returns (bytes32 message) { + string memory encodedChallenge = Base64Url.encode(abi.encodePacked(challenge)); + /* solhint-disable quotes */ + bytes memory clientDataJson = abi.encodePacked( + '{"type":"webauthn.get","challenge":"', + encodedChallenge, + '",', + clientDataFields, + "}" + ); + /* solhint-enable quotes */ + + message = sha256(abi.encodePacked(authenticatorData, sha256(clientDataJson))); + } + + /** + * @notice Checks that the required authenticator data flags are set. + * @param authenticatorData The authenticator data. + * @param authenticatorFlags The authenticator flags to check for. + * @return success Whether the authenticator data flags are set. + */ + function checkAuthenticatorFlags( + bytes calldata authenticatorData, + AuthenticatorFlags authenticatorFlags + ) internal pure returns (bool success) { + success = authenticatorData[32] & AuthenticatorFlags.unwrap(authenticatorFlags) == AuthenticatorFlags.unwrap(authenticatorFlags); + } + + /** + * @notice Verifies a WebAuthn signature. + * @param challenge The WebAuthn challenge used in the credential assertion. + * @param signature The encoded WebAuthn signature bytes. + * @param authenticatorFlags The authenticator data flags that must be set. + * @param x The x-coordinate of the credential's public key. + * @param y The y-coordinate of the credential's public key. + * @param verifier The P-256 verifier implementation to use. + * @return success Whether the signature is valid. + */ + function verifySignature( + bytes32 challenge, + bytes calldata signature, + AuthenticatorFlags authenticatorFlags, + uint256 x, + uint256 y, + IP256Verifier verifier + ) internal view returns (bool success) { + success = verifySignature(challenge, castSignature(signature), authenticatorFlags, x, y, verifier); + } + + /** + * @notice Verifies a WebAuthn signature. + * @param challenge The WebAuthn challenge used in the credential assertion. + * @param signature The WebAuthn signature data. + * @param authenticatorFlags The authenticator data flags that must be set. + * @param x The x-coordinate of the credential's public key. + * @param y The y-coordinate of the credential's public key. + * @param verifier The P-256 verifier implementation to use. + * @return success Whether the signature is valid. + */ + function verifySignature( + bytes32 challenge, + Signature calldata signature, + AuthenticatorFlags authenticatorFlags, + uint256 x, + uint256 y, + IP256Verifier verifier + ) internal view returns (bool success) { + if (!checkAuthenticatorFlags(signature.authenticatorData, authenticatorFlags)) { + return false; + } + + bytes32 message = signingMessage(challenge, signature.authenticatorData, signature.clientDataFields); + success = verifier.verifySignatureAllowMalleability(message, signature.r, signature.s, x, y); + } +} diff --git a/modules/passkey/contracts/libraries/WebAuthnFlags.sol b/modules/passkey/contracts/libraries/WebAuthnFlags.sol deleted file mode 100644 index 9564a79db..000000000 --- a/modules/passkey/contracts/libraries/WebAuthnFlags.sol +++ /dev/null @@ -1,42 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -pragma solidity ^0.8.0; - -/** - * @title WebAuthn Authentication Data Flags - * @dev Library that defines constants for WebAuthn verification of the authenticator data. In - * particular, it defines constants representing user flags that are included in an attestation's - * authenticator data. - * @custom:security-contact bounty@safe.global - */ -library WebAuthnFlags { - /** - * @notice Flag indicating user presence (UP). - * @dev A test of user presence is a simple form of authorization gesture and technical process - * where a user interacts with an authenticator by (typically) simply touching it (other - * modalities may also exist), yielding a Boolean result. Note that this does not constitute - * user verification because a user presence test, by definition, is not capable of biometric - * recognition, nor does it involve the presentation of a shared secret such as a password or - * PIN. - * - * See . - */ - bytes1 internal constant USER_PRESENCE = 0x01; - - /** - * @notice Flag indicating user verification (UV). - * @dev The technical process by which an authenticator locally authorizes the invocation of the - * authenticatorMakeCredential and authenticatorGetAssertion operations. User verification MAY - * be instigated through various authorization gesture modalities; for example, through a touch - * plus pin code, password entry, or biometric recognition (e.g., presenting a fingerprint). The - * intent is to distinguish individual users. - * - * Note that user verification does not give the Relying Party a concrete identification of the - * user, but when 2 or more ceremonies with user verification have been done with that - * credential it expresses that it was the same user that performed all of them. The same user - * might not always be the same natural person, however, if multiple natural persons share - * access to the same authenticator. - * - * See . - */ - bytes1 internal constant USER_VERIFICATION = 0x04; -} diff --git a/modules/passkey/contracts/libraries/WebAuthnSignature.sol b/modules/passkey/contracts/libraries/WebAuthnSignature.sol deleted file mode 100644 index f5fce503f..000000000 --- a/modules/passkey/contracts/libraries/WebAuthnSignature.sol +++ /dev/null @@ -1,44 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -pragma solidity ^0.8.0; - -/** - * @title WebAuthn Signature Format - * @dev Library for reading the standard Safe WebAuthn signature format from calldata bytes. - * @custom:security-contact bounty@safe.global - */ -library WebAuthnSignature { - /** - * @notice The WebAuthn signature data format. - * @dev WebAuthn signatures are expected to be the ABI-encoded bytes of the following structure. - * @param authenticatorData The authenticator data from the WebAuthn credential assertion. - * @param clientDataFields The additional fields from the client data JSON. This is the comma - * separated fields as they appear in the client data JSON from the WebAuthn credential - * assertion after the leading {type} and {challenge} fields. - * @param r The ECDSA signature's R component. - * @param s The ECDSA signature's S component. - */ - struct Data { - bytes authenticatorData; - bytes clientDataFields; - uint256 r; - uint256 s; - } - - /** - * @notice Casts calldata bytes to a WebAuthn signature data structure. - * @param signature The calldata bytes of the WebAuthn signature. - * @return data A pointer to the signature data in calldata. - * @dev This method casts the dynamic bytes array to a signature calldata pointer without - * additional verification. This is not a security issue for the WebAuthn implementation, as any - * signature data that would be represented from an invalid `signature` value, could also be - * encoded by a valid one. It does, however, mean that callers into the WebAuthn signature - * verification implementation might not validate as much of the data that they pass in as they - * would expect, but we do not believe this to be an issue. - */ - function cast(bytes calldata signature) internal pure returns (Data calldata data) { - // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { - data := signature.offset - } - } -} diff --git a/modules/passkey/contracts/test/4337ContractsImports.sol b/modules/passkey/contracts/test/4337Dependencies.sol similarity index 100% rename from modules/passkey/contracts/test/4337ContractsImports.sol rename to modules/passkey/contracts/test/4337Dependencies.sol diff --git a/modules/passkey/contracts/test/TestP256Lib.sol b/modules/passkey/contracts/test/TestP256Lib.sol index d1d5b5039..ac0b40ade 100644 --- a/modules/passkey/contracts/test/TestP256Lib.sol +++ b/modules/passkey/contracts/test/TestP256Lib.sol @@ -1,8 +1,7 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity ^0.8.0; -import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; -import {P256} from "../libraries/P256.sol"; +import {IP256Verifier, P256} from "../libraries/P256.sol"; contract TestP256Lib { using P256 for IP256Verifier; diff --git a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol b/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol index ab9895500..373f76caa 100644 --- a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol +++ b/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol @@ -2,8 +2,7 @@ /* solhint-disable one-contract-per-file */ pragma solidity ^0.8.0; -import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; -import {P256} from "../libraries/P256.sol"; +import {IP256Verifier, P256} from "../libraries/P256.sol"; contract TestWebAuthnSignerFactory { function createSigner(address verifier, uint256 x, uint256 y) external returns (TestWebAuthnSigner signer) { diff --git a/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol b/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol deleted file mode 100644 index be45c783c..000000000 --- a/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol +++ /dev/null @@ -1,104 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -pragma solidity >=0.8.0; - -import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; -import {IWebAuthnVerifier} from "../interfaces/IWebAuthnVerifier.sol"; -import {P256} from "../libraries/P256.sol"; -import {Base64Url} from "../vendor/FCL/utils/Base64Url.sol"; - -/** - * @title WebAuthnVerifier - * @dev A contract that implements a WebAuthn signature verification following the precompile's interface. - * The contract inherits from `P256VerifierWithWrapperFunctions` and provides wrapper functions for WebAuthn signatures. - * - * This contract is designed to allow verifying signatures from WebAuthn-compatible devices, such as biometric authenticators. - * It works by generating a signing message based on the authenticator data, challenge, and client data fields, and then verifying the signature using the P256 elliptic curve. - * - * The contract provides two main functions: - * - `verifyWebAuthnSignatureAllowMalleability`: Verifies the signature of a WebAuthn message using P256 elliptic curve, allowing for signature malleability. - * - `verifyWebAuthnSignature`: Verifies the signature of a WebAuthn message using the P256 elliptic curve, checking for signature malleability. - * - * Both functions take the authenticator data, authenticator flags, challenge, client data fields, r and s components of the signature, and x and y coordinates of the public key as input. - * The `verifyWebAuthnSignature` function also checks for signature malleability by ensuring that the s component is less than the curve order n/2. - * @custom:security-contact bounty@safe.global - */ -contract WebAuthnVerifier is IWebAuthnVerifier { - using P256 for IP256Verifier; - - IP256Verifier internal immutable P256_VERIFIER; - - constructor(IP256Verifier verifier) { - P256_VERIFIER = verifier; - } - - /** - * @dev Generates a signing message based on the authenticator data, challenge, and client data fields. - * @param authenticatorData Authenticator data. - * @param challenge Challenge. - * @param clientDataFields Client data fields. - * @return message Signing message. - */ - function signingMessage( - bytes calldata authenticatorData, - bytes32 challenge, - bytes calldata clientDataFields - ) internal pure returns (bytes32 message) { - string memory encodedChallenge = Base64Url.encode(abi.encodePacked(challenge)); - /* solhint-disable quotes */ - bytes memory clientDataJson = abi.encodePacked( - '{"type":"webauthn.get","challenge":"', - encodedChallenge, - '",', - clientDataFields, - "}" - ); - /* solhint-enable quotes */ - message = sha256(abi.encodePacked(authenticatorData, sha256(clientDataJson))); - } - - /** - * @inheritdoc IWebAuthnVerifier - */ - function verifyWebAuthnSignatureAllowMalleability( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256 r, - uint256 s, - uint256 qx, - uint256 qy - ) public view returns (bool success) { - // check authenticator flags, e.g. for User Presence (0x01) and/or User Verification (0x04) - if ((authenticatorData[32] & authenticatorFlags) != authenticatorFlags) { - return false; - } - - bytes32 message = signingMessage(authenticatorData, challenge, clientDataFields); - - success = P256_VERIFIER.verifySignatureAllowMalleability(message, r, s, qx, qy); - } - - /** - * @inheritdoc IWebAuthnVerifier - */ - function verifyWebAuthnSignature( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256 r, - uint256 s, - uint256 qx, - uint256 qy - ) public view returns (bool success) { - // check authenticator flags, e.g. for User Presence (0x01) and/or User Verification (0x04) - if ((authenticatorData[32] & authenticatorFlags) != authenticatorFlags) { - return false; - } - - bytes32 message = signingMessage(authenticatorData, challenge, clientDataFields); - - success = P256_VERIFIER.verifySignature(message, r, s, qx, qy); - } -} diff --git a/modules/passkey/src/deploy/launchpad.ts b/modules/passkey/src/deploy/launchpad.ts index 437965b1d..f693e3861 100644 --- a/modules/passkey/src/deploy/launchpad.ts +++ b/modules/passkey/src/deploy/launchpad.ts @@ -6,7 +6,7 @@ const deploy: DeployFunction = async ({ deployments, getNamedAccounts }) => { const entryPoint = await deployments.get('EntryPoint') - await deploy('Safe256BitECSignerLaunchpad', { + await deploy('SafeECDSASignerLaunchpad', { from: deployer, args: [entryPoint.address], log: true, diff --git a/modules/passkey/src/deploy/verifiers.ts b/modules/passkey/src/deploy/verifiers.ts index 0120fad94..ec377d776 100644 --- a/modules/passkey/src/deploy/verifiers.ts +++ b/modules/passkey/src/deploy/verifiers.ts @@ -14,19 +14,12 @@ const deploy: DeployFunction = async ({ deployments, getNamedAccounts }) => { log: true, }) - const FCLP256Verifier = await deploy('FCLP256Verifier', { + await deploy('FCLP256Verifier', { from: deployer, args: [], deterministicDeployment: true, log: true, }) - - await deploy('WebAuthnVerifier', { - from: deployer, - args: [FCLP256Verifier.address], - log: true, - deterministicDeployment: true, - }) } export default deploy diff --git a/modules/passkey/test/4337/WebAuthnSigner.spec.ts b/modules/passkey/test/4337/WebAuthnSigner.spec.ts index a236fc79c..dcc42df6b 100644 --- a/modules/passkey/test/4337/WebAuthnSigner.spec.ts +++ b/modules/passkey/test/4337/WebAuthnSigner.spec.ts @@ -12,7 +12,7 @@ describe('WebAuthn Signers [@4337]', () => { }) const setupTests = deployments.createFixture(async ({ deployments }) => { - const { EntryPoint, Safe4337Module, Safe256BitECSignerLaunchpad, SafeProxyFactory, SafeModuleSetup, SafeL2, WebAuthnVerifier } = + const { EntryPoint, Safe4337Module, SafeECDSASignerLaunchpad, SafeProxyFactory, SafeModuleSetup, SafeL2, FCLP256Verifier } = await deployments.run() const [user] = await prepareAccounts() const bundler = bundlerRpc() @@ -21,9 +21,9 @@ describe('WebAuthn Signers [@4337]', () => { const module = await ethers.getContractAt(Safe4337Module.abi, Safe4337Module.address) const proxyFactory = await ethers.getContractAt(SafeProxyFactory.abi, SafeProxyFactory.address) const safeModuleSetup = await ethers.getContractAt(SafeModuleSetup.abi, SafeModuleSetup.address) - const signerLaunchpad = await ethers.getContractAt('Safe256BitECSignerLaunchpad', Safe256BitECSignerLaunchpad.address) + const signerLaunchpad = await ethers.getContractAt('SafeECDSASignerLaunchpad', SafeECDSASignerLaunchpad.address) const singleton = await ethers.getContractAt(SafeL2.abi, SafeL2.address) - const webAuthnVerifier = await ethers.getContractAt('WebAuthnVerifier', WebAuthnVerifier.address) + const verifier = await ethers.getContractAt('IP256Verifier', FCLP256Verifier.address) const WebAuthnSignerFactory = await ethers.getContractFactory('WebAuthnSignerFactory') const signerFactory = await WebAuthnSignerFactory.deploy() @@ -43,7 +43,7 @@ describe('WebAuthn Signers [@4337]', () => { singleton, signerFactory, navigator, - webAuthnVerifier, + verifier, SafeL2, } }) @@ -60,12 +60,12 @@ describe('WebAuthn Signers [@4337]', () => { singleton, signerFactory, navigator, - webAuthnVerifier, + verifier, SafeL2, } = await setupTests() const { chainId } = await ethers.provider.getNetwork() - const webAuthnVerifierAddress = await webAuthnVerifier.getAddress() + const verifierAddress = await verifier.getAddress() const credential = navigator.credentials.create({ publicKey: { @@ -83,14 +83,14 @@ describe('WebAuthn Signers [@4337]', () => { }, }) const publicKey = decodePublicKey(credential.response) - const signerAddress = await signerFactory.getSigner(publicKey.x, publicKey.y, webAuthnVerifierAddress) + const signerAddress = await signerFactory.getSigner(publicKey.x, publicKey.y, verifierAddress) const safeInit = { singleton: singleton.target, signerFactory: signerFactory.target, signerX: publicKey.x, signerY: publicKey.y, - signerVerifier: webAuthnVerifierAddress, + signerVerifier: verifierAddress, setupTo: safeModuleSetup.target, setupData: safeModuleSetup.interface.encodeFunctionData('enableModules', [[module.target]]), fallbackHandler: module.target,