-
Notifications
You must be signed in to change notification settings - Fork 557
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
Grant KeePassXC access to its config directory #5453
Conversation
Without this, KeePassXC can't save its config and it gets reset on each run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will break a lot to everything.
Please test your changes.
keeassxc.profile is intentional a non-whitelisting profile. If you want to change this, you first have to say why and second do it correct.
I though that if a single |
Works perfect for me. I'd love to hear how this breaks other things. |
Where is your database stored? In |
In |
Also at a bare minimum you could have looked at the profile and see that there are other paths like |
Please continue, I honestly don't know where this is going. |
Do you want me to include the cache path here too? |
You see the issues? Everyone will require your keepassxc.local. You need to test this profile, without other modifications. |
What's the alternative? Grant keepassxc access to all of |
If keepass had a "standard" location for its files, I'd use that, but it doesn't. |
Yes. That's what we currently do. If you want to change this, you need to argue why. And how this can be done without to much breakage. |
Also your PR title is wrong because keepassxc already has access to it's config directory. |
If we're going to grant unrestricted access to |
@WhyNotHugo commented on Nov 5:
A "whitelisting profile" usually means a profile that whitelists something in This distinction is made because while system paths can mostly be known ahead Which is why whitelisting profiles are usually made sparingly. In this case, if a user puts their database in ~/ or ~/Documents (which are |
I'm fine with adding |
|
This is up to you. In the Thread Model of the most of use you will not need to sandbox it because KeePassXC is likely a trusted component as it has access to all your passwords and secrets. |
Huh, indeed, with a clean profile all of $HOME is accessible. I must have mistrusted and had another issue when setting this up. It's still quite trivial to escape the sandbox (e.g.: edit neovim's startup file and tell it to execute arbitrary commands), but that's not the scope of this issue. Sorry for the noise, thanks for the clarifications. |
Why do we sandbox if >50% of profiles can be escaped? |
I'd assume we both want to reduce that number as much as possible. The fact that there is a clear escape mechanism for KeePassXC (as documented above) kinda negates a lot of its use. On the topic of hardening this profile, how about:
Wouldn't this effectively result in |
This is also the case w/o the tmpfs. |
@WhyNotHugo commented on Nov 5:
If you actually tried your suggestion before posting it, you would have your $ firejail --quiet --noprofile \
--tmpfs='${HOME}/.config' \
--whitelist='${HOME}/.config/keepassxc' find | LC_ALL=C sort
.
./.Xauthority
./.asoundrc
./.bashrc
./.config
./.inputrc |
Is there any way to mount I think the intent of what I'm trying to do here is obvious: mount an empty tmpfs in I'm basically looking for something like bubblewrap's |
The whitelist mounts the tmpfs on the top level directory, so |
Without this, KeePassXC can't save its config and it gets reset on each run.