search

davidearl / webauthn

An implementation of webauthn in PHP on the server side (e.g Yubico 2 and Google Titan keys)
https://webauthn.davidearl.uk
MIT License
129 stars 24 forks source link

Changed authenticatorData flag checking to properly check flag #11

Closed ikkerens closed 5 years ago

ikkerens commented 5 years ago

After attempting to use the lib on Edge I had noticed all my requests were being denied with cannot decode key response (2c). Upon investigation I had noticed that the lib checked directly against the variable not being 0x1 and errorring consequently.

However, this would mean that if any other flags would passed, this check would immediately fail, as is the case with Microsoft Edge. In my attempts, I had found that it would return the value: 5, consequently 0b101. According to the spec this means that the following flags have been set:

The attached commit changes the detection to only check for bit 0 being set, as opposed to checking for equality.

davidearl commented 5 years ago

Thanks, I’ll add this at the weekend. Given I translated this from the JavaScript examples, rather suggests one of those may be wrong, but I’ll cross check back with that.

On Mon, 15 Apr 2019 at 15:02, Rens Rikkerink notifications@github.com wrote:

After attempting to use the lib on Edge I had noticed all my requests were being denied with cannot decode key response (2c). Upon investigation I had noticed that the lib checked directly against the variable not being 0x1 and errorring consequently.

However, this would mean that if any other flags would passed, this check would immediately fail, as is the case with Microsoft Edge. In my attempts, I had found that it would return the value: 5, consequently 0b101. According to the spec https://www.w3.org/TR/webauthn/#authenticator-data this means that the following flags have been set:

  • User Present (UP) (the one the library wants to check for)
  • User Verified (UV) (probably as a consequence of me entering the PIN)

The attached commit changes the detection to only quick for bit 0 being set, as opposed to checking for equality.

You can view, comment on, or merge this pull request online at:

https://github.com/davidearl/webauthn/pull/11 Commit Summary

  • Changed authenticatorData flag checking to properly check flag

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/davidearl/webauthn/pull/11, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAIGwCDReLX8BlswxHZbhWE4D_AxTYLks5vhIZngaJpZM4cwDjv .

davidearl commented 5 years ago

Sorry for the delay in adding this to the main source

davidearl commented 5 years ago

This test seems to match the current JS implementation example (at https://github.com/jcjones/webauthn.bin.coffee/blob/master/driver.js ). Whether this was my mistake in translating it or has been changed since I haven't checked - most likely my misreading of the original code. However I see that there have been a few changes to accommodate Android in there which may need carrying forward to my PHP, so I'll review those at some stage.