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
132 stars 24 forks source link

Unused parameter ? #20

Closed hregis closed 5 years ago

hregis commented 5 years ago

Hello

you have "$username" and "$userid" for parameters with this function :

$webauthn->prepare_challenge_for_registration($username, $userid)

this is just for your database json system ? it's not use in u2f authentication if i use mysql or others ?

davidearl commented 5 years ago

In principle, a security key can display the username for which it is requesting authentication, which is why it is there. I've not heard of anything that does this.Nevertheless it is part of the spec that a username as well as id userid is part of the challenge process (actually there are three - user id, name and display name, but I used $username for both name and display name).

Unless you are doing something that only lets you in and no one else, you would surely have a user name of some kind (perhaps their email address) in whatever back end database you are using identifying who is being registered. If you really are protecting something that relies only on the key, and not them giving a user name, then I think you could put anything here IIRC, but you'd have to have some way to identify the key used. Currently this code only verifies the key against keys registered for a set of usernames, it doesn't identify an account based purely on the key used.

BTW, my apologies, this morning I accepted several pull requests which contain breaking changes. They were regularising the code using class and function name conventions, which change the locations of files, class names, function names (camel case) and the namespace (\davidearl => \Davidearl). The old code is in branch 0.1.0, but I suggest updating while the code is still relatively new.

hregis commented 5 years ago

there are no worries, I thank you for your work. What I wanted to say is that if we want to add this type of authentication in an application that already manages authentication by login / password, these parameters are useless? thank you

davidearl commented 5 years ago

The id certainly isn't useless - it is used as part of the information that is passed to the key to generate a response (otherwise the same key could be used as a second factor with any account on the same system, which defeats the point of it).

The name is required by the webauthn spec, but I have not seen it used as the spec suggests, that it is displayed to the person doing the authentication, but that's only because there aren't many kinds of key around yet I think. You do need to provide it, as the key expects to receive it. This isn't an invention of this code, it comes from the spec of what the key (or the code that accesses the key - most keys are passive I think) expects to be provided with. You might consider it useless, given the keys we currently have, but the spec demands it.

hregis commented 5 years ago

@davidearl ok thank you