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

bugfix: do not interact with dbus directory if dbus proxy is disabled #6591

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

powerjungle
Copy link
Contributor

No description provided.

Copy link
Collaborator

@kmk3 kmk3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, does the current code cause any issues or is this intended to just
keep /run/firejail cleaner?

@powerjungle
Copy link
Contributor Author

powerjungle commented Dec 28, 2024

It did cause an issue when I was running firejail inside a chroot environment. That's not important, but I also realized there's no point in that code running if we disable the dbus proxy code anyway, just to keep things cleaner.

@powerjungle powerjungle force-pushed the fix/dbus-folder-creation branch from dbf0059 to 40ce858 Compare December 28, 2024 16:05
@kmk3 kmk3 added the bugfix This fixes a bug label Dec 29, 2024
@powerjungle powerjungle force-pushed the fix/dbus-folder-creation branch from 40ce858 to b5d4ea3 Compare December 29, 2024 22:52
@kmk3
Copy link
Collaborator

kmk3 commented Jan 4, 2025

It did cause an issue when I was running firejail inside a chroot
environment. That's not important, but I also realized there's no point in
that code running if we disable the dbus proxy code anyway, just to keep
things cleaner.

I see, but it is currently marked as a bugfix and it's not immediately obvious
what the bug is/what are the practical effects of creating/bind-mounting the
directory.

Could you add those details to the commit message?

By the way, firejail in a chroot is likely an uncommon environment, but if you
have any tips, use cases, etc about it, feel free to add them to the wiki.

@powerjungle
Copy link
Contributor Author

powerjungle commented Jan 4, 2025

I see, but it is currently marked as a bugfix and it's not immediately obvious what the bug is

Sorry about that. Well the bug was, trying to run firejail inside of a firejail --chroot=/somedir --noprofile causes an assert to fail which checks whether the mode for /run/firejail/dbus is correct, which in this case means 0755, but the issue is that when inside of a "firejail chroot", that directory is mounted with mode 0400 probably to isolate the chroot better, which makes sense. Even when running sudo firejail --chroot=/somedir --noprofile, the /run/firejail/dbus directory still has the mode 0400.

I said that it's not important, because this use case shouldn't change the fact that it shouldn't create and care about the directory if firejail is built with dbus disabled. It's just a small cleanup. I probably shouldn't have called it a bugfix, but I just felt like it might cause issues in some other weird situations as well.

By the way, firejail in a chroot is likely an uncommon environment, but if you have any tips, use cases, etc about it, feel free to add them to the wiki.

Oh I forgot about the wiki, that's not a bad idea. Yeah I realize it's uncommon, actually I realized it even had a chroot argument very recently. I think it's a nice and relatively user friendly way to setup a "development environment" where you can install all dev packages in there and just isolate it from the "host system". Also testing out unstable software in that environment is really nice. Yes there are LXC and chroot/schroot, but all of those are a lot more tricky in my opinion and since I'm already using firejail for half of my programs anyway, why not use it for a simple "isolated environment" too. Yes, there are virtual machines too, but they're waaay too heavy for most daily use cases in my opinion, especially if you work on a laptop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This fixes a bug
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants