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.

Sign up for GitHub

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

Jump to bottom

nogroups + wrc prints confusing messages #4930

Closed
rusty-snake opened this issue Feb 11, 2022 · 2 comments · Fixed by #4933
Closed

nogroups + wrc prints confusing messages #4930

rusty-snake opened this issue Feb 11, 2022 · 2 comments · Fixed by #4933
Labels
bug Something isn't working
Projects
Release 0.9.70

Comments

@rusty-snake
Copy link
Collaborator

rusty-snake commented Feb 11, 2022 •
edited by kmk3

Description

nogroups + wrc prints confusing messages.

Steps to Reproduce

  1. Create test.profile (ordering doesn't matter)
include whitelist-run-common.inc
nogroups
  1. test, run and see
$ firejail --profile=./test.profile groups
Reading profile ./test.profile
Reading profile /etc/firejail/whitelist-run-common.inc
Parent pid 1234, child pid 1235
Warning: logind not detected, nogroups command ignored     <--- is a lie
Warning: cleaning all supplementary groups
Child process initialized in 30.00 ms
rusty-snake    <---- running `groups` outside of the sandbox shows more so groups are actually cleaned

Parent is shutting down, bye...

Expected behavior

  1. Warning: logind not detected is only shown when there is no logind
  2. Warning: nogroups command ignored is only shown when it is relly ignored

Actual behavior

  1. Warning: logind not detected is shown on logind systems
  2. Warning: nogroups command ignored; cleaning all supplementary groups makes no sense and confuses users.

Additional context

Environment

  • Fedora Linux 35 (of course with systemd/logind)
firejail version 0.9.69 (03395e1)

Compile time support:
	- always force nonewprivs support is disabled
	- AppArmor support is disabled
	- AppImage support is enabled
	- chroot support is disabled
	- D-BUS proxy support is enabled
	- file transfer support is disabled
	- firetunnel support is disabled
	- networking support is disabled
	- output logging is disabled
	- overlayfs support is disabled
	- private-home support is disabled
	- private-cache and tmpfs as user enabled
	- SELinux support is enabled
	- user namespace support is enabled
	- X11 sandboxing support is disabled

Edit by @kmk3: Formatting.
@glitsj16
Copy link
Collaborator

Reproduced on Arch Linux. I can definately agree on Expected behavior as described above. Nice find 👍.

@kmk3
Copy link
Collaborator

kmk3 commented Feb 11, 2022

@rusty-snake commented on Feb 11:

Description

nogroups + wrc prints confusing messages.

Steps to Reproduce

  1. Create test.profile (ordering doesn't matter)
include whitelist-run-common.inc
nogroups
  1. test, run and see
$ firejail --profile=./test.profile groups
Reading profile ./test.profile
Reading profile /etc/firejail/whitelist-run-common.inc
Parent pid 1234, child pid 1235
Warning: logind not detected, nogroups command ignored     <--- is a lie
Warning: cleaning all supplementary groups
Child process initialized in 30.00 ms
rusty-snake    <---- running `groups` outside of the sandbox shows more so groups are actually cleaned

Parent is shutting down, bye...

Good catch.

I think this probably happens because check_can_drop_all_groups is called
multiple times and at different places (so it probably runs both before and
after the whitelisting).

IIRC I had tried originally to make the checks run only once a startup, which
would be less likely to fail, but it didn't work for some reason.

I think I'll just comment these messages for now until a better solution is
found.

kmk3 added a commit to kmk3/firejail that referenced this issue Feb 11, 2022
@kmk3
Disable/comment message about nogroups being ignored
1db6740 
Added on commit 7abce0b ("Fix keeping certain groups with nogroups",
2021-11-30) / PR netblue30#4732.

As reported by @rusty-snake on netblue30#4930, conflicting messages are printed
when using whitelist-run-common.inc with nogroups:

    $ cat test.profile
    include whitelist-run-common.inc
    nogroups
    $ firejail --profile=./test.profile groups
    Reading profile ./test.profile
    Reading profile /etc/firejail/whitelist-run-common.inc
    Parent pid 1234, child pid 1235
    Warning: logind not detected, nogroups command ignored     <--- is a lie
    Warning: cleaning all supplementary groups
    Child process initialized in 30.00 ms
    rusty-snake    <---- running `groups` outside of the sandbox shows more so groups are actually cleaned

    Parent is shutting down, bye...

This probably happens because wrc causes /run/systemd to be hidden in
the sandbox and because check_can_drop_all_groups is called multiple
times, seemingly both before and after the whitelisting goes into
effect.  So disable the message about nogroups being ignored, but keep
the message about cleaning all supplementary groups (which is unlikely
to be printed unless it really happens).

Fixes netblue30#4930.
@kmk3 kmk3 mentioned this issue Feb 11, 2022
Merged
@kmk3 kmk3 added this to To do in Release 0.9.70 via automation Feb 11, 2022
@kmk3 kmk3 moved this from To do to In progress in Release 0.9.70 Feb 11, 2022
@kmk3 kmk3 added the bug Something isn't working label Feb 16, 2022
kmk3 added a commit that referenced this issue Feb 16, 2022
@kmk3
RELNOTES: add bugfix/ci/docs
b2c954a 
Relates to #4912 #4916 #4930 #4933.
@kmk3 kmk3 moved this from In progress to Done (on RELNOTES) in Release 0.9.70 Feb 16, 2022
@rusty-snake rusty-snake mentioned this issue Feb 17, 2022
Open
kmk3 added a commit to kmk3/firejail that referenced this issue Apr 22, 2022
@kmk3
Stop warning on safe supplementary group clean
30c1534 
When nogroups is used, the following warning may be issued (potentially
multiple times, as drop_privs may be called more than once):

    Warning: cleaning all supplementary groups

But the warning is being shown even when it seems that all supplementary
groups can be safely dropped (and are thus dropped), which is likely a
common scenario.  This commit prevents the warning from being printed in
that case, making it so that it is only shown in the non-happy paths (as
was the case on firejail 0.9.66).

Misc: The added code was copied from drop_privs.

This amends commit 7abce0b ("Fix keeping certain groups with
nogroups", 2021-11-30) / PR netblue30#4732.

Kind of relates to netblue30#4930.
@kmk3 kmk3 mentioned this issue Apr 22, 2022
Merged
@kmk3 kmk3 mentioned this issue May 16, 2022
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
No open projects
Release 0.9.70
  
Done (on RELNOTES)
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Disable/comment message about nogroups being ignored kmk3/firejail
3 participants
@glitsj16 @kmk3 @rusty-snake