Skip to content

Commit

Permalink
chore: improve natspec and clean code (#10)
Browse files Browse the repository at this point in the history
  • Loading branch information
@xenoliss
xenoliss authored Mar 8, 2024
1 parent f5da6b8 commit b7a1823
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 114 deletions.
166 changes: 84 additions & 82 deletions src/WebAuthn.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,107 +3,110 @@ pragma solidity ^0.8.0;

import {Base64} from "solady/utils/Base64.sol";
import {FCL_ecdsa} from "FreshCryptoLib/FCL_ecdsa.sol";
import {FCL_Elliptic_ZZ} from "FreshCryptoLib/FCL_elliptic.sol";
import {LibString} from "solady/utils/LibString.sol";

/// @title WebAuthn
///
/// @notice A library for verifying WebAuthn Authentication Assertions, built off the work
/// of Daimo.
/// of Daimo.
///
/// @dev Attempts to use the RIP-7212 precompile for signature verification.
/// If precompile verification fails, it falls back to FreshCryptoLib.
/// If precompile verification fails, it falls back to FreshCryptoLib.
///
/// @author Coinbase (https://github.com/base-org/webauthn-sol)
/// @author Daimo (https://github.com/daimo-eth/p256-verifier/blob/master/src/WebAuthn.sol)
library WebAuthn {
using LibString for string;

struct WebAuthnAuth {
/// @dev https://www.w3.org/TR/webauthn-2/#dom-authenticatorassertionresponse-authenticatordata
/// @dev The WebAuthn authenticator data.
/// See https://www.w3.org/TR/webauthn-2/#dom-authenticatorassertionresponse-authenticatordata.
bytes authenticatorData;
/// @dev https://www.w3.org/TR/webauthn-2/#dom-authenticatorresponse-clientdatajson
/// @dev The WebAuthn client data JSON.
/// See https://www.w3.org/TR/webauthn-2/#dom-authenticatorresponse-clientdatajson.
string clientDataJSON;
/// The index at which "challenge":"..." occurs in clientDataJSON
/// @dev The index at which "challenge":"..." occurs in `clientDataJSON`.
uint256 challengeIndex;
/// The index at which "type":"..." occurs in clientDataJSON
/// @dev The index at which "type":"..." occurs in `clientDataJSON`.
uint256 typeIndex;
/// @dev The r value of secp256r1 signature
uint256 r;
/// @dev The s value of secp256r1 signature
uint256 s;
}

/// @dev Bit 0, User present bit in authenticatorData
bytes1 constant AUTH_DATA_FLAGS_UP = 0x01;
/// @dev Bit 2, User verified bit in authenticatorData
bytes1 constant AUTH_DATA_FLAGS_UV = 0x04;
/// @dev secp256r1 curve order / 2 for malleability check
uint256 constant P256_N_DIV_2 = 57896044605178124381348723474703786764998477612067880171211129530534256022184;
address constant VERIFIER = address(0x100);
bytes32 constant EXPECTED_TYPE_HASH = keccak256('"type":"webauthn.get"');

/**
* @notice Verifies a Webauthn Authentication Assertion as described
* in https://www.w3.org/TR/webauthn-2/#sctn-verifying-assertion.
*
* @dev We do not verify all the steps as described in the specification, only ones relevant
* to our context. Please carefully read through this list before usage.
* Specifically, we do verify the following:
* - Verify that authenticatorData (which comes from the authenticator,
* such as iCloud Keychain) indicates a well-formed assertion with the user present bit set.
* If requireUserVerification is set, checks that the authenticator enforced
* user verification. User verification should be required if,
* and only if, options.userVerification is set to required in the request
* - Verifies that the client JSON is of type "webauthn.get", i.e. the client
* was responding to a request to assert authentication.
* - Verifies that the client JSON contains the requested challenge.
* - Finally, verifies that (r, s) constitute a valid signature over both
* the authenicatorData and client JSON, for public key (x, y).
*
* We make some assumptions about the particular use case of this verifier,
* so we do NOT verify the following:
* - Does NOT verify that the origin in the clientDataJSON matches the
* Relying Party's origin: It is considered the authenticator's
* responsibility to ensure that the user is interacting with the correct
* RP. This is enforced by most high quality authenticators properly,
* particularly the iCloud Keychain and Google Password Manager were
* tested.
* - Does NOT verify That c.topOrigin is well-formed: We assume c.topOrigin
* would never be present, i.e. the credentials are never used in a
* cross-origin/iframe context. The website/app set up should disallow
* cross-origin usage of the credentials. This is the default behaviour for
* created credentials in common settings.
* - Does NOT verify that the rpIdHash in authData is the SHA-256 hash of an
* RP ID expected by the Relying Party: This means that we rely on the
* authenticator to properly enforce credentials to be used only by the
* correct RP. This is generally enforced with features like Apple App Site
* Association and Google Asset Links. To protect from edge cases in which
* a previously-linked RP ID is removed from the authorised RP IDs,
* we recommend that messages signed by the authenticator include some
* expiry mechanism.
* - Does NOT verify the credential backup state: This assumes the credential
* backup state is NOT used as part of Relying Party business logic or
* policy.
* - Does NOT verify the values of the client extension outputs: This assumes
* that the Relying Party does not use client extension outputs.
* - Does NOT verify the signature counter: Signature counters are intended
* to enable risk scoring for the Relying Party. This assumes risk scoring
* is not used as part of Relying Party business logic or policy.
* - Does NOT verify the attestation object: This assumes that
* response.attestationObject is NOT present in the response, i.e. the
* RP does not intend to verify an attestation.
*
* @param challenge The challenge that was provided by the relying party
* @param requireUserVerification A boolean indicating whether user verification is required
* @param webAuthnAuth The WebAuthnAuth struct containing the authenticatorData, origin, crossOriginAndRemainder, r, and s
* @param x The x coordinate of the public key
* @param y The y coordinate of the public key
* @return A boolean indicating authentication assertion passed validation
*/
function verify(
bytes memory challenge,
bool requireUserVerification,
WebAuthnAuth memory webAuthnAuth,
uint256 x,
uint256 y
) internal view returns (bool) {
/// @dev Bit 0 of the authenticator data struct, corresponding to the "User Present" bit.
/// See https://www.w3.org/TR/webauthn-2/#flags.
bytes1 private constant AUTH_DATA_FLAGS_UP = 0x01;

/// @dev Bit 2 of the authenticator data struct, corresponding to the "User Verified" bit.
/// See https://www.w3.org/TR/webauthn-2/#flags.
bytes1 private constant AUTH_DATA_FLAGS_UV = 0x04;

/// @dev Secp256r1 curve order / 2 used as guard to prevent signature malleability issue.
uint256 private constant P256_N_DIV_2 = FCL_Elliptic_ZZ.n / 2;

/// @dev The precompiled contract address to use for signature verification in the “secp256r1” elliptic curve.
/// See https://github.com/ethereum/RIPs/blob/master/RIPS/rip-7212.md.
address private constant VERIFIER = address(0x100);

/// @dev The expected type (hash) in the client data JSON when verifying assertion signatures.
/// See https://www.w3.org/TR/webauthn-2/#dom-collectedclientdata-type
bytes32 private constant EXPECTED_TYPE_HASH = keccak256('"type":"webauthn.get"');

///
/// @notice Verifies a Webauthn Authentication Assertion as described
/// in https://www.w3.org/TR/webauthn-2/#sctn-verifying-assertion.
///
/// @dev We do not verify all the steps as described in the specification, only ones relevant to our context.
/// Please carefully read through this list before usage.
///
/// Specifically, we do verify the following:
/// - Verify that authenticatorData (which comes from the authenticator, such as iCloud Keychain) indicates
/// a well-formed assertion with the user present bit set. If `requireUV` is set, checks that the authenticator
/// enforced user verification. User verification should be required if, and only if, options.userVerification
/// is set to required in the request.
/// - Verifies that the client JSON is of type "webauthn.get", i.e. the client was responding to a request to
/// assert authentication.
/// - Verifies that the client JSON contains the requested challenge.
/// - Verifies that (r, s) constitute a valid signature over both the authenicatorData and client JSON, for public
/// key (x, y).
///
/// We make some assumptions about the particular use case of this verifier, so we do NOT verify the following:
/// - Does NOT verify that the origin in the `clientDataJSON` matches the Relying Party's origin: tt is considered
/// the authenticator's responsibility to ensure that the user is interacting with the correct RP. This is
/// enforced by most high quality authenticators properly, particularly the iCloud Keychain and Google Password
/// Manager were tested.
/// - Does NOT verify That `topOrigin` in `clientDataJSON` is well-formed: We assume it would never be present, i.e.
/// the credentials are never used in a cross-origin/iframe context. The website/app set up should disallow
/// cross-origin usage of the credentials. This is the default behaviour for created credentials in common settings.
/// - Does NOT verify that the `rpIdHash` in `authenticatorData` is the SHA-256 hash of the RP ID expected by the Relying
/// Party: this means that we rely on the authenticator to properly enforce credentials to be used only by the correct RP.
/// This is generally enforced with features like Apple App Site Association and Google Asset Links. To protect from
/// edge cases in which a previously-linked RP ID is removed from the authorised RP IDs, we recommend that messages
/// signed by the authenticator include some expiry mechanism.
/// - Does NOT verify the credential backup state: this assumes the credential backup state is NOT used as part of Relying
/// Party business logic or policy.
/// - Does NOT verify the values of the client extension outputs: this assumes that the Relying Party does not use client
/// extension outputs.
/// - Does NOT verify the signature counter: signature counters are intended to enable risk scoring for the Relying Party.
/// This assumes risk scoring is not used as part of Relying Party business logic or policy.
/// - Does NOT verify the attestation object: this assumes that response.attestationObject is NOT present in the response,
/// i.e. the RP does not intend to verify an attestation.
///
/// @param challenge The challenge that was provided by the relying party.
/// @param requireUV A boolean indicating whether user verification is required.
/// @param webAuthnAuth The `WebAuthnAuth` struct.
/// @param x The x coordinate of the public key.
/// @param y The y coordinate of the public key.
///
/// @return `true` if the authentication assertion passed validation, else `false`.
function verify(bytes memory challenge, bool requireUV, WebAuthnAuth memory webAuthnAuth, uint256 x, uint256 y)
internal
view
returns (bool)
{
if (webAuthnAuth.s > P256_N_DIV_2) {
// guard against signature malleability
return false;
Expand Down Expand Up @@ -134,8 +137,7 @@ library WebAuthn {
}

// 17. If user verification is required for this assertion, verify that the User Verified bit of the flags in authData is set.
if (requireUserVerification && (webAuthnAuth.authenticatorData[32] & AUTH_DATA_FLAGS_UV) != AUTH_DATA_FLAGS_UV)
{
if (requireUV && (webAuthnAuth.authenticatorData[32] & AUTH_DATA_FLAGS_UV) != AUTH_DATA_FLAGS_UV) {
return false;
}

Expand Down
49 changes: 17 additions & 32 deletions test/Webauthn.fuzz.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.0;

import {FCL_ecdsa} from "FreshCryptoLib/FCL_ecdsa.sol";
import {FCL_Elliptic_ZZ} from "FreshCryptoLib/FCL_elliptic.sol";

import {WebAuthn} from "../src/WebAuthn.sol";

Expand All @@ -12,14 +13,16 @@ contract WebAuthnFuzzTest is Test {

string constant testFile = "/test/fixtures/assertions_fixture.json";

uint256 private constant P256_N_DIV_2 = FCL_Elliptic_ZZ.n / 2;

/// @dev `WebAuthn.verify` should return `false` when `s` is above P256_N_DIV_2.
function test_Verify_ShoulReturnFalse_WhenSAboveP256_N_DIV_2() public {
string memory rootPath = vm.projectRoot();
string memory path = string.concat(rootPath, testFile);
string memory json = vm.readFile(path);
uint256 count = abi.decode(json.parseRaw(".count"), (uint256));

for (uint256 i = 0; i < count; i++) {
for (uint256 i; i < count; i++) {
(
string memory jsonCaseSelector,
bytes memory challenge,
Expand All @@ -32,17 +35,11 @@ contract WebAuthnFuzzTest is Test {
console.log("Veryfing", jsonCaseSelector);

// Only interested in s > P256_N_DIV_2 cases.
if (webAuthnAuth.s <= WebAuthn.P256_N_DIV_2) {
if (webAuthnAuth.s <= P256_N_DIV_2) {
webAuthnAuth.s = FCL_ecdsa.n - webAuthnAuth.s;
}

bool res = WebAuthn.verify({
challenge: challenge,
requireUserVerification: uv,
webAuthnAuth: webAuthnAuth,
x: x,
y: y
});
bool res = WebAuthn.verify({challenge: challenge, requireUV: uv, webAuthnAuth: webAuthnAuth, x: x, y: y});

// Assert the verification failed to guard against signature malleability.
assertEq(res, false, string.concat("Failed on ", jsonCaseSelector));
Expand All @@ -58,7 +55,7 @@ contract WebAuthnFuzzTest is Test {
string memory json = vm.readFile(path);
uint256 count = abi.decode(json.parseRaw(".count"), (uint256));

for (uint256 i = 0; i < count; i++) {
for (uint256 i; i < count; i++) {
(
string memory jsonCaseSelector,
bytes memory challenge,
Expand All @@ -71,20 +68,14 @@ contract WebAuthnFuzzTest is Test {
console.log("Veryfing", jsonCaseSelector);

// Only interested in s <= P256_N_DIV_2 case
if (webAuthnAuth.s > WebAuthn.P256_N_DIV_2) {
if (webAuthnAuth.s > P256_N_DIV_2) {
webAuthnAuth.s = FCL_ecdsa.n - webAuthnAuth.s;
}

// Unset the `up` flag.
webAuthnAuth.authenticatorData[32] = webAuthnAuth.authenticatorData[32] & bytes1(0xfe);

bool res = WebAuthn.verify({
challenge: challenge,
requireUserVerification: uv,
webAuthnAuth: webAuthnAuth,
x: x,
y: y
});
bool res = WebAuthn.verify({challenge: challenge, requireUV: uv, webAuthnAuth: webAuthnAuth, x: x, y: y});

// Assert the verification failed because the `up` flag was not set.
assertEq(res, false, string.concat("Failed on ", jsonCaseSelector));
Expand All @@ -93,15 +84,15 @@ contract WebAuthnFuzzTest is Test {
}
}

/// @dev `WebAuthn.verify` should return `false` when `requireUserVerification` is `true` but the
/// @dev `WebAuthn.verify` should return `false` when `requireUV` is `true` but the
/// authenticator did not set the `uv` flag.
function test_Verify_ShoulReturnFalse_WhenUserVerifictionIsRequiredButTestWasNotPerformed() public {
string memory rootPath = vm.projectRoot();
string memory path = string.concat(rootPath, testFile);
string memory json = vm.readFile(path);
uint256 count = abi.decode(json.parseRaw(".count"), (uint256));

for (uint256 i = 0; i < count; i++) {
for (uint256 i; i < count; i++) {
(
string memory jsonCaseSelector,
bytes memory challenge,
Expand All @@ -118,13 +109,13 @@ contract WebAuthnFuzzTest is Test {
continue;
}

if (webAuthnAuth.s > WebAuthn.P256_N_DIV_2) {
if (webAuthnAuth.s > P256_N_DIV_2) {
webAuthnAuth.s = FCL_ecdsa.n - webAuthnAuth.s;
}

bool res = WebAuthn.verify({
challenge: challenge,
requireUserVerification: true, // Set UV to required to ensure false is returned
requireUV: true, // Set UV to required to ensure false is returned
webAuthnAuth: webAuthnAuth,
x: x,
y: y
Expand All @@ -137,7 +128,7 @@ contract WebAuthnFuzzTest is Test {
}
}

/// @dev `WebAuthn.verify` should return `true` when `s` is below `P256_N_DIV_2` and `requireUserVerification`
/// @dev `WebAuthn.verify` should return `true` when `s` is below `P256_N_DIV_2` and `requireUV`
/// "matches" with the `uv` flag set by the authenticator.
function test_Verify_ShoulReturnTrue_WhenSBelowP256_N_DIV_2() public {
string memory rootPath = vm.projectRoot();
Expand All @@ -146,7 +137,7 @@ contract WebAuthnFuzzTest is Test {

uint256 count = abi.decode(json.parseRaw(".count"), (uint256));

for (uint256 i = 0; i < count; i++) {
for (uint256 i; i < count; i++) {
(
string memory jsonCaseSelector,
bytes memory challenge,
Expand All @@ -159,17 +150,11 @@ contract WebAuthnFuzzTest is Test {
console.log("Veryfing", jsonCaseSelector);

// Only interested in s <= P256_N_DIV_2 cases
if (webAuthnAuth.s > WebAuthn.P256_N_DIV_2) {
if (webAuthnAuth.s > P256_N_DIV_2) {
webAuthnAuth.s = FCL_ecdsa.n - webAuthnAuth.s;
}

bool res = WebAuthn.verify({
challenge: challenge,
requireUserVerification: uv,
webAuthnAuth: webAuthnAuth,
x: x,
y: y
});
bool res = WebAuthn.verify({challenge: challenge, requireUV: uv, webAuthnAuth: webAuthnAuth, x: x, y: y});

// Assert the verification succeeded.
assertEq(res, true, string.concat("Failed on ", jsonCaseSelector));
Expand Down

0 comments on commit b7a1823

Please sign in to comment.