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

Some warnings should be errors #3355

Open
rdiez opened this issue Apr 14, 2020 · 12 comments
Open

Some warnings should be errors #3355

rdiez opened this issue Apr 14, 2020 · 12 comments
Labels
enhancement New feature request

Comments

@rdiez
Copy link

rdiez commented Apr 14, 2020

You can argue that Firejail is an important security tool. If it has been asked to do something, and it cannot, it should fail. Instead, it tends to drop the specify security measure and happily carry on. I have seen this behaviour with the following warnings:

  1. Warning: you are not allowed to change /tmp to read-write

If such configuration fails, applications may stop working correctly. It would be better to stop straight away, so that the issue is promptly recognised and fixed before it causes trouble.

Does this happen too if Firejail has been asked to make something "read-only". If so, that would be a serious issue. You believe that you have placed protections in place, and suddenly, on some system, they may be ignored.

  1. Warning: cannot create a new user namespace, going forward without it...

If I specifically do not want to leak other usernames to the application, Firejail should not just ignore me and leak them anyway.

I am using Ubuntu 18.04.4 LTS, which comes with Firejail version 0.9.52, a rather old version indeed. So maybe this has already been improved in the meantime.

@glitsj16
Copy link
Collaborator

See note in #3354 on how to get the latest stable release.

If such configuration fails, applications may stop working correctly. It would be better to stop straight away, so that the issue is promptly recognised and fixed before it causes trouble.

That sounds reasonable. There's been some recent work on letting users decide on how to deal with seccomp filtering for instance (see #3301). Plenty of work left to broaden the scope of such finer-grained user control.

Does this happen too if Firejail has been asked to make something "read-only". If so, that would be a serious issue. You believe that you have placed protections in place, and suddenly, on some system, they may be ignored.

Firejail follows the general Linux file permission model/rukes AFAICT. In your ${HOME} you should be able to make anything read-only or read-write. But if your user tries to change access mode on something without having the required privileges, it will fail. So answering your question: it depends on the specifics...

@rusty-snake
Copy link
Collaborator

Does this happen too if Firejail has been asked to make something "read-only".

No.

@rusty-snake
Copy link
Collaborator

If firejail would hardfail on these, it would be broken for some users.

@rdiez
Copy link
Author

rdiez commented Apr 15, 2020

I would recommend stop using Firejail, o any security tool for that matter, which, when instructed to protect some areas, it then skips the protections due to some external system or configuration mismatch. You just cannot rely on such a tool!

@rusty-snake
Copy link
Collaborator

--read-write is not a protection, its a exception.

@rdiez
Copy link
Author

rdiez commented Apr 15, 2020

But --noroot is, which is what generates the "cannot create a new user namespace" warning.

I am starting to distrust Firejail. What other protection measures could be skipped like this?

@glitsj16
Copy link
Collaborator

I am starting to distrust Firejail. What other protection measures could be skipped like this?

Personally I think it's a good thing to raise these concerns. Distrust and security tool in the same sentence isn't anathema. So, after repeatedly going over #3354, #3355, #3356 and #3357, I decided to label them for now and await discussion involving other collaborators.

@glitsj16 glitsj16 added the enhancement New feature request label Apr 15, 2020
@matu3ba
Copy link
Contributor

matu3ba commented Apr 15, 2020

@glitsj16

I am starting to distrust Firejail. What other protection measures could be skipped like this?

Personally I think it's a good thing to raise these concerns. Distrust and security tool in the same sentence isn't anathema. So, after repeatedly going over #3354, #3355, #3356 and #3357, I decided to label them for now and await discussion involving other collaborators.

You might want to create a tracking issue collecting a list of options related to PR #3301 and schedule a short meeting to find consensus on the project status and goal.
How to simplify things from the user side and specify program behavior would also come to my mind, but that is up to you.

@smitsohu
Copy link
Collaborator

Letting noroot fail hard would force us to remove the option from all profiles.

IMHO this behavior should be documented in the man pages. Maybe it would also be good to provide more feedback as to why creating a new user namespace failed.

Another idea: would it make sense to add something like an (opt-in) fatal-warnings option?

@smitsohu
Copy link
Collaborator

Another idea: would it make sense to add something like an (opt-in) fatal-warnings option?

... which down the road is likely to create pressure to turn warnings into mere messages.

@rdiez
Copy link
Author

rdiez commented Apr 16, 2020

More ideas:

The profile configuration files could be treated differently than command-line options. Or such default configuration files could start with some "ignore-errors" or "warn-if-error" switch, which is only valid for the profile file it is in.

Providing default configuration files for all Linux distributions is never going to work properly. Like I said, if a particular distribution changes something, chances are that breakage will be noticed much later, leaving users unprotected for quite some time. I would say that Firejail profiles should come from distributions, or even within each software package itself.

Or maybe the configuration files just need to become more flexible. Distributions could provide a top-level configuration file indicating what options will work (or won't) on their distributions.

Managing security settings is hard. Note how polkit ended up introducing a JavaScript engine to manage authorization rules (!).

I just believe it is important to have a way to run Firejail in some "strict" mode you can really rely upon.

@rusty-snake
Copy link
Collaborator

Providing default configuration files for all Linux distributions is never going to work properly. Like I said, if a particular distribution changes something, chances are that breakage will be noticed much later, leaving users unprotected for quite some time.

Then we will end up with only a few profiles per distro. That is even more unprotected.

I would say that Firejail profiles should come from distributions, or even within each software package itself. Or maybe the configuration files just need to become more flexible. Distributions could provide a top-level configuration file indicating what options will work (or won't) on their distributions.

Distriputors and app-developers will not do that.

Anyway, turning some warning into errors (e.g. noroot) sound now reasonable to me. So users need to explicitly opt-out by adding ignore noroot to globals.local. Hardfailing on the cli and/or a fatal-warnings options are also good.

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

5 participants