Skip to content

Commit

Permalink
♻️ improve the creation signature
Browse files Browse the repository at this point in the history 
- Take into account the prefix used to identify the signature
- Rename `loginHash` to `usernameHash`
- Add the address of the `account` concerned by the signature in the signature
- Add the ID of the chain concerned by the creation of the account in the signature
- Remove the `_checkAccountExistence` function from the `AccountFactory` contract
  • Loading branch information
@qd-qd
qd-qd committed Mar 15, 2024
1 parent 5804a8b commit 67b93ce
Show file tree
Hide file tree
Showing 11 changed files with 252 additions and 172 deletions.
16 changes: 9 additions & 7 deletions src/Account.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ contract Account is Initializable, BaseAccount {
// - 4 bytes for the selector of the factory function called --NOT_USED--
// - 32 bytes for the X coordinate of the public key
// - 32 bytes for the Y coordinate of the public key
// - 32 bytes for the loginHash
// - 32 bytes for the usernameHash
// - 32 bytes for the credIdHash
// - X bytes for the signature --NOT_USED--
//
Expand All @@ -189,27 +189,29 @@ contract Account is Initializable, BaseAccount {
//
// Note that any change in the factory's function signature will break the signature validation of this account!
address userOpFactory = address(bytes20(initCode[:20]));
uint256 pubKeyX = uint256(bytes32(initCode[24:56]));
uint256 pubKeyY = uint256(bytes32(initCode[56:88]));
bytes32 loginHash = bytes32(initCode[88:120]);
uint256 pubX = uint256(bytes32(initCode[24:56]));
uint256 pubY = uint256(bytes32(initCode[56:88]));
bytes32 usernameHash = bytes32(initCode[88:120]);
bytes32 credIdHash = bytes32(initCode[120:152]);

// 4. check the factory is the same than the one stored here
if (userOpFactory != factory) return Signature.State.FAILURE;

// 5. recreate the message and try to recover the signer
bytes memory message = abi.encode(Signature.Type.CREATION, loginHash, pubKeyX, pubKeyY, credIdHash);
bytes memory message =
abi.encode(Signature.Type.CREATION, usernameHash, pubX, pubY, credIdHash, address(this), block.chainid);

// 6. fetch the expected signer from the factory contract
address expectedSigner = AccountFactory(factory).admin();

// 7. Check the signature is valid and revert if it is not
if (Signature.recover(expectedSigner, message, signature) == false) return Signature.State.FAILURE;
// NOTE: The signature prefix, added manually to identify the signature, is removed before the recovering process
if (Signature.recover(expectedSigner, message, signature[1:]) == false) return Signature.State.FAILURE;

// 8. Check the signer is the same than the one stored by the factory during the account creation process
// solhint-disable-next-line var-name-mixedcase
(bytes32 $credIdHash, uint256 $pubkeyX, uint256 $pubkeyY) = SignerVaultWebAuthnP256R1.get(credIdHash);
if ($credIdHash != credIdHash || $pubkeyX != pubKeyX || $pubkeyY != pubKeyY) return Signature.State.FAILURE;
if ($credIdHash != credIdHash || $pubkeyX != pubX || $pubkeyY != pubY) return Signature.State.FAILURE;

return Signature.State.SUCCESS;
}
Expand Down
75 changes: 34 additions & 41 deletions src/AccountFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity >=0.8.20 <0.9.0;

import { ERC1967Proxy } from "@openzeppelin/proxy/ERC1967/ERC1967Proxy.sol";
import "src/utils/Signature.sol" as Signature;
import { Account } from "./Account.sol";
import { Account as SmartAccount } from "./Account.sol";

// TODO: Implement an universal registry and use it to store the value of `admin`
// FIXME: createAndInitAccount() Understand the implications of the ban system of the function
Expand All @@ -25,10 +25,10 @@ contract AccountFactory {
address public immutable admin;

event AccountCreated(
bytes32 loginHash, address account, bytes32 indexed credIdHash, uint256 pubKeyX, uint256 pubKeyY
bytes32 usernameHash, address account, bytes32 indexed credIdHash, uint256 pubKeyX, uint256 pubKeyY
);

error InvalidSignature(bytes32 loginHash, bytes signature);
error InvalidSignature(bytes32 usernameHash, bytes signature);

/// @notice Deploy the implementation of the account and store it in the storage of the factory. This
/// implementation will be used as the implementation reference for all the proxies deployed by this
Expand All @@ -48,42 +48,35 @@ contract AccountFactory {
admin = _admin;

// deploy the implementation of the account
Account account = new Account(entryPoint, webAuthnVerifier);
SmartAccount account = new SmartAccount(entryPoint, webAuthnVerifier);

// set the address of the implementation deployed
accountImplementation = payable(address(account));
}

/// @notice This function check if an account already exists based on the loginHash given
/// @param loginHash The keccak256 hash of the login of the account
/// @return The address of the account if it exists, address(0) otherwise
function _checkAccountExistence(bytes32 loginHash) internal view returns (address) {
// calculate the address of the account based on the loginHash and return it if it exists
address calulatedAddress = getAddress(loginHash);
return calulatedAddress.code.length > 0 ? calulatedAddress : address(0);
}

/// @notice This function check if the signature is signed by the correct entity
/// @param pubKeyX The X coordinate of the public key of the first signer. We use the r1 curve here
/// @param pubKeyY The Y coordinate of the public key of the first signer. We use the r1 curve here
/// @param loginHash The keccak256 hash of the login of the account
/// @param usernameHash The keccak256 hash of the login of the account
/// @param credIdHash The hash of the WebAuthn credential ID of the signer. Check the specification
/// @param signature Signature made off-chain. Its recovery must match the admin.
/// @return True if the signature is legit, false otherwise
/// @dev Incorrect signatures are expected to lead to a revert by the library used
function _isSignatureLegit(
uint256 pubKeyX,
uint256 pubKeyY,
bytes32 loginHash,
bytes32 usernameHash,
bytes32 credIdHash,
address accountAddress,
bytes calldata signature
)
internal
view
returns (bool)
{
// recreate the message signed by the admin
bytes memory message = abi.encode(Signature.Type.CREATION, loginHash, pubKeyX, pubKeyY, credIdHash);
bytes memory message = abi.encode(
Signature.Type.CREATION, usernameHash, pubKeyX, pubKeyY, credIdHash, accountAddress, block.chainid
);

// try to recover the address and return the result
return Signature.recover(admin, message, signature);
Expand All @@ -93,70 +86,70 @@ contract AccountFactory {
/// or returns the address of an existing account based on the parameter given
/// @param pubKeyX The X coordinate of the public key of the first signer. We use the r1 curve here
/// @param pubKeyY The Y coordinate of the public key of the first signer. We use the r1 curve here
/// @param loginHash The keccak256 hash of the login of the account
/// @param usernameHash The keccak256 hash of the login of the account
/// @param credIdHash The hash of the WebAuthn credential ID of the signer. Check the specification
/// @param signature Signature made off-chain. Its recovery must match the admin.
/// The loginHash is expected to be the hash used by the recover function.
/// The usernameHash is expected to be the hash used by the recover function.
/// @return The address of the account (either deployed or not)
function createAndInitAccount(
uint256 pubKeyX,
uint256 pubKeyY,
bytes32 loginHash,
bytes32 usernameHash,
bytes32 credIdHash,
bytes calldata signature
)
external
returns (address)
{
// check if the account is already deployed and return prematurely if it is
address alreadyDeployedAddress = _checkAccountExistence(loginHash);
if (alreadyDeployedAddress != address(0)) {
return alreadyDeployedAddress;
}
// 1. get the address of the account if it exists
address accountAddress = getAddress(usernameHash);

// 2. check if the account is already deployed and return prematurely if it is
if (accountAddress.code.length > 0) return accountAddress;

// check if the signature is valid
if (_isSignatureLegit(pubKeyX, pubKeyY, loginHash, credIdHash, signature) == false) {
revert InvalidSignature(loginHash, signature);
// 3. check if the signature is valid
if (_isSignatureLegit(pubKeyX, pubKeyY, usernameHash, credIdHash, accountAddress, signature) == false) {
revert InvalidSignature(usernameHash, signature);
}

// deploy the proxy for the user. During the deployment, the initialize function in the implementation contract
// 4. deploy the proxy for the user. During the deployment, the initialize function in the implementation
// is called using the `delegatecall` opcode
Account account = Account(
SmartAccount account = SmartAccount(
payable(
new ERC1967Proxy{ salt: loginHash }(
accountImplementation, abi.encodeWithSelector(Account.initialize.selector)
new ERC1967Proxy{ salt: usernameHash }(
accountImplementation, abi.encodeWithSelector(SmartAccount.initialize.selector)
)
)
);

// set the first signer of the account using the parameters given
// 5. set the initial signer of the account using the parameters given
account.addFirstSigner(pubKeyX, pubKeyY, credIdHash);

emit AccountCreated(loginHash, address(account), credIdHash, pubKeyX, pubKeyY);

// 6. emit the event and return the address of the deployed account
emit AccountCreated(usernameHash, address(account), credIdHash, pubKeyX, pubKeyY);
return address(account);
}

/// @notice This utility function returns the address of the account that would be deployed
/// @dev This is the under the hood formula used by the CREATE2 opcode
/// @param loginHash The keccak256 hash of the login of the account
/// @param usernameHash The keccak256 hash of the login of the account
/// @return The address of the account that would be deployed
function getAddress(bytes32 loginHash) public view returns (address) {
function getAddress(bytes32 usernameHash) public view returns (address) {
return address(
uint160(
uint256(
keccak256(
abi.encodePacked(
bytes1(0xff), // init code hash prefix
address(this), // deployer address
loginHash, // the salt used to deploy the contract
usernameHash, // the salt used to deploy the contract
keccak256( // the init code hash
abi.encodePacked(
// creation code of the contract deployed
type(ERC1967Proxy).creationCode,
// arguments passed to the constructor of the contract deployed
abi.encode(
accountImplementation, abi.encodeWithSelector(Account.initialize.selector)
accountImplementation, abi.encodeWithSelector(SmartAccount.initialize.selector)
)
)
)
Expand All @@ -176,12 +169,12 @@ contract AccountFactory {
// - CREATE2 is used to deploy the proxy for our users. The formula of this deterministic computation
// depends on these parameters:
// - the address of the factory
// - the loginHash
// - the usernameHash
// - the implementation of the ERC1967Proxy (included in the init code hash)
// - the arguments passed to the constructor of the ERC1967Proxy (included in the init code hash):
// - the address of the implementation of the account
// - the signature selector of the initialize function present in the account implementation (first 4-bytes)
// - the loginHash
// - the usernameHash
//
// - Once set, it's not possible to change the account implementation later.
// - Once deployed by the constructor, it's not possible to change the instance of the account implementation.
Expand Down
65 changes: 49 additions & 16 deletions test/BaseTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,47 @@ pragma solidity >=0.8.20 <0.9.0;
import { Test } from "forge-std/Test.sol";
import { VmSafe } from "forge-std/Vm.sol";
import { MessageHashUtils } from "@openzeppelin/utils/cryptography/MessageHashUtils.sol";
import { AccountFactory } from "src/AccountFactory.sol";
import "src/utils/Signature.sol" as Signature;

struct ValidCreationParams {
uint256 pubKeyX;
uint256 pubKeyY;
string login;
bytes32 loginHash;
bytes32 usernameHash;
bytes32 credIdHash;
bytes signature;
address signer;
bytes signature;
}

contract BaseTest is Test {
VmSafe.Wallet internal signer;

// store a set of valid creation parameters that can be used in tests
ValidCreationParams internal validCreate;

// use a random username hash
bytes32 internal __usernameHash = 0x975f1d0637811aba12c41f7b0d68ee42aa76a3a1cba43b96f60f0e3ea2f2206a;

// generate a credId hash that corresponds to the login
bytes32 internal __credIdHash = 0x511deddb8b836e8ced2f5e8a4ee0ac63ae4095b426398cc712aa772a4c68a099;

// generate a set of valid creation parameters
constructor() {
// generate the wallet for the secret signer
VmSafe.Wallet memory signer = vm.createWallet(4337);

// set a random login and hash it
string memory login = "samus-aran";
bytes32 loginHash = keccak256(bytes(login));
signer = vm.createWallet(4337);

// generate a credId hash that corresponds to the login
bytes32 credIdHash = 0x511deddb8b836e8ced2f5e8a4ee0ac63ae4095b426398cc712aa772a4c68a099;
// TODO: remove valid create here

// recreate the message to sign
bytes memory message = abi.encode(0x00, loginHash, signer.publicKeyX, signer.publicKeyY, credIdHash);
bytes memory message = abi.encode(
Signature.Type.CREATION,
__usernameHash,
signer.publicKeyX,
signer.publicKeyY,
__credIdHash,
address(32),
block.chainid
);

// hash the message with the EIP-191 prefix
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(message);
Expand All @@ -45,11 +57,10 @@ contract BaseTest is Test {
validCreate = ValidCreationParams({
pubKeyX: signer.publicKeyX,
pubKeyY: signer.publicKeyY,
login: login,
loginHash: loginHash,
credIdHash: credIdHash,
signature: signature,
signer: signer.addr
usernameHash: __usernameHash,
credIdHash: __credIdHash,
signer: signer.addr,
signature: signature
});
}

Expand All @@ -62,6 +73,28 @@ contract BaseTest is Test {
_;
}

function _craftCreationSignature(address factoryAddress) internal returns (bytes memory signature) {
address accountAddress = AccountFactory(factoryAddress).getAddress(validCreate.usernameHash);

// recreate the message to sign
bytes memory message = abi.encode(
Signature.Type.CREATION,
__usernameHash,
signer.publicKeyX,
signer.publicKeyY,
__credIdHash,
accountAddress,
block.chainid
);

// hash the message with the EIP-191 prefix
bytes32 hash = MessageHashUtils.toEthSignedMessageHash(message);

// sign the hash of the message and get the signature
(uint8 v, bytes32 r, bytes32 s) = vm.sign(signer.privateKey, hash);
signature = abi.encodePacked(r, s, v);
}

function boundP256R1(uint256 x) internal pure returns (uint256) {
return x % P256R1_MAX;
}
Expand Down
Loading

0 comments on commit 67b93ce

Please sign in to comment.