Wednesday, July 10, 2013

Synergy: Integer Overflow, Key Reuse, IV Reuse

Synergy is a software that lets you share your mouse and keyboard between multiple computers. Until recently, it didn't have any support for encryption, which left users' keystrokes and mouse movements vulnerable to sniffing on the local network. Users worked around this limitation by tunneling Synergy's communications through a VPN or SSH. Lots of users complained, and the Synergy team decided to encrypt the connections. Instead of using a well-known protocol like TLS, they invented their own.

The most obvious problem is that they are re-using the same key and IV for encrypted communications in both directions. This is absolutely fatal for modes like CTR, OFB, GCM, and probably CFB, which are the ones Synergy supports. Update: A tool that exploits this vulnerability is available here.

Second, they derive the IV from the password, and because of an integer overflow bug, when the length of the password is congruent to its double mod 256, the key and the IV are the same. If the IV were the same size as the output of the hash function they're using (SHA256), you'd be able to recover the key for all passwords longer than 128 characters.

 if (!options.m_pass.empty()) {  
     createKey(m_key, options.m_pass, kKeyLength, static_cast<UInt8>(options.m_pass.length()));  
     byte iv[CRYPTO_IV_SIZE];  
     createKey(iv, options.m_pass, CRYPTO_IV_SIZE, static_cast<UInt8>(options.m_pass.length()) * 2);  
     setEncryptIv(iv);  
     setDecryptIv(iv);  
  }  

 void  
 CCryptoStream::createKey(byte* out, const CString& password, UInt8 keyLength, UInt8 hashCount)  
 {  
      assert(keyLength <= SHA256::DIGESTSIZE);  
      byte temp[SHA256::DIGESTSIZE];  
      byte* in = reinterpret_cast<byte*>(const_cast<char*>(password.c_str()));  
      SHA256().CalculateDigest(temp, in, password.length());  
      byte* tempKey = new byte[SHA256::DIGESTSIZE];  
      for (int i = 0; i < hashCount; ++i) {  
           memcpy(tempKey, temp, SHA256::DIGESTSIZE);  
           SHA256().CalculateDigest(temp, tempKey, SHA256::DIGESTSIZE);  
      }  
      delete[] tempKey;  
      memcpy(out, temp, keyLength);  
 }  

There is no attempt to detect or prevent replay attacks, and except for GCM mode, there is no authentication or data integrity guarantees (which you really want in something that's accepting keyboard commands).

The developers seem to be aware of how bad their crypto is, but for some reason refuse to fix or disable it.

Lessions learned:
  • Never invent your own network encryption protocols. Even professional cryptographers can't get it right the first time (SSL 3.0, TLS 1.0). Always use a good TLS library.
  • Always generate a random IV at the time of encryption.
  • Don't use a key for anything other than a key. All other uses will leak information about it.
  • It's better to have no encryption than to have broken encryption with a false sense of security.

5 comments:

  1. > It's better to have no encryption than to have broken encryption with a false sense of security.

    That's an old concept but it's terribly wrong in practice. Para 1 evidences the costs to users, and the above protocol looks good enough to defeat idle sniffing.

    It's also simply better to have broken encryption than no encryption, because the former can be fixed, the latter can't be. Eg., points 2 and 3.

    > Never invent your own network encryption protocols. Even professional cryptographers can't get it right the first time (SSL 3.0, TLS 1.0). Always use a good TLS library.

    Again, this old truism does not survive a tussle with reality. TLS is hard to use and even professionals won't get that right either. As an example, TLS is a very singular purpose tool, and if your model doesn't match its model, you're in a world of pain.

    ReplyDelete
    Replies
    1. The problem is that users had safe workarounds (SSH tunneling), and disabled those workarounds to use the broken crypto. It's not enough to defeat idle sniffing. The keystreams are exactly the same, so just XOR the tx and rx ciphertexts together, and you get the XOR of the tx and rx plaintexts. Known plaintexts in either one will give you the plaintext in the other.

      I agree TLS is hard to use, and that's one of the reasons why they invented their own, but this just highlights the need for usable cryptography libraries. If they used TLS-PSK, they could avoid all of the X.509 and PKI complexity.

      Delete
    2. Hi Winston,

      Yes, that's a problem. But it's not an overall loss, rather it is a potential weakening for some people in exchange for an overall strengthening for everyone (else). Analysing this equation is hard. Choosing to benefit the ssh-endowed minority over the majority is easy but probably wrong.

      On defeating idle sniffing: by that I mean, someone looking at the stream using standard tools, and no more. Even poorly implemented crypto has a benefit here because it forces the attacker to do more: analyse the crypto, find the breach, and develop a tool to crack it. Which is often enough to push the attacker onto other attacks.

      Is there any evidence that anyone cares that much? Most of the time there isn't.

      In the cryptographic world there is a paranoia about perfection. But in the business world, and other realities, "near enough" is often good enough, because you are looking to improve all of your risks, not eliminate only some of them. In the overall scheme of any business or relationships, there are so many other things that go wrong that the difference between 99% and 100% in the crypto is typically irrelevant.

      That's not to say we as professionals don't strive for the 100%. Rather, those amateurs & newbies that do a 99% job also deliver value -- to their users.

      And, hopefully, you and other auditors can help them even more, by reviewing and improving their efforts! Thanks for that!

      Delete
  2. Will you include Salt with it's RSA exponent of 1 and Cryptocat's horribly bad RNG?

    ReplyDelete