Fixes bug #1185 where the password from the CLI wouldn't be send to KeePassXC #1336
Conversation
src/main.cpp
Outdated
| password = in.readLine(); | ||
| } | ||
| mainWindow.openDatabase(filename, password, parser.value(keyfileOption)); | ||
| mainWindow.openDatabase(filename, parser.value(pwstdinOption), parser.value(keyfileOption)); |
There was a problem hiding this comment.
I don't think this is working. You just made the --pw-stdin option effectively a --password option. The reason why we read the password from STDIN instead of the command line is that with the latter option your password would appear in the process list (possibly readable by other users) and in your shell history. You should therefore never provide passwords on the command line.
There was a problem hiding this comment.
I'm trying to get it to work with stdin but it never sends the password at all, lines 145-149 effectively did nothing. I'm not really sure why it doesn't function the way intended, but so far the only method I've found for getting the password to the input box is to do it more directly like I have done.
There was a problem hiding this comment.
It works on other platforms, but not on Windows.
There was a problem hiding this comment.
I'm not entirely sure what I should do then. If it's working for other platforms but not for Windows, then I don't know why it's acting the way it is.
There was a problem hiding this comment.
The problem is that GUI applications on Windows don't have an attached console like *nix applications have.
There was a problem hiding this comment.
Would something like:
#ifdef Q_OS_WIN
mainWindow.openDatabase(filename, parser.value(pwstdinOption), parser.value(keyfileOption));
#else
mainWindow.openDatabase(filename, password, parser.value(keyfileOption));
#endif
be a good enough solution here so all platforms have a working option?
|
Also there are some strange discrepancies between the src folder on the Github site and the actual keepassxc-2.2.4-src.tar.xz file, notably line 103 Translator::installTranslator(); is Translator::installTranslators(); , and the #if defined(Q_OS_WIN) I assume this is something in development. |
|
2.2.4 is built from the 2.2.4 tag. 😉 |
|
Ahh,...I must be getting sleepy. What do you think of the new proposed solution? It would allow both the other two OSes to continue to have the previous working solution, but would give Windows users a functioning option to send their password through to the application. |
|
I'm not a fan of passwords on the command line. It's just bad security practice. |
|
I think this is the only way of getting that function to work on Windows tbh. |
|
I'm getting a compilation error about the lack of missing argon2 libraries when I try to compile now, did something major change with the dev branch? |
|
Yes, we merged KDBX4. Also please don't merge in changes. Use rebasing, otherwise we need to edit out the merge commits before merging into develop. |
|
Okay, I did a full reinstall of my env but I still get: objcopy: 'ARGON2_SYS_LIBRARIES-NOTFOUND': No such file I guess there's missing information on the wiki as it's out of date now? |
|
You need libargon2-0-dev. The wiki hasn't been updated yet. |
|
Bad news that this bug fix was postponed to 2.4.0. |
|
@Remonli I don't think the fix I'm using it going to be used in a release build tbh. |
|
@TheChiefMeat Sorry, I don't understand, do you mean this fix will not be included even in future release |
|
@Remonli I'm not one of the main contributors, so I can't push the code to a release. I think the developers of KeePassXC aren't satisfied with how I fixed it, so it's going to be left as it is. |
|
@TheChiefMeat please rebase on develop, squash all your commits, and fix your whitespace and this can be merged. |
|
Well...I can't seem to do any of those things as I deleted the fork thinking this wouldn't ever actually go through due to the fact that the password was on the command line... I guess I'll need to reopen the pull request, apparently this has been an issue with github for ages. |
Description
Fixes bug #1185 where the password from the CLI wouldn't be send to KeePassXC
Motivation and context
Original bug thread: https://github.com/keepassxreboot/keepassxc/issues/1185
I wanted to be able to more easily launch my database.
How has this been tested?
Ran the changed code on two Windows 10 64-bit machines.
The new code works in all the below tested configurations:
To test use the following:
keepassxc --pw-stdin PASSWORD --keyfile G:\KEYFILE.key D:\KeePass\Database.kdbx
keepassxc --keyfile G:\KEYFILE.key D:\KeePass\Database.kdbx --pw-stdin PASSWORD
Types of changes
Checklist: