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

Proposed by Ulrike Uhlig
Status: Rejected
Rejected by: Christian Boltz
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
AppArmor Developers 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.
Revision history for this message
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

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

05c0f96... by Ulrike Uhlig

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

a8b1ce6... by Ulrike Uhlig

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.

Revision history for this message
intrigeri (intrigeri) wrote :

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

review: Needs Fixing
Revision history for this message
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).

Revision history for this message
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 :)

Revision history for this message
intrigeri (intrigeri) wrote :
Revision history for this message
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?

Revision history for this message
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.

Revision history for this message
intrigeri (intrigeri) wrote :

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

Revision history for this message
intrigeri (intrigeri) wrote :
Revision history for this message
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.

Revision history for this message
Christian Boltz (cboltz) wrote :

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

Revision history for this message
intrigeri (intrigeri) wrote :

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

Thanks!

Unmerged commits

a8b1ce6... by Ulrike Uhlig

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

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

058cbba... by Ulrike Uhlig

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

677d1b1... by Ulrike Uhlig

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
diff --git a/ubuntu/17.04/usr.bin.thunderbird b/ubuntu/17.04/usr.bin.thunderbird
index e74e9f5..ab9a9d9 100644
--- a/ubuntu/17.04/usr.bin.thunderbird
+++ b/ubuntu/17.04/usr.bin.thunderbird
@@ -175,6 +175,20 @@ profile thunderbird /usr/lib/thunderbird/thunderbird {
175 /{usr/,}bin/uname Uxr,175 /{usr/,}bin/uname Uxr,
176 /usr/bin/locale Uxr,176 /usr/bin/locale Uxr,
177177
178 # We want people to open attachments.
179 # This should work for most users, not only GNOME users.
180 # Thunderbird should use *-open but it does not, so we need to allow
181 # launching programs from /usr/bin.
182 # Programs that have a profile though should be called using Px,
183 # the others using sanitized_helper at least.
184 /usr/bin/gnome-open rmix,
185 /usr/bin/xdg-open rmix,
186 /usr/bin/exo-open rmix,
187 /{usr/,}bin/* Cx -> sanitized_helper,
188 /usr/bin/evince Pix,
189 /usr/bin/totem Pix,
190
191
178 /usr/bin/gpg Cx -> gpg,192 /usr/bin/gpg Cx -> gpg,
179193
180 profile gpg {194 profile gpg {

Subscribers

People subscribed via source and target branches