Tails

#1 Updated by intrigeri 2015-04-29 09:05:24

• Subject changed from TBB should only offer download when opening a file to Tor Browser should not propose to open downloaded files with external applications
• Assignee set to intrigeri
• Target version set to Tails_1.4.1
• QA Check deleted (Dev Needed)

emmapeel wrote:
> When clicking on a file link, Tor Browser shows a menu asking what to do with the file.
>
> Since the Browser confinement many of the default options (for example, LibreOfficeWriter for .odt files) cannot work.

Good catch! Indeed, that’s a UX regression brought by the AppArmor confinement. I agree we should simply not allow the “Open with” option, or display a warning that explains why it won’t work (whichever is the most realistic one to implement).

I had a quick look, and it seems that we could implement the warning by patching the torbutton.popup.external.* strings, and setting the extensions.torbutton.launch_warning preference to its default true value (we’re currently explicitly setting it to false). I’ll give it a try for 1.4.1 or 1.5. Not sure if we can do that without patching Torbutton, which is too bad since we’re in the process of stopping to patch it :/

> If I remember correctly, the TorBrowserBundle just offers a little ‘Save file’ menu instead of the one offered by Iceweasel with default app options.

I don’t think so. Last time I checked, Tor Browser outside of Tails merely warns with “If this file is untrusted, you should either save it to view while offline or in a VM, or consider using a transparent Tor proxy like Tails LiveCD or torsocks.” when opening a downloaded file.

#2 Updated by intrigeri 2015-06-06 14:21:56

• Assignee changed from intrigeri to anonym
• QA Check set to Info Needed

Elaborating on my previous idea, I see two options:

1. we could override those strings to modify the text displayed in the dialog; what remains to be seen is whether this can be done without patching Torbutton itself; if it can, well, fine, let’s do that, and perhaps even for 1.4.1 (even though I doubt we’ll have a way to translate these strings in time); otherwise:
2. we could patch torbutton’s _confirmLaunch function (in src/components/external-app-blocker.js) so that it checks if it’s running if Tails, and if that’s the case, then it displays a different dialog that explains why one must save the file in this context. Checking if running in Tails can already be done by checking if extensions.torbutton.lastBrowserVersion is set to “Tails”, but perhaps a new, dedicated pref would be nicer (see how it was done in TorBirdy) — this will have to be discussed upstream. IMO such changes would be more appropriate for the next major release.

anonym, any idea if one can override strings (in this case, those that are in src/chrome/locale/*/torbutton.properties in the Torbutton source tree) “from the outside”, that is without directly modifying those files? Perhaps you’re aware of some location where we can drop override files? Otherwise I’ll go ask the Tor Browser dev team.

#3 Updated by anonym 2015-06-09 11:09:04

• Assignee changed from anonym to intrigeri

intrigeri wrote:
> I agree we should simply not allow the “Open with” option,

This would be great, but I looked around quite a bit, and this doesn’t seem to be possible. I think it would be if we could hide all MIME info from the browser. There’s the “helpers.{global,private}_mime_types_file” prefs but setting them to the empty string doesn’t help. I enabled some logging with:

NSPR_LOG_MODULES=all:5 tor-browser

and it seems the browser asks GNOME about MIME info(I saw “Got MIMEInfo from GNOME registry ...”). I’m not sure how or even if that can be disabled. Do you have any ideas?

> or display a warning that explains why it won’t work (whichever is the most realistic one to implement).
[…]
> # we could override those strings to modify the text displayed in the dialog; what remains to be seen is whether this can be done without patching Torbutton itself; if it can, well, fine, let’s do that, and perhaps even for 1.4.1 (even though I doubt we’ll have a way to translate these strings in time); otherwise:
> # we could patch torbutton’s _confirmLaunch function (in src/components/external-app-blocker.js) so that it checks if it’s running if Tails, and if that’s the case, then it displays a different dialog that explains why one must save the file in this context. Checking if running in Tails can already be done by checking if extensions.torbutton.lastBrowserVersion is set to “Tails”, but perhaps a new, dedicated pref would be nicer (see how it was done in TorBirdy) — this will have to be discussed upstream. IMO such changes would be more appropriate for the next major release.

A third option would be to make everything auto-download (without any dialog) to the default directory. It will require us to generate a .tor-browser/profile.default/mimeTypes.rdf with entries for each MIME type. That seems quite doable by using e.g. /etc/mime.types as input. Firefox is perhaps a bit too quiet about such downloads; there’s just an animation of the “download arrow” that fades into the menu button. Not sure if that’s enough. Ah, but I just realized that this will make it possible for websites to force downloads on Tails’ users which is just terrible. Can this idea be salvaged?

> anonym, any idea if one can override strings (in this case, those that are in src/chrome/locale/*/torbutton.properties in the Torbutton source tree) “from the outside”, that is without directly modifying those files? Perhaps you’re aware of some location where we can drop override files? Otherwise I’ll go ask the Tor Browser dev team.

I’m pretty sure you cannot do that but it could still be worth asking, I suppose.

#4 Updated by intrigeri 2015-06-09 19:02:18

• Assignee changed from intrigeri to anonym

>> I agree we should simply not allow the “Open with” option,

> This would be great, but I looked around quite a bit, and this doesn’t seem to be possible.

Well, it seems to me that this is exactly what Torbutton does when extensions.torbutton.launch_warning is true, so I don’t understand how it can be impossible. Did I miss anything?

> I think it would be if we could hide all MIME info from the browser. There’s the “helpers.{global,private}_mime_types_file” prefs but setting them to the empty string doesn’t help. I enabled some logging with:
>

> NSPR_LOG_MODULES=all:5 tor-browser
> 

> and it seems the browser asks GNOME about MIME info(I saw “Got MIMEInfo from GNOME registry ...”). I’m not sure how or even if that can be disabled. Do you have any ideas?

No idea, sorry.

> A third option would be to make everything auto-download (without any dialog) to the default directory. It will require us to generate a .tor-browser/profile.default/mimeTypes.rdf with entries for each MIME type. That seems quite doable by using e.g. /etc/mime.types as input. Firefox is perhaps a bit too quiet about such downloads; there’s just an animation of the “download arrow” that fades into the menu button. Not sure if that’s enough. Ah, but I just realized that this will make it possible for websites to force downloads on Tails’ users which is just terrible. Can this idea be salvaged?

This feels wrong indeed, and I’ve no idea how it can be salvaged.

=> I’ll ask the Tor Browser devs, and some Mozilla people if this fails.

#5 Updated by anonym 2015-06-09 20:45:44

• Assignee changed from anonym to intrigeri

intrigeri wrote:
> >> I agree we should simply not allow the “Open with” option,
>
> > This would be great, but I looked around quite a bit, and this doesn’t seem to be possible.
>
> Well, it seems to me that this is exactly what Torbutton does when extensions.torbutton.launch_warning is true, so I don’t understand how it can be impossible. Did I miss anything?

When I tested this (and I re-tried it just now to be extra sure) setting that pref only shows an extra warning prompt. Once passed the warning prompt, the normal Firefox download prompt is shown, including the “Open with” option.

Also it should be clarified that with “[disabling the ”Open with" option] doesn’t seem to be possible" I just meant that I couldn’t find a simple way to do it, like a pref, or editing mimeTypes.rdf. I’m sure you can hack it away with some extension-magic.

#6 Updated by intrigeri 2015-06-10 08:14:16

• Assignee changed from intrigeri to anonym

> When I tested this (and I re-tried it just now to be extra sure) setting that pref only shows an extra warning prompt. Once passed the warning prompt, the normal Firefox download prompt is shown, including the “Open with” option.

Ouch, OK. I must have been testing with unsupported files then. Just so that we’re on the same page: what exact file/URL are you downloading?

#7 Updated by anonym 2015-06-10 12:20:32

• Assignee changed from anonym to intrigeri
• QA Check changed from Info Needed to Dev Needed

intrigeri wrote:
> > When I tested this (and I re-tried it just now to be extra sure) setting that pref only shows an extra warning prompt. Once passed the warning prompt, the normal Firefox download prompt is shown, including the “Open with” option.
>
> Ouch, OK. I must have been testing with unsupported files then. Just so that we’re on the same page: what exact file/URL are you downloading?

The Tails 1.4 signature, for instance. The Tails 1.4 ISO image sometimes gives me a dialog with only “Cancel” and “Save File” buttons, which I guess is what we want. However, remember that it’s up to the HTTP server to report MIME types (yay! attack vector, btw?) so the MIME situation will depend on which mirror is picked, and hence the “sometimes”. :)

There are a couple of extensions that modifies this dialog that may be used for inspiration/guidance:

• OpenDownload²
• Download Dialog Tweak (sources

OTOH, they add stuff to the dialog, while we want to remove stuff. In fact, we want to remove all radio button options but the “save file” one, which would be ugly. So in the end we really want the version of this dialog that only have the “Save File” button (and “Cancel”). It seems that internally these are a completely different dialogs: unknownContentType.xul is the one with radio buttons; I’m not sure where the one we want comes from. Or it is that one but it’s modified dynamically somewhere in the JavaScript/XUL/etc spaghetti. Urgh.

#9 Updated by intrigeri 2015-06-28 02:16:29

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

Clearly too late to fix that for 1.4.1 now.

#11 Updated by intrigeri 2015-08-08 11:38:31

• Deliverable for set to 264
• Deliverable for set to 199

#12 Updated by intrigeri 2015-08-09 00:26:04

• Target version changed from Tails_1.5 to Tails_1.6
• QA Check deleted (Dev Needed)
• Deliverable for deleted (199)

For 1.6 I’ll try to experiment with the cheapest possible solution.

#13 Updated by intrigeri 2015-09-22 12:17:55

• Priority changed from Normal to Elevated
• Target version changed from Tails_1.6 to Tails_1.7

(Raising priority for the cheap way. If it proves to need lots of efforts, we’ll see.)

#14 Updated by intrigeri 2015-11-01 06:16:08

• Status changed from Confirmed to In Progress
• Target version changed from Tails_1.7 to Tails_1.8
• % Done changed from 0 to 10

Summed up what I think are our best options, asked for advice: https://lists.torproject.org/pipermail/tbb-dev/2015-November/000315.html

#19 Updated by anonym 2015-11-02 01:15:03

• Assignee changed from intrigeri to anonym

intrigeri wrote:
> > I wonder if it’s Good Enough to simply not say anything. After all, if the server omits the MIME type, users would get only the “Save as” option even without my patch.
>
> I’m in favour of just saying nothing about the option that isn’t shown. It’s not as if one could have counted on it to be working in the last 6 months.

Good! Let’s get our patch merged, then.

> >> So, now I’m hesitating. Patching Torbutton’s string is something I can probably manage to do in the next few months. Anything else, well, I don’t know.
>
> > Similarly, if we want to make it settable via a string pref (as you suggested on tbb-dev@) we have the same issue with losing localization support.
>
> JFTR in the “alternate strings” option I suggested, those strings would live in TBB’s repo and be l10n’ed.

Ah, I mostly remembered the last paragraph of you suggestion 2 sent to tbb-dev@.

> Whatever: your solution (the patch) feels much better, so let’s just see what the TBB people say.

ACK.

> I suspect they’ll want a ticket for better tracking (and review process).

Sure, but I thought since the discussion had started, perhaps we could see if they settle on something first. I guess they might be interested in something similar. While the option is not broken for them, it’s potentially dangerous, and the warning they show is perhaps not enough. Maybe we could come up with something that will work for both of us?

#21 Updated by intrigeri 2015-12-05 14:15:21

• Target version changed from Tails_1.8 to Tails_2.0

A tiny bit more work on the patch was requested on https://trac.torproject.org/projects/tor/ticket/17502.

#24 Updated by anonym 2016-01-08 17:22:48

• % Done changed from 10 to 30
• Feature Branch set to feature/9285-no-external-applications-in-tor-browser

The feature branch sets the pref, but it’s not as easy as that, possibly…

sajolida wrote:
> While working on this we should also adjust the verification instructions in /doc/get/verify_the_iso_image_using_gnome. They will get deprecated once the installation assistant will be released but they will most likely be moved as an advanced topics for some time. I can do the adjustement.

What sajolida is getting at is that with this fix, the signature verification instructions differ between Tails and other distros (Ubuntu, Debian, Fedora, etc) in the “Using Linux with Gnome” case. But since they will be deprecated, the change from the feature branch is not an issue. Right, sajolida?

However, the “Importing an OpenPGP key from a website” scenario in our automated test suite also breaks. Presumably it was introduced with that part of the documentation in mind, so perhaps we can drop it? BTW. it was never really important for the Tails use case any way since we ship the signature. Instead, what about if I remove that scenario and make one for downloading a file in general? I’m thinking:

    Given I have started Tails from DVD and logged in and the network is connected
    When I start the Tor Browser
    Then the Tor Browser has started and loaded the startup page
    When I download some file in the Tor Browser
    Then I get the browser download dialog
    When I save the file to the default Tor Browser Downloads directory
    Then the file is saved to the default Tor Browser Downloads directory

I think I’ll just proceed doing this.

#27 Updated by sajolida 2016-01-08 18:48:52

• related to Bug #10882: Update OpenPGP verifications to Tor Browser not opening applications added

#28 Updated by anonym 2016-01-08 21:25:49

• Assignee changed from anonym to intrigeri
• % Done changed from 30 to 50
• QA Check set to Ready for QA

intrigeri wrote:
> > Instead, what about if I remove that scenario and make one for downloading a file in general?
>
> Sounds like a good idea!

Yay, done! Would you like to review’n’merge? Otherwise, please assign it to kytv (after all, most of the branch actually is test suite-related… :))!

#29 Updated by intrigeri 2016-01-09 11:38:52

• Assignee changed from intrigeri to anonym
• QA Check changed from Ready for QA to Info Needed

> Yay, done! Would you like to review’n’merge?

I can try to do that today, but I’d like an explicit confirmation from you that the conclusion of the above discussion is: we can merge these code & test suite changes without bothering about the doc for now (and the doc update is tracked by the Bug #10882 follow-up ticket). Is this correct?

#31 Updated by anonym 2016-01-09 13:47:11

• QA Check changed from Info Needed to Dev Needed

intrigeri wrote:
> > Yay, done! Would you like to review’n’merge?
>
> I can try to do that today, but I’d like an explicit confirmation from you that the conclusion of the above discussion is: we can merge these code & test suite changes without bothering about the doc for now (and the doc update is tracked by the Bug #10882 follow-up ticket). Is this correct?

So I looked at what sajolida said to me elsewhere again, and I realize that indeed there is a possibility for a timing issue depending on when we decide to release 2.0. He says that the assistant will be released “some time next week” (i.e. 2016-01-11 - 2016-01-17) and we very well may end up releasing 2.0 before that. Hmm.

Perhaps we indeed must coordinate a bit more with sajolida, so let’s wait a bit with the merge (but feel free to review’n’test now). To be continued!

> Code review passes. I’ve pushed a few (untested) commits on top of yours, if you had a quick look before I test and merge the branch, it would be great :)

They look good. Thanks!

#32 Updated by intrigeri 2016-01-09 14:00:04

• Assignee changed from anonym to sajolida
• QA Check changed from Dev Needed to Info Needed

> So I looked at what sajolida said to me elsewhere again, and I realize that indeed there is a possibility for a timing issue depending on when we decide to release 2.0. He says that the assistant will be released “some time next week” (i.e. 2016-01-11 - 2016-01-17) and we very well may end up releasing 2.0 before that. Hmm.

Worst case we break the key import step of the ISO verification doc from inside Tails, right? If it’s the case, it’s mitigated by the fact that Tails already has the right key.

> Perhaps we indeed must coordinate a bit more with sajolida,

Right. Reassigning to him.

sajolida, do we need a doc update before we merge this branch, or can it stay broken until the IA is released?

#33 Updated by intrigeri 2016-01-09 15:03:35

• % Done changed from 50 to 60

anonym: test suite almost passes for me (features/torified_browsing.feature features/unsafe_browser.feature) so I would merge (modulo sajolida’s input). The only failures I see seem to be caused by CSS changes on our website (I see "TorBrowserSavedStartupPage.png" after at most 10 seconds and I see "TailsOfflineDocHomepage.png" after at most 10 seconds).

#34 Updated by anonym 2016-01-09 19:36:42

• Assignee changed from sajolida to intrigeri

intrigeri wrote:
> Worst case we break the key import step of the ISO verification doc from inside Tails, right? If it’s the case, it’s mitigated by the fact that Tails already has the right key.

Good point! Any way, it seems our new release plans makes this a non issue. => unassigning sajolida (based on the assumption that the assistant will be released before Jan 26).

> anonym: test suite almost passes for me (features/torified_browsing.feature features/unsafe_browser.feature) so I would merge (modulo sajolida’s input). The only failures I see seem to be caused by CSS changes on our website (I see "TorBrowserSavedStartupPage.png" after at most 10 seconds and I see "TailsOfflineDocHomepage.png" after at most 10 seconds).

These are of course not related to this branch, so I’m asking you to ignore these results.

However, would this also be a good time to adapt Tor Browser’s AppArmor profile accordingly? E.g. remove all “external tools” we added for the purpose of opening downloaded files in, like seahorse-tool (I think that’s the only one actually). What do you think?

#35 Updated by intrigeri 2016-01-09 20:02:21

• QA Check changed from Info Needed to Dev Needed

> However, would this also be a good time to adapt Tor Browser’s AppArmor profile accordingly? E.g. remove all “external tools” we added for the purpose of opening downloaded files in, like seahorse-tool (I think that’s the only one actually). What do you think?

Right, I’ll do that.

#36 Updated by intrigeri 2016-01-10 00:15:47

• Assignee changed from intrigeri to anonym
• % Done changed from 60 to 70
• QA Check changed from Dev Needed to Ready for QA

#37 Updated by anonym 2016-01-11 11:43:44

• Status changed from In Progress to Fix committed
• % Done changed from 70 to 100

Applied in changeset commit:c661e5c80bf26cff779caf2fd2e83a5bd3e95798.

#38 Updated by anonym 2016-01-11 11:48:09

• Assignee deleted (anonym)
• QA Check changed from Ready for QA to Pass

intrigeri wrote:
> > However, would this also be a good time to adapt Tor Browser’s AppArmor profile accordingly? E.g. remove all “external tools” we added for the purpose of opening downloaded files in, like seahorse-tool (I think that’s the only one actually). What do you think?
>
> Right, I’ll do that.

I opened Bug #10891 for that so I can close this ticket and merge the branch (so it will end up in Tails 2.0~rc1).

#39 Updated by anonym 2016-01-27 13:31:27

• Status changed from Fix committed to Resolved

#40 Updated by sajolida 2016-02-13 18:34:02

• Subject changed from Tor Browser should not propose to open downloaded files with external applications to Tor Browser should not propose to open downloads with external applications