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

docs: manpage should explain precedence of CLI options vs profile settings #6358

Closed
smheidrich opened this issue May 26, 2024 · 7 comments · Fixed by #6359
Closed

docs: manpage should explain precedence of CLI options vs profile settings #6358

smheidrich opened this issue May 26, 2024 · 7 comments · Fixed by #6359
Labels
documentation Issues and pull requests related to the documentation

Comments

@smheidrich
Copy link
Contributor

smheidrich commented May 26, 2024

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

As far as I can tell, settings in a profile always take precedence over settings provided via CLI arguments. Is that right? And as far as I can tell, this isn't documented anywhere in the manpage. Even if I'm wrong, the inverse statement isn't documented in the manpage either (again: AFAICT).

Describe the solution you'd like

The precedence of profiles vs CLI options should be explained in the manpage. Probably in the section at the beginning, either just before or just after the profile discovery algorithm is explained.

Describe alternatives you've considered

None.

Additional context

None.

Relates to:

@rusty-snake rusty-snake added the documentation Issues and pull requests related to the documentation label May 26, 2024
@rusty-snake
Copy link
Collaborator

Firejails options work by staring with a minimal sandbox and then adding stuff like mounts, seccomp filters, process attributes, …. The most options do not have an inverse, hence there is no precedence, either they are present somewhere or nowhere.

There is only a small number of options that have any kind of inverse or an single value option (most are flags or collections):

  • ignore
  • noblacklist/blacklist
  • nowhitelist/whitelist
  • private/private /foo/bar
  • private-cwd
  • protocol
  • caps*
  • dbus-{user,system}
  • hostname
  • join-or-start
  • name
  • oom
  • [not complete list]

The override behaviour of each of them has to be documented separately cause they do not follow any global logic. Secondly the usage of --profile influences the cli vs. profile semantic.

Documenting override behaviour is not limited to cli vs. profile. Also inside profiles (and include chains) it is non-intuitive. Given the following profile, how will ${DOCUMENTS} end up?

a) blacklisted (ro/rw does not matter)
b) not blacklisted and read-only
c) not blacklisted and read-write [right answer]

noblacklist ${DOCUMENTS}
blacklist ${DOCUMENTS}
read-only ${DOCUMENTS}
read-write ${DOCUMENTS}

@glitsj16
Copy link
Collaborator

glitsj16 commented May 26, 2024

As far as I can tell, settings in a profile always take precedence over settings provided via CLI arguments. Is that right?

Not what I'm seeing:

$ firejail --ignore=quiet --ignore='include disable-common.inc' echo "testing"
Reading profile /etc/firejail/default.profile
Reading profile /etc/firejail/globals.local
Reading profile /etc/firejail/disable-programs.inc

** Note: you can use --noprofile to disable default.profile **

firejail version 0.9.73

Parent pid 16527, child pid 16528

Warning: An abstract unix socket for session D-BUS might still be available. Use --net or remove unix from --protocol set.
Base filesystem installed in 52.42 ms
Child process initialized in 117.45 ms
testing

Parent is shutting down, bye...

Observations: default.profile contains include disable-common.inc and CLI can and does override that when desired...

Do you have a reproducers for your assumed behaviour?

I do agree this might need extra clarity in documentation, cfr. @rusty-snake's remarks.

@smheidrich
Copy link
Contributor Author

Thanks for the replies!

@rusty-snake

The most options do not have an inverse, hence there is no precedence, either they are present somewhere or nowhere.

There is only a small number of options that have any kind of inverse or an single value option (most are flags or collections):

So what happens for collections without an inverse like e.g. --protocol? Is the end result just the union of values given at each point where it was specified? If at least these collections follow a simple logic like that, that's something that could be explained in the manpage. And likewise for flags without an inverse ("if it's there once, it applies").

The override behaviour of each of them has to be documented separately cause they do not follow any global logic.

There is probably a way to say that in the manpage, too. Maybe something like:

If options that are neither simple flags nor simple collections are provided multiple times, the exact behavior is option-specific and usually explained in the option's own documentation.


@glitsj16 I arrived at that conclusion because I wasn't able to un-blacklist nc (blacklisted in disable-common.inc) by doing --noblacklist=/usr/bin/nc (plus --noblacklist for each of that symlink's targets, recursively). I similarly wasn't able to disallow UNIX domain socket connections via --protocol=inet, because default.profile already contains protocol unix,inet,inet6. In each case, commenting out those lines in the profiles resolved it.

But given @rusty-snake's comment, it seems it's much more complicated than I thought.


I'll write a PR with a draft of what I think might be an OK explanation, then we can refine it from there.

@smheidrich
Copy link
Contributor Author

Sorry, I only now saw that at least for --protocol, the behavior is already specified in the manpage:

Multiple protocol commands are allowed and they accumulate.

And searching for accumulate in the manpage, I see a lot of other options have similar sentences. In that case, disregard what I wrote about collections - that is actually fine as-is.

Then maybe just 1 sentence to explain the precedence for multiple or conflicting options is explained in the option's own docs, done.

@glitsj16
Copy link
Collaborator

I arrived at that conclusion because I wasn't able to un-blacklist nc (blacklisted in disable-common.inc) by doing --noblacklist=/usr/bin/nc (plus --noblacklist for each of that symlink's targets, recursively). I similarly wasn't able to disallow UNIX domain socket connections via --protocol=inet, because default.profile already contains protocol unix,inet,inet6. In each case, commenting out those lines in the profiles resolved it.

I can see the confusion.

For the nc case, it's important to note that disable-common.inc refers to it as blacklist ${PATH}/nc, not /usr/bin/nc. Once you keep that in mind things work fine on CLI:

$ firejail --ignore=quiet --ignore='blacklist ${PATH}/nc' nc -h
$ firejail --ignore=quiet --noblacklist='${PATH}/nc' nc -h

For overriding protocol you need (1) a new protocol option AND (2) stop that from being overriden by the profile via a additional --ignore=protocol.

HTH

@rusty-snake
Copy link
Collaborator

rusty-snake commented May 26, 2024

e.g. --protocol?

protocol is crazy, see

/* Comma separated list is processed so that:
* "item" -> adds item to list
* "-item" -> removes item from list
* "+item" -> adds item to list
* "=item" -> clear list, add item
*
* For example:
* ,a,,,b,,,c, -> a,b,c
* a,,b,,,c,a -> a,b,c
* a,b,c,-a -> b,c
* a,b,c,-a,a -> b,c,a
* a,+b,c -> a,b,c
* a,b,=c,d -> c,d
* a,b,c,= ->
*/

caps* and seccomp vs. seccomp syscall might be too a bit complicated.

For the most collections it will be the union. But be careful what a collection is.

foo a,b,c
bar α
bar β
bar γ
baz א,ב,ג
  • foo is a collection where its option accepts multiple elements
  • bar is a collection where its option accepts a single element
  • baz is a scalar where its option accepts a collection

Think of List[Box(Object()), Box(Object()), Box(Object())] vs. Box(List[Object(), Object(), Object()]). I.e. internal representation != external representation.

I'll write a PR with a draft of what I think might be an OK explanation, then we can refine it from there.

Every bit of clarity helps. Even if only the simple case are covered first.

For overriding protocol you need (1) a new protocol option AND (2) stop that from being overriden by the profile via a additional --ignore=protocol.

That works. In general and for all options. protocol has a second way, see above.

@smheidrich
Copy link
Contributor Author

Thanks for the replies again!

@glitsj16 That makes sense. And I just saw that --noblacklist='${PATH}/nc' works as well, so maybe the --noblacklist example in the manpage (which uses a hardcoded nc path and hence doesn't work for the default profile) could be updated to use that. I'll put that in the PR.

@rusty-snake I can see why this has been difficult to document 😮 The +/-/= syntax in that comment doesn't seem to work on the command line though (at least when I tried with --protocol=-unix just now), so is that something only available in profiles or internally?

kmk3 pushed a commit to smheidrich/firejail that referenced this issue Jun 8, 2024
@kmk3 kmk3 changed the title manpage should explain precedence of CLI options vs profile settings docs: manpage should explain precedence of CLI options vs profile settings Jun 10, 2024
kmk3 added a commit that referenced this issue Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues and pull requests related to the documentation
Projects
Status: Done (on RELNOTES)
Development

Successfully merging a pull request may close this issue.

3 participants