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

Merge disable-passwordmgr.inc in disable-common.inc or disable-programs.inc #4454

Closed
rusty-snake opened this issue Aug 6, 2021 · 2 comments · Fixed by #4461
Closed

Merge disable-passwordmgr.inc in disable-common.inc or disable-programs.inc #4454

rusty-snake opened this issue Aug 6, 2021 · 2 comments · Fixed by #4461

Comments

@rusty-snake
Copy link
Collaborator

While working on #4157 I was wondering why why treat configurations of password-managers special. (At least of KPXC) there are no secrets in the blacklisted paths, only config/state files. There are more "problematic" files in disable-programs.inc like ~/.mozilla (your firefox profile) or ~/.config/mpv (code execution). Should we merge disable-passwordmgr.inc in disable-programs.inc?

@reinerh
Copy link
Collaborator

reinerh commented Aug 6, 2021

I think a reason to have it separate could have been that it's "easier" to just include disable-passwordmgr.inc everywhere without having to fear to break something (as the blacklisted file are very secret, it's good to include it whereever possible). Including disable-programs.inc can probably not be done so "blindly".

But I'm fine with merging it with one of your suggested files. I even just noticed that the other two files are included about the same time as disable-passwordmgr:

$ rg ^include.*disable-prog etc | wc -l
586
$ rg ^include.*disable-common etc | wc -l
597
$ rg ^include.*disable-passw etc | wc -l
583

@rusty-snake
Copy link
Collaborator Author

as the blacklisted file are very secret

That's the point, they aren't secret (at least keepassxc).
There's no reason other programs need to access these file.

~/.config/keepassxc/keepassxc.ini
[General]
ConfigVersion=1

[Browser]
CustomProxyLocation=

[GUI]
AdvancedSettings=true
CompactMode=true
HidePreviewPanel=true

[KeeShare]
Active="<?xml version=\"1.0\"?>\n<KeeShare xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">\n  <Active/>\n</KeeShare>\n"
Foreign="<?xml version=\"1.0\"?>\n<KeeShare xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">\n  <Foreign/>\n</KeeShare>\n"
Own="<?xml version=\"1.0\"?>\n<KeeShare xmlns:xsd=\"http://www.w3.org/2001/XMLSchema\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\">\n  <PrivateKey/>\n  <PublicKey/>\n</KeeShare>\n"
QuietSuccess=true

[PasswordGenerator]
AdditionalChars=
ExcludedChars=

[Security]
LockDatabaseIdle=true
LockDatabaseIdleSeconds=300

If bitwarden/lastpass/... users can say something it would be great.

But I'm fine with merging it with one of your suggested files. I even just noticed that the other two files are included about the same time as disable-passwordmgr:

$ grep -L "^include disable-passwdmgr" $(grep -l "^include disable-programs.inc" /etc/firejail/*.profile) | wc -l
21
$ grep -L "^include disable-programs" $(grep -l "^include disable-passwdmgr.inc" /etc/firejail/*.profile) | wc -l
17

I don't think that these few are worth to have an own disable include.
Maybe a allow-passwdmgr.inc makes more sense. If there are any programs that would use this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (on RELNOTES)
Development

Successfully merging a pull request may close this issue.

2 participants