keepassxreboot / keepassxc

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

Can't save native messaging script for Browser Plugin #10755

Closed OnSlbWXt closed 5 months ago

OnSlbWXt commented 5 months ago

Overview

Trying to use Browser Integration for Firefox causes the error: image

Steps to Reproduce

  1. Install KeePassXC as admin to the default location
  2. Open keePassXC as a normal user
  3. Activate Browser integration
  4. Get Error Message

Expected Behavior

No Error Message and integration works

Actual Behavior

Error Message and integration doesn't work

Context

The issue is, that KeePassXC tries to place the json files into the installation directory in C:\Program Files\KeePassXC\, but a normal User has no write permissions there (as it should be). Proposed fix: As the registry key for that script is already set in HKEY_CURRENT_USER, it would be smart, to put the json into %userprofile%\AppData\Local\KeePassXC\ instead. (or at least as a fallback)

The issue persists in version 2.7.8 as stated below. The issue persists after uninstalling, rebooting and reinstalling. Installation method is either:

winget install --scope machine --disable-interactivity  --silent --exact --id KeePassXCTeam.KeePassXC

or

wget https://github.com/keepassxreboot/keepassxc/releases/download/2.7.8/KeePassXC-2.7.8-Win64.msi
msiexec /i KeePassXC-2.7.8-Win64.msi /QUIET

KeePassXC - Version 2.7.7 Revision: 68e2dd8

Operating System: Windows 10 Pro and 11 Pro Desktop Env: N/A Windowing System: N/A

droidmonkey commented 5 months ago

We definitely do not put the json files in program files. Are you running the portable version from within program files?

OnSlbWXt commented 5 months ago

No, I am using the winget version using the command:

winget install --scope machine --disable-interactivity  --silent --exact --id KeePassXCTeam.KeePassXC

(which installs https://github.com/keepassxreboot/keepassxc/releases/download/2.7.8/KeePassXC-2.7.8-Win64.msi)

Resulting folder contains image

OnSlbWXt commented 5 months ago

Also thank you for closing this bug, as you apprently are saving to Program Files without intending to, which only further clarifies this as a bug

Also be informed, the issue persists with version 2.7.8

KeePassXC - Version 2.7.8 Revision: f6757d3

Qt 5.15.11 Debugging mode is disabled.

Operating system: Windows 10 Version 2009 CPU architecture: x86_64 Kernel: winnt 10.0.19045

Enabled extensions:

Cryptographic libraries:

droidmonkey commented 5 months ago

How do you know it is trying to save to program files?

Pingger commented 5 months ago

As I am also experiencing the issue: Very easily! Just given "Everyone" write permissions on the keypassxc folder in Program Files. The Error disappears and json files start to appear in the program files folder. That guess comes from the fact, that the registry keys mentioned in this Troubleshooting Guide also point to the json to be in that folder. (while the guide also says it should be appdata local, that is not what actually happens)

Issue is: Some people (like me) don't want other programs messing with the program files. Write access to the program files is restricted to administrators for a reason

A somewhat better workaround: give write permission, start keepass, take write permission and delete permission away. Get errors on startup, but browser integration works.

droidmonkey commented 5 months ago

Are you also installed using winget?

What you describe is definitely not the coded or desired mode of operation. The only explanation I have is that you are running in portable mode for some reason, even though it is installed into program files.

droidmonkey commented 5 months ago

https://github.com/keepassxreboot/keepassxc/blob/develop/src%2Fbrowser%2FNativeMessageInstaller.cpp#L212-L216

Screenshot_20240514_161036_GitHub.png

Why is that file there?

Pingger commented 5 months ago

No, normal msi installation, but in a corporate environment, and we are using that keepassxc.ini in a shortcut ([...]\keepassxc.exe /config [...]), that is rolled out to users. (To enforce some settings) Apparently it is not removed when uninstalling and reinstalling, ...

Also: A Portable-Installation is denoted by a '.portable' file in the application folder based on your portable-zip-file and portableapps.com's version. Not by the keepassxc.ini being there. So that check should be

if (QFile::exists(QCoreApplication::applicationDirPath() + QStringLiteral("/.portable"))) {
    basePath = QCoreApplication::applicationDirPath();
} else {
    basePath = QStandardPaths::writableLocation(QStandardPaths::DataLocation);
}

But yes, after renaming the file to 'keepassxc_corporate.ini' and pointing to that, the issue is resolved.

OnSlbWXt commented 5 months ago

Yes, also a corporate environment. Using a keepassxc.ini in the application directory for defaults. the ini is placed there on system setup and when a user logins we copy it to the user's directory using a startup script. our workaround for #2189

OnSlbWXt commented 5 months ago

I created a pull request aiming to fix the issue.

droidmonkey commented 5 months ago

Thanks, that code I referenced is ancient so you should have seen this in prior versions. We used to detect portable mode by the presence of the keepassxc.ini file. Thank you for finding this bug 😁

Also please, in the future it is important to include the context of your deployment environment.

OnSlbWXt commented 5 months ago

Yeah, I was certain, that there were no leftovers after uninstalling and then reinstalling, without adding the keepassxc.ini there. So from my perspective, that was a "clean" installation and an issue out of the box.

droidmonkey commented 5 months ago

Guaranteed you had leftovers, look at the modified dates

OnSlbWXt commented 5 months ago

yes, the dates don't match, but the folders were deleted and recreated, yet their last modified is even older. Windows Last-Modified can't be trusted