-
Notifications
You must be signed in to change notification settings - Fork 5
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
Store Credential id with passkey #270
base: main
Are you sure you want to change the base?
Changes from all commits
722019f
9671aa8
6648ffd
fc1f2a3
397ade6
43dd3e2
d1650a9
cbd3ebb
430a4da
1b752db
20ff4c3
4aab44f
5514906
8ea2b8b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,49 +28,89 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { | |
bytes32 private constant LOW_S_MAX = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8; | ||
bytes32 private constant HIGH_R_MAX = 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551; | ||
|
||
event PasskeyCreated(address indexed keyOwner, string originDomain); | ||
event PasskeyCreated(address indexed keyOwner, string originDomain, bytes credentialId); | ||
event PasskeyRemoved(address indexed keyOwner, string originDomain, bytes credentialId); | ||
|
||
// The layout is unusual due to EIP-7562 storage read restrictions for validation phase. | ||
mapping(string originDomain => mapping(address accountAddress => bytes32)) public lowerKeyHalf; | ||
mapping(string originDomain => mapping(address accountAddress => bytes32)) public upperKeyHalf; | ||
mapping(string originDomain => mapping(bytes credentialId => mapping(address accountAddress => bytes32 publicKey))) | ||
public lowerKeyHalf; | ||
mapping(string originDomain => mapping(bytes credentialId => mapping(address accountAddress => bytes32 publicKey))) | ||
public upperKeyHalf; | ||
|
||
// so you can check if you are using this passkey on this or related domains | ||
mapping(string originDomain => mapping(bytes credentialId => address accountAddress)) public keyExistsOnDomain; | ||
|
||
struct PasskeyId { | ||
string domain; | ||
bytes credentialId; | ||
} | ||
|
||
/// @notice Runs on module install | ||
/// @param data ABI-encoded WebAuthn passkey to add immediately, or empty if not needed | ||
function onInstall(bytes calldata data) external override { | ||
if (data.length > 0) { | ||
require(addValidationKey(data), "WebAuthValidator: key already exists"); | ||
(bytes memory credentialId, bytes32[2] memory rawPublicKey, string memory originDomain) = abi.decode( | ||
data, | ||
(bytes, bytes32[2], string) | ||
); | ||
require(addValidationKey(credentialId, rawPublicKey, originDomain), "WebAuthValidator: key already exists"); | ||
} | ||
} | ||
|
||
/// @notice Runs on module uninstall | ||
/// @param data ABI-encoded array of origin domains to remove keys for | ||
function onUninstall(bytes calldata data) external override { | ||
string[] memory domains = abi.decode(data, (string[])); | ||
for (uint256 i = 0; i < domains.length; i++) { | ||
string memory domain = domains[i]; | ||
lowerKeyHalf[domain][msg.sender] = 0x0; | ||
upperKeyHalf[domain][msg.sender] = 0x0; | ||
PasskeyId[] memory passkeyIds = abi.decode(data, (PasskeyId[])); | ||
for (uint256 i = 0; i < passkeyIds.length; i++) { | ||
PasskeyId memory passkeyId = passkeyIds[i]; | ||
_removeValidationKey(passkeyId.credentialId, passkeyId.domain); | ||
} | ||
} | ||
|
||
/// @notice Adds a WebAuthn passkey for the caller | ||
/// @param key ABI-encoded WebAuthn public key to add | ||
/// @return true if the key was added, false if it was updated | ||
function addValidationKey(bytes calldata key) public returns (bool) { | ||
(bytes32[2] memory key32, string memory originDomain) = abi.decode(key, (bytes32[2], string)); | ||
bytes32 initialLowerHalf = lowerKeyHalf[originDomain][msg.sender]; | ||
bytes32 initialUpperHalf = upperKeyHalf[originDomain][msg.sender]; | ||
function removeValidationKey(bytes calldata credentialId, string calldata domain) external { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason for having an internal/external pair of functions instead of having one public one? |
||
return _removeValidationKey(credentialId, domain); | ||
} | ||
|
||
// we might want to support multiple passkeys per domain | ||
lowerKeyHalf[originDomain][msg.sender] = key32[0]; | ||
upperKeyHalf[originDomain][msg.sender] = key32[1]; | ||
function _removeValidationKey(bytes memory credentialId, string memory domain) internal { | ||
lowerKeyHalf[domain][credentialId][msg.sender] = 0x0; | ||
upperKeyHalf[domain][credentialId][msg.sender] = 0x0; | ||
if (keyExistsOnDomain[domain][credentialId] == msg.sender) { | ||
keyExistsOnDomain[domain][credentialId] = address(0); | ||
} | ||
emit PasskeyRemoved(msg.sender, domain, credentialId); | ||
} | ||
|
||
// we're returning true if this was a new key, false for update | ||
bool keyExists = uint256(initialLowerHalf) == 0 && uint256(initialUpperHalf) == 0; | ||
/// @notice Adds a WebAuthn passkey for the caller | ||
/// @param credentialId unique public identifier for the key | ||
/// @param rawPublicKey ABI-encoded WebAuthn public key to add | ||
/// @param originDomain the domain this associated with | ||
/// @return true if the key was added, false if one already exists | ||
function addValidationKey( | ||
bytes memory credentialId, | ||
bytes32[2] memory rawPublicKey, | ||
string memory originDomain | ||
) public returns (bool) { | ||
bytes32 initialLowerHalf = lowerKeyHalf[originDomain][credentialId][msg.sender]; | ||
bytes32 initialUpperHalf = upperKeyHalf[originDomain][credentialId][msg.sender]; | ||
if (uint256(initialLowerHalf) != 0 || uint256(initialUpperHalf) != 0) { | ||
return false; | ||
} | ||
if (keyExistsOnDomain[originDomain][credentialId] != address(0)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since it is impossible (from what I see) to have a zero key, is this check (and hence mapping) really necessary? |
||
// this key already exists on the domain (but it was zero before?) | ||
return false; | ||
} | ||
if (rawPublicKey[0] == 0 && rawPublicKey[1] == 0) { | ||
// empty keys aren't valid, if attempting to clear, use remove | ||
return false; | ||
} | ||
|
||
lowerKeyHalf[originDomain][credentialId][msg.sender] = rawPublicKey[0]; | ||
upperKeyHalf[originDomain][credentialId][msg.sender] = rawPublicKey[1]; | ||
keyExistsOnDomain[originDomain][credentialId] = msg.sender; | ||
MiniRoman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
emit PasskeyCreated(msg.sender, originDomain); | ||
emit PasskeyCreated(msg.sender, originDomain, credentialId); | ||
|
||
return keyExists; | ||
return true; | ||
} | ||
|
||
/// @notice Validates a WebAuthn signature | ||
|
@@ -103,9 +143,12 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { | |
/// @param fatSignature The signature to validate (authenticator data, client data, [r, s]) | ||
/// @return true if the signature is valid | ||
function webAuthVerify(bytes32 transactionHash, bytes memory fatSignature) internal view returns (bool) { | ||
(bytes memory authenticatorData, string memory clientDataJSON, bytes32[2] memory rs) = _decodeFatSignature( | ||
fatSignature | ||
); | ||
( | ||
bytes memory authenticatorData, | ||
string memory clientDataJSON, | ||
bytes32[2] memory rs, | ||
bytes memory credentialId | ||
) = _decodeFatSignature(fatSignature); | ||
|
||
// prevent signature replay https://yondon.blog/2019/01/01/how-not-to-use-ecdsa/ | ||
if (rs[0] <= 0 || rs[0] > HIGH_R_MAX || rs[1] <= 0 || rs[1] > LOW_S_MAX) { | ||
|
@@ -139,8 +182,8 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { | |
// as passkeys are linked to domains, so the storage mapping reflects that | ||
string memory origin = root.at('"origin"').value().decodeString(); | ||
bytes32[2] memory pubkey; | ||
pubkey[0] = lowerKeyHalf[origin][msg.sender]; | ||
pubkey[1] = upperKeyHalf[origin][msg.sender]; | ||
pubkey[0] = lowerKeyHalf[origin][credentialId][msg.sender]; | ||
pubkey[1] = upperKeyHalf[origin][credentialId][msg.sender]; | ||
// This really only validates the origin is set | ||
if (uint256(pubkey[0]) == 0 || uint256(pubkey[1]) == 0) { | ||
return false; | ||
|
@@ -180,8 +223,20 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator { | |
|
||
function _decodeFatSignature( | ||
bytes memory fatSignature | ||
) private pure returns (bytes memory authenticatorData, string memory clientDataSuffix, bytes32[2] memory rs) { | ||
(authenticatorData, clientDataSuffix, rs) = abi.decode(fatSignature, (bytes, string, bytes32[2])); | ||
) | ||
private | ||
pure | ||
returns ( | ||
bytes memory authenticatorData, | ||
string memory clientDataSuffix, | ||
bytes32[2] memory rs, | ||
bytes memory credentialId | ||
) | ||
{ | ||
(authenticatorData, clientDataSuffix, rs, credentialId) = abi.decode( | ||
fatSignature, | ||
(bytes, string, bytes32[2], bytes) | ||
); | ||
} | ||
|
||
/// @notice Verifies a message using the P256 curve. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's combine these into a single mapping that maps to e.g.
bytes32[2]