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

✨ add verify to the "WebAuthn x P256R1" vault #14

Merged
merged 3 commits into from
Jan 10, 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
2 changes: 2 additions & 0 deletions .github/workflows/quality-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ jobs:
uses: actions/checkout@v3
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
Expand Down Expand Up @@ -46,6 +47,7 @@ jobs:
uses: actions/checkout@v3
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/release-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ jobs:
steps:
- name: Checkout code
uses: actions/checkout@v2
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Create Release
id: create_release
Expand All @@ -34,6 +37,7 @@ jobs:
uses: actions/checkout@v3
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/static-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ jobs:
uses: actions/checkout@v3
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Set up Python3
uses: actions/setup-python@v4
Expand Down
6 changes: 4 additions & 2 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
uses: actions/checkout@v3
with:
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
Expand Down Expand Up @@ -67,7 +68,8 @@ jobs:
- name: "Check out the repo"
uses: "actions/checkout@v3"
with:
submodules: "recursive"
submodules: recursive
token: ${{ secrets.SMOOTH_QDQD_PAT_RIVATE_REPOSITORY }}

- name: "Install Foundry"
uses: "foundry-rs/foundry-toolchain@v1"
Expand All @@ -79,7 +81,7 @@ jobs:
# contracts in the test/ and script/ directory are excluded fron the report
# the precompute internal version of the library is also excluded from the report as
# it is highly experimental and to meant to be used at all
run: "forge coverage --report lcov && lcov --remove lcov.info \
run: "forge coverage --ir-minimum --report lcov && lcov --remove lcov.info \
-o lcov.info 'test/*' 'script/*'"

- name: "Add coverage summary"
Expand Down
13 changes: 8 additions & 5 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
[submodule "lib/forge-std"]
branch = "v1"
path = "lib/forge-std"
url = "https://github.com/foundry-rs/forge-std"
branch = v1
path = lib/forge-std
url = git@github.com:foundry-rs/forge-std.git
[submodule "lib/account-abstraction"]
path = lib/account-abstraction
url = https://github.com/eth-infinitism/account-abstraction
url = git@github.com:eth-infinitism/account-abstraction.git
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
url = git@github.com:OpenZeppelin/openzeppelin-contracts.git
[submodule "lib/webauthn.git"]
path = lib/webauthn.git
url = git@github.com:0x90d2b2b7fb7599eebb6e7a32980857d8/webauthn.git
1 change: 1 addition & 0 deletions lib/webauthn.git
Submodule webauthn.git added at cd44a5
1 change: 1 addition & 0 deletions remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
@openzeppelin/=lib/openzeppelin-contracts/contracts/
ds-test/=lib/forge-std/lib/ds-test/src/
forge-std/=lib/forge-std/src/
@webauthn/=lib/webauthn.git/src/
63 changes: 63 additions & 0 deletions src/SignerVaultWebAuthnP256R1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity >=0.8.20 <0.9.0;

import { StorageSlotRegistry } from "src/StorageSlotRegistry.sol";
import { WebAuthn256r1 } from "@webauthn/WebAuthn256r1.sol";

/// @title A signer vault for WebAuthn signers using the p256r1 curve.
/// @notice Use this library to store and retrieve WebAuthn signers.
Expand Down Expand Up @@ -60,7 +61,7 @@
bytes32 slot = getSignerStartingSlot(clientIdHash);

// store the signer's data in the vault
assembly ("memory-safe") {

Check warning on line 64 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 64 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(slot, clientIdHash)
sstore(add(slot, 1), pubKeyX)
sstore(add(slot, 2), pubKeyY)
Expand All @@ -73,13 +74,13 @@
/// Notably, in WebAuthn contexts, this data is dynamically sized and unpredictable in length.
/// @return WP256r1signer A struct containing all the signer's data.
/// Returns an empty WP256r1signer (bytes32(0), uint256(0), uint256(0)) if no signer is found.
function get(bytes memory clientId) internal view returns (WebAuthnP256R1Signer memory WP256r1signer) {

Check warning on line 77 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Variable name must be in mixedCase

Check warning on line 77 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Variable name must be in mixedCase
bytes32 slot = getSignerStartingSlot(keccak256(clientId));
bytes32 clientIdHash;
uint256 pubkeyX;
uint256 pubkeyY;

assembly ("memory-safe") {

Check warning on line 83 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 83 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
clientIdHash := sload(slot)
pubkeyX := sload(add(slot, 1))
pubkeyY := sload(add(slot, 2))
Expand All @@ -94,7 +95,7 @@
/// In WebAuthn contexts, this client ID is dynamically sized and unpredictable in length.
/// @return WP256r1signer A struct containing all the signer's data. Reverts with SignerNotFound error if no signer
/// is found.
function tryGet(bytes memory clientId) internal view returns (WebAuthnP256R1Signer memory WP256r1signer) {

Check warning on line 98 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Variable name must be in mixedCase

Check warning on line 98 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Variable name must be in mixedCase
WP256r1signer = get(clientId);

if (WP256r1signer.clientIdHash == bytes32(0)) {
Expand All @@ -110,7 +111,7 @@
uint256 pubkX;
uint256 pubkY;

assembly ("memory-safe") {

Check warning on line 114 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases

Check warning on line 114 in src/SignerVaultWebAuthnP256R1.sol

View workflow job for this annotation

GitHub Actions / lint

Avoid to use inline assembly. It is acceptable only in rare cases
pubkX := sload(add(slot, 1))
pubkY := sload(add(slot, 2))
}
Expand Down Expand Up @@ -164,4 +165,66 @@
pubKeyY := sload(add(slot, 2))
}
}

/// @notice Verify ECDSA signature though WebAuthn on the secp256r1 curve
/// @dev This function is a wrapper around the WebAuthn256r1 library.
/// Note the required interactions with the precompiled contract can revert the transaction
/// @param authenticatorDataFlagMask This is a bit mask that will be used to validate the flag in the
/// authenticator data. The flag is located at byte 32 of the authenticator
/// data and is used to indicate, among other things, wheter the user's
/// presence/verification ceremonies have been performed.
/// This argument is not expected to be exposed to the end user, it is the
/// responsibility of the caller to enforce the value of the flag for their flows.
///
/// Here are some flags you may want to use depending on your needs.
/// - 0x01: User presence (UP) is required. If the UP flag is not set, revert
/// - 0x04: User verification (UV) is required. If the UV flag is not set, revert
/// - 0x05: UV and UP are both accepted. If none of them is set, revert
///
// Read more about UP here: https://www.w3.org/TR/webauthn-2/#test-of-user-presence
// Read more about UV here: https://www.w3.org/TR/webauthn-2/#user-verification
/// @param authenticatorData The authenticator data structure encodes contextual bindings made by the authenticator.
/// Described here: https://www.w3.org/TR/webauthn-2/#authenticator-data
/// @param clientData This is the client data that was signed. The client data represents the
/// contextual bindings of both the WebAuthn Relying Party and the client.
/// Described here: https://www.w3.org/TR/webauthn-2/#client-data
/// @param clientChallenge This is the challenge that was sent to the client to sign. It is
/// part of the client data. In a classic non-EVM flow, this challenge
/// is generated by the server and sent to the client to avoid replay
/// attack. In our context, as we already have the nonce for this purpose
/// we use this field to pass the arbitrary execution order.
/// This value is expected to not be encoded in Base64, the encoding is done
/// during the verification.
/// @param clientChallengeOffset The offset of the client challenge in the client data
/// @param r uint256 The r value of the ECDSA signature.
/// @param s uint256 The s value of the ECDSA signature.
/// @param qx The x value of the public key used for the signature
/// @param qy The y value of the public key used for the signature
/// @return bool True if the signature is valid, false otherwise
function verify(
bytes1 authenticatorDataFlagMask,
bytes calldata authenticatorData,
bytes calldata clientData,
bytes calldata clientChallenge,
uint256 clientChallengeOffset,
uint256 r,
uint256 s,
uint256 qx,
uint256 qy
)
internal
returns (bool)
{
return WebAuthn256r1.verify(
authenticatorDataFlagMask,
authenticatorData,
clientData,
clientChallenge,
clientChallengeOffset,
r,
s,
qx,
qy
);
}
}
87 changes: 87 additions & 0 deletions test/unit/SignerVault/signerVaultWebAuthnP256R1.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,66 @@ contract SignerVault__WebAuthnP256R1 is BaseTest {

assertEq(implementation.root(), 0x766490bc3db2290d3ce2c7c2b394a53399f99517ba4974536d11869c06dc8900);
}

// @dev the role of this test is not to test the `webauthn` library but to check it has been integrated correctly
function test_ShouldReturnTrueIfVerifyCorrectWebauthnPayload() external {
// it should return true if verify correct webauthn payload
assertTrue(
implementation.verify(
// authenticatorDataFlagMask
0x01,
// authenticatorData
hex"f8e4b678e1c62f7355266eaa4dc1148573440937063a46d848da1e25babbd20b010000004d",
// clientData
hex"7b2274797065223a22776562617574686e2e676574222c226368616c6c656e67"
hex"65223a224e546f2d3161424547526e78786a6d6b61544865687972444e583369"
hex"7a6c7169316f776d4f643955474a30222c226f726967696e223a226874747073"
hex"3a2f2f66726573682e6c65646765722e636f6d222c2263726f73734f726967696e223a66616c73657d",
// clientChallenge
hex"353a3ed5a0441919f1c639a46931de872ac3357de2ce5aa2d68c2639df54189d",
// clientChallengeOffset
0x24,
// r
45_847_212_378_479_006_099_766_816_358_861_726_414_873_720_355_505_495_069_909_394_794_949_093_093_607,
// s
55_835_259_151_215_769_394_881_684_156_457_977_412_783_812_617_123_006_733_908_193_526_332_337_539_398,
// qx
114_874_632_398_302_156_264_159_990_279_427_641_021_947_882_640_101_801_130_664_833_947_273_521_181_002,
// qy
32_136_952_818_958_550_240_756_825_111_900_051_564_117_520_891_182_470_183_735_244_184_006_536_587_423
)
);
}

// @dev the role of this test is not to test the `webauthn` library but to check it has been integrated correctly
function test_ShouldReturnFalseIfVerifyIncorrectWebauthnPayload() external {
// it should return false if verify incorrect webauthn payload
assertFalse(
implementation.verify(
// authenticatorDataFlagMask
0x01,
// authenticatorData
hex"f8e4b678e1c62f7355266eaa4dc1148573440937063a46d848da1e25babbd20b010000004d",
// clientData
hex"7b2274797065223a22776562617574686e2e676574222c226368616c6c656e67"
hex"65223a224e546f2d3161424547526e78786a6d6b61544865687972444e583369"
hex"7a6c7169316f776d4f643955474a30222c226f726967696e223a226874747073"
hex"3a2f2f66726573682e6c65646765722e636f6d222c2263726f73734f726967696e223a66616c73657d",
// clientChallenge
hex"353a3ed5a0441919f1c639a46931de872ac3357de2ce5aa2d68c2639df54189d",
// clientChallengeOffset
0x24,
// r -- INCORRECT
42_847_212_378_479_006_099_766_816_358_861_726_414_873_720_355_505_495_069_909_394_794_949_093_093_607,
// s
55_835_259_151_215_769_394_881_684_156_457_977_412_783_812_617_123_006_733_908_193_526_332_337_539_398,
// qx
114_874_632_398_302_156_264_159_990_279_427_641_021_947_882_640_101_801_130_664_833_947_273_521_181_002,
// qy
32_136_952_818_958_550_240_756_825_111_900_051_564_117_520_891_182_470_183_735_244_184_006_536_587_423
)
);
}
}

/// @notice this contract is a wrapper around the SignerVaultWebAuthnP256R1 library
Expand Down Expand Up @@ -346,4 +406,31 @@ contract SignerVaultWebAuthnP256R1TestWrapper {
function remove(bytes32 clientIdHash) external {
SignerVaultWebAuthnP256R1.remove(clientIdHash);
}

function verify(
bytes1 authenticatorDataFlagMask,
bytes calldata authenticatorData,
bytes calldata clientData,
bytes calldata clientChallenge,
uint256 clientChallengeOffset,
uint256 r,
uint256 s,
uint256 qx,
uint256 qy
)
external
returns (bool)
{
return SignerVaultWebAuthnP256R1.verify(
authenticatorDataFlagMask,
authenticatorData,
clientData,
clientChallenge,
clientChallengeOffset,
r,
s,
qx,
qy
);
}
}
2 changes: 2 additions & 0 deletions test/unit/SignerVault/signerVaultWebAuthnP256R1.tree
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ SignerVault__WebAuthnP256R1
├── it should return false if no signer exists given a client id
├── it should remove a stored signer based on a client id hash
├── it should return the stored pubkey associated to a client id hash
├── it should return true if verify correct webauthn payload
├── it should return false if verify incorrect webauthn payload
└── it should always use the same root
Loading