Improve KeePassHTTP security

[sfunk1x]

After looking through the issues list for KeePassHTTP, I noticed issue #258 and it's criticism of the encryption of the connection between the HTTP clients and the HTTP server. I'm not seeing where this has been addressed in KeePassXC, so I'd like to bring this up here.

The concept of generating a set of self-signed certificates to ensure a TLS connection on either the exposed LAN connection (remote) or on localhost seems like it would go a long way to securing against potential password leak without using a roll-your-own-crypto approach, which seems to be what's going on currently.

[TheZ3ro]

Note that on KeePassXC by default KeePassHTTP is disabled at compile-time, is disable at runtime, and listen only on localhost.

The security is not a roll-your-own-crypto based, simply KeePassHTTP is an API to access the kdbx, that is encrypted with AES-CBC. Asks for the master passwords, decrypt the database, sends the password to client.

Now, for making a CBC Padding oracle attack you need to have a padding oracle in a decryption method. I don't know the KeePassHTTP protocol well but the only decryption is done when asking for the master password and unlocking the db (I think). Also I don't know KeePassHTTP but the padding oracle can be done only if the server report a padding error.

The flaw in KeePassHTTP is not described in detail and is only "theoretical" so there is no need to panic IMHO. Should be nice a talk with @Quiark

[Quiark]

Unfortunately I have a working exploit code for KeePassHTTP (that I'm not going to share until this hole is resolved). The KeepassHTTP protocol is not very good and there are fields that are being decrypted and where padding attack can be applied. And even if padding error is not reported, a timing attack could still be probably used.

TLS may be an option (though I realize how complicated it can be, especially in C++). Maybe TweetNaCl could be used. But I ended up not using browser integration at all because I see it as a too large security risk (issues found by taviso in other password managers could potentially apply here as well).

[TheZ3ro]

If you share to me the exploit code privately (you can get my email in m profile->website or from a GPG key in a verified commit) I can better understand where the flaw is, detemine if it's in the protocol or in our implementation, and decide if needs TLS or just a patch in our implementation.

Also note that our KeePassHTTP implementation is different from https://github.com/pfn/keepasshttp

I will dive more into our implementation in the next few days to see if a Padding Oracle apply somewhere.

For an eventual TLS implementation we can maybe rely on https://doc.qt.io/qt-5/ssl.html and https://github.com/GuiTeK/Qt-SslServer

[TheZ3ro]

Summary: the key exchange in KeePassHTTP protocol between the browser and the server (KeePass/X/XC) happens in pure plain-text. This is a bad security practice, if an attacker can succeed in this, he can use the KeePassHTTP protocol to request almost all of your passwords. (limited to the one with an URL set or an URL in the title)

All the users that are using KeePass/X/XC are recommended to stop using KeePassHTTP plugin. At least ensure that the server (KeePass/X/XC) is ONLY listening on your PC only (host: 127.0.0.1). Listening on 127.0.0.1 limits attacks coming from outside your network, but malicious application on your computer can still intercept the protocol's key exchange.

Thanks also to @Quiark

[pfn]

If you have a malicious application on your system capable of sniffing localhost traffic, then you have already lost the security war. Game over, full stop.

Using tls and installing a cert in the browser is not practical unless shipping you're own tls implementation.

[pfn]

Your... Stupid autocorrect.

Anyway, if you're using nacl, sounds like using your own tls implementation is the plan

[TheZ3ro]

We can't just insert TLS if chromeIPass and PassIFox are not designed for it. In KeePassXC, the KeePassHTTP protocol is listening on 127.0.0.1 and disabled by default but lot of implementation are listening over 0.0.0.0 and accepting requests across the web

[phoerious]

Using plain HTTP is not generally a problem. Security can be implemented on any layer. That's how OTR chats work, that's how email encryption works. Not using HTTPS can sometimes even be a good idea, especially when you want or need to avoid the hassle that evolves around the TLS PKI. The problem here is rather this is a poorly designed (and also completely undocumented) proprietary crypto protocol. It would be much better to use a proven standard protocol.

[phoerious]

Due to the popularity of this feature, I'm thinking about rescheduling this issue to 2.1.1 and declaring it as a bugfix therefore. Any thoughts on that @droidmonkey @TheZ3ro ?

[TheZ3ro]

I've already said that in https://github.com/keepassxreboot/keepassxc/issues/177#issuecomment-274528136 A bugfix is obviously needed. We should also continue to release 2 separate release with and without KeePassHTTP support.

[phoerious]

I started something in the hotfix/147-keepasshttp branch. I removed the hostname setting and only bind to localhost now. I also added a warning when the user tries to configure a port below 1024.

[eugenesan]

Hi all,

I don't understand what is all this noise about...

1. Where is exactly the security "hole" everybody talking about?
2. Is referenced by Quiark exploit able to "suck" the passwords without me interacting with the KeePassX UI?
3. Are we responsible for the cases where user overrides default localhost binding?

I tend to agree with https://github.com/pfn, once your localhost is compromised you are already screwed and if you're messing with default network binding you are on your own...

[phoerious]

Running this protocol over a remote network connection is inherently insecure. KeePassX(C) does not only target expert users with in-depth cryptography and security knowledge and we therefore need to take good precautions that people don't expose their passwords. Also even if the user configured the application correctly, but for some reason binding to the specified hostname and port fails, KeePassXC silently falls back to listening to 0.0.0.0 which is a no-go. My patch removes all that and enforces running on localhost or not at all. That is the only configuration in which this protocol has no known exploits.

[eugenesan]

@phoerious nobody argues with the fact that configuring non-local binding is a security issue. The problem is the way the issue is presented and treated by developers.

This feature by it's dangerous nature is targeting toward more advanced/cautious users.

Regarding your modifications, I partially agree with your reasoning. Fallback to 0.0.0.0 is silent only to user using the GUI. I agree I should have added the warning to GUI also. Note, your modification breaks systems with IPv6.

[phoerious]

No, it does not break IPv6 systems. That would require disabling IPv4 completely (which we can assume is not the case and even then I'm not sure if lo wouldn't still accept requests over IPv4).

[TheZ3ro]

@eugenesan

1. If there is an error binding 127.0.0.1, Default fallback to 0.0.0.0 -> key is exchanged in plain-text
2. Once you have the key you can follow the protocol and request every password you want (with website set in title)
3. If the default fallback is 0.0.0.0 then yes.

I tend to agree with https://github.com/pfn, once your localhost is compromised you are already screwed and if you're messing with default network binding you are on your own...

What the purpose of having a password-protected offline KDBX file if when my PC is compromised everyone can read my passwords? Just store them in plain-text and don't even bother with passwords managers.

[eugenesan]

@TheZ3ro

1. I thought HTTP used asymetric crypto and TLS handshake therefor plaintext key is not a problem.
2. I still not sure there is no enforcement of IF and which password is pulled for each request, unless you did put the checkbox on "Remember my choice" which IS the rea security flaw.
3. The default is "localhost" and "0.0.0.0" is the fallback with warning (currently in console only which should be fixed).

[phoerious]

Just to rule out any misunderstandings: the KeePassHTTP protocol works over plain (!) HTTP, not HTTPS. It is therefore not TLS encrypted. The encryption is handled by the sending and receiving parties without using any official crypto protocol. The catch with this is that the messages are unauthenticated and the handshake is done without encryption in plaintext. Therefore intercepting that handshake enables you to decrypt all follow-up traffic. Furthermore, a padding oracle attack is possible on the AES-CBC cipher texts due to the lack of authentication.

[TheZ3ro]

@eugenesan

1. HTTP does not use any crypto or TLS. The key is sended in the associate request base64 encoded (here the reference https://github.com/pfn/keepasshttp)
2. Depends on users preference. Attacker can still sniff get-logins request-response and decrypt passwords with the key that he sniffed previously
3. If localhost:19455 is already binded what will happened?

[eugenesan]

@TheZ3ro 1,2. If that's the case, there is a reason to be worried ( A LOT :-o )

3. It warns the user (only in console) and falls back to the default of the Qt network stack which is 0.0.0.0.

I always assumed original protocol developer did the crypto correctly...

[phoerious]

That's what we are talking about. This protocol is poorly designed and must therefore not be used over any remote network connection.

And BTW it does not fall back to any Qt defaults. We don't even use Qt for running the network daemon at the moment. That is handled by libmicrohttpd and the fallback to 0.0.0.0 was hardcoded.

[eugenesan]

@phoerious

1. I am not sure the code currently in this repo inclueds all the intended fixes it originally had. Please see recent commits at https://github.com/eugenesan/keepassx/commits/2.0-http-totp-2.0.3
2. libmicrohttpd handles the binding while Qt network stack handles the IP:PORT availability and DNS. And once more, 0.0.0.0 is only a fallback, just remove it or request user interaction to solve that.

[eugenesan]

As an immediate resolution I suggest disabling 0.0.0.0 fallback. But a more serious issue is a lack of proper TLS-style crypto, that should be discussed with browser side maintainer. KeePassX project can't handle it by it's own.

[phoerious]

pfn made it pretty clear, that TLS will not be an option, so we have to work with what we have. Therefore restricting to localhost is our only option.

[eugenesan]

@phoerious Well, I am not sure how to comment on pfn's opinion regarding TLS but I agree regarding localhost.

Note, I still think localhost is a DNS record "localhost" and not IP address 127.0.0.1 and should not be treated as such.

[eugenesan]

@pfn Earlier, I've agreed with you regarding concept of loosing to "local sniffing agent" but now, after some thought gymnastics, I think I've missed important part of the problem. I still agree regarding regarding real "local sniffing agent" but since half of the variability is in the browser it is essentially a "remote sniffing agent" and must be treated with at least secure handshake.

[phoerious]

That's a different factor, but a secure handshake won't help you there. The browser extension has the decryption key for retrieved passwords. Therefore, if your browser extension is compromised, an attacker can read the passwords even if a proper cryptographic protocol had been used. Letting your browser query passwords from your database opens lots of attack vectors which cannot be mitigated in any way except not using KeePassHTTP in the first place.

I added another commit to my branch which disables the plugin by default so that users have to enable it deliberately.

[pfn]

A secure handshake over localhost is unnecessary. The key exchange must never occur over the Internet. That's basically all.

On Mon, Jan 23, 2017, 6:15 PM Eugene San notifications@github.com wrote:

@pfn https://github.com/pfn Earlier, I've agreed with you regarding concept of loosing to "local sniffing agent" but now, after some thought gymnastics, I think I've missed important part of the problem. I still agree regarding regarding real "local sniffing agent" but since half of the variability is in the browser it is essentially a "remote sniffing agent" and must be treated with at least secure handshake.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274681371, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxTJ_ApXSPJOJCwRw8YxjPfcKnZxaks5rVV7BgaJpZM4LeEMj .

[phoerious]

Also the cipher text must not go over a network connection since it does not follow the Encrypt-then-MAC idiom and the CBC cipher is therefore vulnerable to timed padding oracle attacks (even if the server does not produce any particular error message).

[pfn]

I would like to see proof of the latter.

On Mon, Jan 23, 2017, 6:35 PM Janek Bevendorff notifications@github.com wrote:

Also the cipher text must not go over a network connection since it does not follow the Encrypt-then-MAC idiom and the CBC cipher is therefore vulnerable to timed padding oracle attacks (even if the server does not produce any particular error message).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274684254, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxbXKnVVqsJk3pWG3xOQBmNoWF4IQks5rVWNcgaJpZM4LeEMj .

[eugenesan]

@phoerious I am afraid we'll have to rely on Browsers enforcing security/sandboxing of the extensions. I do worry about silent sniffing of "approved by user passwords" but I am worried more about other extensions or/and JS fetched from the web being able to decipher/fetch the key and fetch the passwords silently (by overcoming KeePassX UI requests) or/and using timing attack to fetch specific passwords. This is where we need cooperation of @pfn.

Update: I do agree with https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274684254

[eugenesan]

Since this issue affect, many project I think this discussion should be taken out of this issue context. For one we should hear the option of KeeFoxRPC author https://github.com/luckyrat.

[pfn]

Browsers have lots of sandbox functionality. The key stored by the browser extensions is generally unavailable to other extensions (Firefox is weird, all extensions basically have full, complete access to everything. Chrome is well sandboxed). Websites are banned from making requests to localhost (Cross origin protection, and this is enabled across all browsers), etc.

Yes, a malicious copy of the browser extensions (or in the case of Firefox, a 3rd party extension) can pretend to be legitimate extensions, perform key exchange and then access passwords.

On Mon, Jan 23, 2017, 6:36 PM Eugene San notifications@github.com wrote:

@phoerious https://github.com/phoerious I am afraid we'll have to rely on Browsers enforcing security/sandboxing of the extensions. I do worry about silent sniffing of "approved by user passwords" but I am worried more about other extensions or/and JS fetch from web being able to decipher/fetch the key and fetch the passwords silently (by overcoming KeePassX UI requests) or/and using timing attack to fetch specific passwords. This where we need cooperation of @pfn https://github.com/pfn.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274684455, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxaQg16oJ8nKQrnR6A77rdwD4raNlks5rVWOkgaJpZM4LeEMj .

[eugenesan]

What if we:

1. Add a secure handshake
2. Add some form of "user side" authorization like it is done in many protocols as zrtsp and signal. Meaning a random user readable text appears on both sides, in our case in the extention and the KeePass*'s password fetch approval UI?

[phoerious]

@pfn you can use a side-channel attack by measuring timings to find out whether a code decrypted correctly or not. I'm not quite sure what happens when the ciphertext is invalid in our case, but it's surely not handled properly. Encyption modes that require padding ALWAYS need message authentication and if the MAC of a message fails, no attempt must be done to decrypt the message. Cloudflare has a pretty good read about the weaknesses of CBC: https://blog.cloudflare.com/padding-oracles-and-the-decline-of-cbc-mode-ciphersuites/

@eugenesan If a website manages to break out of the sandbox and make requests to localhost, it is probably also able to retrieve the encryption key from the browser extension. And if we talk about secure handshakes, we also need to talk about authenticated encryption. Otherwise our secure handshake isn't work anything.

[droidmonkey]

You would need to modify the browser plugin to enable this security. Certainly not impossible (it is hosted on Github), but will take a lot of coordination and people to update their browser plugins.

[eugenesan]

@phoerious Leak of the key is a serious issue but user interaction is an addition line of defense. While the protection of the key is the job of the browser it should be possible to enhance browser and KeePass plugins to provide UI protection and it will make the job of the "rouge" agent substantially more difficult since altering UI would require modifying the code of the browser plugin which is not a trivial task.

[Quiark]

If TLS is a no-go (should not be too hard to do in .NET) then I would recommend, crypto-wise, to at least switch to libsodium and/or TweetNaCl (that's the JS version) and use secret_box constructions for the entire message (not individual fields) and ensure you have a nonce there. The library and API are deliberately designed to avoid problems such as CBC oracle and it even allows you to implement a public-key exchange.

út 24. 1. 2017 v 11:15 odesílatel Eugene San notifications@github.com napsal:

@phoerious https://github.com/phoerious Leak of the key is a serious issue but user interaction is an addition line of defense. While the protection of the key is the job of the browser it should be possible to enhance browser and KeePass plugins to provide UI protection and it will make a job of the "rouge" agent substantially since altering UI would require modifying the code of the browser plugin which is not a trivial task.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274689875, or mute the thread https://github.com/notifications/unsubscribe-auth/AADmw-5ffPvlMcaMlkTzaaD5roa_s14tks5rVWy1gaJpZM4LeEMj .

[phoerious]

@eugenesan I'm not quite sure what your attack scenario is. But in our case the main weakness is the protocol. The rest is more of a general security concern of whether you want a browser plugin to have access to your database or not.

@Quiark I would also prefer a real crypto protocol with full message encryption as it would prevent leaking of arbitrary information. However, that requires huge changes to the browser plugins. I'm willing to go that path if there is support from the browser add-on developers, but only then. Partial solutions don't work. 

[TheZ3ro]

Please guys, here we are discussing KeePassHTTP implementation in KeePassXC. If you want to talk about KeePassHTTP protocol the issue is this -> https://github.com/pfn/keepasshttp/issues/258

[pfn]

I'm looking to hand off development of the browser extensions (and keepasshttp for that matter). There will be minimal support for any protocol changes from me.

On Tue, Jan 24, 2017, 2:15 AM TheZ3ro notifications@github.com wrote:

Please guys, here we are discussing KeePassHTTP implementation in KeePassXC. If you want to talk about KeePassHTTP protocol the issue is this -> pfn/keepasshttp#258 https://github.com/pfn/keepasshttp/issues/258

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274762763, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxXVMJFwJbcTbnGgmC3pXKM8zTpCVks5rVc9cgaJpZM4LeEMj .

[phoerious]

Does that mean, KeepassHTTP won't be maintained anymore? 

[pfn]

It will be maintained enough to be sufficient for my needs. But not enough for any features or protocol enhancements.

On Tue, Jan 24, 2017, 6:53 AM Janek Bevendorff notifications@github.com wrote:

Does that mean, KeepassHTTP won't be maintained anymore?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/keepassxreboot/keepassxc/issues/147#issuecomment-274825853, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxTlrIGBfUwLAQfDDaBx6Xslhnfgbks5rVhCDgaJpZM4LeEMj .