protect against mitm attacks

[totaam]

Originally recorded in #197 (only mildly related):
*1) Wait until a new client tries to connect and intercept
*2) Connect your own client to the server, 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]

I don't want to go down the route of ssh and having to verify host fingerprints and such, so here are a few better options that rely solely on the shared secret:

• EAP-PSK
• EAP-EKE
• SPEKE

In the meantime, this is seriously mitigated by r1981 which encrypts all traffic except for the initial hello packet. A mitm attacker can still intercept and forward all traffic between the client and server, but since the AES key is derived from the secret, the attacker has no way of decrypting the traffic, and no way of modifying it either.

[totaam]

2012-10-13 15:21:26: lindi commented

---

How about just using SSL?

[totaam]

meh - I don't really like ssl, the whole certificate business is complicated, it would take some effort to do this right and to document how to set things up properly.

I like SPEKE: "Unlike unauthenticated Diffie-Hellman, SPEKE prevents man in the middle attack by the incorporation of the password. An attacker who is able to read and modify all messages between Alice and Bob cannot learn the shared key K and cannot make more than one guess for the password in each interaction with a party that knows it."

It's simple and effective.

We're not far off that, modifying our code to accommodate it shouldn't be too hard.

[totaam]

2012-10-13 15:44:27: lindi commented

---

I'm bit worried that if you try to create your own transport method you are going to end up implementing a lot of bugs.

[totaam]

2012-10-13 16:01:54: lindi commented

---

Maybe you have read this already but if not it is a good read:

http://web.archive.org/web/20110620022453/http://chargen.matasano.com/chargen/2009/7/22/if-youre-typing-the-letters-a-e-s-into-your-code-youre-doing.html

I have not even looked at how you do padding in your code :)

[totaam]

Great read thanks. We're safe from padding oracle, and none of the data is under user control (although as discussed previously, the key's salt is), I chose zero-byte-padding for simplicity and also because we don't need any of the features of PKCS and friends.

The good thing is that we don't have to do anything with the transport at all, unlike EAP, SPEKE relies on just one random integer sent across from both ends.
(it really is beautifully simple)

Shame that there is nothing in python for doing AES CCM Mode.
Some more useful pointers:

• Lessons learned and misconceptions regarding encryption and cryptology
• Authenticated encryption

[totaam]

2012-10-14 12:18:18: antoine commented

---

SPEKE is pretty awesome, and more importantly we know it is safe:

#!/usr/bin/env python

import hashlib
from Crypto.Util.number import getStrongPrime, bytes_to_long
from Crypto.Random import random

def test_SPEKE(*args):
	password = "secret"
	p = getStrongPrime(768)
	h = hashlib.sha1()
	h.update(password)
	print("hash(%s)=%s=%s" % (password, h.hexdigest(), bytes_to_long(h.digest())))
	sq = bytes_to_long(h.digest())**2
	print("hash^2=%s" % sq)
	g = sq % p
	print("g=%s" % g)
	ga = 0
	MIN_I = 2**10
	MAX_I = 2**15
	while ga<=2 or ga>=p-2:
		a = random.randint(MIN_I, MAX_I)
		print("a=%s" % a)
		ga = (g**a) % p
		print("(g**a)%%p=%s" % ga)
	gb = 0
	while gb<=2 or gb>=p-2:
		b = random.randint(MIN_I, MAX_I)
		print("b=%s" % b)
		gb = (g**b) % p
		print("(g**b)%%p=%s" % gb)
	ak = ((gb % p)**a) % p
	print("aK=%s" % ak)
	bk = ((ga % p)**b) % p
	print("bK=%s" % bk)
        assert ak==bk

def main():
	test_SPEKE()


if __name__ == "__main__":
	main()

Note: still need to copy the elgamal code to use a "SafePrime" and not a "StrongPrime", and maybe use password stretching? (not sure it would make any difference here as the hash value is still deterministic from the input and that is all we use it for)

[totaam]

2012-10-16 14:35:57: lindi commented

---

Good luck :)

[totaam]

2012-10-21 04:40:20: mvrable commented

---

I just recently started looking at Xpra, and like many aspects of it. But the current attempts to implement cryptography worry me. I'd like to repeat a concern that lindi has expressed: that implementing your own secure transport layer is difficult; even experts make mistakes and it's very easy to end up with something with security problems.

Unfortunately, some of what I've seen so far in the code doesn't inspire confidence. Among the issues thus far:

• "since the AES key is derived from the secret, the attacker has no way
  of decrypting the traffic, and no way of modifying it either": not
  true as the attacker can still change data (without decrypting it) and
  replay old traffic. You seem to have realized this (you're talking
  about authenticated encryption now), but I haven't seen what design
  you intend to use. Probably from Python, the easiest approach is to
  add a MAC to each of the packets (be sure to encrypt then MAC, not
  some other order), and the MAC should also include a counter to
  protect against replay attacks.

• In the current client_base.py code, using UUIDs to generate salt and IVs
  isn't a good idea. UUIDs have some fixed content (they aren't
  completely random), and by using hexadecimal strings you're cutting the
  number of random bits in half. The Cyrpto.Random module can already
  give you cryptographically-strong random data, so you should be using
  that instead.

• The padding used in protocol.py is I think non-standard. You could
  instead use CTR mode for the cipher, which doesn't require any padding
  at all (and is secure as long as you are very careful to never repeat an
  IV).

• If you do choose to use something like SPEKE (which I'm not familiar
  with, though at first glance it seems reasonable), I hope you're
  planning to improve the code snippet you posted:

  • No need to randomly generate a prime number each time to define the
    group. Diffie-Hellman key exchange works just fine with a well-known
    shared prime number. There are some standard ones you could use, or
    "openssl dhparam" can generate a safe value, and then you can
    hard-code it in the Xpra source. I'd probably go larger than 768
    bits.
  • Picking a random value between 2^10^ and 2^15^ is a serious problem
    (it's easy for an attacker to exhaustively check all those values).
    You want to pick random values from the entire allowable range.
  • The implementation is very inefficient, computing g^a^ and only
    reducing modulo p at the end. Take a look at the Python pow()
    builtin. (But even this shouldn't be used carelessly for serious
    cryptography, since it doesn't take care to avoid timing or other
    side-channel attacks. It might be safe here, but I'd have to think to
    be more certain.)

I'll need to think over the initial handshake in more detail; not too much point in commenting on the current design now as it looks likely to change in the near future.

I may see if I can implement something better and if so send a patch, but if you make further changes yourself I'd at least advise waiting to release a version with an encrypted connection until the design can be reviewed in more detail.

[totaam]

2012-10-21 05:22:37: antoine commented

---

Please bear in mind that we will not release anything until SPEKE key exchange (or something like it) is implemented.
Nonetheless, let me try to reply to your queries:

(...) not true as the attacker can still change data (without decrypting it) and replay old traffic
If you change data the packet becomes invalid and the connection is dropped. You only get one attempt. Or am I missing something?
(and we are talking about SPEKE-like key exchange setup, right?)

(...) but I haven't seen what design you intend to use
I don't have pretty diagrams ;)
But the general idea is to hook something based on the PoC code into the handshake code.

(...) and the MAC should also include a counter to protect against replay attacks
With CBC mode, I fail to see how this provides any benefits: the replayed packets would be invalid and the connection dropped. And since we are trying to prevent both eavesdropping and password stealing, if you can modify traffic you can already get the connection to drop. Nothing gained here?

(...) using UUIDs to generate salt and IVs isn't a good idea
Generally no, but these values are public anyway, so although we should try make them stronger, I don't think that there is an inherently big risk here: salts don't matter much as long as they are there. (but I will change this to Crypto.Random when I get around to this ticket, hex strings were just easier to pass around multi-language code - thanks for reminding me)

The padding used in protocol.py is I think non-standard. You could instead use CTR mode for the cipher, which doesn't require any padding at all (and is secure as long as you are very careful to never repeat an IV)
I'm pretty sure null padding is standard, I will check. CTR mode is harder to get right, and is IMHO not worth the effort - although it would save an average of 16 bytes per packet (security vs space).

I hope you're planning to improve the code snippet you posted
It took me about 10 minutes to write as a PoC, so definitely...

No need to randomly generate a prime number each time (...)
Thanks, I might do that, although I really don't like "hardcoding" any kind of value. I'll have to think about that (comments welcome) as there are pros and cons: hardcoding makes the code easier, but should any crypto vulnerability rely on pre-calculating tables with known primes this could also make it more vulnerable. On the other hand, letting the server choose the prime number could also expose the clients to more attacks. Not an easy one.

Picking a random value between 2^10^ and 2^15^ is a serious problem (it's easy for an attacker to exhaustively check all those values). You want to pick random values from the entire allowable range.
Again, this was a 10min PoC sample code, but I'm not sure about this one:

• the range discarded is very small comparatively (1/2^5^) so there is almost nothing lost by excluding small values and small values make the calculations far less costly for an attacker (not that there are any known attacks - but still),
• using very large values becomes onerous (noticeably), and apparently for no benefit
  Ideas?

The implementation is very inefficient (...)
Again, PoC code generally is.

(...) Python pow() builtin. (But even this shouldn't be used carelessly for serious cryptography, since it doesn't take care to avoid timing or other side-channel attacks. It might be safe here, but I'd have to think to be more certain)
Yes, that's one of the beautiful things about SPEKE that I liked when I first saw it: A and B are public, so the time it takes to calculate power() does not really matter when it comes to timing attacks. (although as mentioned above, I would be weary of using really low cheap/easy values still)

I'll need to think over the initial handshake in more detail; not too much point in commenting on the current design now as it looks likely to change in the near future
Yes, I chose SPEKE because it fits very well with the handshake we currently have. I don't think many things will need to change there. (and even less if the prime number is fixed)

I may see if I can implement something better and if so send a patch
Please do - I am currently on holiday

but if you make further changes yourself I'd at least advise waiting to release a version with an encrypted connection until the design can be reviewed in more detail
We generally stick to a monthly release cycle (more or less), and this is just the beginning of the new cycle. It would not be unusual to ship code that is disabled until properly reviewed and tested (and sometimes dropped completely).

[totaam]

2012-11-08 00:20:27: mvrable uploaded file xpra.patch (31.4 KiB)

Proposed new encryption/authentication protocol

[totaam]

2012-11-08 00:25:16: mvrable commented

---

I just attached a patch to this ticket (xpra.patch), with a prototype for a new key exchange and encryption protocol for Xpra. This isn't quite ready to be checked in, but has most of the basics there.

• I have some problem with the initial handshake still, I think, because in testing the connection authenticates but then the client disconnects with the error message "xpra: Fatal IO error 2 (No such file or directory) on X server :0." I'm still debugging.

• Some code still needs a bit more cleanup.

• I'd like to add authentication for the initial packets in the exchange: basically, compute a hash of all data sent before the connection switches over to being encrypted, then communicate that hash when the connection is secure. That will prevent a man-in-the-middle from tampering with the initial handshake (without the tampering being evident).

• I'd like to revisit the key exchange. Right now it's a simple Diffie-Hellman key exchange with a little bit of authentication, but this definitely leaks some information about the password and isn't based on an existing protocol. Using something based on EAP-EKE, SPEKE, or some existing protocol would be better. (This should be easy to do within the framework, just hasn't been done yet.)

[totaam]

2012-11-15 03:39:56: antoine commented

---

Good read: Introduction to Cryptography from Stanford Online.

[totaam]

2013-02-11 16:31:04: antoine commented

---

Now fairly high on the TODO list now that 0.9 dev work starts.

[totaam]

2013-03-20 14:23:05: antoine commented

---

Ouch, still not done - definitely for 0.10 then! For sure this time.

[totaam]

2013-05-16 14:27:52: antoine commented

---

OK, finally had a look... sorry for taking so long.

• I like the separation between crypto code and network, will keep that no matter what
• I'm not sure about the dedicated packet type for key-exchange (which should block the hello until complete as per TODO item) - might be easier to overload the hello packet as per the current password code (it might make it easier to fail gracefully with a meaningful error message) - actually I'm not sure, maybe this is ok too, I will have to try both to see
• it would be nice to split mac from the encryption step, or at least be able to choose the mac algo - no biggie, could be done later too
• encrypt then/and/before mac: while encrypt-then-mac is theoretically better, it is also somewhat harder to get right. meh, I don't think it is worth the hassle of trying to support anything else, is it? in any case:
• the whole key exchange is there for doing things like DH, which is insecure... whereas authenticated encryption, like SPEKE and others like GCM are secure and do not need any key exchange step at all. What bothers me is the level of complexity in adding a key exchange which does not add value. I quote:
  The need for Authenticated Encryption emerged from observation that securely compositing a confidentiality mode with an authentication mode could be error prone and difficult.[1][2] This was confirmed by a number of practical attacks has been introduced into production protocols and applications by incorrect implementation, or lack of, authentication (including SSL/TLS).[3]
  In summary: I'm just really not keen on key exchange since it can be avoided.
  I do understand that some algos may be patented in the USA, but GCM is not, so maybe that's a good way forward?

Edit PS: aes in gcm mode in python

[totaam]

2013-05-20 05:27:15: mvrable commented

---

Replying to [comment:16 antoine]:

OK, finally had a look... sorry for taking so long.

I have also been meaning for quite some time to go back and work on my patches a bit more; apologies that I have been busy and haven't gotten to that.

A few other comments:

• I'm not sure about the dedicated packet type for key-exchange (which should block the hello until complete as per TODO item) - might be easier to overload the hello packet as per the current password code (it might make it easier to fail gracefully with a meaningful error message) - actually I'm not sure, maybe this is ok too, I will have to try both to see

I'll have to go back to look at exactly why I set this up this way. One thing I was trying to support was key exchange protocols that might take multiple round trips

• it would be nice to split mac from the encryption step, or at least be able to choose the mac algo - no biggie, could be done later too

I have no problem with letting the MAC be chosen separately from the encryption algorithm.

• encrypt then/and/before mac: while encrypt-then-mac is theoretically better, it is also somewhat harder to get right. meh, I don't think it is worth the hassle of trying to support anything else, is it? in any case:

I don't feel like encrypt-then-mac is much more difficult, and we should only support one variant so I'd go with that.

In summary: I'm just really not keen on key exchange since it can be avoided.

You need some method to establish the key used to encrypt and MAC. It's best if this is a session key (used for a single connection only) since this avoids some potential attacks (that could occur if the same key was used in multiple sessions). The session key could be derived from some master key (based on a shared password) and per-session nonce values. The advantage of a DH-like key exchange is that it provides forward secrecy (if the master key/password is revealed, that wouldn't allow an attacker who had recorded network traffic to decrypt the past traffic). DH is not necessary if you give up forward secrecy.

Whatever is done, I would again strongly recommend using a session-specific key for encryption, once the handshaking is all done.

I do understand that some algos may be patented in the USA, but GCM is not, so maybe that's a good way forward?

AES-GCM is essentially AES-CTR for encryption combined with a MAC that can be optimized very well given appropriate hardware support. But, since I don't think we have a native AES-GCM implementation in Python, I would still suggest separating the encrypt and MAC steps that have Python support. My personal vote would be for something like in the current patch: AES-CTR encryption combined HMAC-SHA-256. There should be no patent problems with any of the above.

[totaam]

2013-07-11 17:52:16: antoine commented

---

Clarification: re-reading comment:17, I'm not against DH per se, or at least not variants like ECDH... just plain DH. And as long as the mechanism is "DH-like" (like you said) then I have not problem with it.
[[BR]]
Even though I still think that the "forward secrecy" is not a very serious issue for our use case: getting access to the session passwords plus the full network capture is a tall order - reaching this goal and being able to see the session being replayed would not be the most interesting thing for the attacker IMO.

[[BR]]
FYI: 0.10 is getting nearer, so this looks like getting delayed again..

[totaam]

2013-09-13 06:41:19: antoine commented

---

I really really want to get something done about this ticket for this milestone, so raising the priority.
I think we want to try something incremental, probably starting with a better separation of the network code proper and the encryption layer and the consumers. (a bit like what was done in the patch attached to this ticket)
The number one thing to implement would have been MAC... except GCM makes this redundant.
OTOH the multiple round-trips for establishing session keys may be getting in the way of things like #426 (though making it all part of the "hello" packet might help)

Also, here's a good (general information) pointer/review: State of encryption from Kurt Roeckx's journal

[totaam]

2014-05-29 16:54:10: gschwind commented

---

Added a ticket #584 to discuss the API for new crypto modules, an Issue raised by discussions within this ticket.

[totaam]

FYI: the recent changes in trunk make the protocol class much easier to work with: the compression and encoding aspects have been moved to separate classes. The same should be done with the crypto bits, I have only moved the bare minimum so far.

[totaam]

Too big a job, not a priority.
If you are worried about security issues, use SSH mode.

[totaam]

2015-10-16 03:34:56: antoine commented

---

Like I said:

No need to randomly generate a prime number each time (...)
Thanks, I might do that, although I really don't like "hardcoding" any kind of value. I'll have to think about that (comments welcome) as there are pros and cons: hardcoding makes the code easier, but should any crypto vulnerability rely on pre-calculating tables with known primes this could also make it more vulnerable. On the other hand, letting the server choose the prime number could also expose the clients to more attacks. Not an easy one.
[[BR]]
And the NSA and others are doing exactly that:
https://weakdh.org/imperfect-forward-secrecy.pdf

[totaam]

2015-11-14 13:35:48: antoine commented

---

We have a winner, including python code: [https://en.wikipedia.org/wiki/Secure_Remote_Password_protocol]

And we can even use this in the HTML5 client: [https://code.google.com/p/srp-js/]

[totaam]

SSL will do: #1252. See wiki Encryption for details.

Note: if you can send your AES key to each end securely, the AES option is still probably still safer for most: figuring out how to configure SSL properly is hard.