From 6430626c33a34af53a10d3fc3bcdfa11e960a5e8 Mon Sep 17 00:00:00 2001 From: qd-qd Date: Wed, 10 Jan 2024 16:51:02 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9E=95=20install=20our=20webauthn=20libr?= =?UTF-8?q?ary=20as=20a=20submodule?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This library will be used by the SignerVaultWebAuthnP256R1 library to verify a Webauthn payload. This commit also update the configuration of the CI to support the installation of a library from one of our private repository --- .github/workflows/quality-checks.yml | 2 ++ .github/workflows/release-package.yml | 4 ++++ .github/workflows/static-analysis.yml | 1 + .github/workflows/tests.yml | 4 +++- .gitmodules | 13 ++++++++----- lib/webauthn.git | 1 + remappings.txt | 1 + 7 files changed, 20 insertions(+), 6 deletions(-) create mode 160000 lib/webauthn.git diff --git a/.github/workflows/quality-checks.yml b/.github/workflows/quality-checks.yml index 21f2014..8b53009 100644 --- a/.github/workflows/quality-checks.yml +++ b/.github/workflows/quality-checks.yml @@ -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 @@ -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 diff --git a/.github/workflows/release-package.yml b/.github/workflows/release-package.yml index 704c0bc..8716c93 100644 --- a/.github/workflows/release-package.yml +++ b/.github/workflows/release-package.yml @@ -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 @@ -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 diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index c4a4d2c..eb079a6 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -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 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 3690485..b124a6b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -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 @@ -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" diff --git a/.gitmodules b/.gitmodules index 68102d3..f9e00bf 100644 --- a/.gitmodules +++ b/.gitmodules @@ -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 diff --git a/lib/webauthn.git b/lib/webauthn.git new file mode 160000 index 0000000..cd44a50 --- /dev/null +++ b/lib/webauthn.git @@ -0,0 +1 @@ +Subproject commit cd44a5068eaa9f83f67ea61c7228a865d78616e0 diff --git a/remappings.txt b/remappings.txt index 0b387fd..b71602c 100644 --- a/remappings.txt +++ b/remappings.txt @@ -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/ From 503363d44d892f1916867d2fb9c0290424424614 Mon Sep 17 00:00:00 2001 From: qd-qd Date: Wed, 10 Jan 2024 16:51:54 +0100 Subject: [PATCH 2/3] =?UTF-8?q?=E2=9C=A8=20add=20verify=20to=20SignerVault?= =?UTF-8?q?WebAuthnP256R1=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The library is now able to verify webauthn payload, making the vault library complete. --- src/SignerVaultWebAuthnP256R1.sol | 63 ++++++++++++++ .../signerVaultWebAuthnP256R1.t.sol | 87 +++++++++++++++++++ .../signerVaultWebAuthnP256R1.tree | 2 + 3 files changed, 152 insertions(+) diff --git a/src/SignerVaultWebAuthnP256R1.sol b/src/SignerVaultWebAuthnP256R1.sol index f906cb8..3d12bb4 100644 --- a/src/SignerVaultWebAuthnP256R1.sol +++ b/src/SignerVaultWebAuthnP256R1.sol @@ -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. @@ -164,4 +165,66 @@ library SignerVaultWebAuthnP256R1 { 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 + ); + } } diff --git a/test/unit/SignerVault/signerVaultWebAuthnP256R1.t.sol b/test/unit/SignerVault/signerVaultWebAuthnP256R1.t.sol index 81e0044..a7f6f19 100644 --- a/test/unit/SignerVault/signerVaultWebAuthnP256R1.t.sol +++ b/test/unit/SignerVault/signerVaultWebAuthnP256R1.t.sol @@ -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 @@ -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 + ); + } } diff --git a/test/unit/SignerVault/signerVaultWebAuthnP256R1.tree b/test/unit/SignerVault/signerVaultWebAuthnP256R1.tree index fc776d3..4dcc6ba 100644 --- a/test/unit/SignerVault/signerVaultWebAuthnP256R1.tree +++ b/test/unit/SignerVault/signerVaultWebAuthnP256R1.tree @@ -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 From 7a636b0d572d35a2178a5709828d2aad9b999425 Mon Sep 17 00:00:00 2001 From: qd-qd Date: Wed, 10 Jan 2024 17:57:30 +0100 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=91=B7=20use=20the=20ir-minimum=20fla?= =?UTF-8?q?g=20for=20the=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The introduction of the fat webauthn library throw stack error issue when running the coverage command. This commit use the experimental `ir-minimum` flag to avoid this --- .github/workflows/tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b124a6b..7adf330 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -81,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"