diff --git a/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol b/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol index c9de10c94..5e286e496 100644 --- a/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol +++ b/modules/passkey/contracts/4337/Safe256BitECSignerLaunchpad.sol @@ -5,15 +5,16 @@ import {IAccount} from "@account-abstraction/contracts/interfaces/IAccount.sol"; import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/PackedUserOperation.sol"; import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol"; import {SafeStorage} from "@safe-global/safe-contracts/contracts/libraries/SafeStorage.sol"; -import {SignatureValidatorConstants} from "../SignatureValidatorConstants.sol"; + import {ICustom256BitECSignerFactory} from "../interfaces/ICustomSignerFactory.sol"; -import {ISafeSetup} from "../interfaces/ISafe.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. */ -contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage, SignatureValidatorConstants { +contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage { bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)"); // keccak256("SafeSignerLaunchpad.initHash") - 1 @@ -197,7 +198,7 @@ contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage, SignatureValidato ) returns (bytes4 magicValue) { // The timestamps are validated by the entry point, therefore we will not check them again - validationData = _packValidationData(magicValue != EIP1271_MAGIC_VALUE, validUntil, validAfter); + validationData = _packValidationData(magicValue != ERC1271.MAGIC_VALUE, validUntil, validAfter); } catch { validationData = _packValidationData(true, validUntil, validAfter); } @@ -219,7 +220,7 @@ contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage, SignatureValidato address[] memory owners = new address[](1); owners[0] = ICustom256BitECSignerFactory(signerFactory).createSigner(signerX, signerY, signerVerifier); - ISafeSetup(address(this)).setup(owners, 1, setupTo, setupData, fallbackHandler, address(0), 0, payable(address(0))); + ISafe(address(this)).setup(owners, 1, setupTo, setupData, fallbackHandler, address(0), 0, payable(address(0))); } (bool success, bytes memory returnData) = address(this).delegatecall(callData); diff --git a/modules/passkey/contracts/SignatureValidatorConstants.sol b/modules/passkey/contracts/SignatureValidatorConstants.sol deleted file mode 100644 index 06d5f1d25..000000000 --- a/modules/passkey/contracts/SignatureValidatorConstants.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable one-contract-per-file */ -pragma solidity >=0.7.0 <0.9.0; - -/** - * @title SignatureValidatorConstants - * @dev This contract defines the constants used for EIP-1271 signature validation. - */ -contract SignatureValidatorConstants { - // bytes4(keccak256("isValidSignature(bytes32,bytes)") - bytes4 internal constant EIP1271_MAGIC_VALUE = 0x1626ba7e; - - // bytes4(keccak256("isValidSignature(bytes,bytes)") - bytes4 internal constant LEGACY_EIP1271_MAGIC_VALUE = 0x20c13b0b; -} diff --git a/modules/passkey/contracts/WebAuthnSigner.sol b/modules/passkey/contracts/WebAuthnSigner.sol index fba58f569..1ccfd5a63 100644 --- a/modules/passkey/contracts/WebAuthnSigner.sol +++ b/modules/passkey/contracts/WebAuthnSigner.sol @@ -1,21 +1,15 @@ // SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable one-contract-per-file */ pragma solidity >=0.8.0; -import {SignatureValidatorConstants} from "./SignatureValidatorConstants.sol"; -import {ICustom256BitECSignerFactory} from "./interfaces/ICustomSignerFactory.sol"; -import {SignatureValidator} from "./SignatureValidator.sol"; -import {IWebAuthnVerifier, WebAuthnConstants} from "./verifiers/WebAuthnVerifier.sol"; - -struct SignatureData { - bytes authenticatorData; - bytes clientDataFields; - uint256[2] rs; -} +import {SignatureValidator} from "./base/SignatureValidator.sol"; +import {IWebAuthnVerifier} from "./interfaces/IWebAuthnVerifier.sol"; +import {WebAuthnFlags} from "./libraries/WebAuthnFlags.sol"; +import {WebAuthnSignature} from "./libraries/WebAuthnSignature.sol"; /** - * @title WebAuthnSigner + * @title WebAuthn Safe Signature Validator * @dev A contract that represents a WebAuthn signer. + * @custom:security-contact bounty@safe.global */ contract WebAuthnSigner is SignatureValidator { uint256 public immutable X; @@ -38,114 +32,18 @@ contract WebAuthnSigner is SignatureValidator { * @inheritdoc SignatureValidator */ function _verifySignature(bytes32 message, bytes calldata signature) internal view virtual override returns (bool isValid) { - SignatureData calldata signaturePointer; - // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { - signaturePointer := signature.offset - } + WebAuthnSignature.Data calldata data = WebAuthnSignature.cast(signature); return WEBAUTHN_SIG_VERIFIER.verifyWebAuthnSignatureAllowMalleability( - signaturePointer.authenticatorData, - WebAuthnConstants.AUTH_DATA_FLAGS_UV, + data.authenticatorData, + WebAuthnFlags.USER_VERIFICATION, message, - signaturePointer.clientDataFields, - signaturePointer.rs, + data.clientDataFields, + data.r, + data.s, X, Y ); } } - -/** - * @title WebAuthnSignerFactory - * @dev A factory contract for creating and managing WebAuthn signers. - */ -contract WebAuthnSignerFactory is ICustom256BitECSignerFactory, SignatureValidatorConstants { - // @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)))); - 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); - - if (_hasNoCode(signer) && _validVerifier(verifier)) { - WebAuthnSigner created = new WebAuthnSigner{salt: bytes32(0)}(qx, qy, verifier); - require(address(created) == signer); - } - } - - /** - * @dev Checks if the given verifier address contains code. - * @param verifier The address of the verifier to check. - * @return A boolean indicating whether the verifier contains code or not. - */ - function _validVerifier(address verifier) internal view returns (bool) { - // The verifier should contain code (The only way to implement a webauthn verifier is with a smart contract) - return !_hasNoCode(verifier); - } - - // @inheritdoc ICustom256BitECSignerFactory - function isValidSignatureForSigner( - uint256 qx, - uint256 qy, - address verifier, - bytes32 message, - bytes calldata signature - ) external view override returns (bytes4 magicValue) { - if (checkSignature(verifier, message, signature, qx, qy)) { - magicValue = EIP1271_MAGIC_VALUE; - } - } - - /** - * @dev Checks if the provided account has no code. - * @param account The address of the account to check. - * @return True if the account has no code, false otherwise. - */ - function _hasNoCode(address account) internal view returns (bool) { - uint256 size; - // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { - size := extcodesize(account) - } - return size == 0; - } - - /** - * @dev Checks the validity of a signature using WebAuthnVerifier. - * @param verifier The address of the WebAuthnVerifier contract. - * @param dataHash The hash of the data being signed. - * @param signature The signature to be verified. - * @param qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @return A boolean indicating whether the signature is valid or not. - */ - function checkSignature( - address verifier, - bytes32 dataHash, - bytes calldata signature, - uint256 qx, - uint256 qy - ) internal view returns (bool) { - SignatureData calldata signaturePointer; - // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { - signaturePointer := signature.offset - } - - return - IWebAuthnVerifier(verifier).verifyWebAuthnSignatureAllowMalleability( - signaturePointer.authenticatorData, - WebAuthnConstants.AUTH_DATA_FLAGS_UV, - dataHash, - signaturePointer.clientDataFields, - signaturePointer.rs, - qx, - qy - ); - } -} diff --git a/modules/passkey/contracts/WebAuthnSignerFactory.sol b/modules/passkey/contracts/WebAuthnSignerFactory.sol new file mode 100644 index 000000000..a8ac73a7d --- /dev/null +++ b/modules/passkey/contracts/WebAuthnSignerFactory.sol @@ -0,0 +1,86 @@ +// 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 {ERC1271} from "./libraries/ERC1271.sol"; +import {WebAuthnFlags} from "./libraries/WebAuthnFlags.sol"; +import {WebAuthnSignature} from "./libraries/WebAuthnSignature.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)))); + 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); + + if (_hasNoCode(signer) && _validVerifier(verifier)) { + WebAuthnSigner created = new WebAuthnSigner{salt: bytes32(0)}(qx, qy, verifier); + require(address(created) == signer); + } + } + + // @inheritdoc ICustom256BitECSignerFactory + function isValidSignatureForSigner( + uint256 qx, + uint256 qy, + address verifier, + bytes32 message, + bytes calldata signature + ) 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 + ) + ) { + magicValue = ERC1271.MAGIC_VALUE; + } + } + + /** + * @dev Checks if the given verifier address contains code. + * @param verifier The address of the verifier to check. + * @return A boolean indicating whether the verifier contains code or not. + */ + function _validVerifier(address verifier) internal view returns (bool) { + // The verifier should contain code (The only way to implement a webauthn verifier is with a smart contract) + return !_hasNoCode(verifier); + } + + /** + * @dev Checks if the provided account has no code. + * @param account The address of the account to check. + * @return True if the account has no code, false otherwise. + */ + function _hasNoCode(address account) internal view returns (bool) { + uint256 size; + // solhint-disable-next-line no-inline-assembly + assembly ("memory-safe") { + size := extcodesize(account) + } + return size == 0; + } +} diff --git a/modules/passkey/contracts/SignatureValidator.sol b/modules/passkey/contracts/base/SignatureValidator.sol similarity index 80% rename from modules/passkey/contracts/SignatureValidator.sol rename to modules/passkey/contracts/base/SignatureValidator.sol index d6cf792ec..d2816abc8 100644 --- a/modules/passkey/contracts/SignatureValidator.sol +++ b/modules/passkey/contracts/base/SignatureValidator.sol @@ -1,13 +1,14 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.0; -import {SignatureValidatorConstants} from "./SignatureValidatorConstants.sol"; +import {ERC1271} from "../libraries/ERC1271.sol"; /** - * @title ISafeSigner - * @dev A interface for smart contract Safe owners that supports multiple `isValidSignature` versions. + * @title Signature Validator Base Contract + * @dev A interface for smart contract Safe owners that supports multiple ERC-1271 `isValidSignature` versions. + * @custom:security-contact bounty@safe.global */ -abstract contract SignatureValidator is SignatureValidatorConstants { +abstract contract SignatureValidator { /** * @dev Validates the signature for the given data. * @param data The signed data bytes. @@ -16,7 +17,7 @@ abstract contract SignatureValidator is SignatureValidatorConstants { */ function isValidSignature(bytes memory data, bytes calldata signature) external view returns (bytes4 magicValue) { if (_verifySignature(keccak256(data), signature)) { - magicValue = LEGACY_EIP1271_MAGIC_VALUE; + magicValue = ERC1271.LEGACY_MAGIC_VALUE; } } @@ -28,7 +29,7 @@ abstract contract SignatureValidator is SignatureValidatorConstants { */ function isValidSignature(bytes32 message, bytes calldata signature) external view returns (bytes4 magicValue) { if (_verifySignature(message, signature)) { - magicValue = EIP1271_MAGIC_VALUE; + magicValue = ERC1271.MAGIC_VALUE; } } diff --git a/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol b/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol index 0cbab63e4..b68f8c920 100644 --- a/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol +++ b/modules/passkey/contracts/interfaces/ICustomSignerFactory.sol @@ -4,7 +4,8 @@ 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. + * 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 { /** @@ -42,6 +43,7 @@ interface ICustomECSignerFactory { /** * @title ICustom256BitECSignerFactory * @dev Interface for creating and verifying ECDSA signers using 256-bit elliptic curves. + * @custom:security-contact bounty@safe.global */ interface ICustom256BitECSignerFactory { /** diff --git a/modules/passkey/contracts/interfaces/IP256Verifier.sol b/modules/passkey/contracts/interfaces/IP256Verifier.sol new file mode 100644 index 000000000..f233e40c9 --- /dev/null +++ b/modules/passkey/contracts/interfaces/IP256Verifier.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: LGPL-3.0-only +/* solhint-disable payable-fallback */ +pragma solidity ^0.8.0; + +/** + * @title P-256 Elliptic Curve Verifier. + * @dev P-256 verifier contract that follows the EIP-7212 EC verify precompile interface. For more + * details, refer to the EIP-7212 specification: + * @custom:security-contact bounty@safe.global + */ +interface IP256Verifier { + /** + * @notice A fallback function that takes the following input format and returns a result + * indicating whether the signature is valid or not: + * - `input[ 0: 32]`: message + * - `input[ 32: 64]`: signature r + * - `input[ 64: 96]`: signature s + * - `input[ 96:128]`: public key x + * - `input[128:160]`: public key y + * + * The output is a Solidity ABI encoded boolean value indicating whether or not the signature is + * valid. Specifically, it returns 32 bytes with a value of `0x00..00` or `0x00..01` for an + * invalid or valid signature respectively. + * + * Note that this function does not follow the Solidity ABI format (in particular, it does not + * have a 4-byte selector), which is why it requires a fallback function and not regular + * Solidity function. Additionally, it has `view` function semantics, and is expected to be + * called with `STATICCALL` opcode. + * + * @param input The encoded input parameters. + * @return output The encoded signature verification result. + */ + fallback(bytes calldata input) external returns (bytes memory output); +} diff --git a/modules/passkey/contracts/interfaces/ISafe.sol b/modules/passkey/contracts/interfaces/ISafe.sol index 7f2d89f7e..016a2ffca 100644 --- a/modules/passkey/contracts/interfaces/ISafe.sol +++ b/modules/passkey/contracts/interfaces/ISafe.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity >=0.8.0 <0.9.0; -interface ISafeSetup { +interface ISafe { function setup( address[] calldata _owners, uint256 _threshold, diff --git a/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol b/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol new file mode 100644 index 000000000..2ba86b4f6 --- /dev/null +++ b/modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol @@ -0,0 +1,56 @@ +// 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/ERC1271.sol b/modules/passkey/contracts/libraries/ERC1271.sol new file mode 100644 index 000000000..25d7fb288 --- /dev/null +++ b/modules/passkey/contracts/libraries/ERC1271.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity ^0.8.0; + +/** + * @title ERC-1271 Magic Values + * @dev Library that defines constants for ERC-1271 related magic values. + * @custom:security-contact bounty@safe.global + */ +library ERC1271 { + /** + * @notice ERC-1271 magic value returned on valid signatures. + * @dev Value is derived from `bytes4(keccak256("isValidSignature(bytes32,bytes)")`. + */ + bytes4 internal constant MAGIC_VALUE = 0x1626ba7e; + + /** + * @notice Legacy EIP-1271 magic value returned on valid signatures. + * @dev This value was used in previous drafts of the EIP-1271 standard, but replaced by + * {MAGIC_VALUE} in the final version. + * + * Value is derived from `bytes4(keccak256("isValidSignature(bytes,bytes)")`. + */ + bytes4 internal constant LEGACY_MAGIC_VALUE = 0x20c13b0b; +} diff --git a/modules/passkey/contracts/verifiers/IP256Verifier.sol b/modules/passkey/contracts/libraries/P256.sol similarity index 68% rename from modules/passkey/contracts/verifiers/IP256Verifier.sol rename to modules/passkey/contracts/libraries/P256.sol index 8850644da..2ba320a15 100644 --- a/modules/passkey/contracts/verifiers/IP256Verifier.sol +++ b/modules/passkey/contracts/libraries/P256.sol @@ -1,44 +1,15 @@ // SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable payable-fallback */ pragma solidity ^0.8.0; -/** - * @title P-256 Elliptic Curve Verifier. - * @dev P-256 verifier contract that follows the EIP-7212 EC verify precompile interface. For more - * details, refer to the EIP-7212 specification: - * @custom:security-contact bounty@safe.global - */ -interface IP256Verifier { - /** - * @notice A fallback function that takes the following input format and returns a result - * indicating whether the signature is valid or not: - * - `input[ 0: 32]`: message - * - `input[ 32: 64]`: signature r - * - `input[ 64: 96]`: signature s - * - `input[ 96:128]`: public key x - * - `input[128:160]`: public key y - * - * The output is a Solidity ABI encoded boolean value indicating whether or not the signature is - * valid. Specifically, it returns 32 bytes with a value of `0x00..00` or `0x00..01` for an - * invalid or valid signature respectively. - * - * Note that this function does not follow the Solidity ABI format (in particular, it does not - * have a 4-byte selector), which is why it requires a fallback function and not regular - * Solidity function. Additionally, it has `view` function semantics, and is expected to be - * called with `STATICCALL` opcode. - * - * @param input The encoded input parameters. - * @return output The encoded signature verification result. - */ - fallback(bytes calldata input) external returns (bytes memory output); -} +import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; /** - * @title P-256 Elliptic Curve Verifier. - * @dev P-256 verifier contract that follows the EIP-7212 EC verify precompile interface. For more - * details, refer to the EIP-7212 specification: + * @title P-256 Elliptic Curve Verification Library. + * @dev Library P-256 verification with contracts that follows the EIP-7212 EC verify precompile + * interface. See + * @custom:security-contact bounty@safe.global */ -library P256VerifierLib { +library P256 { /** * @notice P-256 curve order n divided by 2 for the signature malleability check. * @dev By convention, non-malleable signatures must have an `s` value that is less than half of diff --git a/modules/passkey/contracts/libraries/WebAuthnFlags.sol b/modules/passkey/contracts/libraries/WebAuthnFlags.sol new file mode 100644 index 000000000..9564a79db --- /dev/null +++ b/modules/passkey/contracts/libraries/WebAuthnFlags.sol @@ -0,0 +1,42 @@ +// 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 new file mode 100644 index 000000000..f5fce503f --- /dev/null +++ b/modules/passkey/contracts/libraries/WebAuthnSignature.sol @@ -0,0 +1,44 @@ +// 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/BadP256Verfier.sol b/modules/passkey/contracts/test/BadP256Verfier.sol index 5c6ff4be7..0c304c2be 100644 --- a/modules/passkey/contracts/test/BadP256Verfier.sol +++ b/modules/passkey/contracts/test/BadP256Verfier.sol @@ -3,7 +3,7 @@ /* solhint-disable payable-fallback */ pragma solidity ^0.8.0; -import {IP256Verifier} from "../verifiers/IP256Verifier.sol"; +import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; contract BadP256Verifier is IP256Verifier { enum Behaviour { diff --git a/modules/passkey/contracts/test/TestP256VerifierLib.sol b/modules/passkey/contracts/test/TestP256Lib.sol similarity index 80% rename from modules/passkey/contracts/test/TestP256VerifierLib.sol rename to modules/passkey/contracts/test/TestP256Lib.sol index 62b693bed..d1d5b5039 100644 --- a/modules/passkey/contracts/test/TestP256VerifierLib.sol +++ b/modules/passkey/contracts/test/TestP256Lib.sol @@ -1,10 +1,11 @@ // SPDX-License-Identifier: LGPL-3.0-only pragma solidity ^0.8.0; -import {IP256Verifier, P256VerifierLib} from "../verifiers/IP256Verifier.sol"; +import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; +import {P256} from "../libraries/P256.sol"; -contract TestP256VerifierLib { - using P256VerifierLib for IP256Verifier; +contract TestP256Lib { + using P256 for IP256Verifier; function verifySignature( IP256Verifier verifier, diff --git a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol b/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol index 6ef92be30..ab9895500 100644 --- a/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol +++ b/modules/passkey/contracts/test/TestWebAuthnSignerFactory.sol @@ -2,7 +2,8 @@ /* solhint-disable one-contract-per-file */ pragma solidity ^0.8.0; -import {IP256Verifier, P256VerifierLib} from "../verifiers/IP256Verifier.sol"; +import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; +import {P256} from "../libraries/P256.sol"; contract TestWebAuthnSignerFactory { function createSigner(address verifier, uint256 x, uint256 y) external returns (TestWebAuthnSigner signer) { @@ -11,7 +12,7 @@ contract TestWebAuthnSignerFactory { } contract TestWebAuthnSigner { - using P256VerifierLib for IP256Verifier; + using P256 for IP256Verifier; struct SignatureData { bytes authenticatorData; diff --git a/modules/passkey/contracts/verifiers/FCLP256Verifier.sol b/modules/passkey/contracts/verifiers/FCLP256Verifier.sol index 95c33c46a..a8491300a 100644 --- a/modules/passkey/contracts/verifiers/FCLP256Verifier.sol +++ b/modules/passkey/contracts/verifiers/FCLP256Verifier.sol @@ -3,8 +3,8 @@ /* solhint-disable payable-fallback */ pragma solidity 0.8.24; +import {IP256Verifier} from "../interfaces/IP256Verifier.sol"; import {FCL_ecdsa} from "../vendor/FCL/FCL_ecdsa.sol"; -import {IP256Verifier} from "./IP256Verifier.sol"; /** * @title P-256 Elliptic Curve Verifier Based on The FreshCryptoLib P-256 Implementation. @@ -14,7 +14,7 @@ contract FCLP256Verifier is IP256Verifier { /** * @inheritdoc IP256Verifier */ - fallback(bytes calldata input) external returns (bytes memory) { + fallback(bytes calldata input) external returns (bytes memory output) { if (input.length != 160) { return abi.encodePacked(uint256(0)); } @@ -34,6 +34,6 @@ contract FCLP256Verifier is IP256Verifier { y := calldataload(128) } - return abi.encode(FCL_ecdsa.ecdsa_verify(message, r, s, x, y)); + output = abi.encode(FCL_ecdsa.ecdsa_verify(message, r, s, x, y)); } } diff --git a/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol b/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol index 1b3c7a512..be45c783c 100644 --- a/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol +++ b/modules/passkey/contracts/verifiers/WebAuthnVerifier.sol @@ -1,77 +1,11 @@ // SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable one-contract-per-file */ pragma solidity >=0.8.0; -import {IP256Verifier, P256VerifierLib} from "./IP256Verifier.sol"; +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 WebAuthnConstants - * @dev Library that defines constants for WebAuthn verification. - */ -library WebAuthnConstants { - /** - * @dev Constants representing the flags in the authenticator data of a WebAuthn verification. - * - * - `AUTH_DATA_FLAGS_UP`: User Presence (UP) flag in the authenticator data. - * - `AUTH_DATA_FLAGS_UV`: User Verification (UV) flag in the authenticator data. - * - `AUTH_DATA_FLAGS_BE`: Attested Credential Data (BE) flag in the authenticator data. - * - `AUTH_DATA_FLAGS_BS`: Extension Data (BS) flag in the authenticator data. - */ - bytes1 internal constant AUTH_DATA_FLAGS_UP = 0x01; - bytes1 internal constant AUTH_DATA_FLAGS_UV = 0x04; - bytes1 internal constant AUTH_DATA_FLAGS_BE = 0x08; - bytes1 internal constant AUTH_DATA_FLAGS_BS = 0x10; -} - -/** - * @title IWebAuthnVerifier - * @dev Interface for verifying WebAuthn signatures. - */ -interface IWebAuthnVerifier { - /** - * @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 rs The signature components. - * @param qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @return A boolean indicating whether the signature is valid. - */ - function verifyWebAuthnSignatureAllowMalleability( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256[2] calldata rs, - uint256 qx, - uint256 qy - ) external view returns (bool); - - /** - * @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 rs The signature components. - * @param qx The x-coordinate of the public key. - * @param qy The y-coordinate of the public key. - * @return A boolean indicating whether the signature is valid. - */ - function verifyWebAuthnSignature( - bytes calldata authenticatorData, - bytes1 authenticatorFlags, - bytes32 challenge, - bytes calldata clientDataFields, - uint256[2] calldata rs, - uint256 qx, - uint256 qy - ) external view returns (bool); -} - /** * @title WebAuthnVerifier * @dev A contract that implements a WebAuthn signature verification following the precompile's interface. @@ -86,8 +20,11 @@ interface IWebAuthnVerifier { * * 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) { @@ -120,25 +57,18 @@ contract WebAuthnVerifier is IWebAuthnVerifier { } /** - * @dev Verifies the signature of a WebAuthn message using P256 elliptic curve, allowing for signature malleability. - * @param authenticatorData Authenticator data. - * @param authenticatorFlags Authenticator flags. - * @param challenge Challenge. - * @param clientDataFields Client data fields. - * @param rs R and S components of the signature. - * @param qx X coordinate of the public key. - * @param qy Y coordinate of the public key. - * @return result Whether the signature is valid. + * @inheritdoc IWebAuthnVerifier */ function verifyWebAuthnSignatureAllowMalleability( bytes calldata authenticatorData, bytes1 authenticatorFlags, bytes32 challenge, bytes calldata clientDataFields, - uint256[2] calldata rs, + uint256 r, + uint256 s, uint256 qx, uint256 qy - ) public view returns (bool result) { + ) 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; @@ -146,29 +76,22 @@ contract WebAuthnVerifier is IWebAuthnVerifier { bytes32 message = signingMessage(authenticatorData, challenge, clientDataFields); - result = P256VerifierLib.verifySignatureAllowMalleability(P256_VERIFIER, message, rs[0], rs[1], qx, qy); + success = P256_VERIFIER.verifySignatureAllowMalleability(message, r, s, qx, qy); } /** - * @dev Verifies the signature of a WebAuthn message using the P256 elliptic curve, checking for signature malleability. - * @param authenticatorData Authenticator data. - * @param authenticatorFlags Authenticator flags. - * @param challenge Challenge. - * @param clientDataFields Client data fields. - * @param rs R and S components of the signature. - * @param qx X coordinate of the public key. - * @param qy Y coordinate of the public key. - * @return result Whether the signature is valid. + * @inheritdoc IWebAuthnVerifier */ function verifyWebAuthnSignature( bytes calldata authenticatorData, bytes1 authenticatorFlags, bytes32 challenge, bytes calldata clientDataFields, - uint256[2] calldata rs, + uint256 r, + uint256 s, uint256 qx, uint256 qy - ) public view returns (bool result) { + ) 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; @@ -176,6 +99,6 @@ contract WebAuthnVerifier is IWebAuthnVerifier { bytes32 message = signingMessage(authenticatorData, challenge, clientDataFields); - result = P256VerifierLib.verifySignature(P256_VERIFIER, message, rs[0], rs[1], qx, qy); + success = P256_VERIFIER.verifySignature(message, r, s, qx, qy); } } diff --git a/modules/passkey/test/verifiers/P256VerfierLib.spec.ts b/modules/passkey/test/libraries/P256.spec.ts similarity index 57% rename from modules/passkey/test/verifiers/P256VerfierLib.spec.ts rename to modules/passkey/test/libraries/P256.spec.ts index eee79b16b..f98868197 100644 --- a/modules/passkey/test/verifiers/P256VerfierLib.spec.ts +++ b/modules/passkey/test/libraries/P256.spec.ts @@ -3,49 +3,49 @@ import { deployments, ethers } from 'hardhat' import { Account } from '../utils/p256' -describe('P256VerifierLib', function () { +describe('P256', function () { const setupTests = deployments.createFixture(async ({ deployments }) => { const { FCLP256Verifier } = await deployments.fixture() const verifier = await ethers.getContractAt('FCLP256Verifier', FCLP256Verifier.address) - const P256VerifierLib = await ethers.getContractFactory('TestP256VerifierLib') - const verifierLib = await P256VerifierLib.deploy() + const P256Lib = await ethers.getContractFactory('TestP256Lib') + const p256Lib = await P256Lib.deploy() const account = new Account() - return { verifier, verifierLib, account } + return { verifier, p256Lib, account } }) it('Should return true on valid signature', async function () { - const { verifier, verifierLib, account } = await setupTests() + const { verifier, p256Lib, account } = await setupTests() const message = ethers.id('hello world') const { r, s } = account.sign(message) const { x, y } = account.publicKey - expect(await verifierLib.verifySignature(verifier, message, r, s, x, y)).to.be.true + expect(await p256Lib.verifySignature(verifier, message, r, s, x, y)).to.be.true }) it('Should return false on invalid signature', async function () { - const { verifier, verifierLib } = await setupTests() + const { verifier, p256Lib } = await setupTests() - expect(await verifierLib.verifySignature(verifier, ethers.ZeroHash, 1, 2, 3, 4)).to.be.false + expect(await p256Lib.verifySignature(verifier, ethers.ZeroHash, 1, 2, 3, 4)).to.be.false }) it('Should check for signature signature malleability', async function () { - const { verifier, verifierLib, account } = await setupTests() + const { verifier, p256Lib, account } = await setupTests() const message = ethers.id('hello world') const { r, highS } = account.sign(message) const { x, y } = account.publicKey - expect(await verifierLib.verifySignature(verifier, message, r, highS, x, y)).to.be.false - expect(await verifierLib.verifySignatureAllowMalleability(verifier, message, r, highS, x, y)).to.be.true + expect(await p256Lib.verifySignature(verifier, message, r, highS, x, y)).to.be.false + expect(await p256Lib.verifySignatureAllowMalleability(verifier, message, r, highS, x, y)).to.be.true }) it('Should return false for misbehaving verifiers', async function () { - const { verifierLib, account } = await setupTests() + const { p256Lib, account } = await setupTests() const message = ethers.id('hello world') const { r, s } = account.sign(message) @@ -58,8 +58,8 @@ describe('P256VerifierLib', function () { for (const behaviour of [WRONG_RETURNDATA_LENGTH, INVALID_BOOLEAN_VALUE, REVERT]) { const verifier = await BadP256Verifier.deploy(behaviour) - expect(await verifierLib.verifySignature(verifier, message, r, s, x, y)).to.be.false - expect(await verifierLib.verifySignatureAllowMalleability(verifier, message, r, s, x, y)).to.be.false + expect(await p256Lib.verifySignature(verifier, message, r, s, x, y)).to.be.false + expect(await p256Lib.verifySignatureAllowMalleability(verifier, message, r, s, x, y)).to.be.false } }) })