Settings dialog surfaces clear-text passphrase (Firefox)

[selfawaresoup]

Expected Behavior

No sensitive information should be visible without unlocking the KeyPassXC database.

Current Behavior

In the Settings dialog of the Firefox extension, when navigating to "Connected Databases" the "identifier" column contains the Keypass database passphrase in clear text.

It's unclear to me how the passphrase is even stored anywhere and this seems like a significant security issue.

I created a new throw-away password database in KeyPassXC and connected it to the Firefox extension. Here's a screenshot showing the unlocking passphrases in clear-text for all connected databases:

This information is also visible when the databases are locked. But it really shouldn't ever be visible.

Possible Solution

unclear

Identify how and why the clear-text passphrase is stored at all and stop doing that. And of course don't display it anywhere.

Steps to Reproduce (for bugs)

1. Set up KeyPassXC in macOS
2. Install Firefox extension
3. open extension's settings page
4. navigate to "connected databases"

Debug info

KeePassXC - 2.7.8
KeePassXC-Browser - 1.9.0.5
Operating system: macOS Sonoma 14.5 (ARM)
Browser: Mozilla Firefox 127.0
Browser: Chrome/Firefox/Vivaldi/Chromium

[droidmonkey]

You set the database name to its password? That's on you haha. Go to Database -> Database Settings -> Set a new database name.

Or is that the identifier you used when connecting to keepassxc? Either way, don't use your password there.

[selfawaresoup]

No, the names of the databases are not the same as the passphrases, that would be ridiculous.

Now maybe take this report a bit more seriously and stop joking around.

[droidmonkey]

I can't take it seriously because that is not what happens.

[selfawaresoup]

are you fucking kidding me? Is this how you handle security issues? by just ignoring them?

[droidmonkey]

[selfawaresoup]

You know what, I wanted to try and have a constructive discussion here but you seem more interested in ridicule which does not inspire a lot of trust in the security software you're working on.

This is how you demotivate people from contributing anything. Great job.

[droidmonkey]

Are you putting your password in that text field or are you not? There is nothing constructive about this, just need facts. If you voluntarily put your password in that box then, yes, it will appear in the area of the UI that you screenshotted. That is easily avoided by not putting your password as the database identifier.

[selfawaresoup]

I can't go back in time and check. But let's assume that that is what happened.

That's still a dangerous UI pattern right there. When connecting a new database (which is probably locked as it should be) popping up a text input that looks very similar to the normal unlock dialog is very risky design. You actually did a massive improvement on it right there in your screenshot annotation by putting a very appropriate warning in it.

[droidmonkey]

You cannot connect a locked database.

The dialog very clearly states what is desired.

The input is also not masked (ie, *******), so there is no indication that a password should be there.

We could put in a sample text "underlay", but that often confuses users as well into using the same text as the sample text for every connection which leads to overwriting the connection id. So in this case we won't make any changes.

[varjolintu]

@selfawaresoup The only way your password can be visible here is that it has been written in that popup when asking for an identifier. The master password is not stored to any variable, so it's impossible to even send it to the extension side. And the whole point of how the extension behaves is so that it doesn't need to know the master password at all, and will never ask for it.

You can delete the connection from KeePassXC side (Database Settings) and then from the extension, and connect the database again with a new identification string.

[jdittrich]

We could put in a sample text "underlay", but that often confuses users as well into using the same text as the sample text for every connection which leads to overwriting the connection id. So in this case we won't make any changes.

I agree that a sample text is not helpful here, but I also see the risk that people put their password there (well, it clearly happed!) and I am not surprised – a lot of actions are habitual.

Would it be a possibility suggesting an ID, similarly to how meet.jit.si suggests an meeting ID?

[varjolintu]

@jdittrich keepassxreboot/keepassxc#5772

[MasinAD]

In my setup, it is already suggesting an ID. I would prefer sane defaults in the input field at this point, e.g. prefilled with hostname, browser and (database name|database root folder name|database filename) → wmde-103180_vivaldi_wmde

Training colleagues to use KeePassXC is part of my job, and the screen asking for a database name always baffled me as naming the database doesn't seem relevant anywhere. I think I am in the minority of those users using multiple databases, so those names do appear in the tabs of KeePassXC. But until now I didn't notice the name appears here, too. Which isn't that useful for single database users as they already might have forgotten the name they issued for their database (creating the database and discovering the browser extension can be at vastly different points in time). The name does appear in the window title bar, though.

I wouldn't call that exactly "discoverable" from a user's point of view, but it is there. I understand from a UI PoV that "always show tabs even if only one database" isn't really attractive.

Unlocking the database initiated by the extension looks different enough, I guess:

If I interpret this correctly, the connection request window is created by KeePassXC and not by the browser extension. I haven't put much thought into this before but I always had the feeling that clicking "Connect" or "Link" (it's called "Verbinden" in German which might translate to both) is an interaction I have with the extension. Prefilling the ID field as suggested above might not be possible because of that as KeePassXC maybe doesn't know which browser the extension is running in. But at least after establishing the connection the extension knows the identifier name so there's some metadata communication.

There are some UI issues. IMHO nothing serious but I'd really welcome if they were addressed.

The text in the connection request dialog could read something like "Connection requested for the currently active database: %s", emphasizing it's about active vs. inactive databases.

If there was a way to provide the browser name, state it clearly: "KeePassXC Browser in Vivaldi requests a connection to the currently active database: %s"

In keepassxreboot/keepassxc#5772 someone mentioned the database name field being empty causes a rather confusing string in this dialog. In my opinion, each database should have a name and users should be forced to give it a name. The request should error out with "Cannot connect to unnamed databases. Click here to name it. [Database Settings]". But I understand that's no good UX so I proposed several levels of graceful degradation above. The root folder can be unnamed, too 😮. In my eyes, the root folder should initially be named after the database, but keeping database and root folder name in sync afterwards isn't part of the KDBX 4.0 specs, I guess.

[jdittrich]

@MasinAD probably best to put the "knowing the database name" into a separate issue (this one is closed anyway).

[droidmonkey]

I discussed this in another issue requesting suggestions for connection id's. Ultimately, it is a lot of extra code to make proper suggestions. This is generally speaking a one time action. I really don't see the return on investment for any major improvements to this process.

[MasinAD]

@droidmonkey I completely understand it. I wouldn't set it to high prio, too. Maybe put it on a list for when there's some revamping of the workflow.

Unfortunately, I'm not very good at C++, so I cannot help. Some googling led me to

[static] QString QHostInfo::​localHostName()
Returns the host name of this machine.

And if I understand QInputDialog correctly, something like keydialog.textValue = "%1_%2".arg(db->metadata()->name().toHtmlEscaped(), QHostInfo::​localHostName()) would prefill the text input with at least database name and hostname. @varjolintu mentioned in keepassxreboot/keepassxc#5772 (comment) that support for browser identification in the protocol was deemed a possibility 10 months ago. I don't know the current state, though.

I remember times when I had to re-connect on a more or less regular basis because something went awry. But since a year or two the browser extension's connection is quite solid, so it's mostly a non-issue for me. But I'd think of me as one of the more proficient users of KeePassXC. I'm not the target audience for this kind of code but first-time users discovering the possibility of the browser extension.

I just ask to at least consider this feedback if you ever come to re-working this procedure. Or tell your price for us to pay for it ;-).

[varjolintu]

The best I can do here is to add some improvements to the Protocol V2 that could help implementing this.