Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

packet encryption for tcp sockets #197

Closed
totaam opened this issue Oct 10, 2012 · 23 comments
Closed

packet encryption for tcp sockets #197

totaam opened this issue Oct 10, 2012 · 23 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Oct 10, 2012

If someone wants to use a password to protect a session exposed with "--bind-tcp=", chances are do not want to be snooped upon either.

We should use a block cipher with the same password and encrypt all traffic (quite cheap).

Here is a good tutorial: Symmetric Encryption with PyCrypto

The cost will be:

  • padding on each packet (padded to 32 bytes for AES): so on average 16 bytes per packet
  • CPU time spent encrypting/decrypting stuff: low I think, can easily run automated tests to verify

The main difficulty is that our packet header sends the data size and now we will have a padded size and actual size...

@totaam
Copy link
Collaborator Author

totaam commented Oct 10, 2012

2012-10-10 20:50:26: lindi commented


Hmm. If you only use AES then it is easy to do man-in-the-middle attacks or to corrupt the data. To combat against man-in-the-middle you need some sort of public key infrastructure and to combat against corruption you need some sort of MAC.

@totaam
Copy link
Collaborator Author

totaam commented Oct 11, 2012

  • we do have HMAC already, that's why this encryption mode will only be enabled with "use a password to protect a session"
  • this makes MiM attacks pointless since all you're going to get is a broken packet

@totaam
Copy link
Collaborator Author

totaam commented Oct 11, 2012

done in r1907 - see changeset log message for details

@totaam totaam closed this as completed Oct 11, 2012
@totaam
Copy link
Collaborator Author

totaam commented Oct 12, 2012

2012-10-12 15:46:48: lindi commented


I can still mitm:

  1. Wait until a new client tries to connect
  2. Connect your own client to the server but without "cipher" set, obtain "challenge" from server
  3. Feed "challenge" to the original client, wait for "challenge_response"
  4. Feed "challenge_response" to the server and allow your own client to interact with the server
  5. Drop the original client

@totaam
Copy link
Collaborator Author

totaam commented Oct 12, 2012

No you cannot, the salt is per client now, which means that one client's challenge is irrelevant to anyone else.

@totaam
Copy link
Collaborator Author

totaam commented Oct 12, 2012

2012-10-12 16:05:39: lindi commented


Hmm, do you mean "cipher.key_salt" by "the salt"? If yes, this should not be a problem since the attacker can just disable encryption in his client completely? I don't see a way to force the server to only allow encrypted connections.

@totaam
Copy link
Collaborator Author

totaam commented Oct 12, 2012

By salt I mean the salt used for the initial HMAC password verification step.

The cipher salt is not very important and can be made public since it isn't the "secret" used with AES (the password is - after stretching), it just has to be random: it is only there to prevent rainbow tables attacks.

There is no way to force clients to use encrypted connections, but I don't see how that helps in any way since you still have to get past the password authentication - or am I missing something? To only allow encrypted connections you would need a pre-arranged secret (cert, passphrase, whatnot) - which we do not have.

Edit: And just to clarify: once a client has requested encryption, any non-encrypted packets at either end will cause a disconnection (this is covered in the changelog message)

@totaam
Copy link
Collaborator Author

totaam commented Oct 12, 2012

2012-10-12 21:03:17: lindi commented


In my attack you connect to the server only once and thus obtain only one "challenge". You can then mimic a real server and trick a real client into providing you the right "challenge_response".

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

Hah, so this has nothing to do with AES at all, this is just plain mitm against password auth, with or without encryption makes no difference.
In this case the description in comment:6 is correct, but this is the wrong ticket for it, i've created #198 for this.

The only way to defeat that is with a "hostkey" ala ssh, so the client can be certain it is talking to

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

2012-10-13 08:36:56: lindi commented


Well it is related to encryption support since people might think that they are safe if they always use "--encryption=AES" in their clients. This attack shows that you can mitm and open an unencrypted connection to the server.

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

OK, so it is related but only for people who make the wrong assumptions.. (I can include myself in that since I was mistaken in comment:4 as I assumed the comment was related to AES when in fact it was related to HMAC password auth)

So, to clarify, this ticket (as per the original ticket text) prevents snooping: anyone eavesdropping on the connection between a client and server cannot decrypt the data.
It does not prevent mitm, which will be addressed in #198

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

2012-10-13 13:52:10: lindi commented


I think the encryption part has at least one more issue. Since you use PBKDF2 to generate the key and the client can choose the salt you can easily build a rainbow table that for example decrypts all xpra traffic that uses 8-character alphanumeric passwords. Since there is no forward secrecy you can start collecting data today and then decrypt the communications once your table is ready.

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

Lindi - thanks for taking the time to look into this.

I understand what you're saying, except for the part that says "Since you use PBKDF2 to generate the key and the client can choose the salt". Isn't the problem here the fact that the salt can be intercepted, not that the client can choose it? I mean, the clients do choose safe enough random salts, the problem here is that an attacker can see what the value of the salt is/was, right? What good would it do to an attacker to re-use a previously used salt?

For the record, this is what happens now (as of r1981 - see #198):

  • client sends an unencrypted hello with its encryption request (iv, salt)
  • server responds with a challenge message encrypted with the generated key, this packet includes the server's own encryption request (iv, salt)
  • client sends the challenge response encrypted with the server's key, requesting a new encryption (new iv, new salt)
  • server responds with hello, requesting a new encryption (new iv, new salt)

The fact that we change the key does not really help against rainbow tables since after cracking the first key the attacker will then know what the password was, it can generate the new key easily enough (since it will be able to decrypt the new salt and iv)

Now, I do understand your concern about offline rainbow tables, but PBKDF2 with 1000 iterations is relatively expensive, and then you still have to test each key until you find one that works (by trying to decrypt a sample packet then parsing it), is it really worth worrying about? I am tempted to just make a note of it in the man page: use strong passwords... After all it is a --password-file switch, and winswitch uses truly random 16 byte values in there.

I can't think of anything else we can do, can you? (well apart from implementing the algorithms mentioned in #198 that is)

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

2012-10-13 15:12:56: lindi commented


I think you got my point but just to be clear: The attacker can connect to the server and send an "insecure" salt for which the attacker has precomputed a rainbow table. The rainbow table has direct mapping from encrypted data block back to the password so there is no need to parse anything.

This of course assumes that the attacker can predict the plaintext of the first encrypted block. Here it seems to be

   50 01 00 00 00 00 04 38  c2 85 68 65 6c 6c 6f 3c P......8 ..hello<

which looks pretty predictable to me.

@totaam
Copy link
Collaborator Author

totaam commented Oct 13, 2012

Gotcha, so in this case, we can do this: avoid using encryption in step-2 above, we send a random salt to the client with the encryption request, then we combine this salt with the one provided by the client to make the server key. The client has no way of pre-calculating since it won't know the actual salt used until the connection is made. Right?

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

2012-10-14 11:10:12: lindi commented


How do you make sure that the mitm attacker can't just change the salt to some fixed value?

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

I'm not following you... the mitm can change the salt, but since this salt is combined with the salt from the other end (be it client or server), and obviously not with a dumb xor.. then he has no way of knowing the actual salt used until the connection is made, hence no way of using any precomputed rainbow tables.

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

2012-10-14 11:45:04: lindi commented


I think I'd need to see a proper protocol specification to understand your idea :/

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

2012-10-14 11:56:33: lindi commented


But basically: you need to send the salt before you can do encryption "securely" but you also need to setup "secure" encryption before you can safely send the salt. I don't see how you can make this work.

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

No you don't. The encryption uses the secret, and both salts.
Now an attacker can choose just one of those, but that won't help him precompute anything.

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

In any case, this is looking more and more like a home made crypto algo, which is a recipe for disaster, and as it happens SPEKE (#198) is actually very easy to implement, and it fits very well within the current initial packet exchange format, so I will just drop all this and replace it with SPEKE: the initial packets do the key exchange, each end can verify the other has the password and we use an AES secret key based on that (the SPEKE derived 'K').

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

2012-10-14 13:12:02: lindi commented


I have never used SPEKE so it's hard to comment. I guess I'll try to look at it later but probably only after you have actually released a version of xpra that uses it. It's kind of frustrating that you change the algorithm just when I have spent so much time analyzing it :)

@totaam
Copy link
Collaborator Author

totaam commented Oct 14, 2012

Sorry, it's not all wasted anyway because there are many similarities, the only major change is how the key is derived.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant