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

profiles: keepassxc: add new socket location #6391

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

Conversation

qdii
Copy link
Contributor

@qdii qdii commented Jun 22, 2024

KeePassXC browser extension look for KeePassXC socket in the /run/user/app directory (https://github.com/keepassxreboot/keepassxc/blob/6b1ab1a5edd66ac10706a2fb5af34ec9458a901d/src/browser/BrowserShared.cpp#L41).

Unfortunately, /run/user/app seems to be blacklisted in disable-common.inc under the flatpak section (

blacklist ${RUNUSER}/app
), as a result, KeePassXC extension cannot find a socket.

Fixes #5447.

Relates to #3952.

@@ -33,6 +33,7 @@ whitelist ${HOME}/.mozilla
# Note: Start KeePassXC before Firefox and keep it open to allow communication between them.
#whitelist ${RUNUSER}/kpxc_server
#whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer
#noblacklist ${RUNUSER}/app
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should also be added to {cachy-browser,floorp,librewolf}.profile.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should also be added to {cachy-browser,floorp,librewolf}.profile.

If this applies to multiple browsers, I think it would be better to improve the
comment then move it to firefox-common.profile.

Then copy it to chromium-common.profile as suggested above.

I could do the refactor after this PR.

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

Add support to {cachy-browser,floorp,librewolf}.profile

@kmk3 kmk3 changed the title Fix browser communication with KeepassXC extension profiles: keepassxc: document new socket location Jun 22, 2024
Comment on lines 34 to 36
#whitelist ${RUNUSER}/kpxc_server
#whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer
#noblacklist ${RUNUSER}/app
Copy link
Collaborator

@kmk3 kmk3 Jun 22, 2024

Choose a reason for hiding this comment

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

Suggested change
#whitelist ${RUNUSER}/kpxc_server
#whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer
#noblacklist ${RUNUSER}/app
#noblacklist ${RUNUSER}/app
#whitelist ${RUNUSER}/app/org.keepassxc.KeePassXC
#whitelist ${RUNUSER}/kpxc_server
#whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer

Sort and whitelist the new socket location.

Also, add the following to firefox-common-addons.profile:

-# Prevent whitelisting in ${RUNUSER}
+# Prevent blacklisting and whitelisting in ${RUNUSER}
+noblacklist ${RUNUSER}/app
 ignore whitelist ${RUNUSER}/*firefox*
 ignore whitelist ${RUNUSER}/psd/*firefox*
+ignore whitelist ${RUNUSER}/app/org.keepassxc.KeePassXC
 ignore whitelist ${RUNUSER}/kpxc_server
 ignore whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer
 ignore include whitelist-runuser-common.inc

Copy link
Collaborator

Choose a reason for hiding this comment

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

@qdii

The sort and whitelisting should also apply to the comments in the other
changed profiles.

Also the changes to firefox-common-addons.profile are still missing.

etc/profile-a-l/chromium-common.profile Outdated Show resolved Hide resolved
@@ -33,6 +33,7 @@ whitelist ${HOME}/.mozilla
# Note: Start KeePassXC before Firefox and keep it open to allow communication between them.
#whitelist ${RUNUSER}/kpxc_server
#whitelist ${RUNUSER}/org.keepassxc.KeePassXC.BrowserServer
#noblacklist ${RUNUSER}/app
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should also be added to {cachy-browser,floorp,librewolf}.profile.

If this applies to multiple browsers, I think it would be better to improve the
comment then move it to firefox-common.profile.

Then copy it to chromium-common.profile as suggested above.

I could do the refactor after this PR.

Copy link
Collaborator

@glitsj16 glitsj16 left a comment

Choose a reason for hiding this comment

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

Requested changes addressed for me...

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.

Note that the following issue is still unresolved:

Also, the current commit message has very long lines (>72 characters) and is
not consistent with the usual format:

$ git show --pretty='%s%n%n%b' -s d8e01fe62f8c5910b23411acbe42e5447846b3f8
Add comment to include `noblacklist ${RUNUSER}/app` in browser profiles

KeePassXC browser extension look for KeePassXC in the /run/user/app directory (https://github.com/keepassxreboot/keepassxc/blob/6b1ab1a5edd66ac10706a2fb5af34ec9458a901d/src/browser/BrowserShared.cpp\#L41). Unfortunately, /run/user/app seems to be blacklisted in disable-common.inc under the flatpak section (https://github.com/netblue30/firejail/blob/b89ec818926b4bcd3a58bb4e2a67b68a8090ba1c/etc/inc/disable-common.inc\#L667), as a result, KeePassXC extension cannot connect to it.

$ git show --pretty='%b' -s d8e01fe62f8c5910b23411acbe42e5447846b3f8 | tr -d '\n' | wc -m
481

Suggestion:

Squash all commits, then reword the message to:

profiles: keepassxc: document new socket location

The KeePassXC browser extension looks for the KeePassXC socket at
`${RUNUSER}/app/org.keepassxc.KeePassXC`[1].

But `${RUNUSER}/app` seems to be blacklisted in disable-common.inc under the
flatpak section[2], so the KeePassXC extension cannot connect to it.

Fixes #5447.

[1] https://github.com/keepassxreboot/keepassxc/blob/6b1ab1a5edd66ac10706a2fb5af34ec9458a901d/src/browser/BrowserShared.cpp#L41
[2] https://github.com/netblue30/firejail/blob/b89ec818926b4bcd3a58bb4e2a67b68a8090ba1c/etc/inc/disable-common.inc#L667

(The link lines are still long, but the URLs cannot be line-wrapped)

See the following for details:

Let me know if you have any questions.

@qdii
Copy link
Contributor Author

qdii commented Jul 3, 2024

Done.

Thanks for the link to the article, that will definitely help :)

@kmk3 kmk3 changed the title profiles: keepassxc: document new socket location profiles: keepassxc: add new socket location Jul 7, 2024
The KeePassXC browser extension looks for the KeePassXC socket at
`${RUNUSER}/app/org.keepassxc.KeePassXC`[1].

But `${RUNUSER}/app` seems to be blacklisted in disable-common.inc under the
flatpak section[2], so the KeePassXC extension cannot connect to it.

Fixes netblue30#5447.

[1] https://github.com/keepassxreboot/keepassxc/blob/6b1ab1a5edd66ac10706a2fb5af34ec9458a901d/src/browser/BrowserShared.cpp#L41
[2] https://github.com/netblue30/firejail/blob/b89ec818926b4bcd3a58bb4e2a67b68a8090ba1c/etc/inc/disable-common.inc#L667
Comment on lines +66 to +70
mkdir ${RUNUSER}/app/org.keepassxc.KeePassXC
whitelist ${RUNUSER}/app/org.keepassxc.KeePassXC
whitelist /usr/share/keepassxc
include whitelist-run-common.inc
include whitelist-runuser-common.inc
Copy link
Collaborator

Choose a reason for hiding this comment

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

@rusty-snake on Nov 5, 2022:

whitelist ${RUNUSER}/app/org.keepassxc.KeePassXC

This will break X11, D-Bus, ...

Do you think the whitelisting is safe with whitelist-run-user-common.inc
included?

@kmk3 kmk3 added this to In progress in Release 0.9.74 via automation Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Release 0.9.74
  
In progress
Development

Successfully merging this pull request may close these issues.

firefox: cannot communicate with KeePassXC
3 participants