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

Display name content rules? #593

Closed
r12a opened this issue Sep 27, 2017 · 10 comments · Fixed by #951
Closed

Display name content rules? #593

r12a opened this issue Sep 27, 2017 · 10 comments · Fixed by #951
Assignees
Labels
i18n-needs-resolution Issue the Internationalization Group has raised and looks for a response on. stat:pr-open subtype:impl-cons subtype:underspecifiedBehaviors
Milestone

Comments

@r12a
Copy link

r12a commented Sep 27, 2017

[This review was done against https://www.w3.org/TR/2016/WD-webauthn-20160531/#iface-account, and the comment was raised by Addison Phillips. The i18n WG agreed to forward as a WG comment.]

There are no rules (other than that they are DOMStrings) associated with display names. Should there be any content restrictions?

@r12a r12a added the i18n-needs-resolution Issue the Internationalization Group has raised and looks for a response on. label Sep 27, 2017
@AngeloKai
Copy link
Contributor

What kind of content restrictions do you recommend? Is it to restrict to certain text encoding such as UTF-16, etc.?

@AngeloKai
Copy link
Contributor

All browsers have some form of policy regarding how they deal with international DOMString. If we agree on something, we can all take a bug fix to address. Assigning the issue to PR milestone.

@equalsJeffH @nadalin @YubicoDemo for tracking.

@AngeloKai AngeloKai added this to the PR milestone Oct 25, 2017
@emlun
Copy link
Member

emlun commented Feb 28, 2018

For some additional context, the current description of the user.displayName field is:

displayName, of type DOMString
A human-friendly name for the user account, intended only for display. For example, "Alex P. Müller" or "田中 倫". The Relying Party SHOULD let the user choose this, and SHOULD NOT restrict the choice more than necessary.

@selfissued
Copy link
Contributor

@jcjones will review this with engineers at Mozilla. There doesn't seem to be a need to add any restrictions.

@jcjones
Copy link
Contributor

jcjones commented Feb 28, 2018

After consulting with a Mozilla wizard (@zbraniecki), the current definition of these displayName fields will prohibit browsers from including them in UI that does not have clear boundaries. E.g., not in notification or permission boxes. E.g., we could include the displayNames in a management interface where there are grids/borders/graphical elements, but not run it together into a compressed-space like a prompt.

(Note: Firefox is not including anything but the origin in any of its prompts in its first release for these reasons.)

@zbraniecki suggests that if we want to let browsers use this information for prompts, that we should write / refer to some guidance on how to present the strings, given that the attacker can do three major things with a DOMString:

play with position and directionality (so the string may appear on the far right of the field rather than left, or in the middle), make things invisible within the string or confusing (emojis), or may the string contain characters that look like other characters.

My poor synopsis of some sample guidance would be advice to always use UI elements to provide a clear boundary around these strings, and not allow overflow into other elements, etc.

@zbraniecki
Copy link

You could also do simpleDisplayName (ASCII, min 3 characters long?) and fullDisplayName (unicode, any non-empty string) and provide separate guidelines for those and suggest that if only one is provided the other is casted onto it (with stripping in case of unicode->ascii)

@stpeter
Copy link

stpeter commented Feb 28, 2018

Although https://tools.ietf.org/html/rfc8266 might be overkill here, it addresses some of the core issues and challenges with display names...

@jcjones
Copy link
Contributor

jcjones commented Mar 7, 2018

7 March 2018 WG call: @equalsJeffH agrees with @stpeter. @jcjones to write a PR that references RFC 8266's nickname profile for the relevant DOMStrings.

jcjones added a commit to jcjones/webauthn that referenced this issue Apr 23, 2018
The RP provides 'PublicKeyCredentialUserEntity/displayName' and 'PublicKeyCredentialEntity/name',
both of which are intended for display by User Agent. As DOMString objects, these could be
manipulated by a malicious RP to try and confuse the user about what is being displayed, so
User Agents should be careful in how they display these fields.

This PR points to RFC 8266 for its guidance on showing those fields. This is guidance that
browser vendors already follow for other specifications, so it's nothing new -- it merely
codifies what should be.
@equalsJeffH
Copy link
Contributor

equalsJeffH commented Jun 14, 2018

Ok, it seems there are two facets to this issue:

  1. corralling the unicode content of the "name"-ish domstrings
  2. providing implementer guidance regarding how to display/present these string values in order to mitigate effects of possibly malicious string content.

PR #951 is an attempt to address (1) and is an alternative to pr #878.

I have done some modest searching around WHATWG and W3C specs regarding how one might specify guidance per @jcjones's suggestion above of "[advise] to always use UI elements to provide a clear boundary around these strings, and not allow overflow into other elements, etc...", but have been unable to find anything useful -- does anyone have any clues?

@equalsJeffH
Copy link
Contributor

this was addressed by PR #951

equalsJeffH added a commit that referenced this issue Jul 13, 2018
…#951)

given #951 (comment) where we decided on the 11-Jul webauthn call to go ahead and merge this PR as-is, and @stpeter's nominal ok of the presentation warning #951 (comment), I'm merging this.  if anyone feels there are problems with it, please submit specific new issues.

* employ PRECIS RFC8264 et al for 'name'-ish domstring values

* address emlun's review comment

* remove reference to 'preparation', 'enforcement' includes it

* re-do section references per selfissued

* client-side normativity to SHOULD

* add presentation admonition wrt name-ish strings
WebAuthnBot pushed a commit that referenced this issue Jul 13, 2018
WebAuthnBot pushed a commit that referenced this issue Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-needs-resolution Issue the Internationalization Group has raised and looks for a response on. stat:pr-open subtype:impl-cons subtype:underspecifiedBehaviors
Projects
None yet
8 participants