This repository has been archived by the owner. It is now read-only.

Security #28

Open
varjolintu opened this Issue Oct 16, 2017 · 46 comments

Comments

Projects
None yet
5 participants
@varjolintu
Owner

varjolintu commented Oct 16, 2017

Security

Any questions, improvement suggestions or general discussion about security. Ask and comment freely. I also wanted to clarify why I did the things and improvements this way. The document will be updated randomly when new ideas appear.

HTTP or sockets -> native messaging

When a browser extension starts a process using native messaging first it loads a JSON config file that holds the path to the binary file and a list of allowed browser extensions. So when comparing this to the plain passifox/chromeipass and KeePassHTTP it much more secure. To gain access to your database or the KeePassXC process a malicous browser extension will not be accepted and native messaging returns an error. To "hack" the connection between native messaging browser extension and KeePassXC you'd need a rogue application that must change the JSON file content and provide an incorrect or malformed version of the KeePassXC itself. The downside is that the KeePassXC-Browser extension basically controls the KeePassXC process and it cannot be started beforehand or the connection fails. In short, it's pretty secure after all.

Proxy application support

This new browser intergration also supports Unix domain sockets (named pipes under Windows) that could handle any messages between KeePassXC-Browser and KeePassXC with the keepassxc-proxy application. This method is much safer than the original KeePassHTTP provides. When using a proxy application through Unix domain sockets or the old KeePassHTTP method it's of course possible that a malicious application captures your keys and acts like a "verified" client. Currently there's no safe method to ensure if the client is valid without any extra user interaction. Still, libsodium provides a slightly better encryption so the connection doesn't rely on just one base64 encoded key that's visible in packet capture.. Instead it uses a keypair where only public keys are transferred but you need the secret key(s) to encrypt or decrypt the data.

Other improvements

The version 0.3.4 added a feature that you can lock the database from the popup without needing to go back to KeePassXC. With 0.3.5 the database states are received from KeePassXC instead of polling the state continously.

The previous versions (including the original passifox/chromeipass) didn't update the active tab credentials even when there were changes with the databases. This means that if your database has locked because of a timeout or you have switched to another database tab the active tab/page still holds valid credentials in the memory from the previously active database. This is no good. Now the credentials will be cleared if the database is locked or switched to a locked one. Also, if you have two databases open the credentials are received automatically from the active database even if you switch from one to another. All this happens without needing to reload the page.

@achmetinternet

This comment has been minimized.

achmetinternet commented Nov 4, 2017

I am using your unofficial rc4 fork but the extension asks me to update?

You use an old version of KeePassXC.
Please download the latest version from keepassxc.org.

If I do not update, am I running a security risk?

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Nov 4, 2017

This is because my fork is using the 2.2.0 release as its base and I haven't merged the changes of 2.2.1 and 2.2.2. Mostly the fixes are for Yubikey and snap/AppImage issues, so if you don't use those I'd say it's quite safe.

Maybe I'll make another branch that doesn't conflict with the PR branch and release the changes with 2.2.2 as base.

You can find the list of fixes here:
https://keepassxc.org/blog/2017-10-02-2.2.1-released/
https://keepassxc.org/blog/2017-10-22-2.2.2-released/

@rugk

This comment has been minimized.

rugk commented Dec 14, 2017

Looking through the protocol description, 1. already strikes me:

keepassxc-browser generates a key pair (with public and secret key) and transfers the public key to KeePassXC

And how does it do so securely? Assuming all encryption afterwards is secure, this can still be the point where the whole system fails.
So the MD should describe how this is done. FYI: It needs to be done in a way, so an attacker (in your case a malicious application, right?) cannot intercept and modify the key.

IMHO the easiest way would be to tell the user to copy and paste that from one application to another one. Of course, such a key would be useful to store, so one does not need to copy & paste it each time the browser is started.

So as you use Libsodium, I hope you use all libsodium's functions for nounce generation and such stuff. AFAIK it is supposed to make it easy, so good…

When keepassxc-browser sends a message it is encrypted with KeePassXC's public key, a random generated nonce and keepassxc-browser's secret key.

AFAIK it should not matter, but why do you randomly create it in one direction and just increment it when sending messages in another direction?

Whatever reason you may have, it needs to be documented. Undocumented cryptostuff is not good…

nonce - 24 bytes long random data, base64 encoded. This must be the same when responding to a request.

Attention: No, please don't do so. A nonce reuse is a nonce reuse (have a look at what bad things you find), better don't do it. Even if the messages are the completely different ones, just don't… you never know who comes up with some clever ideas, yeah…
Just use an ID or so if you want to identify the request, not abuse the nounce for that.
Because I'd say the nounce is a thing to prevent replay attacks here. To associate requests, just include an ID inside of the encrypted payload, so an attacker cannot even see it. Then an attacker also cannot fake the ID or so, if the ID is also a cryptograhically secure random number.

And looking at the examples a final word: From a stance of TLS, your protocol is really metadata heavy. All action names and client IDs are also submitted in plaintext. While one may not care in general and it is likely also just to protect localhost communication, some users may want to use it for remote communcation. In that I see a problem, and if it is just how much/when you request passwords or so.
And I see no reason why KeePassXC would need to see the request action or so in plaintext.

Disclaimer. Not an expert, so you know.


Finally: Of course you should be aware that designing such a protocol can be dangerous. Thanks to an easy libsodium crypto library, you may avoid many attacks, but nevertheless even Libsodium may not rescue you in each case.
So just FYI, there is also Noise or so, a protocol (usually intended/used for messengers), but also based on Libsodium, and it may work in your case, too.


But generally a word of praise here: It's very good you are documenting that protocol. The doc needs to be extended, but generally the whole thing looks nice.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Dec 15, 2017

Looking through the protocol description, 1. already strikes me:

keepassxc-browser generates a key pair (with public and secret key) and transfers the public key to KeePassXC

And how does it do so securely? Assuming all encryption afterwards is secure, this can still be the point where the whole system fails.
So the MD should describe how this is done. FYI: It needs to be done in a way, so an attacker (in your case a malicious application, right?) cannot intercept and modify the key.

The public key (like all other messages) is transferred through native messaging. It's more secure than a plain TCP socket. But the public key itself is transferred in plain-text. Secret keys are not transferred, ever. And you need both (+ nonce) to do proper encryption/decryption.

IMHO the easiest way would be to tell the user to copy and paste that from one application to another one. Of course, such a key would be useful to store, so one does not need to copy & paste it each time the browser is started.

Using the clipboard is really not so secure, so nope. And this is just one extra step for the user that doesn't actually make anything more secure. Also, it's not wise to store a public key and then use the same key over and over again (I suppose that's what you meant, and for example passifox actually does this..). This is why I chose a different kind of solution: If there's no saved database in the browser extension, the new database is stored with the current public key as the identifier. It is not used again in the next browser session. The key is only used for identification. A new key pair is generated each time for the actual messaging.

AFAIK it should not matter, but why do you randomly create it in one direction and just increment it when sending messages in another direction?

Because currently all messages are asynchronous and some responses can take more time than others, and at this point there's no buffer for these responses. This can mess the incrementation counter. It's not the best solution, I know.

Whatever reason you may have, it needs to be documented. Undocumented cryptostuff is not good…

nonce - 24 bytes long random data, base64 encoded. This must be the same when responding to a request.

This was old stuff in the documentation. Updated.

And looking at the examples a final word: From a stance of TLS, your protocol is really metadata heavy. All action names and client IDs are also submitted in plaintext. While one may not care in general and it is likely also just to protect localhost communication, some users may want to use it for remote communcation. In that I see a problem, and if it is just how much/when you request passwords or so.
And I see no reason why KeePassXC would need to see the request action or so in plaintext.

Action names are necessary for some operations, for example change-public-key or lock-database. But I agree, it could be removed from other messages in the future. Using the clientID is also important because that way the encrypted message can be redirected to a class that holds the correct key pair for decryption. Otherwise you couldn't know which client sent the message and which key pair to use. This is only for situations when you have multiple connections active simultaneously. If you are just using a single connection the clientID has no meaning. So in that case it's just useless metadata.

Disclaimer. Not an expert, so you know.

Me neither. That's why these kind of conversations are vital.

But generally a word of praise here: It's very good you are documenting that protocol. The doc needs to be extended, but generally the whole thing looks nice.

Thank you. And the other feedback too! I appreciate it.

@rugk

This comment has been minimized.

rugk commented Dec 15, 2017

The public key (like all other messages) is transferred through native messaging.

Ah, great. Yeah, I assume STDIN/STDOUT is save. But in that case, you would not need to encrypt the data through native messaging anywhere?
I mean after the public keys are exchanged, you still use NM to exchange the data, don't you? So if that channel is already secure (i.e. cannot be read or modified by other applications) theoretically no encryption is needed. (Maybe when the proxy is used this get's different, does not it? How does the proxy communicate to KeePassXC?)

Using the clipboard is really not so secure, so nope.

Yeah, thinking about it of course applications can modify the clipboard… So, good you already have a better solution, NM.

If there's no saved database in the browser extension, the new database is stored with the current public key as the identifier. It is not used again in the next browser session.

Ah great. This should be stated more explicitly in the docs, I thought it might store something long-term.

So in that case it's just useless metadata.

Yeah, okay, clientID makes sense. But as you stated, the action-name must not be sent in plain-text… (not such bad, but yeah…, it would be one bit less of meta data 😉 )

This was old stuff in the documentation. Updated.

It's still there in the master branch:

nonce - 24 bytes long random data, base64 encoded. This must be the same when responding to a request.

Also it seems you do not use a nonce for each request. E.g. the reply to get-databasehash does not contain a nonce, but it is also not encrypted in general.

Also the change-public-keys request from the browser contains a nonce. AFAIK this request in not encrypted, so a nonce does not really help here for anything. Also the reply to this request contains no nonce.

So as you use NM let's still start with an attacker. What's the thread model here?

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Dec 15, 2017

Ah, great. Yeah, I assume STDIN/STDOUT is save. But in that case, you would not need to encrypt the data through native messaging anywhere?
I mean after the public keys are exchanged, you still use NM to exchange the data, don't you? So if that channel is already secure (i.e. cannot be read or modified by other applications) theoretically no encryption is needed. (Maybe when the proxy is used this get's different, does not it? How does the proxy communicate to KeePassXC?)

Yes. Probably the encryption is not even needed with just native messaging. But it's needed because of the proxy behaviour. It communicates through Unix sockets and named pipes (under Windows) and those can be monitored. Of course those sockets are created in XDG_RUNTIME_DIR instead of /tmp/ when possible. With pure native messaging everything's a bit more secure. That's why I wanted to leave the KeePassXC fork the feature to connect to it directly.

Ah great. This should be stated more explicitly in the docs, I thought it might store something long-term.

Yes, I will update the document with more details in the future. The fixes are still in the develop branch but I'm making a new release soon and it'll merged to the master.

Also it seems you do not use a nonce for each request. E.g. the reply to get-databasehash does not contain a nonce, but it is also not encrypted in general.

Also the change-public-keys request from the browser contains a nonce. AFAIK this request in not encrypted, so a nonce does not really help here for anything. Also the reply to this request contains no nonce.

There are situations that you need to be able to see if the extension is connected to KeePassXC and currently the method for this is getting the database hash. Maybe a separate command for this would be better in the future. The current model is not optimal. I think it should be the following:

  1. Ask is KeePassXC is present and ready for key change
  2. Change the keys
  3. Query the database hash

I'm going to fix that in the future.

So as you use NM let's still start with an attacker. What's the thread model here?

Local access. After that it's game over. Or using a computer that you don't control (have root rights). Nothing really guards against that.

@rugk

This comment has been minimized.

rugk commented Dec 15, 2017

Local access. After that it's game over. Or using a computer that you don't control (have root rights). Nothing really guards against that.

Okay, yeah, it is game over then. But is not that what you are trying to protect against with the encryption? Not from the root attacker, but from other applications which e.g. run as a different user or as the same user. That's why you'd want to encrypt communication over these sockets at all, is not it?

Local sockets are generally a trusted thing on Linux. (one can even "authenticate" the applications connecting to Unix sockets on Linux as it seems) E.g. consider MySQL/MariaDB servers, they also use sockets or local pipes to communicate with applications. And usually one does not use an additional layer of encryption there.

Still one thing, because I don't know so much about sockets/named pipes: Applications could listen there, but could not do Man-in-the-middle attacks and modify data? If not that would be bad, because then they could tamper with the handshake there and replace the public keys exchanged via change-public-keys.
But, in any case, that means that one still should not sent the data of these sockets through the network, as there MITM attacks are always possible.

Also, BTW, generally: When NM itself is secure and does not need encryption (as it seems), you could also do all that encryption in the proxy application and do not need to do it in the browser.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Dec 15, 2017

Yes, every data that goes through any socket should be encrypted because the connection is not direct. At first the sockets were all UDP. The encryption was surely needed with those, and even when using Unix sockets now I think the encryption is good to have.

I prefer to do the encryption elsewhere than in the proxy application. Proxies should be simple, secure and effective, just passing messages through. This opens a possibility to make your own proxy and deliver the data through for example TCP sockets if you want to. You want encryption there.

What I really like about native messaging is that you could only use the certain extension to open a certain binary. This ensures that even if you manage to install a malicious browser extension, it cannot start the KeePassXC binary or proxy without tampering local files.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

Would you be open to continuing the discussion on this? I don't want to waste your time if you're not, but otherwise, I'm not convinced that there is a security risk, and the use of native messaging has made the usage of the extension massively painful.

If you are open to further discussion:

What I don't understand is, under what scenario would a rogue application be able to extract data from KeePass(XC), but be unable to tamper with files on the disk?

  • If you're referring to a virus with admin privileges, it can do anything; there's nothing you can do.
  • If you're referring to a virus without admin privileges, it can still modify KeePass, read/write your user files, or modify your browser profile, so there is still nothing you can do.
  • If we're talking about a rogue browser extension, it will not have the key required to communicate with KeePass(XC)—which is the entire point of having that key. Sockets and native messaging will make no difference here.

So if you don't mind me asking—what is the exact attack you are trying to prevent?

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

@mehrdadn I'll suggest you check these threads for some info about the weaknesses of keepasshttp:
keepassxreboot/keepassxc#147
pfn/keepasshttp#258

Using native messaging with updated encryption and better key changing method eliminates those weaknesses. Even if you had local access to the computer and a possibility to sniff Unix sockets you cannot decrypt the messages. And of course the Unix sockets are created with QLocalServer::UserAccessOption flag where access is restricted to the same user as the process that created the socket. If you want to eliminate even the Unix sockets you can connect to KeePassXC directly via STDIN/STDOUT pipe (when proxy is diabled).

Because there's a possibility to create a custom proxy that forwards the traffic through network, proper key change and encryption are definitely needed.

While there might not be an exact attack method to prevent (maybe sniffing the localhost is the closest), all this makes it much harder. And harder to setup too. A few extra steps are needed. While using the extension might be a little more painful, it's soon getting updates which makes the experience slightly more smoother.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

I'll take a look at those links (thanks) but I'm confused why you say proper key exchange and encryption are needed—I agree with that, and I never suggested otherwise. I'm talking about native messaging vs. sockets, not plaintext vs. encryption. The entire point of encryption (which I agree is good, and which you already seem to have) is to make it so that it doesn't matter whether anyone is intercepting the traffic actively or passively—they won't be able to decrypt anything. The transport mechanism makes no difference here!

Also note that I am not suggesting the KeePassHTTP protocol itself was secure—I am just saying that, with a protocol that is secure, the transport mechanism is irrelevant (pretty much by definition).

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

I understand. I was just making my point clear. Those threads will give you some answers why plain HTTP sockets can be insecure.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

I'm reading them, but just as I just mentioned, their complaints are that KeePass's protocol is insecure. People are trying to solve the problem via proper handshakes, key exchange, authentication, and the like. Nobody has even suggested that changing the transport mechanism to native messaging is the solution to their problems!

In fact this comment makes this point explicitly clear:

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.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

The issue is more about securing the transfer mechanism between a browser extension and an application. It's worth noticing that both 1Password and LastPass also use native messaging with their desktop application. If you have a possibility for a slightly more secure transferring method, why not use it?

I agree that while HTTP is not generally a problem, a listening socket is not restricted to a certain user. Also, any browser extension can make HTTP requests to localhost. I prefer to have a closed system where no other application or browser extension outside your user can access KeePassXC.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

I guess I'm saying you should not use it because of the combination of these two reasons:

  • You yourself admit it is only ever-so-slightly more secure.
  • The price you pay is that far more people will be left less secure than with a socket-based version, because they won't be able to actually use a program with this many hoops to jump through.

To be explicit: by all means, if you can make native messaging seamless enough that it doesn't make a difference usability-wise, I have no complaint against it. I am not claiming the technology itself is less secure.

But what I am saying is that "is it more secure?" is the wrong metric to use.

Think about what it would be like if you made the user have to type in Please Let me See My PaSsWoRd every time before seeing their password. That would make it very difficult for someone standing by your shoulder to accidentally reveal your password by pressing (say) Ctrl-Shift-U. It would be more secure! And the longer it is, the more secure it would be! So does that mean you should have people type an essay before they can use your program? No, that would be nonsense.

Remember that it's ALWAYS possible to make something more secure than it currently is. That mere fact does not justify taking that action. The real question is: what are the trade-offs? If you make something so secure that it's unusable, then you don't actually improve security; to the contrary, you can end up with a less secure world than you would be in if people had the ever-so-marginally-less-secure version of your program.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

I see your point. To answer your question (what are the trade-offs):

The price you pay is that far more people will be left less secure than with a socket-based version, because they won't be able to actually use a program with this many hoops to jump through.

I disagree with the usability here. There are only two new things people will have to remember:

  1. After enabling the Browser Integration from KeePassXC's settings enable the checkbox for your browser. This could be of course automatic, but then many are not happy if an application writes config files to applications you don't even have installed. Of course, a possibility is to check which one's are installed.
  2. When KeePassXC is closed, browser loses the connection. After KeePassXC is reopened, user needs to reconnect the extension. There was an automatic reconnect feature at some point but we had to revert it because of a bug in Chromium (I've found a workaround but it needs more testing). Users who keep KeePassXC in the background all times will not have to reload.

Native messaging makes some things more complex, and while many things are already automatic, the few manual things doesn't make the extension unusable. Most of all, I've tried to keep this as near as chromeipass/passifox experience as possible.

You yourself admit it is only ever-so-slightly more secure.

Maybe I expressed this way too lightly. It's more secure, not just slightly.

What is the biggest trade-off in your opinion?

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

Encryption makes the protocol itself secure, but it does not prevent user errors. The initial key exchange is the most fragile point and using a protocol over a network interface may lead to other users on the system fooling you into accepting their key. Sockets, on the other hand, are protected by your user privileges and no other user can attack it (unless they exploit a kernel vulnerability). So it is not primarily the encryption that makes things secure with local sockets only adding something to it. It is actually the other way round. What the encryption allows you to do (if you really want to) is to forward the socket over an insecure channel to another system (which is basically the same as using a normal network interface with all its problems).

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@varjolintu Well, I'm happy to reply, but before I do that, can I ask you the following four simple questions?

  1. Whom you want your users to be?
  2. Whom you think your current users are?
  3. Whom do you think your users could be with native messaging?
  4. Whom do you think your users could be with local socket communication?

Here's a list of user bases I can think of:

a. Security experts
b. Browser extension developers
c. Web developers
d. Software developers
e. Technical users in general
f. Everyday users in general

On my end, the answers to these questions would be 1 = F, 2 = A or B, 3 = B or C, 4 = D or E.
But I need to know this because it both affects my responses and because a strong disagreement over this makes the discussion pretty moot.

(Edit: I mean the bulk of your users, sorry. I'm sure there are exceptions in every category.)

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@phoerious I'm confused, are you arguing against sockets, or against exposing these externally? I'm talking about localhost:19455-vs.-native-messaging, not about 0.0.0.0-vs-127.0.0.1. I agree the initial key exchange is important (and there are a million ways to make it happen without exposing it to other users on a system, including asking the user to copy-paste via clipboard) but I can't tell what you're actually arguing for or against.

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

Anything that goes over a network interface can be listened to and potentially be attacked by other users on the system. That may not be such a big deal on your home PC, but it will be when you use it on a shared server, for example. And I'm not even talking about exposing it publicly to other machines in the network. By using sockets, we circumvent this attack vector completely.
Anyway, I don't understand this discussion. From a user's perspective, the technology does not matter. If you have a problem, then it's because something is not yet working as smoothly as it should be. This is not a technology discussion. This is a discussion about individual bugs.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

Anything that goes over a network interface can be listened to and potentially be attacked by other users on the system. [...] By using sockets, we circumvent this attack vector completely.

I don't understand what you're saying if I'm being honest. Network interfaces are used via sockets. How can network interfaces be insecure but secured via sockets?!

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

A local (Unix) socket is not a network interface. It is an IPC connection between two or more programs. Only because you can use sockets to talk to the network that doesn't mean that sockets and network interfaces are the same.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

Ugh, I see, okay. You say "socket" to mean UNIX socket and I say "socket" to mean TCP socket... let's try to clarify this in the future because this gets very confusing/contradictory.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

@mehrdadn

  1. A-F
  2. A-F (or B-E)
  3. A-F
  4. A-F

The point for such a large scale is that the new integration should be simple enough for anyone, but allow some advanced configurations so developers and experts can benefit from it. This is my opinion with any widely used open source software. I'm not sure how this categorizing would help our conversation. It's already going off the tracks.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@varjolintu: I asked because I'm in rather disbelief that category F will ever be the bulk of your users with the current number of hoops users have to jump through, let alone the current ones, so we need to agree on whether this is the case or not, because if you have data to the contrary then I have no point to make—you've already achieved the goal.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 9, 2018

@mehrdadn Everyday users in general don't care if there are TCP sockets or native messaging underneath. For them it's just enough everything works. Do I assume correctly that this is the point of view where you find the extension unusable?

If that category is not going to be the bulk of the users then those few additional steps in setup won't matter, right? Most of those people leave a review with words "It doesn't work" or you never hear anything from them. Other categories are be quite active writing issues and feedback, which I'm very thankful of.

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

I happily quote myself from above:

Anyway, I don't understand this discussion. From a user's perspective, the technology does not matter. If you have a problem, then it's because something is not yet working as smoothly as it should be. This is not a technology discussion. This is a discussion about individual bugs.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@phoerious To respond to your initial comment after the clarification... so I understand the attack scenario you're worried about is someone logging onto a server, trying to use a password manager, but suddenly having it hijacked by another user's program on that server? Wouldn't simply making the user copy-paste a key take care of that for the first setup?

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

Copy&Paste is another rather insecure channel. Thinking in terms of confinement by containerization, the clipboard is one of the worst channels to use for a key exchange. It is also also extremely volatile, because anything on the system can intermittently replace values stored in it. I also don't understand how copying and pasting something from somewhere to somewhere else would be any more user-friendly. It's actually quite a clunky way with a lot of hoops to jump through.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@varjolintu @phoerious Yes, like I said earlier, if they were equally seamless, I wouldn't be here trying to change your mind. The problem was that this setup as I see it is not seamless. The fact that it's confused users enough that you've had to write your own guide explaining how to do it is itself enough evidence of that fact. Otherwise if it's so easy why did you need to write a guide for it?

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

We needed a guide for KeePassHTTP as well.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@phoerious [previous comment] You call it insecure without describing the actual threat you are trying to mitigate. What is the threat?

@phoerious [last comment] Sigh... OK, I give up. KeePassXC-Browser with native messaging is way more awesome and less confusing than chromeIPass. Is that something you genuinely believe that now you're arguing against it?

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

You call it insecure without describing the actual threat you are trying to mitigate. What is the threat?

The clipboard is shared by all applications of a user (and when using legacy protocols like X11, I wouldn't even trust in guarantees on separation between users). Sockets can be protected by mandatory access control and only specific applications can be allowed access. Confinement does not work with a shared clipboard. Period.

Sigh... OK, I give up. KeePassXC-Browser with native messaging is way more awesome and less confusing than chromeIPass. Is that something you genuinely believe that now you're arguing against it?

Dude, I don't understand what you are complaining about. Literally the only thing that has changed is that on the settings page there is one more checkbox for selecting your browser. Everything else is exactly the same. Maybe it is less polished yet, but that is not a technology problem. If you have anything constructive about this issue, please let us know. But complaining about a technology choice when the problem is not actually the technology doesn't get you anywhere.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@varjolintu to reply to your earlier comment:

I disagree with the usability here. There are only two new things people will have to remember:

  1. After enabling the Browser Integration from KeePassXC's settings enable the checkbox for your browser. This could be of course automatic, but then many are not happy if an application writes config files to applications you don't even have installed. Of course, a possibility is to check which one's are installed.

This is indeed annoying but indeed less of an issue so I won't dwell on it here.

  1. When KeePassXC is closed, browser loses the connection. After KeePassXC is reopened, user needs to reconnect the extension. There was an automatic reconnect feature at some point but we had to revert it because of a bug in Chromium (I've found a workaround but it needs more testing). Users who keep KeePassXC in the background all times will not have to reload.

This is where I find things a little crazy right now....

First of all, it is completely unreasonable and counterintuitive for either the browser or the password manager to behave poorly if the user happens to want to start the password manager before the browser, or to close the browser before the password manager, or anything else implied by some kind of parent-child relationship between the browser and the password manager. This itself is far less likable than a config file written somewhere the user could care less about. And I dare say that no user (even a technical one who isn't familiar with Chrome internals) will expect or find it intuitive for Chrome to suddenly launch KeePassXC, if they don't find it outright frightening.

Second, proxies are extra programs that you now have to maintain too. For me they've crashed constantly. And then suddenly everything connecting the two stops working until I restart both the browser and the password manager. I have absolutely no idea what is going on. I can't imagine a nontechnical user will fare better. And then it's another component users have to keep updated. And another program with security holes.

Third, all those extra options in the other tab make things even more confusing. When something doesn't work (which it inevitably won't because the majority of users won't read guides before using your extension!!), users will try toggling them, and that will confuse them further as to what the correct settings are, and also make it even harder to get things working. I am saying this from my personal experience today, and I did read the guides. I can only imagine how easy laypeople will find it.

Native messaging makes some things more complex, and while many things are already automatic, the few manual things doesn't make the extension unusable.

I mean, no, it's not unusable. You could make it even harder to use and it will still be usable. The only problem is, it will be usable by fewer people...

Most of all, I've tried to keep this as near as chromeipass/passifox experience as possible.

Not trying to discount your efforts, but I'm saying the experience I had was far more painful than with chromeIPass, and this was both times I went through it (first with KeePass, now with KeePassXC).

You yourself admit it is only ever-so-slightly more secure.

Maybe I expressed this way too lightly. It's more secure, not just slightly.

I mean, you were correct the first time

What is the biggest trade-off in your opinion?

What is "the biggest" trade-off will probably very person-to-person. I'm not sure I can pin down a single one that applies to everyone. But I know that for me personally (even as a developer) the craziest part was the fact that the browser itself (not just the extension) felt the need to "know" about the password manager, and suddenly feels the liberty to launch or close the password manger at will. I use my password manager for half a dozen different things, and I close Chrome for so many reasons that don't include logging off. Chrome is not some privileged rose that gets to manage other things on my computer. It's just nuts, far more nuts than the extension writing a config file in Chrome's folder.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@phoerious if what you consider to be your intended average user has even heard of X11 (or Wayland, or Unix-domain sockets, or whatever), I rest my case...

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

You don't have to have heard of X11 to use the clipboard on Linux. I don't understand what one has to do with the other.
Besides, I haven't seen a single argument from your side where native messaging is the reason why something doesn't work and which couldn't be fixed without using something else.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@phoerious

You don't have to have heard of X11 to use the clipboard on Linux. I don't understand what one has to do with the other.

I guess I need to be more explicit here. What I was trying to say that if your average users are Linux users, they are not average users. Hence we go back to the earlier point I was making about the target audience not being the average user. You can make life as hard as you want on Linux because those users are used to this anyway, but on actual OSes that normal people use (i.e. Windows), an insecure protocol like X11 isn't a thing, so you don't need extra mitigations meant for X11.

Besides, I haven't seen a single argument from your side where native messaging is the reason why something doesn't work

I just wrote quite an extensive reply to @varjolintu's points, which as far as I could tell were consequences of him using native messaging. Did you not read them?

...and which couldn't be fixed without using something else.

Well yes, I didn't list such a thing because... did you not read the following?

To be explicit: by all means, if you can make native messaging seamless enough that it doesn't make a difference usability-wise, I have no complaint against it.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

@varjolintu To reply to your last comment:

Everyday users in general don't care if there are TCP sockets or native messaging underneath.

Right, I don't either. That's why I have written the following so many times now:

To be explicit: by all means, if you can make native messaging seamless enough that it doesn't make a difference usability-wise, I have no complaint against it.

For them it's just enough everything works. Do I assume correctly that this is the point of view where you find the extension unusable?

Yes... I think? Not sure if I understood your question.

If that category is not going to be the bulk of the users then those few additional steps in setup won't matter, right? Most of those people leave a review with words "It doesn't work" or you never hear anything from them. Other categories might be quite active writing issues and feedback, which I'm very thankful of.

I'm afraid I don't understand where you're going here...

@phoerious

This comment has been minimized.

phoerious commented Jun 9, 2018

Why do you make assumptions about our user base? About one third, if not more, of our users use some version of Linux. And even if it were only ten, if we provide a Linux version, we cannot suddenly make it less secure. We also don't say "sorry, but we don't support encryption for KDBX files on Linux, but that doesn't matter, because most people use Windows anyway."

I think I stop responding here. You made your case and I explained to you why native messaging is inherently more secure (and I didn't even mention that we got rid of a whole HTTP server library thanks to it) and not the problem you think it is. You say our browser integration isn't yet working smoothly, which I do not object to. But stop telling us that the reason for this is native messaging, because that is just not the case. We are working on fixing the remaining problems, but we will not revert to a less secure (and overall way more complex) HTTP interface. That is a final decision.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 9, 2018

if we provide a Linux version, we cannot suddenly make it less secure.

I find it sad and quite frustrating that you refuse to read what I actually write and instead fabricate your own visions of what I must have written. This is perhaps the 3rd or 4th time you are replying with complete disregard of what I actually just wrote in my preceding comment. I did NOT say you should make the Linux version less secure. I wrote the exact opposite, suggesting you can keep the X11 mitigations for Linux users without subjecting the Windows users to them needlessly. Here was my exact quote:

You can make life as hard as you want on Linux because those users are used to this anyway, but on actual OSes that normal people use (i.e. Windows), an insecure protocol like X11 isn't a thing, so you don't need extra mitigations meant for X11.

So yes, I think it makes sense for me to stop responding to you here as well.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 10, 2018

@varjolintu: FYI, it occurred to me that for local connections, a TCP server should be able to tell what the user ID of the client is. So preventing other users from connecting should be possible on the KeePass(XC) side even over TCP, if you have any influence on that side.

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 10, 2018

@mehrdadn I had to think twice to even reply because I'm still a little confused what the problem you are trying to describe actually is?:

  1. Native messaging is secure but bad for user experience
  2. We should've kept using TCP sockets
  3. KeePassXC-Browser is hard to setup (it isn't)
  4. All above and you have problems with the proxy crashing (I would've appreciated if you'd make a separate issue about this instead of this long thread)

First of all, it is completely unreasonable and counterintuitive for either the browser or the password manager to behave poorly if the user happens to want to start the password manager before the browser, or to close the browser before the password manager, or anything else implied by some kind of parent-child relationship between the browser and the password manager. This itself is far less likable than a config file written somewhere the user could care less about. And I dare say that no user (even a technical one who isn't familiar with Chrome internals) will expect or find it intuitive for Chrome to suddenly launch KeePassXC, if they don't find it outright frightening.

The only thing that has changed is that if user starts KeePassXC after the browser you have to press Reload for the connection to be established. This is going to be automatic in the later releases. And yes, KeePassXC-Browser actually starts KeePassXC if you disable the proxy, but that's because of native messaging. The whole idea is to create a secure pipe between the browser extension and an application. That's the way it works.

What is "the biggest" trade-off will probably very person-to-person. I'm not sure I can pin down a single one that applies to everyone. But I know that for me personally (even as a developer) the craziest part was the fact that the browser itself (not just the extension) felt the need to "know" about the password manager, and suddenly feels the liberty to launch or close the password manger at will. I use my password manager for half a dozen different things, and I close Chrome for so many reasons that don't include logging off. Chrome is not some privileged rose that gets to manage other things on my computer. It's just nuts, far more nuts than the extension writing a config file in Chrome's folder.

KeePassXC-Browser doesn't have the liberty of launching and closing you password manager, unless you tampered with the settings and you knew what you were doing (or not). The browser migration document clearly says:

You can also use KeePassXC without the proxy. That way you have direct communication between KeePassXC and your browser. The disadvantage is that your browsers starts a new KeePassXC instance when you connect. For that to work, you need to close any running KeePassXC instance before you click Connect.

If you have a problem with the proxy, make an issue about it.

FYI, it occurred to me that for local connections, a TCP server should be able to tell what the user ID of the client is. So preventing other users from connecting should be possible on the KeePass(XC) side even over TCP, if you have any influence on that side.

Still, the TCP socket is open for every user. Unix sockets are restricted to the current user.

And btw, is damn easy to read your clipboard with JavaScript or any other 3rd party software. If you are a developer you can verify this just by testing it yourself.

This thread is about security, but mainly your complaints are related to UX and bugs.

@rugk

This comment has been minimized.

rugk commented Jun 10, 2018

I also think the issues described are clearly usability things and do not belong to this issue. Better open a new issue for it and try out the proxy before. It seems as if the proxy was not used in the complains.

@mehrdadn

This comment has been minimized.

mehrdadn commented Jun 10, 2018

@varjolintu

Everything I've been complaining about here has been a result of experiencing bad UX, which as far as I could tell is caused by native messaging. TCP servers, etc. were only relevant as potential solutions to that due to the nicer UX I found of them, but that was all.

The security relevance was that I was arguing that the improvement to security was marginal or nonexistent. (My stance changed in the process of writing this reply, though... please read all of this reply until the end.)

And yes, KeePassXC-Browser actually starts KeePassXC if you disable the proxy, but that's because of native messaging. That's the way it works.

..."but"? It's not "but"; that was exactly the point. If using native messaging resulted in my garage door opening, you wouldn't reply "Yes, my extension actually turns on your car in the garage if you disable the proxy, but because of native messaging. That's the way it works."... that wouldn't make any logical sense. Yes, I get that that that's how it works... that's exactly why I was complaining about it! If it worked differently then I wouldn't need to be complaining about it.

And btw, is damn easy to read your clipboard with JavaScript or any other 3rd party software. If you are a developer you can verify this just by testing it yourself.

OK, this is interesting. This made me have to look into what's possible in a browser. And now that I have, I see you're correct that there is a threat here. But, funny enough, it's not what you mentioned. (!)

First, let's get 3rd party software out of the way. It's irrelevant. Remember I've been talking about Windows the entire time. (I'm not saying anything about Linux because for all I care you could keep it as-is.) On Windows, you cannot read another user's clipboard, because the sessions and desktops are separate. And if you have malicious 3rd party software running in your own account, the game is already over.

Now we get to JS on a malicious website that just happens to be loaded when you're trying to connect the extension. But even on that front, reading the clipboard is only a problem if you go back to using symmetric ciphers like chromeIPass did, which I never suggested. If you're doing it properly (say, DH key-exchange, or whatever) you could make potential eavesdropping on the clipboard irrelevant.

However, thinking about this leads us to the actual threats, which are different from what you mentioned and which I also admittedly (until now) did not realize at the time of my writing were possible from within the browser:

  1. Randomly writing to the clipboard, and
  2. Potentially being able to access services on localhost from within the browser.

The combination of the two could let a man-in-the-middle attack happen from within a browser, which is a possible attack. So I see that this is one actual threat that is mitigated by the allowed_origins policy of native messaging, which is finally actually somewhat compelling...

@rugk

This comment has been minimized.

rugk commented Jun 10, 2018

All you describe are are not security issues and solely usability issues. Please open a new issue. Also all threats you describe (clipboard, other user' access) are not a problem, because they actually are solved by native messaging. And BTW, there are no differences regarding this between the OSes

@varjolintu

This comment has been minimized.

Owner

varjolintu commented Jun 10, 2018

@mehrdadn

..."but"? It's not "but"; that was exactly the point. If using native messaging resulted in my garage door opening, you wouldn't reply "Yes, my extension actually turns on your car in the garage if you disable the proxy, but because of native messaging. That's the way it works."... that wouldn't make any logical sense. Yes, I get that that that's how it works... that's exactly why I was complaining about it! If it worked differently then I wouldn't need to be complaining about it.

Then the complaints should be forwarded to Google and Mozilla. The logic is that native messaging opens a direct STDIN/STDOUT pipe to an application outside a browser. In this case it's the keepassxc-proxy which handles the traffic to and from KeePassXC. If you have an alternative approach or improvement suggestions to the proxy method, file an issue or a feature request.

OK, this is interesting. This made me have to look into what's possible in a browser. And now that I have, I see you're correct that there is a threat here. But, funny enough, it's not what you mentioned. (!)

I didn't, until you brought up a possibility to copy/paste a key. I thought it's common knowledge that clipboard is not safe.

I think we are done. If you are unhappy with the UX or you have bugs, file an issue. Continue on this thread if you have something to say against the security of native messaging and Unix sockets, or the way we are implementing them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.