Xpra: Ticket #198: protect against mitm attacks

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



Sat, 13 Oct 2012 13:50:56 GMT - Antoine Martin: status, summary changed

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:

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.


Sat, 13 Oct 2012 14:21:25 GMT - Timo Juhani Lindfors:

How about just using SSL?


Sat, 13 Oct 2012 14:32:05 GMT - Antoine Martin:

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.


Sat, 13 Oct 2012 14:44:26 GMT - Timo Juhani Lindfors:

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.


Sat, 13 Oct 2012 15:01:53 GMT - Timo Juhani Lindfors:

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 :)


Sat, 13 Oct 2012 16:48:04 GMT - Antoine Martin:

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:


Sun, 14 Oct 2012 11:18:18 GMT - Antoine Martin:

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)


Tue, 16 Oct 2012 13:35:56 GMT - Timo Juhani Lindfors:

Good luck :)


Sun, 21 Oct 2012 03:40:20 GMT - mvrable:

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:

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.


Sun, 21 Oct 2012 04:22:36 GMT - Antoine Martin:

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 210 and 215 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:

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).


Thu, 08 Nov 2012 00:20:27 GMT - mvrable: attachment set

Proposed new encryption/authentication protocol


Thu, 08 Nov 2012 00:25:16 GMT - mvrable:

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.


Thu, 15 Nov 2012 03:39:55 GMT - Antoine Martin:

Good read: Introduction to Cryptography from Stanford Online.


Mon, 11 Feb 2013 16:31:04 GMT - Antoine Martin: milestone changed

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


Wed, 20 Mar 2013 14:23:05 GMT - Antoine Martin:

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


Wed, 20 Mar 2013 14:47:14 GMT - Antoine Martin: milestone changed


Thu, 16 May 2013 13:27:52 GMT - Antoine Martin:

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

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


Mon, 20 May 2013 04:27:14 GMT - mvrable:

Replying to 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'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

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

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.


Thu, 11 Jul 2013 16:52:16 GMT - Antoine Martin:

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.
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.


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


Tue, 16 Jul 2013 05:40:56 GMT - Antoine Martin: status, milestone changed


Fri, 13 Sep 2013 05:41:19 GMT - Antoine Martin: priority changed

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: Kurt Roeckx's journal


Mon, 20 Jan 2014 11:48:58 GMT - Antoine Martin: owner, status, milestone changed

Still not done - grr, re-scheduling again.


Mon, 03 Mar 2014 15:08:01 GMT - Antoine Martin: milestone changed

And again.


Sat, 17 May 2014 11:19:57 GMT - Antoine Martin: milestone changed

And again.


Thu, 29 May 2014 15:54:09 GMT - Benoit Gschwind:

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


Tue, 29 Jul 2014 10:48:01 GMT - Antoine Martin:

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.


Wed, 22 Apr 2015 04:01:55 GMT - Antoine Martin: milestone changed

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


Fri, 16 Oct 2015 02:34:55 GMT - Antoine Martin:

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.


And the NSA and others are doing exactly that: https://weakdh.org/imperfect-forward-secrecy.pdf


Sat, 14 Nov 2015 13:35:48 GMT - Antoine Martin: milestone changed

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/


Wed, 16 Mar 2016 05:20:46 GMT - Antoine Martin: milestone changed


Tue, 12 Jul 2016 16:52:22 GMT - Antoine Martin: milestone changed

Milestone renamed


Tue, 02 Aug 2016 10:19:06 GMT - Antoine Martin: status changed; resolution set

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.


Sat, 23 Jan 2021 04:48:12 GMT - migration script:

this ticket has been moved to: https://github.com/Xpra-org/xpra/issues/198