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.

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

Changed authenticatorData flag checking to properly check flag #11

Merged
merged 1 commit into from
May 7, 2019
Merged

Changed authenticatorData flag checking to properly check flag #11

merged 1 commit into from
May 7, 2019

Conversation

ikkerens
Copy link
Contributor

@ikkerens ikkerens commented Apr 15, 2019

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:

  • 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 check for bit 0 being set, as opposed to checking for equality.

@davidearl
Copy link
Owner

davidearl commented Apr 15, 2019 via email

@davidearl davidearl merged commit fbdd894 into davidearl:master May 7, 2019
@davidearl
Copy link
Owner

Sorry for the delay in adding this to the main source

@davidearl
Copy link
Owner

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.

@ikkerens ikkerens deleted the patch-1 branch May 7, 2019 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants