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.

Sign up for GitHub

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

Jump to bottom

chore: improve natspec and clean code #10

Merged
merged 1 commit into from
Mar 8, 2024
Merged

chore: improve natspec and clean code #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
166 changes: 84 additions & 82 deletions src/WebAuthn.sol
View file
Open in desktop
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
Open in desktop
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