Bug #11964

Discuss if Thunderbird AppArmor profile should prevent users from opening attachments

Added by Anonymous 2016-11-19 10:32:17 . Updated 2018-08-17 15:17:25 .

Status:
Rejected
Priority:
Elevated
Assignee:
Category:
Target version:
Start date:
2016-11-19
Due date:
% Done:

30%

Feature Branch:
icedove:tails/stretch
Type of work:
User interface design
Blueprint:

Starter:
Affected tool:
Email Client
Deliverable for:

Description

We need to document this or fix it in Debian.

First step: research what’s in the log and if it can be fixed.
Second step: Document it or upstream the fix to Debian.

Subtasks

Related issues

Related to Tails - Bug #10750: Ship an AppArmor profile for Icedove in Tails Resolved 2016-02-05
Related to Tails - Bug #11973: Confine Thunderbird with AppArmor Resolved 2016-11-20
Blocked by Tails - Bug #11712: Have Icedove built from Stretch with our patchset applied in Tails 3.0 Resolved 2016-08-24

History

#1 Updated by intrigeri 2016-11-19 10:34:40

  • related to Bug #10750: Ship an AppArmor profile for Icedove in Tails added

#2 Updated by intrigeri 2016-12-06 16:17:25

I guess that’s strongly related to the UX work that we’ve done when we confined Tor Browser.

#3 Updated by Anonymous 2017-01-17 15:30:15

  • blocks Bug #11973: Confine Thunderbird with AppArmor added

#4 Updated by intrigeri 2017-03-03 07:48:55

  • Target version set to Tails_3.0

I believe this will be a behavior change in Tails 3.0, that will include the AppArmor profile => setting Target version accordingly so it’s on our radar.

#5 Updated by intrigeri 2017-03-03 07:49:11

  • blocked by deleted (Bug #11973: Confine Thunderbird with AppArmor)

#6 Updated by intrigeri 2017-03-03 07:49:31

  • related to Bug #11973: Confine Thunderbird with AppArmor added

#7 Updated by Anonymous 2017-03-17 09:38:02

  • Priority changed from Low to Elevated

#8 Updated by Anonymous 2017-03-17 14:35:39

I’ve seen that Carsten has marked this as fixed here https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=855346 but I need to investigate how it was fixed exactly.

#9 Updated by Anonymous 2017-03-18 09:47:51

After some research and testing, this is not fixed in Debian.

The idea would be to make the Debian profile more permissive and allow launching eog, file-roller, libre-office, evince, gedit.

But in Tails, we will ideally want Icedove not to display the possibility of opening attachments directly. It should rather use the same fix as Torbrowser (upstreamed in FF50), using

// Given our AppArmor sandboxing, Tor Browser will not be allowed to
// open external applications, so let's not offer the option to the user,
// and instead only propose them to save downloaded files.
pref("browser.download.forbid_open_with", true);

found in: /etc/tor-browser/profile/preferences/0000tails.js

https://bugzilla.mozilla.org/show_bug.cgi?id=1281959

#10 Updated by Anonymous 2017-03-18 16:09:01

I’ve proposed a more permissive file Upstream: https://code.launchpad.net/~u-d/apparmor-profiles/+git/apparmor-profiles/+merge/320276

This concerns only the Debian side of things. In Tails, we will want to remove this part of the profile.

Next steps: work on integrating the patch for Tails’ Icedove.

#11 Updated by Anonymous 2017-03-19 18:30:16

  • Assignee set to anonym
  • QA Check set to Ready for QA
  • Feature Branch set to icedove:tails/jessie

u wrote:
> After some research and testing, this is not fixed in Debian.
>
> The idea would be to make the Debian profile more permissive and allow launching eog, file-roller, libre-office, evince, gedit.
>
> But in Tails, we will ideally want Icedove not to display the possibility of opening attachments directly. It should rather use the same fix as Torbrowser (upstreamed in FF50), using
> […]
>
> https://bugzilla.mozilla.org/show_bug.cgi?id=1281959

I’ll continue to take care of the Debian side of things.

Meanwhile, I’d like anonym to review and test my modifications in the tails/jessie branch.

#12 Updated by intrigeri 2017-04-02 09:02:33

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 10

Note that this new restriction may require some new documentation => please coordinate this with sajolida before merging :)

#13 Updated by intrigeri 2017-04-02 09:06:30

  • related to deleted (Bug #11973: Confine Thunderbird with AppArmor)

#14 Updated by intrigeri 2017-04-02 09:06:41

  • blocks Bug #11712: Have Icedove built from Stretch with our patchset applied in Tails 3.0 added

#15 Updated by intrigeri 2017-04-18 15:08:25

  • Target version changed from Tails_3.0 to Tails_3.0~rc1

#16 Updated by anonym 2017-05-14 14:31:17

  • Type of work changed from Research to Code

u wrote:
> Meanwhile, I’d like anonym to review and test my modifications in the tails/jessie branch.

Where did you get the patch you imported? I’m wondering, cause it contains two syntax errors! From your commit 7e3ba02 that imports the patch:

+-    if (shouldntRememberChoice && !this.openWithDefaultOK()) {
++    if (shouldntRememberChoice && !this.openWithDefaultOK()) ||
++        Services.prefs.getBoolPref("browser.download.forbid_open_with")) {)

As you can see, the patched file will contain one syntax error because there is an opening parenthesis missing (so the || becomes dangling) which seems to have been misplaced to the opening of the conditional’s body (where it is unmatched) causing yet another syntax error. It should be:

+-    if (shouldntRememberChoice && !this.openWithDefaultOK()) {
++    if ((shouldntRememberChoice && !this.openWithDefaultOK()) ||
++        Services.prefs.getBoolPref("browser.download.forbid_open_with")) {

and that was indeed how my original patch looked when I submitted to the Tor Browser devs (that they got upstreamed into Firefox). Neither the patch on the Mozilla tracker, nor the one in Tor Browser has this syntax error. Interesting! :)

Any way, I’ve fixed it in the tails/stretch branch that I’m currently working on. I’ll build new packages and test the this soon.

#17 Updated by anonym 2017-05-15 15:53:47

  • % Done changed from 10 to 30
  • QA Check changed from Ready for QA to Dev Needed
  • Feature Branch changed from icedove:tails/jessie to icedove:tails/stretch tails:feature/11712-thunderbird

My fix worked. However, if I right-click on an attached .txt, select “Open”, then it opens in Gedit; in other words, we are still missing the AppArmor profile overrides. I casually tried adding these to /etc/apparmor.d/local/usr.bin.thunderbird:

  audit deny /usr/bin/exo-open xr,
  audit deny /usr/lib/@{multiarch}/xfce4/exo-1/exo-helper-1 xr,
  audit deny /etc/xdg/xdg-xubuntu/xfce4/helpers.rc r,
  audit deny /etc/xdg/xfce4/helpers.rc r,


but it did seemingly nothing. I’ll look into it with intrigeri, I suppose, unless you, u, see this and fix it ASAP (please push to the Tails feature branch in that case).

#18 Updated by intrigeri 2017-05-16 07:00:27

Before I review’n’merge this, I’d like to see a summary of what the intent is, i.e. what operations will be accepted/denied (and when denied, what’s the user story you have in mind). Thanks in advance!

#19 Updated by intrigeri 2017-05-16 16:34:25

  • Subject changed from Icedove's AppArmor profile prevents users from opening attachments to Icedove's AppArmor profile should prevent users from opening attachments

Retitling to express what we’re trying to achieve here.

#20 Updated by intrigeri 2017-05-16 16:35:17

  • blocked by deleted (Bug #11712: Have Icedove built from Stretch with our patchset applied in Tails 3.0)

#21 Updated by intrigeri 2017-05-16 16:35:27

  • blocked by Bug #11712: Have Icedove built from Stretch with our patchset applied in Tails 3.0 added

#22 Updated by intrigeri 2017-05-16 16:41:17

  • Target version changed from Tails_3.0~rc1 to Tails_3.0

It seems that the AppArmor changes needed to prevent users from opening attachments have not been done anywhere yet. Correct? So let’s postpone to 3.0. This doesn’t block Bug #11712 AFAICT (we can very well first ship a permissive profile, and later harden it).

#23 Updated by anonym 2017-05-16 17:34:28

  • Feature Branch changed from icedove:tails/stretch tails:feature/11712-thunderbird to icedove:tails/stretch

I’ve reverted the only commit relevant to this ticket in tails:feature/11712-thunderbird. So commit:5d848fb956b4628ee79006a2a994d212b69fca48 should be reverted to enable this again.

#24 Updated by intrigeri 2017-05-18 14:53:56

  • related to Bug #11973: Confine Thunderbird with AppArmor added

#25 Updated by intrigeri 2017-05-18 15:03:07

intrigeri wrote:
> It seems that the AppArmor changes needed to prevent users from opening attachments have not been done anywhere yet.

I was wrong, sorry! As documented on Bug #11973#note-24 the AppArmor profile included in Stretch already prevents attachments from being opened on Tails/Stretch. Assuming this is what we wanted (and IIRC it was), next steps are:

  • revert commit:5d848fb956b4628ee79006a2a994d212b69fca48 to hide the “Open with…” option in the dialog that opens when one double-clicks an attachment
  • have the end-user doc explain why we think this behaviour is preferable, and how one is supposed to open their attachments, as requested earlier (Bug #11964#note-12)

I’ll let you folks pick a suitable target version: I’d be fine with merging this in 3.0 if our doc writers are fine with it and a mergeable branch (including doc!) is ready by the end of May: this means it won’t see much testing (except if we do a 3.0~rc2, not clear yet) and it will leave very little time for translators to do their job, but well… I really would like us to do a step forward on this topic, both to protect our users from some attacks, and to make us developers happy to have delivered something :)

#26 Updated by intrigeri 2017-05-18 15:03:28

  • related to deleted (Bug #11973: Confine Thunderbird with AppArmor)

#27 Updated by intrigeri 2017-05-18 15:03:37

  • blocks Bug #11973: Confine Thunderbird with AppArmor added

#28 Updated by anonym 2017-05-25 14:44:17

intrigeri wrote:
> intrigeri wrote:
> > It seems that the AppArmor changes needed to prevent users from opening attachments have not been done anywhere yet.
>
> I was wrong, sorry! As documented on Bug #11973#note-24 the AppArmor profile included in Stretch already prevents attachments from being opened on Tails/Stretch.

… except for files that will open in Gedit (e.g. .txt files). See Bug #11973#note-27 for details.

#29 Updated by intrigeri 2017-05-25 18:33:05

  • Subject changed from Icedove's AppArmor profile should prevent users from opening attachments to The Thunderbird AppArmor profile should prevent users from opening attachments

#30 Updated by anonym 2017-06-01 16:59:05

  • Target version changed from Tails_3.0 to Tails_3.2

Since this change doesn’t only depend on me, and I doubt I’ll be able to coordinate with everyone else to get this done before 3.0.,I’ll postpone this to the next major release.

#31 Updated by intrigeri 2017-06-29 10:32:47

#32 Updated by intrigeri 2017-09-07 06:39:44

  • Target version changed from Tails_3.2 to Tails_3.5

(As per Bug #11973#note-42.)

#33 Updated by intrigeri 2017-10-01 09:51:38

#34 Updated by intrigeri 2017-10-01 09:51:40

  • blocked by deleted (Feature #13234: Core work 2017Q3: Foundations Team)

#35 Updated by intrigeri 2017-10-01 09:51:58

  • Target version changed from Tails_3.5 to Tails_3.6

(3.4 will be a bugfix release)

#36 Updated by Anonymous 2018-01-19 11:04:12

  • Target version changed from Tails_3.6 to Tails_3.8

#37 Updated by Anonymous 2018-01-19 11:04:29

  • Type of work changed from Code to User interface design

#38 Updated by Anonymous 2018-01-19 11:05:18

  • Subject changed from The Thunderbird AppArmor profile should prevent users from opening attachments to Discuss if Thunderbird AppArmor profile should prevent users from opening attachments

#39 Updated by intrigeri 2018-01-26 15:01:20

  • blocked by deleted (Feature #13245: Core work 2018Q1: Foundations Team)

#40 Updated by intrigeri 2018-01-26 15:01:29

#41 Updated by bertagaz 2018-03-14 11:30:21

  • blocked by deleted (Bug #11973: Confine Thunderbird with AppArmor)

#42 Updated by bertagaz 2018-03-14 11:30:32

  • related to Bug #11973: Confine Thunderbird with AppArmor added

#43 Updated by intrigeri 2018-04-14 12:35:38

  • blocked by deleted (Feature #15139: Core work 2018Q2: Foundations Team)

#44 Updated by intrigeri 2018-04-14 12:35:52

  • Target version changed from Tails_3.8 to Tails_3.10.1

#45 Updated by Anonymous 2018-08-17 14:07:57

  • Assignee changed from anonym to intrigeri

Reassigning to intrigeri, who is possibly in a good position to know what’s the current status of this issue.

#46 Updated by intrigeri 2018-08-17 15:17:25

  • Status changed from In Progress to Rejected
  • Assignee deleted (intrigeri)
  • Target version deleted (Tails_3.10.1)

I’m personally satisfied with the current state of things and don’t feel the need to discuss this further. So whoever wants to change things, please reopen.