Customizing displayName and potential PII leak in user handle

[mikestead]

First off thanks for the work on this library, I can see a lot of thought has gone into it and the tools around it.

I have a question regarding the username parameter when registering on the client, specifically this line.

// ID should not be directly "identifiable" for privacy concerns
id: await utils.sha256(new TextEncoder().encode(username))

https://github.com/passwordless-id/webauthn/blob/main/src/client.ts#L84

A plain client side sha256 would still be seen as PII as no secret salt was included from the server. I double checked the spec after seeing this and it confirmed it.

Relying Party MUST NOT include personally identifying information, e.g., e-mail addresses or usernames, in the user handle. This includes hash values of personally identifying information, unless the hash function is salted with salt values private to the Relying Party, since hashing does not prevent probing for guessable input values.

https://w3c.github.io/webauthn/#sctn-user-handle-privacy

If I'm correct and not misread your code, then I believe you'll need to open up the registration to allow provision of the handle + the PII (displayName, name) separately, and make it clear the handle must not be a raw username or similar.

You may want to let user's of the library know this issue as PII could possibly be leaked (may need GDPR disclosures for those businesses?) so would need clients to re-register customer passkeys? I'm not entirely sure the flows there, still getting to grips with those.

[dagnelies]

Thanks for the kind words.

Regarding the GDPR, I don't know what this has to do with it, so I'll skip that part, but you are welcome to enlighten me.

---

Concerning the displayName, it's made during registration, so during registration you would have to take as input a fixed username and a display name. From a UX perspective, I think almost everyone prefers a single field to register an account. That's an opiniated part of the lib.

---

Concerning the id, also known as "userHandle", it's a bit more messy and you are right to raise the issue.

Citing https://w3c.github.io/webauthn/#sctn-user-handle-privacy:

authenticators MAY reveal user handles without first performing user verification,

Consequently, if someone finds or steals an USB security stick, it may expose these user IDs/handles. Since these are hashed usernames, an attacker may deduct the corresponding plain usernames with rainbow tables or plain brute forcing.

What is not said, is that this id/handle can also be used to overwrite the credential on the authenticator. This can be useful in a variety of situations, from avoiding creating duplicate credentials for a same user to "resetting" it for whatever reason. Perhaps it could also be used in the future to delete the credential? (...since that's currently impossible 😕 ). Also, this is done during registration, and each user should have a single user handle, so registering multiple devices would need to first poll the server "create or get user id given username". However, the latter appears to be an even greater privacy/security issue to me.

However, if user verification is turned off (BTW, it's turned on by default in this lib but turned off by default in the standard), it might even turn out as vulnerability since the attacker also may have found out the username and has the usb key storing the credentials. I consider this a bigger issue than the privacy one.

The alternative would be to simply put some random string in the id. This would lead to multiple user handle when registering multiple devices and duplicate credentials in the case of accidental re-registration of a device. Since this against what the specs mandates (a single user id per account) I guess it would lead to other issues.

[mikestead]

I think I'm understanding the reasoning (still new to this area). Do Discoverable Credentials not cover this case where you don't want to pull a consistent opaque user id from the server?

My general feedback would be that deviating from a security related spec in any way is risky, especially for a public lib. It might be worth checking in with some people knowledgeable in this field to confirm the approach taken is ok. Otherwise you'd want to at least make it clear to people using this lib that you're not following that one piece of the spec and why. Allows them to make an informed choice.

As for GDPR, if there's a chance a company has leaked PII of their customers then they have a duty to disclose that, at least in the EU.

I'd say this one is potentially very minor if people are passing in public usernames (that are then hashed). But there would be a chance of devs passing in an email address I'd imagine. You'd want to make it clear not to do that.

Anyway this was just a heads up, I'll leave it with you and again thanks for the work on this.

[dagnelies]

I think I'm understanding the reasoning (still new to this area). Do Discoverable Credentials not cover this case where you don't want to pull a consistent opaque user id from the server?

It's unrelated. "Discoverable" means you can trigger a browser/platform specific UI (if available) to pick one of the existing discoverable credentials.

My general feedback would be that deviating from a security related spec in any way is risky, especially for a public lib. It might be worth checking in with some people knowledgeable in this field to confirm the approach taken is ok. Otherwise you'd want to at least make it clear to people using this lib that you're not following that one piece of the spec and why. Allows them to make an informed choice.

Agreed. I'm also tempted to put a random value inside. Although the protocol mandates one account / one user id, the id itself is useless in practice. ... Other than to overwrite the credentials. It might be a worthy sacrifice.

As for GDPR, if there's a chance a company has leaked PII of their customers then they have a duty to disclose that, at least in the EU.

Well, this is related to the company's services. If a user looses it's own USB stick, the company cannot be accountable for it.

I'd say this one is potentially very minor if people are passing in public usernames (that are then hashed). But there would be a chance of devs passing in an email address I'd imagine. You'd want to make it clear not to do that.

Anyway this was just a heads up, I'll leave it with you and again thanks for the work on this.

Well, it's like loosing an USB stick with some stuff on it. I personally see the privacy issue of "with enough malicious effort the username/email can be brute forced" as a joke. The security implications on the other hand are worthy of attention.... In particular if user verification is turned off during authentication.

[dagnelies]

After sleeping over it for some time and careful thoughts, I decided to replace the current sha256(username) for the user id with a random UUID in the upcoming version. That will comply with the spec's recommendation and clear all doubts about misuse of brute-forcing the userHandle to obtain the username.

[dagnelies]

...oh man ...it sucks that it creates new credentials every time instead of overwriting them ...it results in the "discoverable" autofill UI to be overcrowded 🤒

This will apply to all the demos and so on since they don't store credentials server side, but the credentials client side will pile up each time the user tries out any of the demos. ...quite annoying.

[dagnelies]

As a side note, for completeness, it reminds me of w3c/webauthn#1763 which I asked in the past to the spec authors. The reasoning of anonymizing the user ID can be summarized as follows:

It's more to do with not revealing the identity of an authenticator's owner without authentication, for example if a security key is stolen or dropped on the street. Authenticators are required to keep name and displayName secret unless they've authenticated the user, but may reveal the user handle without authentication.

As a concrete example, CTAP 2.0 reveals user handles without authentication when called with empty allowCredentials. This is also the default in CTAP 2.1, but many client platforms now set the credProtect extension to keep credential existence secret unless the user passes user verification or the caller already knows the credential ID.

[dagnelies]

Well, in the end I simply added the a salt ...well, kind of, it's more a public constant.

That way, credentials don't pile up upon creation when the same username is used.

Since it is not a private salt, it still makes brute forcing the username from the hash theoretically possible, but this raises the difficulty bar of hacking the username of usb sticks you pick up on the street.

It's not a perfect, ideal solution but I consider it "good enough".

[mikestead]

I'll be straight, a public salt in this context is not much use given the js src is public on any website that uses this, plus it's OSS.

The important detail for people using your lib (again, great lib!) is the original instruction from the spec is still not followed.

unless the hash function is salted with salt values private to the Relying Party, since hashing does not prevent probing for guessable input values

As this update still goes against that guidance, the recommendation of at least making this very visible to anyone considering this library would a fair thing to do.

Maybe allowing devs to pass a custom id via the RegisterOptions would be a nice override option from default.

RE the demos, can you use local storage for a generated id? In all other real world cases a Relaying Party is needed, that's where opaque id state should really be managed.

[dagnelies]

Actually, I agree with all you said. 🙄 ...you are right, I'll reopen it.

RE the demos, can you use local storage for a generated id? In all other real world cases a Relaying Party is needed, that's where opaque id state should really be managed.

Well, not really. People often use "private browsing" to try it, and may also try various browsers, so you'll end up with clean local storages each time, leading to lots of credentials anyway.

[dagnelies]

Added userHandle as registration option. It's included in the example with the recommendation to randomize it and a notice on the parameter description. Good enough for you?