Webauthn verifier #25
Conversation
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
src/WebAuthn.sol
Outdated
| } | ||
|
|
||
| // Check the UV bit | ||
| if (requireUserVerification && (flags & 0x04) != 0x04) { |
There was a problem hiding this comment.
so we are including that check after all?
There was a problem hiding this comment.
I think in Daimo usage we should pass this requireUserVerification = false. It should be the passkey authenticator's job to enforce such conditions (and the good ones we use seem to do it properly already), just leaving this here in case other users want it for any reason, as it also means we have feature parity for current users of FCL's Webauthn implementation.
src/WebAuthn.sol
Outdated
| '"' | ||
| ); | ||
|
|
||
| if (!contains(challenge_property, clientDataJSON, challengeLocation)) { |
| return true; | ||
| } | ||
|
|
||
| function verifySignature( |
There was a problem hiding this comment.
add comment. sommething lke
/**
* Verifies a Webauthn P256 signature. Specifically:
* - Verifies that authenticatorData (which comes from the authenticator, such as iCloud Keychain) indicates a secure signature. If requireUserVerification is set, checks that the authenticator attests that the user was present for signing (such as via FaceID).
* - Verifies that the client JSON contains the given challenge. Ignores everything else in the client JSON.
* - Finally, verifies that (r, s) constitute a valid signature over both the authenicatorData and client JSON, for public key (x, y).
*/There was a problem hiding this comment.
I've added a much more detailed commenting in this file. Let me know if it looks good.
| } | ||
| } | ||
|
|
||
| return string(result); |
|
|
||
| library Base64URL { | ||
| function encode(bytes memory data) public pure returns (string memory) { | ||
| string memory strb64 = Base64.encode(data); |
There was a problem hiding this comment.
alternative approach to consider:
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Base64.sol
^ base64 is pretty small. i think we could turn it into Base64URL by:
- changing two characters in the table
- removing the lines about adding equals signs
- computing a correct (no equals padding) length for the result string
...so ours would be a small diff on theirs and actually shorter
wonder if OZ would include it in a future release, would fit imo
There was a problem hiding this comment.
no strong opinion either way, but I slightly prefer this so we're not touching any assembly / hand wrangling of bytes ourselves. Will go with this for now unless you feel strongly.
| ); | ||
|
|
||
| // Write to JSON | ||
| const filepath = "./vectors_scure_base64url.jsonl"; |
nalinbhardwaj
force-pushed
the
nibnalin/webauthn
branch
from
2f77f6c to
59e8010
Compare
100% coverage |
| bytes memory b64 = bytes(strb64); | ||
|
|
||
| // count and ignore all "=" symbols from the end of the string | ||
| uint256 equalsCount = 0; |
There was a problem hiding this comment.
given that there can be at most 2 equals at the end we could also say
uint256 equalsCount = 0; // Base64 can end with "=" or "=="; Base64URL has no padding.
if (b64[b64.length - 2] == "=") equalsCount = 2;
else if (b64[b64.length - 1] == "=") equalsCount = 1;
Currently manually tested using the input generated in scratchpad daimo-eth/daimo#330