Revert "private-etc: big profile changes"

[kmk3]

This reverts commit 5d0822c and later
commits that touch the same files (which is necessary in order to revert
the commit in question).

There seems to be a non-trivial amount of changes done in error in the
big refactor from commit 5d0822c ("private-etc: big profile changes",
2023-02-05). For example, there are profiles for CLI programs
(including man.profile) and servers that now contain the @X11 group:

$ git grep -l '^private-etc .*@x11' -- etc
# [...]
etc/profile-a-l/email-common.profile:private-etc @tls-ca,@x11,gnupg,hosts.conf,mailname,timezone
etc/profile-m-z/man.profile:private-etc @x11,groff,man_db.conf,manpath.config,sysless
etc/profile-m-z/mutt.profile:private-etc @tls-ca,@x11,Mutt,Muttrc,Muttrc.d,gai.conf,gnupg,gnutls,hosts.conf,mail,mailname,nntpserver,terminfo
etc/profile-m-z/neomutt.profile:private-etc @tls-ca,@x11,Mutt,Muttrc,Muttrc.d,gnupg,hosts.conf,mail,mailname,neomuttrc,neomuttrc.d,nntpserver
etc/profile-m-z/nextcloud.profile:private-etc @tls-ca,@x11,Nextcloud,host.conf,os-release
etc/profile-m-z/nodejs-common.profile:private-etc @tls-ca,@x11,host.conf,mime.types,rpc,services

Note: These are just the ones that I immediately noticed; it is possible
that there are many that I missed.

Part of the issue is that the groups appear to be inconsistent and
rather broad. For example, paths related to 3D graphics (vulkan) and
audio (openal) are in the @games group, which are not used only by games
and not all games use those standards/libraries. As another example,
the @X11 group contains paths related to GTK, KDE and GPU hardware
acceleration, even though those are not necessarily tied to X11 (and
even though hardware acceleration may be used by headless programs).
Replacing the known paths with groups that are not very granular results
in loss of information about what exactly a profile actually needs and
so makes the profiles less self-documenting. Note also that a given
path could potentially belong to multiple groups, which would preclude
using the "etc-cleanup" tool (in its current form at least), as it would
not know which is the correct group to replace the path with.

Command used to revert the changes:

$ git revert \
  1be9bb3c78b3f129eb2a9fefc07211694c700e4e \
  e889db095873197e999c84077fe28c135b49e43c \
  e6f2374d557c94616b9b9db0bcebe0bbd5d78d88 \
  acb0154ea2a71edf935f7c45cc280b0244937336 \
  740f502aeef509ddec89679d2a9fc24270a8c953 \
  5649bd4568f194eb93eaefb7619d92b57fd27e9c \
  2e4e9d13add71bd0b96246e54e209a29583644b6 \
  0f996ea4de584dc061faf21853d61a600da1a1d8 \
  5d0822c52c9a5e631676899e9642911d9143dba8

Note: This reverts commits from PRs #5641 #5642 #5643, most of which are
later re-applied.

Relates to #5610.

[glitsj16]

For example, there are profiles for CLI programs (including man.profile) and servers that now contain the @X11 group

Part of the issue is that the groups appear to be inconsistent and rather broad.

Replacing the known paths with groups that are not very granular results
in loss of information about what exactly a profile actually needs and
so makes the profiles less self-documenting.

Thanks for explaining the gist of your problem/worries with the current state of the private-etc refactoring. These are all sound and valid arguments IMO and we should have a discussion on them.

That being said - and that's also just my opinion - there's always going to be some amount of 'wins and losses' if one wants to introduce a more abstract way of putting together a working private-etc combination. Can one achieve a perfect 1:1 relationship between the 'old' and the 'new'? Probably not. Can the current grouping be improved upon to 'make more sense' regarding the 'nature' of the application involved (CLI vs GUI, client vs server, GNOME vs KDE, game vs work, etcetera)? Probably yes.

Without wanting to stir up some kind of confusion or 'infight', I do feel there's some (growing) friction with the way impactful changes like this one are being introduced. If there had been a foregoing discussion on the criteria used to distinguish sensible groups of files under /etc we can use to abstract-away the very fine-grained yet hard-to-maintain long lines of the past, I think your arguments would still make sense. Boils down to communication styles/preferences I guess. Please don't take this as some kind of personal criticism. I do understand and appreciate the perspective you are and have been taking in Firejail as a project, especially regarding the state of our git tree(s). Maybe I'm reading too much into some of the recent back and forth's. I do hope we can work this out in a constructive way and keep offering the community of users the best possible combination of security profiles we can, being a voluntary community.

Cheers!

[netblue30]

Let's leave this pr open for now, we can revert the profiles at any time, or we can just go back to the point we were in 0.9.72 and do some merging.

An easy way to cover the hundreds of profiles we have, is to build a default group that will work for most console application, and a x11 group for most X11/gnome/kde apps. sound and network are handled automatically, then we add some miscellaneous groups such as tls-ca and games. We can detect all these groups and additional files with "firejail --build".

It is not worth building different groups for kde and gnome, or QT and GTK. The differences in /etc are small, and you might have programs linking in libraries from both sides.

If vulkan starts showing up in non-game apps, we move it in x11 group. We already have there nvidia and ati. Intel and AMD drivers seem to be integrated in x11 (probably under /etc/X11)

Something interesting regarding /etc directory. The following command works just fine for me:

$ firejail --blacklist=/etc warzone2100

If the program cannot access /etc, it will just work with whatever defaults are hardcoded in the program or libraries. I'm seeing this on most programs. Usually, if you or your Linux distribution didn't modify the file in /etc, you don't really need it. Only sound and networking seem to be affected by /etc.

[glitsj16]

Follow-up to my earlier comment. Hopefully an example can convey what I mean more appropriately. An app I use (however briefly) regularly is gnome-logs. It needs access to /etc/machine-id (besides a few other paths under /etc).

Before the big private-etc refactor:

firejail/etc/profile-a-l/gnome-logs.profile

Line 42 in e4f0f91

private-etc alternatives,fonts,ld.so.cache,ld.so.preload,localtime,machine-id

After:

firejail/etc/profile-a-l/gnome-logs.profile

Line 42 in 45a641d

private-etc

So this profile is currently broken (I'll be filing a PR to unbreak it). Although machine-id is now in the @sound group, adding a specific /etc/foo item (regardless of it already being part of a group) is still supported in a .local override. For me this is working fine:

$ grep private-etc /etc/firejail/gnome-logs.local
# file bug report on private-etc, profile currently broken
# temporary fix
#private-etc @sound
private-etc machine-id

So, yes, I do agree that using private-etc machine-id in this case is indeed the more fine-grained (tighter) way to go about it. The refactor does keep that option on the table if one prefers doing so.

[glitsj16]

Thanks for the review request. I'll do that later, if netblue30 decides to go that route. LGTM though, appreciate the effort and time you must have spent preparing this.

[kmk3]

@glitsj16 I appreciate your kind words and concerns; I'll try to address the
more project-meta points later. For now I'll focus on the main points about
restoring the contents of private-etc (and maybe also discuss improvements to
the groups, but that's a secondary concern).

Besides what was said in the first comment, the discarding of information in
private-etc lines has mid- and long- term drawbacks, which I'll try to unpack
here.

Point 1: The Revert

@netblue30 on Feb 8:

Let's leave this pr open for now, we can revert the profiles at any time

As mentioned, it was not trivial to prepare the revert and for any extra commit
which changes profiles and/or the etc-cleanup tool that were to be added to
master, that would complicate the revert and muddy up the history even further.

Note that commits have to be reverted in the reverse order that they were
applied. So any new such commit on master would require resetting the branch,
reverting that commit, rebasing the branch, potentially editing and re-applying
the commit and resolving any conflicts.

While on the other side of things, as far as I understand it, it is relatively
trivial to run the etc-cleanup tool to do the search and replacing and it can
easily be done again at any time in the future.

So if anything I would say the reverse: Let's do the revert for now (which has
an increasing cost for postponing), because we can run the tool at any time
(which seems relatively trivial and has arguably a much smaller cost for
postponing).

In the mean time we can discuss and improve the groups so that when the tool is
executed, we can be more certain that the result is reasonably correct.

or we can just go back to the point we were in 0.9.72 and do some merging.

There would be two main ways of doing this:

• Reverting the >130 commits since 0.9.72 on master at once
• Resetting master to around 0.9.72

The first would effectively invalidate (but not remove) a significant amount of
commits, which would be a blight in the history that would make future analysis
harder.

Note that this PR already reverts a bunch of commits, but doing the above would
affect an order of magnitude more commits.

The second seems like a rather nuclear option, especially for an established
project. It would cause confusion for everyone that has a clone of the project
and could potentially break CI for downstream projects. Also, rewriting master
at all, not to mention a significant portion of it would be rather sketchy
("Why were all these commits suddenly deleted? Did the repository get
compromised?"), which I think would reduce trust in the project overall
(especially if there were less drastic ways to solve the problem).

And in either case, starting over from scratch would cause a mess with the
references between commits, issues and pull requests. As a contributor, I
would find it very confusing if the commits linked in my PRs stopped being the
"real"/final commits, especially if it was due to something completely
unrelated to the PRs. Also, it would complicate release management tasks
(including managing the release board and the RELNOTES).

Additionally, if you considered starting the release from scratch anyway, why
not proceed with the revert from this PR, which is a much less drastic change?

---

Edit: I did not manage to finish writing the remaining points yet but I wanted
to post at least the first one today. I'll try to post the rest tomorrow.

But for reference, the gist of it is that the information that was added to
private-etc lines over time could also be used in other, more impactful ways
(not necessarily related to /etc).

[kmk3]

@glitsj16 on Feb 8:

Before the big private-etc refactor:

firejail/etc/profile-a-l/gnome-logs.profile

Line 42 in e4f0f91

private-etc alternatives,fonts,ld.so.cache,ld.so.preload,localtime,machine-id

After:

firejail/etc/profile-a-l/gnome-logs.profile

Line 42 in 45a641d

private-etc

So this profile is currently broken (I'll be filing a PR to unbreak it).

That's the thing: machine-id is not used only by pulseaudio; it may be used by
programs and libraries for any reason.

gnome-logs.profile had both nosound and private-etc machine-id, and yet
machine-id was indiscriminately removed from its private-etc (and from the
private-etc of every other profile apparently).

This is one of the reasons for the revert: The way that the search and replace
was done is not correct and at the very least it broke an unknown amount of
profiles in an unknown number of ways (while also removing some
hardening-related information). Verifying and fixing every profile seems to be
significantly more work (and many of them could remain broken after the
attempt) than reverting the refactor and fixing the tool (and the underlying
assumptions) and the groups before running it again.

[kmk3]

Programs using GPU acceleration and Games

@netblue30 on Feb 8:

An easy way to cover the hundreds of profiles we have, is to build a default
group that will work for most console application, and a x11 group for most
X11/gnome/kde apps. sound and network are handled automatically, then we add
some miscellaneous groups such as tls-ca and games.

I think that this scheme is likely to lead to confusion because some programs
are Wayland-specific (like sway and rofi). Naming it @gui (like the previous
GUI group name) seems like it would be more accurate. Though I still think
it's too generic/encompassing (I'll expand on this in the next comment).

If vulkan starts showing up in non-game apps, we move it in x11 group. We
already have there nvidia and ati.

Some examples of non-game apps that use or plan to use OpenGL/Vulkan:

GFX tools (Blender), video editors (Da Vinci Resolve), image editors (Krita).

OpenGL is used in some compositors for making desktop effects smoother and
Vulkan is planned to be used in KDE Plasma 6.

Vulkan Video is becoming a thing, which will allow using standardized Vulkan
APIs for video encoding and decoding (in place of things like VA-API and
NVENC). This could be used by any application which plays media files, such as
media players (mpv), web browsers (Firefox) and messaging programs (many
Electron-based ones).

Intel and AMD drivers seem to be integrated in x11 (probably under /etc/X11)

Driver-specific options can be configured in the X server configuration in
/etc/X11/xorg.conf.d/, but I don't think X clients would generally need access
to anything in /etc/X11. I'm fairly sure that configuration for things related
to OpenGL and Vulkan are not done in /etc/X11 at all, especially since they can
be used without X11 (such as headless or in Wayland).

Note also that in the case of Mesa (which is usually the default for at least
AMD and Intel on the desktop), OpenGL/Vulkan are implemented in userspace and
that kernel modesetting has been the default on the desktop for a while (rather
than being done in the X server).

sound and network are handled automatically

That sounds good and I think it would make sense to do the same for 3d.

Suggestion: Create a @3d group and automatically allow everything in it
unless no3d is used. It would contain only paths for GPU acceleration, such
as paths for mesa and for the proprietary nvidia driver.

I think that compared to the @games group, doing the above would be less
ambiguous, would be easier for users and would require less profile
maintenance.

As for the paths used by games, proprietary games tend to be started from
proprietary launchers (such as Steam) and tend to use proprietary frameworks
(such as Unreal Engine/Unity3D), which do not usually have system-wide
configuration paths AFAIK (at least not in /etc).

In the case of tools and frameworks used in fully libre games, I think that
there is a wide variety of them and I'm not sure that they are common enough to
be worth throwing all of them in a group. For example, as of 0.9.72 there are
only 11 profiles that mention openal in private-etc.

[kmk3]

@netblue30

I have some more comments about the groups and etc, but considering at least
the following regressions/inconsistencies that come from the commits relevant
to this PR:

• The @x11 group was added to profiles of non-GUI programs
• machine-id was removed from the private-etc of all profiles, presumably
  with the assumption that it wouldn't be needed by any profile anymore, but at
  least one profile broke due to the removal

And considering that:

• There may be more profiles affected and also other problems not yet
  discovered
• This PR only affects the profiles; it does not affect the source code

Do you see any downsides for reverting the commits in order to fix the above?