Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor WebAuthn Implementation #320

Merged
merged 1 commit into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@ import {PackedUserOperation} from "@account-abstraction/contracts/interfaces/Pac
import {_packValidationData} from "@account-abstraction/contracts/core/Helpers.sol";
import {SafeStorage} from "@safe-global/safe-contracts/contracts/libraries/SafeStorage.sol";

import {ICustom256BitECSignerFactory} from "../interfaces/ICustomSignerFactory.sol";
import {ICustomECDSASignerFactory} from "../interfaces/ICustomECDSASignerFactory.sol";
import {ISafe} from "../interfaces/ISafe.sol";
import {ERC1271} from "../libraries/ERC1271.sol";

/**
* @title SafeOpLaunchpad - A contract for Safe initialization with custom unique signers that would violate ERC-4337 factory rules.
* @dev The is intended to be set as a Safe proxy's implementation for ERC-4337 user operation that deploys the account.
* @title Safe Launchpad for Custom ECDSA Signing Schemes.
* @dev A launchpad account implementation that enables the creation of Safes that use custom ECDSA
* signing schemes that require additional contract deployments over ERC-4337.
* @custom:security-contact [email protected]
*/
contract Safe256BitECSignerLaunchpad is IAccount, SafeStorage {
contract SafeECDSASignerLaunchpad is IAccount, SafeStorage {
bytes32 private constant DOMAIN_SEPARATOR_TYPEHASH = keccak256("EIP712Domain(uint256 chainId,address verifyingContract)");

// keccak256("SafeSignerLaunchpad.initHash") - 1
Expand Down Expand Up @@ -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);
Expand All @@ -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)));
}
Expand Down
38 changes: 13 additions & 25 deletions modules/passkey/contracts/WebAuthnSigner.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subjective nit: I resonate more with isValid than success in this case.

A similar change in SignatureValidator as well.

success = WebAuthn.verifySignature(message, signature, WebAuthn.USER_VERIFICATION, X, Y, VERIFIER);
}
}
59 changes: 23 additions & 36 deletions modules/passkey/contracts/WebAuthnSignerFactory.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}
Expand Down
4 changes: 2 additions & 2 deletions modules/passkey/contracts/base/SignatureValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity >=0.8.0 <0.9.0;

/**
* @title Custom ECDSA Signer Factory
* @dev Interface for a factory contract that can create ERC-1271 compatible signers, and verify
* signatures for custom ECDSA schemes.
* @custom:security-contact [email protected]
*/
interface ICustomECDSASignerFactory {
/**
* @notice Gets the unique signer address for the specified data.
* @dev The unique signer address must be unique for some given data. The signer is not
* guaranteed to be created yet.
* @param x The x-coordinate of the public key.
* @param y The y-coordinate of the public key.
* @param verifier The address of the verifier.
* @return signer The signer address.
*/
function getSigner(uint256 x, uint256 y, address verifier) external view returns (address signer);

/**
* @notice Create a new unique signer for the specified data.
* @dev The unique signer address must be unique for some given data. This must not revert if
* the unique owner already exists.
* @param x The x-coordinate of the public key.
* @param y The y-coordinate of the public key.
* @param verifier The address of the verifier.
* @return signer The signer address.
*/
function createSigner(uint256 x, uint256 y, address verifier) external returns (address signer);

/**
* @notice Verifies a signature for the specified address without deploying it.
* @dev This must be equivalent to first deploying the signer with the factory, and then
* verifying the signature with it directly:
* `factory.createSigner(signerData).isValidSignature(message, signature)`
* @param message The signed message.
* @param signature The signature bytes.
* @param x The x-coordinate of the public key.
* @param y The y-coordinate of the public key.
* @param verifier The address of the verifier.
* @return magicValue Returns the ERC-1271 magic value when the signature is valid. Reverting or
* returning any other value implies an invalid signature.
*/
function isValidSignatureForSigner(
bytes32 message,
bytes calldata signature,
uint256 x,
uint256 y,
address verifier
) external view returns (bytes4 magicValue);
}
87 changes: 0 additions & 87 deletions modules/passkey/contracts/interfaces/ICustomSignerFactory.sol

This file was deleted.

56 changes: 0 additions & 56 deletions modules/passkey/contracts/interfaces/IWebAuthnVerifier.sol

This file was deleted.

Loading
Loading