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

Install provided profiles under LIBDIR/firejail-profiles #4871

Open
rusty-snake opened this issue Jan 20, 2022 · 17 comments
Open

Install provided profiles under LIBDIR/firejail-profiles #4871

rusty-snake opened this issue Jan 20, 2022 · 17 comments
Labels
enhancement New feature request

Comments

@rusty-snake
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

  1. You can not add system-wide overrides for .profile/.inc files
  2. You normally should not modify installed .profile files, moving them out of /etc should make this clear.
  3. If you edit a .profile (instead of using .local) --debug does not indicated this.

Describe the solution you'd like

  • Profiles provided by firejail should be installed under /usr/lib/firejail-profiles (or to whatever --libdir is configured).
  • System-wide customizations (.local and .profile/.inc) will be under /etc/firejail(-profiles). i.e. you can create /etc/firejail(-profiles)/firefox.profile at it will not be overwritten.
  • User customizations are at ~/.config/firejail (nothing changed)

Describe alternatives you've considered

/usr/share (i.e. --datadir) but I think /usr/lib is better for provided configurations.

Additional context

@rusty-snake rusty-snake added the enhancement New feature request label Jan 20, 2022
@reinerh
Copy link
Collaborator

reinerh commented Jan 20, 2022

Describe alternatives you've considered

/usr/share (i.e. --datadir) but I think /usr/lib is better for provided configurations.

I think /usr/share is actually the better place, as this is the place for platform-independent files, while /usr/lib should contain platform-specific files (afaik).

@rusty-snake
Copy link
Collaborator Author

systemd* and NetworkManager use /usr/lib, pipewire uses /usr/share, ... looks like there is no perfect location.

@reinerh
Copy link
Collaborator

reinerh commented Jan 20, 2022

According to FHS:

https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s11.html

The /usr/share hierarchy is for all read-only architecture independent data files.

https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s06.html

/usr/lib includes object files and libraries. [21] On some systems, it may also include internal binaries that are not intended to be executed directly by users or shell scripts. [22]

Applications may use a single subdirectory under /usr/lib. If an application uses a subdirectory, all architecture-dependent data exclusively used by the application must be placed within that subdirectory. [23]

@topimiettinen
Copy link
Collaborator

FHS is getting outdated. Still, profiles probably shouldn't be shared across different architectures, even though they are text files. For example, system calls lists for seccomp may be different and white/blacklisting could refer to architecture dependent paths.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 22, 2022

@rusty-snake commented on Jan 20:

Is your feature request related to a problem? Please describe.

  1. You can not add system-wide overrides for .profile/.inc files

While overriding the upstream profiles with non-packaged files system-wide
could perhaps be a "cleaner" way to configure things, as a user I think it's
fine to just replace an upstream profile with the intended version (either by
manually editing the file or with automation). One benefit of the latter
approach is that the package manager notifies of any changes to the upstream
profile (and another is that this is the usual workflow for system
configuration for most packages AFAIK). Related comment:
#2097 (comment)

  1. You normally should not modify installed .profile files, moving them out
    of /etc should make this clear.

If by "normally should not modify" you mean that .local files are the preferred
form of modification, this could also be made clearer with better
documentation.

Also, user story:

The first thing I do when debugging profiles is to edit/comment things in
/etc/firejail. This usually means editing .profile files; sometimes I only
bother creating a .local file when I'm confident that my changes work (and if
they don't, I just undo them), to avoid unnecessarily creating files in
/etc/firejail.

Additionally, even if not editing the .profile file, currently I can simply do
this to open both the .profile and .local files:

# opens firefox.local, firefox.profile
vim /etc/firejail/firefox.*

To create a .local file:

cd /etc/firejail
vim foo.{profile,local}

To see only the local overrides:

ls -l /etc/firejail/*.local

This is the most important part to me, as having the profiles split across
multiple base dirs would make these workflows more cumbersome.

  1. If you edit a .profile (instead of using .local) --debug does not
    indicated this.

While I agree that a user failing to provide important details on an issue is
annoying, it is still their responsibility to provide enough information to
make a report reproducible.

Maybe this could be mitigated by making it part of the issue template
checklist. Example:

  • I have not modified the relevant .profile files in /etc/firejail

Also, while I agree that users are less likely to modify files in /usr, putting
files in there does not prevent them from doing so. If a package puts its
config files in /usr/(lib|share)/$package, one might just end up using that dir
as if it were /etc/$package anyway if it's more convenient.

Describe the solution you'd like

  • Profiles provided by firejail should be installed under
    /usr/lib/firejail-profiles (or to whatever --libdir is configured).

I think that this would make profiles less discoverable. Whatever is in /etc,
I know that it is intended to be both readable and editable. So if I have a
problem with a program, usually I can just use the result of
pacman -Qlq $program | grep /etc to find out what knobs I can turn.

I don't really expect to ever open anything on /usr/lib unless I want to debug
a binary or something. As far as I know, it usually contains just binaries and
"implementation details".

/usr/share might contain documentation, but again, it's rare that I'll ever
open something from it directly, especially when needing to configure a
program.

  • System-wide customizations (.local and .profile/.inc) will be under
    /etc/firejail(-profiles). i.e. you can create
    /etc/firejail(-profiles)/firefox.profile at it will not be overwritten.

I think having to deal with 2 different base directories for system
configuration would make things more confusing; I really like the simplicity of
how it currently works.

  • User customizations are at ~/.config/firejail (nothing changed)

@kmk3
Copy link
Collaborator

kmk3 commented Jan 22, 2022

@topimiettinen commented on Jan 20:

Still, profiles probably shouldn't be shared across different architectures,
even though they are text files.

If they are not to be shared by all architectures, then how would they be
divided? Would there be profiles that are architecture-specific overrides
(e.g.: amd64/firefox.profile or firefox.amd64.profile)? That does not sound
fun to maintain.

For example, system calls lists for seccomp may be different

Do you mean in the arguments to seccomp?

If seccomp is breaking profiles on certain architectures, maybe we could add
a seccomp subcommand for every supported architecture. Example:

seccomp call1,call2 # generic syscalls expected to be present on every arch
seccomp-amd64 call3,call4 # architecture-specific
seccomp-armv7 call5,call6
seccomp-ia32 call7,call8

(Or whatever names are used for each arch in the kernel)

And if seccomp.block-secondary is used, the commands for the non-native archs
could be no-ops.

and white/blacklisting could refer to architecture dependent paths.

Well, there are already references to e.g.: /usr/lib64, which may not exist for
all architectures and may not exist on all distros. I think that this could be
helped by adding more macros, such as e.g.: ${LIB}.

@topimiettinen
Copy link
Collaborator

@topimiettinen commented on Jan 20:

Still, profiles probably shouldn't be shared across different architectures,
even though they are text files.

If they are not to be shared by all architectures, then how would they be divided? Would there be profiles that are architecture-specific overrides (e.g.: amd64/firefox.profile or firefox.amd64.profile)? That does not sound fun to maintain.

For example, system calls lists for seccomp may be different

Do you mean in the arguments to seccomp?

Most of the system calls are common to all architectures, so in practice there shouldn't be a problem. System call lists also help. Using raw system call numbers ($1234) wouldn't be portable, but I don't think the profiles use them.

If seccomp is breaking profiles on certain architectures, maybe we could add a seccomp subcommand for every supported architecture. Example:

seccomp call1,call2 # generic syscalls expected to be present on every arch
seccomp-amd64 call3,call4 # architecture-specific
seccomp-armv7 call5,call6
seccomp-ia32 call7,call8

(Or whatever names are used for each arch in the kernel)

That could work, or if the architecture specific system calls don't overlap, they could be ignored on other architectures.

And if seccomp.block-secondary is used, the commands for the non-native archs could be no-ops.

Yes.

and white/blacklisting could refer to architecture dependent paths.

Well, there are already references to e.g.: /usr/lib64, which may not exist for all architectures and may not exist on all distros. I think that this could be helped by adding more macros, such as e.g.: ${LIB}.

Yes, but there could be blacklists blocking some of /usr/lib/x86_64-linux-gnu/, /usr/lib/i386-linux-gnu/ and /usr/lib/x32-linux-gnu/ depending on the architecture.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 24, 2022

@topimiettinen commented on Jan 22:

and white/blacklisting could refer to architecture dependent paths.

Well, there are already references to e.g.: /usr/lib64, which may not exist
for all architectures and may not exist on all distros. I think that this
could be helped by adding more macros, such as e.g.: ${LIB}.

Yes, but there could be blacklists blocking some of
/usr/lib/x86_64-linux-gnu/, /usr/lib/i386-linux-gnu/ and
/usr/lib/x32-linux-gnu/ depending on the architecture.

Yes; maybe this could be done automatically when handling
seccomp.block-secondary. Example:

if (arg_seccomp_block_secondary) {
#ifndef __AMD64__
	profile_add("blacklist /usr/lib/x86_64-*");
#endif
#ifndef __ARMV7__
	profile_add("blacklist /usr/lib/armv7-*");
#endif
// ...
}

@topimiettinen
Copy link
Collaborator

@topimiettinen commented on Jan 22:

and white/blacklisting could refer to architecture dependent paths.

Well, there are already references to e.g.: /usr/lib64, which may not exist
for all architectures and may not exist on all distros. I think that this
could be helped by adding more macros, such as e.g.: ${LIB}.

Yes, but there could be blacklists blocking some of
/usr/lib/x86_64-linux-gnu/, /usr/lib/i386-linux-gnu/ and
/usr/lib/x32-linux-gnu/ depending on the architecture.

Yes; maybe this could be done automatically when handling seccomp.block-secondary. Example:

if (arg_seccomp_block_secondary) {
#ifndef __AMD64__
	profile_add("blacklist /usr/lib/x86_64-*");
#endif
#ifndef __ARMV7__
	profile_add("blacklist /usr/lib/armv7-*");
#endif
// ...
}

Good idea, though I'd make it a separate option. This wouldn't help if the profile files refer to the architecture dependent paths, for example blacklist /usr/lib/x86_64-linux-gnu/gtk-3.0. Maybe the paths could be managed with something more generic like ${NATIVE_ARCH} (e.g. x86_64) and ${NON_NATIVE_ARCHS} (e.g. i386, arm etc.), maybe also ${SECONDARY_ARCHS} (i386, x32) for use in paths. So instead of hardcoding this to Firejail, common profile files could use blacklist /usr/lib/${NON_NATIVE_ARCHS}-*.

@kmk3
Copy link
Collaborator

kmk3 commented Jan 25, 2022

@topimiettinen commented on Jan 24:

Good idea, though I'd make it a separate option. This wouldn't help if the
profile files refer to the architecture dependent paths, for example
blacklist /usr/lib/x86_64-linux-gnu/gtk-3.0.

(Continued on #4879)

@rusty-snake
Copy link
Collaborator Author

rpmlint says

W: only-non-binary-in-usr-lib
There are only non binary files in /usr/lib so they should be in /usr/share.

text files can be arch specific too ...

Does lintian something like this?

@reinerh
Copy link
Collaborator

reinerh commented Jan 31, 2022

Lintian does not complain about "only non-binary" files in /usr/lib. I tested by installing the profiles in the firejail-profiles package into /usr/lib/firejail instead of /etc/firejail.

(It has a warning for the other "direction": arch-dependent-file-in-usr-share; and it also currently complains about executables in /usr/lib (instead of libexec): executable-in-usr-lib)

@pirate486743186
Copy link
Contributor

I think intuitively people expect to find configuration files in etc. A better alternative would be to create new folders in /etc/firejail . Some applications are annoyingly hiding some configuration files in var, so that you can't find them.

for example something like this:

/etc/firejail/profiles
/etc/firejail/includes
/etc/firejail/local (higher priority then profiles and includes, lower then .config/firejail)

@reinerh
Copy link
Collaborator

reinerh commented Mar 1, 2022

Just as another example, the recommended path for dbus policy files has recently moved in Debian from /etc/dbus-1/system.d to /usr/share/dbus-1/system.d (see also #1006631).

@gioele
Copy link

gioele commented Jan 7, 2024

Any update on this?

For the record, firejail-profiles is at the moment the Debian package with the second highest number of files installed under /etc (1207).

@rusty-snake
Copy link
Collaborator Author

OT: Out of interest who is the winner?

@gioele
Copy link

gioele commented Jan 7, 2024

OT: Out of interest who is the winner?

libopenzwave1.6 with 1489 files (of which 635 PNG images...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request
Projects
None yet
Development

No branches or pull requests

6 participants