search

coinbase / smart-wallet

MIT License
258 stars 47 forks source link

Passkey signatures failing for some hashes #41

Closed jamesmccomish closed 3 months ago

jamesmccomish commented 3 months ago

I was running tests on a fork of the smart wallet and getting errors for some tests.

I was following the same signing procedure as in this repo, and eventually found that the signature malleability check in WebAuthn-Sol was returning the error due to s being too large.

This test shows that for some hashes the values returned from vm.signP256 are causing the problem. I added a test testValidateSignatureWithPasskeySignerWithSCheck which passes when we take the compliment of s.

Is this something that should be handled by signP256? Maybe I have misunderstood what webAuthn.messageHash should be and that is causing the wrong signatures?

wilsoncusack commented 3 months ago

Hey, thanks for this! Here is how you can normalize s values control for this. We'd expect clients to do this normalization.

jamesmccomish commented 3 months ago

Ah thanks, yea I see it is expected then. Will take it into account from here