Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include AppArmor Abstractions #589

Closed
wants to merge 3 commits into from

Conversation

teward
Copy link
Contributor

@teward teward commented Jun 29, 2021

Fixes #588.

The apparmor rules that are introduced break a few things by not including required abstractions.

  • There are DBUS denies for opening dialog boxes and file open boxes, which need DBUS abstractions to access the user sessions. Fixed by including abstractions/dbus-session (which also implicitly imports abstractions/dbus-session-strict for SystemD user sessions) in the apparmor rules, if the abstractions exist.
  • There are X abstractions which are useful and needed for X display integration. So if those abstractions exist, include them. HOWEVER, there's a problem in Ubuntu where these abstractions are not properly handling the X Display Sockets in /tmp/.X11-unix/* so we need to permit those with a specific override. See the comment block in the profile for details.

Access to DBUS is needed for some of the system behavior for dialog boxes.  There is an inbuilt in Ubuntu for these abstractions, so include if they're present.  This resolves any of the user session DBUS socket connection errors.
X abstractions are needed since we use X Server for the display bits for Firefox Browser.

On Ubuntu/Lubuntu though, in 21.04 and up, the "write" permission is yanked from the X display sockets in `/tmp/.X11-unix/*` though, and is needed, so we'll override it for now in-profile.  This addresses torproject#588.
@teward teward requested a review from intrigeri as a code owner June 29, 2021 19:49
@teward teward changed the title Include AppArmor Abstractions and relevant Overrides Include AppArmor Abstractions Jun 30, 2021
The regression in X11 abstractions has been fixed in upstream apparmor, and is being patched in the next few days or so in Impish and eventually Hirsute.
@@ -8,6 +8,8 @@ profile torbrowser_firefox @{torbrowser_firefox_executable} {
#include <abstractions/gnome>
#include <abstractions/ibus>
#include if exists <abstractions/vulkan>
#include if exists <abstractions/dbus-session>
#include if exists <abstractions/X>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is included by abstractions/gnome already, so I don't understand why we need this. Could you please clarify?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@intrigeri Not every system has GNOME on it. Lubuntu for instance doesn't, and even though the 'gnome' abstractions exist as part of the package apparmor in Ubuntu, the GNOME abstractions don't work and/or are not properly included in a case where GNOME is not running. It became necessary specifically for LXQt and Lubuntu to include that abstractions inclusion for X as well to make sure it behaves.

The DBUS inclusion is needed independently, though, for some dialog box bits to function.

@intrigeri
Copy link
Collaborator

intrigeri commented Aug 13, 2021 via email

@teward
Copy link
Contributor Author

teward commented Feb 23, 2024

Please clarify why this became necessary, that is which exact AppArmor denials happen when the X abstraction is included transitively via the gnome abstraction, but don't happen when the X abstraction is included directly.

This became necessary because while the X inclusion is included in the GNOME abstractions, when you are on an LXQt system or such that does not have GNOME, the X abstractions are not included. Therefore, in that environment, when it relies on the X abstractions that were pulled in via the GNOME abstractions. Line 8 for example is #include <abstractions/gnome> which has in THAT abstraction an inclusion on the X abstractions. But if abstractions/gnome doesn't exist (and it doesn't when GNOME is not installed), you lose the X abstractions that're implicitly included by the GNOME abstractions.

@intrigeri
Copy link
Collaborator

intrigeri commented Feb 26, 2024 via email

@teward
Copy link
Contributor Author

teward commented Feb 26, 2024

But if abstractions/gnome doesn't exist (and it doesn't when GNOME
is not installed)

I'm very surprised to read this. On which distributions is it
the case?

The distributions I'm aware of unconditionally ship
abstractions/gnome with their AppArmor package, regardless of
whether GNOME is installed or not.

FYSA this issue has been moved to the Tor Gitlab.

As of my last testing, Lubuntu which is a flavor of Ubuntu has a highly divergent packageset from standard Ubuntu and ships no GNOME library components whatsoever, which is why it doesnt get included in the defaults. This was noticed in the last release of Lubuntu and the LTS in 2022 as well. To my knowledge this has not changed in Ubuntu and therefore the GNOME abstractions are still not available to AppArmor on Lubuntu. Other flavors of Ubuntu may be affected, but Lubuntu so far has been the only 'oddball'.

Regardless, it makes sense to include the X abstractions anyways rather than rely on the assumption that "every distro will have the GNOME abstractions as a default inclusion" so as to address any case where this assumption is false.

@teward
Copy link
Contributor Author

teward commented Feb 26, 2024

I'm moving tracking of this issue to https://gitlab.torproject.org/tpo/applications/torbrowser-launcher/-/merge_requests/16 - since this is now on the Upstream environment of Gitlab. @intrigeri you've been assigned as reviewer on that issue by boklm so I'm closing this here as there's an equivalent PR up on Gitlab.

@teward teward closed this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AppArmor Issue on non-GNOME environments?
2 participants