chore: add more comments to webauthn verify

Also move the test function to a test contract

Author: cpb8010

src/test/TestWebAuthValidator.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import { WebAuthValidator } from "../validators/WebAuthValidator.sol";
+
+contract WebAuthValidatorTest is WebAuthValidator {
+  /// @notice Verifies a message using the P256 curve.
+  /// @dev Useful for testing the P256 precompile
+  /// @param message The sha256 hash of the authenticator hash and hashed client data
+  /// @param rs The signature to validate (r, s) from the signed message
+  /// @param pubKey The public key to validate the signature against (x, y)
+  /// @return valid true if the signature is valid
+  function p256Verify(
+    bytes32 message,
+    bytes32[2] calldata rs,
+    bytes32[2] calldata pubKey
+  ) external view returns (bool valid) {
+    valid = rawVerify(message, rs, pubKey);
+  }
+}

src/validators/WebAuthValidator.sol

@@ -20,6 +20,8 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
   using JSONParserLib for string;
 
   address private constant P256_VERIFIER = address(0x100);
+
+  // check for secure validation: bit 0 = 1 (user present), bit 2 = 1 (user verified)
   bytes1 private constant AUTH_DATA_MASK = 0x05;
   bytes32 private constant LOW_S_MAX = 0x7fffffff800000007fffffffffffffffde737d56d38bcf4279dce5617e3192a8;
   bytes32 private constant HIGH_R_MAX = 0xffffffff00000000ffffffffffffffffbce6faada7179e84f3b9cac2fc632551;
@@ -103,17 +105,19 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
       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) {
       return false;
     }
 
-    // check if the flags are set
+    // https://developer.mozilla.org/en-US/docs/Web/API/Web_Authentication_API/Authenticator_data#attestedcredentialdata
     if (authenticatorData[32] & AUTH_DATA_MASK != AUTH_DATA_MASK) {
       return false;
     }
 
-    // parse out the important fields (type, challenge, origin, crossOrigin): https://goo.gl/yabPex
+    // parse out the required fields (type, challenge, crossOrigin): https://goo.gl/yabPex
     JSONParserLib.Item memory root = JSONParserLib.parse(clientDataJSON);
+    // challenge should contain the transaction hash, ensuring that the transaction is signed
     string memory challenge = root.at('"challenge"').value().decodeString();
     bytes memory challengeData = Base64.decode(challenge);
     if (challengeData.length != 32) {
@@ -123,11 +127,14 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
       return false;
     }
 
+    // type ensures the signature was created from a validation request
     string memory type_ = root.at('"type"').value().decodeString();
     if (!Strings.equal("webauthn.get", type_)) {
       return false;
     }
 
+    // the origin determines which key to validate against
+    // 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];
@@ -137,6 +144,10 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
       return false;
     }
 
+    // cross-origin validation is optional, but explicitly not supported.
+    // cross-origin requests would be from embedding the auth request
+    // from another domain. The current SSO setup uses a pop-up instead of
+    // an i-frame, so we're rejecting these until the implemention supports it
     JSONParserLib.Item memory crossOriginItem = root.at('"crossOrigin"');
     if (!crossOriginItem.isUndefined()) {
       string memory crossOrigin = crossOriginItem.value();
@@ -181,7 +192,7 @@ contract WebAuthValidator is VerifierCaller, IModuleValidator {
     bytes32 message,
     bytes32[2] calldata rs,
     bytes32[2] calldata pubKey
-  ) external view returns (bool valid) {
+  ) internal view returns (bool valid) {
     valid = callVerifier(P256_VERIFIER, message, rs, pubKey);
   }
 }

test/PasskeyModule.ts

@@ -5,15 +5,15 @@ import { ECDSASigValue } from "@peculiar/asn1-ecc";
 import { AsnParser } from "@peculiar/asn1-schema";
 import { bigintToBuf, bufToBigint } from "bigint-conversion";
 import { assert, expect } from "chai";
+import { randomBytes } from "crypto";
+import { parseEther, ZeroAddress } from "ethers";
 import * as hre from "hardhat";
+import { encodeAbiParameters, Hex, hexToBytes, toHex } from "viem";
 import { SmartAccount, Wallet } from "zksync-ethers";
+import { base64UrlToUint8Array } from "zksync-sso/utils";
 
-import { SsoAccount__factory, WebAuthValidator, WebAuthValidator__factory } from "../typechain-types";
+import { SsoAccount__factory, WebAuthValidator, WebAuthValidator__factory, WebAuthValidatorTest, WebAuthValidatorTest__factory } from "../typechain-types";
 import { ContractFixtures, getProvider, getWallet, LOCAL_RICH_WALLETS, logInfo, RecordedResponse } from "./utils";
-import { base64UrlToUint8Array } from "zksync-sso/utils";
-import { encodeAbiParameters, Hex, hexToBytes, toHex } from "viem";
-import { randomBytes } from "crypto";
-import { parseEther, ZeroAddress } from "ethers";
 
 /**
  * Decode from a Base64URL-encoded string to an ArrayBuffer. Best used when converting a
@@ -36,6 +36,14 @@ async function deployValidator(wallet: Wallet): Promise<WebAuthValidator> {
   return WebAuthValidator__factory.connect(await validator.getAddress(), wallet);
 }
 
+async function deployP256Tester(wallet: Wallet): Promise<WebAuthValidatorTest> {
+  const deployer: Deployer = new Deployer(hre, wallet);
+  const passkeyValidatorArtifact = await deployer.loadArtifact("WebAuthValidatorTest");
+
+  const validator = await deployer.deploy(passkeyValidatorArtifact, []);
+  return WebAuthValidatorTest__factory.connect(await validator.getAddress(), wallet);
+}
+
 /**
  * COSE Keys
  *
@@ -338,9 +346,6 @@ function encodeFatSignature(
     clientDataJSON: string;
     signature: string;
   },
-  contracts: {
-    passkey: string;
-  },
 ) {
   const signature = unwrapEC2Signature(base64UrlToUint8Array(passkeyResponse.signature));
   return encodeAbiParameters(
@@ -358,7 +363,7 @@ function encodeFatSignature(
 }
 
 async function rawVerify(
-  passkeyValidator: WebAuthValidator,
+  passkeyValidator: WebAuthValidatorTest,
   authenticatorData: string,
   clientData: string,
   b64SignedChallange: string,
@@ -370,7 +375,7 @@ async function rawVerify(
   const rs = unwrapEC2Signature(toBuffer(b64SignedChallange));
   const publicKeys = await getPublicKey(publicKeyEs256Bytes);
 
-  return await passkeyValidator.rawVerify(hashedData, rs, publicKeys);
+  return await passkeyValidator.p256Verify(hashedData, rs, publicKeys);
 }
 
 async function verifyKeyStorage(
@@ -390,11 +395,11 @@ function encodeKeyFromHex(hexStrings: [Hex, Hex], domain: string) {
   // the same as the ethers: new AbiCoder().encode(["bytes32[2]", "string"], [bytes, domain]);
   return encodeAbiParameters(
     [
-      { name: 'publicKeys', type: 'bytes32[2]' },
-      { name: 'domain', type: 'string' },
+      { name: "publicKeys", type: "bytes32[2]" },
+      { name: "domain", type: "string" },
     ],
-    [hexStrings, domain]
-  )
+    [hexStrings, domain],
+  );
 }
 
 function encodeKeyFromBytes(bytes: [Uint8Array, Uint8Array], domain: string) {
@@ -428,8 +433,8 @@ async function validateSignatureTest(
     { name: "clientDataJson", type: "string" },
     { name: "rs", type: "bytes32[2]" },
   ],
-    [toHex(authData), sampleClientString, [toHex(rNormalization(generatedSignature.r)), toHex(sNormalization(generatedSignature.s))]]
-  )
+  [toHex(authData), sampleClientString, [toHex(rNormalization(generatedSignature.r)), toHex(sNormalization(generatedSignature.s))]],
+  );
   return await passkeyValidator.validateSignature(transactionHash, fatSignature);
 }
 
@@ -480,7 +485,7 @@ describe("Passkey validation", function () {
       const receipt = await fundTx.wait();
       expect(receipt.status).to.eq(1, "send funds to proxy account");
 
-      return { passKeyModuleContract, sampleDomain, proxyAccountAddress, generatedR1Key, passKeyModuleAddress }
+      return { passKeyModuleContract, sampleDomain, proxyAccountAddress, generatedR1Key, passKeyModuleAddress };
     }
 
     it("should deploy proxy account via factory", async () => {
@@ -523,16 +528,16 @@ describe("Passkey validation", function () {
           ], [
             toHex(authData),
             sampleClientString,
-            [toHex(normalizeR(generatedSignature.r)), toHex(normalizeS(generatedSignature.s))]
-          ])
+            [toHex(normalizeR(generatedSignature.r)), toHex(normalizeS(generatedSignature.s))],
+          ]);
 
           const moduleSignature = encodeAbiParameters(
             [{ name: "signature", type: "bytes" }, { name: "moduleAddress", type: "address" }, { name: "validatorData", type: "bytes" }],
             [fatSignature, passKeyModuleAddress, "0x"]);
           return moduleSignature;
         },
         address: proxyAccountAddress,
-        secret: wallet.privateKey, //generatedR1Key.privateKey,
+        secret: wallet.privateKey, // generatedR1Key.privateKey,
       }, provider);
 
       const aaTransaction = {
@@ -653,7 +658,6 @@ describe("Passkey validation", function () {
           clientDataJSON: ethersResponse.clientData,
           signature: ethersResponse.b64SignedChallenge,
         },
-        { passkey: publicKeys[0] },
       );
 
       const initData = encodeKeyFromHex(publicKeys, "http://localhost:5173");
@@ -671,14 +675,14 @@ describe("Passkey validation", function () {
   // fully expand the raw validation to compare step by step
   describe("P256 precompile comparison", () => {
     it("should verify passkey", async function () {
-      const passkeyValidator = await deployValidator(wallet);
+      const passkeyValidator = await deployP256Tester(wallet);
 
       // 37 bytes
       const authenticatorData = "SZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2MFAAAABQ";
-      const clientData =
-        "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiZFhPM3ctdWdycS00SkdkZUJLNDFsZFk1V2lNd0ZORDkiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUxNzMiLCJjcm9zc09yaWdpbiI6ZmFsc2UsIm90aGVyX2tleXNfY2FuX2JlX2FkZGVkX2hlcmUiOiJkbyBub3QgY29tcGFyZSBjbGllbnREYXRhSlNPTiBhZ2FpbnN0IGEgdGVtcGxhdGUuIFNlZSBodHRwczovL2dvby5nbC95YWJQZXgifQ";
-      const b64SignedChallenge =
-        "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A";
+      const clientData
+        = "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiZFhPM3ctdWdycS00SkdkZUJLNDFsZFk1V2lNd0ZORDkiLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUxNzMiLCJjcm9zc09yaWdpbiI6ZmFsc2UsIm90aGVyX2tleXNfY2FuX2JlX2FkZGVkX2hlcmUiOiJkbyBub3QgY29tcGFyZSBjbGllbnREYXRhSlNPTiBhZ2FpbnN0IGEgdGVtcGxhdGUuIFNlZSBodHRwczovL2dvby5nbC95YWJQZXgifQ";
+      const b64SignedChallenge
+        = "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A";
 
       const verifyMessage = await rawVerify(
         passkeyValidator,
@@ -691,7 +695,7 @@ describe("Passkey validation", function () {
       assert(verifyMessage == true, "valid sig");
     });
     it("should sign with new data", async function () {
-      const passkeyValidator = await deployValidator(wallet);
+      const passkeyValidator = await deployP256Tester(wallet);
       // The precompile expects the fully hashed data
       const preHashedData = await toHash(
         concat([toBuffer(ethersResponse.authenticatorData), await toHash(toBuffer(ethersResponse.clientData))]),
@@ -718,7 +722,7 @@ describe("Passkey validation", function () {
         [generatedSignature.r, generatedSignature.s],
         [generatedX, generatedY],
       );
-      const onChainGeneratedVerified = await passkeyValidator.rawVerify(
+      const onChainGeneratedVerified = await passkeyValidator.p256Verify(
         preHashedData,
         [generatedSignature.r, generatedSignature.s],
         [generatedX, generatedY],
@@ -728,7 +732,7 @@ describe("Passkey validation", function () {
         [recordedR, recordedS],
         [recordedX, recordedY],
       );
-      const onChainRecordedVerified = await passkeyValidator.rawVerify(
+      const onChainRecordedVerified = await passkeyValidator.p256Verify(
         preHashedData,
         [recordedR, recordedS],
         [recordedX, recordedY],
@@ -741,7 +745,7 @@ describe("Passkey validation", function () {
     });
 
     it("should verify other test passkey data", async function () {
-      const passkeyValidator = await deployValidator(wallet);
+      const passkeyValidator = await deployP256Tester(wallet);
 
       const verifyMessage = await rawVerify(
         passkeyValidator,
@@ -755,10 +759,10 @@ describe("Passkey validation", function () {
     });
 
     it("should fail when signature is bad", async function () {
-      const passkeyValidator = await deployValidator(wallet);
+      const passkeyValidator = await deployP256Tester(wallet);
 
-      const b64SignedChallenge =
-        "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A";
+      const b64SignedChallenge
+        = "MEUCIQCYrSUCR_QUPAhvRNUVfYiJC2JlOKuqf4gx7i129n9QxgIgaY19A9vAAObuTQNs5_V9kZFizwRpUFpiRVW_dglpR2A";
       const verifyMessage = await rawVerify(
         passkeyValidator,
         ethersResponse.authenticatorData,
@@ -899,10 +903,10 @@ describe("Passkey validation", function () {
       const partialClientObject = {
         challenge: "jBBiiOGt1aSBy1WAuRGxqU7YzRM5oWpMA9g8MKydjPI",
       };
-      const duplicatedClientString =
-        JSON.stringify(sampleClientObject).slice(0, -1) +
-        "," +
-        JSON.stringify(partialClientObject).slice(1);
+      const duplicatedClientString
+        = JSON.stringify(sampleClientObject).slice(0, -1)
+        + ","
+        + JSON.stringify(partialClientObject).slice(1);
       const authData = toBuffer(ethersResponse.authenticatorData);
       const transactionHash = Buffer.from(sampleClientObject.challenge, "base64url");
       const isValidSignature = await validateSignatureTest(