strengthen encryption layer

[totaam]

Issue migrated from trac ticket # 951

component: network | priority: major | resolution: fixed

2015-08-18 11:14:55: antoine created the issue

---

Without going as far as implementing #198, there are things we should be doing already:

• bug: when using the password-as-encryption-key fallback mode instead of the encryption keyfile option, we allow clients to connect with encryption turned off (because of the way we check for encryption)
• the first packet we send usually enables zlib + bencode (for maximum compatibility) without any encryption, we could encrypt it using pre-defined salt and iv - this would be better than plain text, but may leak something about the key.. it would also not be backwards compatible with older servers, we could add a flag:

• mode=strict : always encrypt
• mode=legacy : as we do now
• mode=lax : encrypt if client requests it (this one is optional, not much going for it)

• the hello packet we send is predictable because it always contains (more or less) the same data, needs a salt (ie: simply adding the current time would help)

[totaam]

2015-08-18 11:21:50: antoine changed status from new to assigned

[totaam]

2015-08-18 11:21:50: antoine commented

---

r10336 makes it at least possible to encrypt the first hello packet, controlled by the env var XPRA_ENCRYPT_FIRST_PACKET=1.
The IV and salt can also be changed using env vars.

The statement in the ticket description about needing more salt may be wrong: when we use encryption (which is when we care about salt), we already send 'iv' and 'salt' which are random.

See also: #933#comment:5

[totaam]

2015-11-09 17:14:38: antoine changed status from assigned to closed

[totaam]

2015-11-09 17:14:38: antoine set resolution to fixed

[totaam]

2015-11-09 17:14:38: antoine commented

---

Encrypting the first packet is not particularly useful: there is little to hide in that packet, especially when also using authentication (we skip a lot of data in that case since we will be sending the second hello with the challenge response), and in fact it may be a bad idea: if we somehow end up repeatedly encrypting with the pre-defined IV, we could expose enough information to help break the key.

See AES encryption of files in Python with PyCrypto: in A word about the initialization vector: it suffices to say that the IV is as important as the salt in hashed passwords, and the lack of correct IV usage led to the cracking of the WEP encryption for wireless LAN.

I have also reviewed our use of salt and iv: we use uuid.uuid4, which is fine for this purpose.
See Answer to: When are you truly forced to use UUID as part of the design?: Version 4 UUIDs are essentially just 16 bytes of randomness pulled from a cryptographically secure random number generator, with some bit-twiddling to identify the UUID version and variant and Frankly, in a single application space without malicious actors, the extinction of all life on earth will occur long before you have a collision, even on a version 4 UUID, even if you're generating quite a few UUIDs per second.

I think this is good enough for now and we should NOT be encrypting the first packet.

See also: #876, #1029