keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
20.07k stars 1.42k forks source link

Snap: Improve Web-browser Native Messaging host functionality #10906

Closed JGCarroll closed 2 weeks ago

JGCarroll commented 3 weeks ago

This commit allows for the snap distribution of KeepassXC to self-manage native messaging manifests This is done by making the binary aware of the snapd environment changes that currently prevent this. Furthermore, the snap sandbox is expanded to the bare minimum needed to access these privileged files.

Please note if running a self-compiled / untrusted KeepassXC snap build (I.E, installed with --dangerous) that you must manually run sudo snap connect keepassxc:browser-native-messaging to grant permissions.

This will work on all distributions that expose /snap/bin/ - such as Ubuntu, Debian, etc. For systems which don't provide /snap/, such as Fedora, follow instructions for enabling "Classic" snaps. e.g., sudo ln -s /var/lib/snapd/snap /snap

Describe your changes in detail, why is this change required?

Currently, if users install KeepassXC as a snap, the "Browser Integration" interface will instruct them to use an external script to set up the Native Messaging hosts. This is a poor user experience as it's a snap specific requirement, and also because it prevents KeepassXC being able clean up the host files (the user cannot "untick" and press ok on any browser).

This helps remove user friction by making things "just work" without compromising in security in any manner, the sandbox is expanded purely to accommodate the exact files required, and all existing certificate checks on the app & extensions themselves, the authentication process, etc, all remain as standard.

Explain large or complex code modifications.

Ultimately, this mirrors the workarounds used for the Flatpak build, the app needs to actively consider that the $HOME path it is presented with is not the real value on the host. By doing so, the files are instead exposed where they need to be to enable other programs to use them.

Whilst this will reduce bug reports and user problems significantly, it's not a magic bullet. There's 2 elements that come into play with compatibility.

1) The browser itself may be sandboxed. If so, native-messaging support isn't guaranteed. It's available on Firefox and Chromium snaps in distributions newer than 22.04, in a "it just works experience". This is done via a downstream patch to the XDG Desktop Portals, and so, it wouldn't be compatible with e.g Fedora using the Firefox snap, until/unless https://github.com/flatpak/xdg-desktop-portal/pull/705 is merged and widely distributed.

Ultimately, most Snap users are Ubuntu users, so considering population demographics, this should still improve the majority of typical use cases.

2) The binary is hardcoded as /bin/snap/keepassxc.proxy, this isn't guaranteed to exist at that location. E.G., on Fedora, users would have to run sudo ln -s /var/lib/snapd/snap /snap, this follows the same semantics as enabling "Classic Snaps" support. This can be done before or after installation of KeepassXC, and KeepassXC itself is still operating under Strict confinement.

Again, considering user demographics, I'd imagine most other distributions use the AppImage, Flatpak, or distro repo versions before considering snap, meaning this shouldn't be a common occurrence relatively.

Screenshots

N/A

Testing strategy

1) Build the Snap 2) Ensure sudo snap connect keepassxc:browser-native-messaging is run, as will be required on any untrusted builds not published from the Snap Store. 3) Enable the browser integration functionality and test it on browsers.

Test Environment, Ubuntu 24.04

Firefox Snap + KeepassXC snap: ✅ Firefox native + KeepassXC snap: ✅ Chromium Snap + KeepassXC Snap: ✅ Google-Chrome native + KeepassXC snap: ✅ Brave Snap + KeepassXC Snap: Failure as Brave has not adopted the XDG Desktop Portal patches, no regression in this patch. Brave native + KeepassXC Snap: ✅

Type of change

JGCarroll commented 3 weeks ago

PR marked as a draft as due Snap Store policy, being able to distribute KeepassXC will require a one time review that could take up to 2 weeks. We would like to minimize the time between acquiring permission from the Snap Store and actually building/uploading any snaps with this functionality as otherwise doing so will keep triggering the review process and deny uploads.

Ideally, the store would agree for this to be autoconnected as it enables major functionality in the app and the permissions required to do so are very specific and within reasonable users expectations.

droidmonkey commented 3 weeks ago

Request for personal-files access was submitted: https://forum.snapcraft.io/t/personal-files-access-request-for-keepassxc/40628

varjolintu commented 3 weeks ago

This looks excellent! Haven't tested it yet though.