-
Notifications
You must be signed in to change notification settings - Fork 588
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
Thought: blacklist all .files and folders by default. #578
Comments
Sounds like an improvement in security! 😄 |
Thanks. That's what I'm hoping. :) |
Will probably conflict with #579 |
You guys are worse than a six year old :) @Fred-Barclay: you are right, all the include system is a mess in this moment. What you propose is also somehow releated to #504. Let's leave it open for now, I'll mark it as an enhancement and go for it in the release after the current one. The thing is I've already started adding private-bin and whitelisting /tmp directory all over the place. Also private-dev and private-etc are in plan. By the time I'm done, everything will be broken. So no messing around with the include files, instead just put in some pull requests with whatever you have in your private profiles. If is working on your setup, it should be fine, and we'll fix it when people start complaining. |
"You guys are worse than a six year old :)" Will do. Will the next release be 0.9.41 or 0.9.42? I noticed you seem to use even-numbered point releases. |
0.9.41 is the development tag. It will be released as 0.9.42 once is ready. |
@Fred-Barclay An alternative implementation of your idea would be to generally switch to whitelisting, and to whitelist all non-hidden directories in the root of user home (like the XDG defaults ~/Videos, ~/Downloads and so on). From a usability perspective the result would be the same, but the approach has the advantage of effectively handling #1011, which is otherwise fairly trivial to exploit. Take only Gnome: How many installations out there have one the files But admittedly rewriting all profiles for whitelisting would be a major effort, and having more restrictive profiles also increases the risk of breaking functionality. |
@SYN-cook @Fred-Barclay Do you think we've sufficiently cleaned up the profiles to close this or should we keep it open to motivate us further? |
Also, I would suggest maybe going the route I do in my personal profiles: only use whitelists and keep them within the specific profiles (see my repository for examples of what I mean). I use a |
We've generally been moving in this direction to the point where I think this can be closed. The profiles have been significantly revamped since the issue was first opened, and we're generally keeping this in mind 🙂 |
We already blacklist most hidden directories and files with disable-common, disable-passwdmgr, and especially disable-programs. Then we noblacklist the files in the appropriate profiles.
This has the weakness of requiring multiple disable.inc files, which can lead to unnecessary duplication, complication, and confusion. It also opens a bit of a security risk in that hidden files that aren't blacklisted are visible to the sandbox. While currently firejail blacklists all the sensitive hidden files, sooner or later there will be one that isn't included in the sandbox that should have been.
Why not just disable them all in a single file? Something like disable-hidden.inc:
blacklist ${HOME}/.*
.Then we can just add the necessary noblacklists to the profiles. We do this already since nearly all hidden files are blacklisted already, so most profiles would be ready-to-go as they are right now. A few profiles will have to be modified (esp. if they need .bashrc which is currently visible to the sandbox) but there is plenty of time for debugging before the next firejail release (right, @netblue30 ? 😄)
And on the positive side, a bit of testing in terminal has shown no problems on my side.
Thoughts, mates? My last big change wasn't welcomed by some so I thought I'd get responses before creating a pull request. A rough mental draft is ready to go depending on the feedback.
The text was updated successfully, but these errors were encountered: