Merge ~u-d/apparmor-profiles:thunderbird/launcher into ~apparmor-dev/apparmor-profiles/+git/apparmor-profiles-old:master

Proposed by Ulrike Uhlig on 2017-03-18
Status: Rejected
Rejected by: Christian Boltz on 2017-10-27
Proposed branch: ~u-d/apparmor-profiles:thunderbird/launcher
Merge into: ~apparmor-dev/apparmor-profiles/+git/apparmor-profiles-old:master
Diff against target: 25 lines (+14/-0)
1 file modified
ubuntu/17.04/usr.bin.thunderbird (+14/-0)
Reviewer Review Type Date Requested Status
intrigeri Needs Fixing on 2017-07-01
AppArmor Developers 2017-03-18 Pending
Review via email: mp+320276@code.launchpad.net

Description of the change

Hi,

I'd like to request a review and merge for this change, which is supposed to solve https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=855346.

In Thunderbird, people should be allowed to view attachments, and for this purpose they should be able to launch some standard applications.

Debian's Thunderbird will soon ship the profile per default and thus I am concerned about usability for normal users.

Cheers,
ulrike

To post a comment you must log in.
Christian Boltz (cboltz) wrote :

"xr" is not a valid permission set (except for deny rules). Please choose which exec mode (Cx, Px, ix, Ux or one of the fallback modes) you want to use ;-)

058cbba... by Ulrike Uhlig on 2017-03-18

Correct permissions. We want these apps to run using their own profile.

05c0f96... by Ulrike Uhlig on 2017-03-18

Correct permissions. We want these apps to run only if they have a profile.

a8b1ce6... by Ulrike Uhlig on 2017-03-19

After some discussion with intrigeri and cboltz, here is a better proposal.

We want people to open attachments.
This should work for most users, not only GNOME users.
Thunderbird should use *-open but it does not, so we need to allow launching programs from /usr/bin.
Programs that have a profile though should be called using Px, the others using sanitized_helper at least.

intrigeri (intrigeri) wrote :

This looks good, but I have one question and would like to see one issue fixed.

review: Needs Fixing
Simon McVittie (smcv) wrote :

On Sat, 01 Jul 2017 at 16:14:03 -0000, intrigeri wrote:
> > + /usr/bin/exo-open rmix,
> > + /{usr/,}bin/* Cx -> sanitized_helper,
> > + /usr/bin/evince Pix,
> > + /usr/bin/totem Pix,
>
> Please use paths that work with merged-/usr i.e. for example
> /{usr/,}bin/evince. I've spent lots of time eliminating all these
> incompatibilities and would rather not see us add new ones now :)

tl;dr I'm not sure this is actually a problem, even with merged /usr.

There are three possible handlings of /usr:

* Separate /usr (traditional setup, e.g. Debian 8): low-level system
  utilities (enough to do basic disaster recovery and mount /usr) are
  in /bin, high-level/large binaries are in /usr/bin

* Merged /usr (Fedora, Debian with usrmerge): Everything is physically
  in /usr/bin, /bin -> /usr/bin is a symlink

* No /usr (Debian GNU/Hurd tried this for a while and decided it was
  too weird even for them): Everything is physically in /bin,
  /usr -> / is a symlink

For basic system utilities (those that are sometimes or always in /bin):

* Hard-coding /bin/sh in AppArmor profiles breaks "merged /usr"
* Hard-coding /usr/bin/sh breaks "separate /usr" and "no /usr"
* Alternations like /{usr/,}bin/sh work for everyone

For "high-level" things like GUIs, that nobody except the "no /usr"
faction would put in /bin:

* Hard-coding /bin/evince breaks everything except the rare "no /usr" case
* Hard-coding /usr/bin/evince breaks the rare "no /usr" case, but I'm
  not convinced that case exists in reality
* Alternations like /{usr/,}bin/evince work for everyone, but are
  unnecessary complexity if the "no /usr" case doesn't really exist

Does the "no /usr" case actually exist in practice? It seems to me that
its only advantage is some debatable conceptual elegance. The major
advantage of merged /usr is that it groups together all large read-only
OS files (/usr/bin, /usr/sbin, /usr/lib*, /usr/share) into a directory
that can easily be a separate read-only mount-point, without including
files that must be in the root filesystem and would prevent it from
being read-only (notably /etc).

intrigeri (intrigeri) wrote :

> On Sat, 01 Jul 2017 at 16:14:03 -0000, intrigeri wrote:
> > > + /usr/bin/exo-open rmix,
> > > + /{usr/,}bin/* Cx -> sanitized_helper,
> > > + /usr/bin/evince Pix,
> > > + /usr/bin/totem Pix,
> >
> > Please use paths that work with merged-/usr i.e. for example
> > /{usr/,}bin/evince. I've spent lots of time eliminating all these
> > incompatibilities and would rather not see us add new ones now :)
>
> tl;dr I'm not sure this is actually a problem, even with merged /usr.

You're right. Sorry I was confused.

Ulrike: so only my other question remains pending :)

Vincas Dargis (talkless) wrote :

Sorry for off-topic, but could you elaborate this:

> tl;dr I'm not sure this is actually a problem, even with merged /usr.

So what are the AppArmor guidelines for these merge/separate usr exactly?

intrigeri (intrigeri) wrote :

> So what are the AppArmor guidelines for these merge/separate usr exactly?

If I got Simon's explanation right: use alternations like /{usr/,}bin/xyz for stuff that's typically shipped in /bin or /lib (in order to support merged-/usr), and don't bother about stuff that's typically shipped in /usr already.

intrigeri (intrigeri) wrote :

Status update: Vincas is going to rebase my last patch on top of the current profile and resubmit – thanks! :)

intrigeri (intrigeri) wrote :

What's the best way to reject this MR in Launchpad? I see I could delete it but it would be nice to keep this discussion archived.

Christian Boltz (cboltz) wrote :

Set the status to "Rejected", like I just did ;-)

intrigeri (intrigeri) wrote :

> Set the status to "Rejected", like I just did ;-)

Thanks!

Unmerged commits

a8b1ce6... by Ulrike Uhlig on 2017-03-19

After some discussion with intrigeri and cboltz, here is a better proposal.

We want people to open attachments.
This should work for most users, not only GNOME users.
Thunderbird should use *-open but it does not, so we need to allow launching programs from /usr/bin.
Programs that have a profile though should be called using Px, the others using sanitized_helper at least.

05c0f96... by Ulrike Uhlig on 2017-03-18

Correct permissions. We want these apps to run only if they have a profile.

058cbba... by Ulrike Uhlig on 2017-03-18

Correct permissions. We want these apps to run using their own profile.

677d1b1... by Ulrike Uhlig on 2017-03-18

Be a little bit permissive when users try to view attachments.

We can't really forbid users to view attachments, but launching external
applications is not allowed by the AA profile for Thunderbird.
For common applications, this should be allowed though.

All uncommon applications should probably be added through a local
profile.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/ubuntu/17.04/usr.bin.thunderbird b/ubuntu/17.04/usr.bin.thunderbird
2index e74e9f5..ab9a9d9 100644
3--- a/ubuntu/17.04/usr.bin.thunderbird
4+++ b/ubuntu/17.04/usr.bin.thunderbird
5@@ -175,6 +175,20 @@ profile thunderbird /usr/lib/thunderbird/thunderbird {
6 /{usr/,}bin/uname Uxr,
7 /usr/bin/locale Uxr,
8
9+ # We want people to open attachments.
10+ # This should work for most users, not only GNOME users.
11+ # Thunderbird should use *-open but it does not, so we need to allow
12+ # launching programs from /usr/bin.
13+ # Programs that have a profile though should be called using Px,
14+ # the others using sanitized_helper at least.
15+ /usr/bin/gnome-open rmix,
16+ /usr/bin/xdg-open rmix,
17+ /usr/bin/exo-open rmix,
18+ /{usr/,}bin/* Cx -> sanitized_helper,
19+ /usr/bin/evince Pix,
20+ /usr/bin/totem Pix,
21+
22+
23 /usr/bin/gpg Cx -> gpg,
24
25 profile gpg {

Subscribers

People subscribed via source and target branches