Bug #9391

Document why we disabled the new circuit view of Tor Browser 4.5

Added by BitingBird 2015-05-13 04:54:40 . Updated 2015-08-26 06:45:47 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Target version:
Start date:
2015-05-13
Due date:
% Done:

100%

Feature Branch:
doc/9391-circuitview
Type of work:
End-user documentation
Blueprint:

Starter:
Affected tool:
Browser
Deliverable for:

Description

The release notes only say “We disabled in Tails the new circuit view of Tor Browser 4.5 for security reasons.”, and plenty of people are wondering what the security reasons are, on irc and tweeter. Maybe a link to Feature #9298 would be better than no explanation?


Subtasks


Related issues

Related to Tails - Feature #9298: How much do we want Tor Browser's per-tab circuit view? Resolved 2015-04-29

History

#1 Updated by BitingBird 2015-05-13 04:55:01

  • related to Feature #9298: How much do we want Tor Browser's per-tab circuit view? added

#2 Updated by intrigeri 2015-05-13 07:46:25

  • Assignee set to anonym
  • QA Check set to Info Needed

> The release notes only say “We disabled in Tails the new circuit view of Tor Browser 4.5 for security reasons.”, and plenty of people are wondering what the security reasons are, on irc and tweeter. Maybe a link to Feature #9298 would be better than no explanation?

Perhaps this would actually be worth a tiny piece of design doc, until Bug #9365 and Bug #9366 are completed?

#3 Updated by intrigeri 2015-05-16 14:05:47

  • Status changed from New to Confirmed
  • Assignee changed from anonym to BitingBird
  • QA Check deleted (Info Needed)

And now Tor Browser users who have read our 1.4 release announce wonder why TB has this feature enabled if it’s dangerous. IMO we should write a few lines in our Tor Browser end-user doc, that state that:

  1. enabling this feature might make attacks on Tor Browser more dangerous than they currently are in the context of Tails; we haven’t had time to evaluate how severe this might be (link to Bug #9365 and Bug #9366);
  2. Vidalia already gives access to the same info in Tails, so it’s not high priority to do the aforementioned research;
  3. the above is specific to Tails: in the context of Tor Browser outside of Tails, 1. enabling or disabling this feature has no security implication as far as we know; and 2. Vidalia is not shipped so this feature provides important UX improvements.

And then we should patch the release notes to point to that new piece of doc.

BitingBird, do you want to take it? Do you need more info?

#4 Updated by BitingBird 2015-05-16 22:23:30

I can take it but I won’t work on this in the next couple of days. I’ll see then if I need more info.

#5 Updated by sajolida 2015-05-18 12:06:02

Damn! The one time I allow myself to say “for security reasons” without explaining why, people jump on me! That’s crazy. I’m glad BitingBird will fix that.

#6 Updated by BitingBird 2015-06-29 01:31:37

  • Target version changed from Tails_1.4.1 to Tails_1.5

Sorry I wasn’t reactive on that. I’ll work on it for next version.

#7 Updated by BitingBird 2015-07-06 07:38:26

  • Status changed from Confirmed to In Progress
  • Feature Branch set to bitingbird:doc/9391-circuitview

#8 Updated by BitingBird 2015-07-06 08:16:14

  • Assignee changed from BitingBird to sajolida
  • % Done changed from 0 to 50
  • QA Check set to Ready for QA

Added in the Tor Browser doc page (created a new section for this, I don’t see where else it could go). Added a link in the release notes for 1.4.

Built, looks good - please review :)

#9 Updated by sajolida 2015-08-04 04:58:16

  • Target version deleted (Tails_1.5)

#10 Updated by BitingBird 2015-08-04 05:17:31

  • Target version set to Tails_1.6

Please don’t just remove target versions. Postponing is fine, but it’s sad if it disappears from our radar.

#11 Updated by sajolida 2015-08-09 11:45:57

  • Assignee changed from sajolida to BitingBird
  • QA Check changed from Ready for QA to Info Needed

Oops, I’m very sorry I didn’t check that before my long delay for review but I think that you forgot to push that branch.

git branch -a | grep bitingbird | grep 9391 gives me nothing :(

#12 Updated by BitingBird 2015-08-10 05:28:22

  • Assignee changed from BitingBird to sajolida
  • QA Check changed from Info Needed to Ready for QA

Oops, pushed now :)

#13 Updated by sajolida 2015-08-10 08:10:45

  • Assignee changed from sajolida to BitingBird
  • QA Check changed from Ready for QA to Dev Needed

Now I see it. Thanks for working on that. I now that I’m usually the one to push for precise and clear content, trying to extract as much technical information as possible into fewer words as possible. But here on the other hand I feel like we should be blurry because the issue at stake doesn’t deserve so many words or so much word on documentation :)

  • About the phrasing of the title. We usually use the gerund in titles to explain how to do things (for example “Chatting with Pidgin & OTR”) here you’re using the gerund for something different and it feels awkward. I think we can find something better, maybe “The circuit view is disabled”? But then later on I’m proposing to remove the title all the way, so I’ll let you decide what’s best.
  • Thanks to your patch, I’m realizing that the different sections of this page are a bit messy. We have many top-level titles with little hierarchy or order between then. For example, your new section makes sense as part of what we say about Torbutton. So what about moving your section at the end of the Torbutton section? And what about doing the same with the “New identity feature” section and put it below “Security slider”? While you are at it we could also make a level-two title out of “HTTP Everywhere” which would then fit as a subsction of “HTTPS Encryption”. Makes sense, no? Bonus point if you add a picture for Torbutton at the beginning of the “Torbutton” section :)
  • I’m a bit worried about taking more space than needed here as we’re explaining something that is in the end “not sure” or “too complicated to be explained in depth” and in any case not something people can act upon. So I’d looked for ways of shortening all this drastically. For example, your currently have five sentences. Sentences 2 and 4 explain that people can do the same already with Vidalia. We can probably merge them and shorten everything.
  • On the other hand, I feel the need to clarify what is the circuit view as people might not know. Maybe we can link to https://people.torproject.org/~mikeperry/images/OnionMenu.jpg, but I wouldn’t embed the image.
  • Ideally this should all fit in a “note” div instead of a full-blown section as it doesn’t really deserve it.
  • Maybe phrasing our problem somehow as “we were not sure whether this new feature would have security implications in the particular context of Tails (see tickets …)” would be enough. I think what we need to do here is to explain that we’re unsure but that’s specific to the context of Tails without giving more technical insight than links to the tickets.
  • Beware of the formatting of your links to tickets as they introduce an extra # when compiled.

My review is far from being clear this time maybe, but then that gives you more freedom to be creative :)

You have 120 words right word, try to bring them down to 60 or less. `wc` is your friend.

#14 Updated by BitingBird 2015-08-11 03:46:11

  • Assignee changed from BitingBird to sajolida
  • QA Check changed from Dev Needed to Ready for QA

Put it as a note, under Torbutton. Also moved HTTPS everywhere under HTTPS encryption, and New identity under security slider.

Didn’t add an image - we could use the one from New identity? Not sure.

Shortened a lot, but probably not as much as you said - I don’t see what else I could remove and still make sense :)

I also didn’t put the link you gave - it would make more sense to me to link to a design doc?

Please review!

#15 Updated by intrigeri 2015-08-11 08:23:04

In passing: please have anonym or I review the final version for technical correctness just before merging (there’s been a history of misunderstandings on this front, let’s be extra careful for once :)

#16 Updated by sajolida 2015-08-17 12:28:41

  • Assignee changed from sajolida to BitingBird
  • Feature Branch changed from bitingbird:doc/9391-circuitview to doc/9391-circuitview

The whole page looks much better structured now and the “note” works better than a full section and is much shorter. Congrats!

Also thanks for doing smaller commits in general, though I could guess from it’s title that 9b97837 was far from being atomic. It’s hard sometimes to split many changes that have been done in a same piece of text into different commits. The trick is sometimes to think a bit before jumping to the text editor and draw a list of things to change (possibly in your head). Then change them one after the other and do a commit each time. But I reckon it’s hard.

Still, I did a bunch more commits on top of yours (a443512..3d4e94f) to either fix formatting, be more precise when I felt it was needed, and shorten a bit more. We’re now at 43 :)

Please have a look and tell me if I can merge.

Also, I reverted 237003a. I think it’s good to do commits dedicated to rewrapping while working on a branch. But I don’t like doing this on text that has been here for a while. For example, I’m not sure how it interacts with translations. And it surely makes stuff like `git blame` less useful. But if you want to rewrap, you can do in on the new stuff for this branch before we merge. Now is the time :)

#17 Updated by BitingBird 2015-08-18 04:40:49

  • Assignee changed from BitingBird to sajolida

I don’t think 2c3e8e82 is a good idea. I’m afraid Tor people would get plenty of users saying “oh my god that’s not safe!”.

#18 Updated by sajolida 2015-08-19 07:00:54

  • Assignee changed from sajolida to BitingBird

Ok, I thought this was already covered by saying “in the particular context of Tails”, but to be more explicit I know added a shorter version of the previous sentence. See a90dd8f. Tell me what you think.

#19 Updated by BitingBird 2015-08-19 07:48:03

  • Assignee changed from BitingBird to intrigeri

You now affirm that it’s safe outside of Tails, I’m not sure about that. So I’ll let intrigeri make the review, and if he’s happy with it, we can merge :)

#20 Updated by intrigeri 2015-08-25 03:21:43

  • Assignee changed from intrigeri to sajolida
  • % Done changed from 50 to 80

Sounds good. Thanks for clarifying the non-Tails aspect, I’m sure this will avoid some pain for Tor user support people.

Just one suggestion: s/we were/we are/. Not sure if “we disabled” should be present tense instead.

Merged current master, pushed to the topic branch.

#21 Updated by sajolida 2015-08-26 04:38:57

> Just one suggestion: s/we were/we are/. Not sure if “we disabled” should be present tense instead.

Of course! Done in 4fd5bd2. Now merging and pushing.

#22 Updated by sajolida 2015-08-26 04:39:18

  • Status changed from In Progress to Resolved
  • % Done changed from 80 to 100

Applied in changeset commit:8af3d164a12fa3fa0f67c4e9e79d734569bdd7ab.

#23 Updated by sajolida 2015-08-26 04:39:45

  • Assignee deleted (sajolida)
  • % Done changed from 100 to 80
  • QA Check deleted (Ready for QA)

#24 Updated by intrigeri 2015-08-26 06:45:47

  • % Done changed from 80 to 100