[tbb-bugs] #23247 [Applications/Tor Browser]: Communicating security expectations for .onion: what to say about different padlock states for .onion services

Tor Bug Tracker & Wiki blackhole at torproject.org
Tue May 22 18:34:23 UTC 2018

#23247: Communicating security expectations for .onion: what to say about different
padlock states for .onion services
 Reporter:  isabela                              |          Owner:
                                                 |  pospeselr
     Type:  project                              |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ux-team, tor-hs,                     |  Actual Points:
  TorBrowserTeam201805                           |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
Changes (by gk):

 * keywords:  ux-team, tor-hs, TorBrowserTeam201805R => ux-team, tor-hs,
 * status:  needs_review => needs_revision


 Overall, looks good to me! Some minor things:

 1) I had trouble parsing your commit message (there are some typos and
 things in the "-" lines missing etc.). https://chris.beams.io/posts/git-
 commit/ might be a useful resource. At least it helped me a lot.

 2) There is code in `browser.js` regarding the PointerLock API that might
 require patching as well (IIRC this or similar code is used or was used to
 show the same info when entering into fullscreen, at least on some
  get pointerlockFsWarningClassName() {
     // Note that the fullscreen warning does not handle
     if (this._uriHasHost && this._isEV) {
       return "verifiedIdentity";
     if (this._uriHasHost && this._isSecure) {
       return "verifiedDomain";
     return "unknownIdentity";

 3) Could you explain why you needed to add `this._sslStatus != null`  in
 } else if (this._uriHasHost && this._isSecure && this._sslStatus != null)
 If that's not obvious I think adding a comment would be useful.

 4) The `const bool` does not really fit into the surrounding code (where
 just `bool` is used). I think we should stick to the latter. Or maybe even
 better just get rid of this variable and do things like
 if (!StringEndsWith(host, NS_LITERAL_CSTRING(".onion")))
 in both places. (In the `parentIsOnion` case, maybe rename `host` to
 `parentHost` then as this makes the context clearer.

 5) I like the strings added to the Page Info window. However, doing it
 that way we have trouble translating them until we got our code upstreamed
 and Mozilla takes care of the translation. This has the unfortunate side-
 effect of mixed english $LANG text.

 We had a similar issue in #20244 (see in particular
 comment:2:ticket:20244) and worked around it with a Torbutton overlay. I
 wonder if that could be a solution for this case as well.

Ticket URL: <https://trac.torproject.org/projects/tor/ticket/23247#comment:51>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

More information about the tbb-bugs mailing list