search

duo-labs / py_webauthn

Pythonic WebAuthn 🐍
https://duo-labs.github.io/py_webauthn
BSD 3-Clause "New" or "Revised" License
856 stars 171 forks source link

Wrong salt length when checking PS256 signatures #54

Closed jonathanverner closed 4 years ago

jonathanverner commented 5 years ago

As I understand it the COSE PS256 signature algorithm is specified in RFC8230 (see Cose Algorithms) and there it states that salt length should be 32 . However, the _verify_signature method from webauthn.py uses PSS.MAX_SIZE :

https://github.com/duo-labs/py_webauthn/blob/a48ab13c06ad2c44cdf226cc6b8483d45b350691/webauthn/webauthn.py#L1365

which leads to signatures failing to verify.

futureimperfect commented 4 years ago

Hi @jonathanverner,

It looks like you’re correct. Can you verify that signatures are verifying successfully if you set salt_length to 32? I suspect this was set to PSS.MAX_LENGTH because that’s what’s recommended in the Cryptography module’s documentation, but I believe @dsanders11 made the change so perhaps he knows better.

https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptography.hazmat.primitives.asymmetric.padding.PSS

dsanders11 commented 4 years ago

@futureimperfect, unfortunately that PR was long enough ago that any specific details in my memory are long gone. I was basing it on a combination of sources, so it's very likely something wa just lost in translation, and I didn't have a device to test PS256 with.

jonathanverner commented 4 years ago

@futureimperfect Sorry it took me so long, I was swamped with other work. Anyway, yes I checked that the signatures are validating correctly with salt length set to 32 (I've also checked the demo at https://webauthn.io/ and there it works too). I've created a pull request.

nickmooney commented 4 years ago

Fixed by merging #59! Thanks all.