profile.template: add missing noautopulse option

[kmk3]

Added on commit 617ff40 ("add --noautopulse arg for complex pulse
setups") / PR #1854.

Note: The template was added after that, on commit cb98aea ("Add
profile templates").

Misc: I noticed that it was missing when comparing it to
contrib/vim/syntax/firejail.vim on commit 22a91ae ("contrib/vim: add
missing noinput command to syn match") / PR #4259.

Cc: @vermeeren (as the author of #1854)

[rusty-snake]

I'm against adding it. It is only used in agetpkg and looks wrong there too.

$ grep noautopulse /etc/firejail/*
/etc/firejail/agetpkg.profile:noautopulse

[kmk3]

@rusty-snake left a comment:

I'm against adding it. It is only used in agetpkg and looks wrong there too.

$ grep noautopulse /etc/firejail/*
/etc/firejail/agetpkg.profile:noautopulse

Why is that? Is it something that should only be used on local profiles?

I thought about adding it for completeness, as it's mentioned on firejail(1)
and firejail-profile(5).

How about commenting that it should not be used then? Example:

--- a/etc/templates/profile.template
+++ b/etc/templates/profile.template
@@ -145,6 +145,7 @@ include globals.local
 #net none
 #netfilter
 #no3d
+##noautopulse (use only on local profiles, see #4269)
 ##nodbus (deprecated, use 'dbus-user none' and 'dbus-system none', see below)
 #nodvd
 #nogroups

[rusty-snake]

noautopulse
Disable automatic ~/.config/pulse init, for complex setups such as remote
pulse servers or non-standard socket paths.

noautopulse isn't for hardening, it is for workarounds => usage in upstream profiles does not make sense.

profile.template should make it easy/easier to contribute new profiles to firejail. Adding .local-only commands would make this more complex.

Moreover there are a lot of commands in firejail-profile(5) "missing" in profile.template. For me that are nowhitelist, blacklist-nolog, bind, private-cwd, private-srv, tmpfs, caps, seccomp.32, seccomp.32.drop, seccomp.keep, seccomp.32.keep, cgroup, cpu, nice, rlimit-as, rlimit-cpu, rlimit-fsize, rlimit-nproc, rlimit-nofile, rlimit-sigpending, timeout, name and deterministic-exit-code. But I ./configure --enable-selinux --enable-force-nonewprivs --disable-chroot --disable-file-transfer --disable-firetunnel --disable-network --disable-output --disable-overlayfs --disable-private-home --disable-x11 which removes lot of commands from the manpage (because they are disabled).

[glitsj16]

I'm against adding it. It is only used in agetpkg and looks wrong there too.

@rusty-snake Correct, I'll take it out.

[kmk3]

It is only used in agetpkg and looks wrong there too.

$ grep noautopulse /etc/firejail/*
/etc/firejail/agetpkg.profile:noautopulse

noautopulse
Disable automatic ~/.config/pulse init, for complex setups such as remote
pulse servers or non-standard socket paths.

noautopulse isn't for hardening, it is for workarounds => usage in upstream
profiles does not make sense.

profile.template should make it easy/easier to contribute new profiles to
firejail. Adding .local-only commands would make this more complex.

Moreover there are a lot of commands in firejail-profile(5) "missing" in
profile.template. For me that are nowhitelist, blacklist-nolog, bind,
private-cwd, private-srv, tmpfs, caps, seccomp.32,
seccomp.32.drop, seccomp.keep, seccomp.32.keep, cgroup, cpu,
nice, rlimit-as, rlimit-cpu, rlimit-fsize, rlimit-nproc,
rlimit-nofile, rlimit-sigpending, timeout, name and
deterministic-exit-code.

So agetpkg was added on #3887. @glitsj16 Do you remember why the option was
included?

My suspicion: Since agetpkg is a CLI tool that does not need access to
basically anything, all of the no options were added. If that's the case,
then it seems like something that I might accidentally do as well, precisely
because even though the option "isn't for hardening", it kind of looks like it
is. As in, nosomething looks like hardening and noautopulse is the only
no option that is not used for that.

Say, an unsuspecting individual glancing over the options might assume that if
both noautopulse and nosound exist, then noautopulse means no pulseaudio
access and that nosound just means no direct hardware access to the sound
device(s), so "why not use both?".

Another factor is that, unlike many of the options that you listed, this one is
a simple toggle, which makes it seem like a hardening "easy win". Put another
way, finding the right arguments for e.g.: seccomp.and rlimit- seems like a
lot of work, but adding what looks like a simple "less sound access" toggle?
Sure, why not?

You also mentioned private-srv, which to me, even after looking at the man
pages, seems like another hardening "easy win" / "nice to have" for a profile.
I have wondered "why not?" and actually used it on local profiles before
(without any specific reason to) and would likely have eventually opened a PR
just like this one to add it to the template until you implied that it's not
meant for general-purpose.

Anyway, I think making it clear on the template itself that noautopulse
should not be used would help avoid confusion.

But I ./configure --enable-selinux --enable-force-nonewprivs --disable-chroot --disable-file-transfer --disable-firetunnel --disable-network --disable-output --disable-overlayfs --disable-private-home --disable-x11 which removes lot of commands from the manpage (because they
are disabled).

In this case, there is no --disable-noautopulse.

[glitsj16]

So agetpkg was added on #3887. @glitsj16 Do you remember why the option was
included?
My suspicion: Since agetpkg is a CLI tool that does not need access to
basically anything, all of the no options were added.

@kmk3 That's fully correct. I added it in an overzealous attempt to make agetpkg as tight as possible. Thanks to @rusty-snake now I know better.

You also mentioned private-srv, which to me, even after looking at the man
pages, seems like another hardening "easy win" / "nice to have" for a profile.
I have wondered "why not?" and actually used it on local profiles before
(without any specific reason to) and would likely have eventually opened a PR
just like this one to add it to the template until you implied that it's not
meant for general-purpose.

I can see private-srv in 8 of the current profiles. It's nice to have a clearcut understanding and profile template to avoid mistakes like this. Thanks to you both for all the work on the template! What do you guys think, shall we take out private-srv?

[rusty-snake]

What do you guys think, shall we take out private-srv?

I would prefer to add blacklist /srv to disable-common.inc. This would cover much much more profiles then adding private-srv none and is unlikely to break anything. It is also the cleaner way, because private-srv none does not mean empty, it means only the directory "none" (and if there is no "none", then it is empty).

private-srv www (for example) is only interesting for server which actually use /srv. And even then whitelist /srv/www+read-only /srv would be preferred in the most cases as private-srv is slow and heavy due to it's copy semantic.

[glitsj16]

I would prefer to add blacklist /srv to disable-common.inc. This would cover much much more profiles then adding private-srv none and is unlikely to break anything.

Makes perfect sense. I'll hold off, you are on top of this apparently 👍.

@kmk3 Apologies for semi-hijacking this thread.

[vermeeren]

Hi, thanks for the CC @kmk3,

I agree that noautopulse option is for non-standard setups, so including it in a tl;dr-type example should not be done in my opinion.

Anyway, I think making it clear on the template itself that noautopulse
should not be used would help avoid confusion.

Personally I would indeed document it somewhere. An explicit "not in the tl;dr version on purpose" avoids confusion about whether it was simply forgotten or not. I have not worked with firejail in some years however, so I am not sure how it should be done for this project, if at all.

Cheers.

[kmk3]

@glitsj16 commented 22 hours ago:

I would prefer to add blacklist /srv to disable-common.inc. This would
cover much much more profiles then adding private-srv none and is unlikely
to break anything.

Makes perfect sense. I'll hold off, you are on top of this apparently +1.

@kmk3 Apologies for semi-hijacking this thread.

No problem; it's good to know more about private-srv, especially since it
seems kind of elusive.

@vermeeren commented 20 hours ago:

Hi, thanks for the CC @kmk3,

I agree that noautopulse option is for non-standard setups, so including it
in a tl;dr-type example should not be done in my opinion.

Anyway, I think making it clear on the template itself that noautopulse
should not be used would help avoid confusion.

Personally I would indeed document it somewhere. An explicit "not in the
tl;dr version on purpose" avoids confusion about whether it was simply
forgotten or not. I have not worked with firejail in some years however, so I
am not sure how it should be done for this project, if at all.

Cheers.

Thanks for the input.

Okay, so if there is no agreement on including noautopulse on
profile.template, then how about adding the following to the man pages:

Note: This is not intended as a hardening option.

It doesn't solve the problem but it's at least something.

Ultimately, I think that the main issue stems from the option itself looking
like it's for hardening. Therefore, I suppose that the only real "fix" would
be to rename it, for example, to keep-config-pulse. This is similar to the
keep-dev-shm and keep-var-tmp options, which are used to "leave a path
alone", just like noautopulse. Alternatively, disable-autopulse (similar
to disable-mnt) would look closer to what it is now, albeit I don't think it
looks quite as intuitive as the former suggestion.

If this makes sense, then both versions could be supported at once for a while,
with noautopulse being deprecated and eventually removed.

I'm usually not a fan of renaming config options, but I think that this one has
caused enough confusion to warrant it.

[glitsj16]

+1 for renaming it to keep-config-pulse.

[rusty-snake]

+1 from me too

[kmk3]

Closing in favor of #4278.