Add DHE_RSA support to tlslite.

third_party/tlslite/tlslite/tlsconnection.py

Index: third_party/tlslite/tlslite/tlsconnection.py
diff --git a/third_party/tlslite/tlslite/tlsconnection.py b/third_party/tlslite/tlslite/tlsconnection.py
index 20cd85bc90f31b456ec45c5d23a746123b962346..07ee862a190cecf2ad40ab0735b277c2182d5cb3 100644
--- a/third_party/tlslite/tlslite/tlsconnection.py
+++ b/third_party/tlslite/tlslite/tlsconnection.py
@@ -24,6 +24,78 @@ from .handshakesettings import HandshakeSettings
 from .utils.tackwrapper import *
 
 
+class KeyExchange(object):
+    def __init__(self, cipherSuite, clientHello, serverHello, privateKey):
+        self.cipherSuite = cipherSuite
+        self.clientHello = clientHello
+        self.serverHello = serverHello
+        self.privateKey = privateKey
+
+    def makeServerKeyExchange():
+        raise NotImplementedError()
+    def processClientKeyExchange(clientKeyExchange):
+        raise NotImplementedError()
+
+
+class RSAKeyExchange(KeyExchange):
+    def makeServerKeyExchange(self):
+        return None

[Ryan Sleevi, 2014/03/31 20:43:57]

is None right? Should it be NotImplemented?

[davidben, 2014/03/31 20:58:20]

RSA doesn't send a ServerKeyExchange. This method is supposed to either return a
ServerKeyExchange to send or None to indicate that this key exchange doesn't
involve sending one. So it is implemented and intentionally returns None.
_serverCertKeyExchange calls makeServerKeyExchange regardless of what
cipherSuite you're using. Possibly that should be documented in a comment
somewhere.

+
+    def processClientKeyExchange(self, clientKeyExchange):
+        premasterSecret = self.privateKey.decrypt(\
+            clientKeyExchange.encryptedPreMasterSecret)
+
+        # On decryption failure randomize premaster secret to avoid
+        # Bleichenbacher's "million message" attack

[Ryan Sleevi, 2014/03/31 20:43:57]

heheheh.

This doesn't really do that, but ok [Yes, aware it's pre-existing code]

[davidben, 2014/03/31 20:58:20]

:-P I make no claims as to whether that comment was actually accurate. 

(I can also remove the comment or the logic altogether if you'd prefer. *shrug*)

[wtc, 2014/04/02 03:34:20]

If self.privateKey.decrypt does RSA blinding, then this should be sufficient,
right?

+        randomPreMasterSecret = getRandomBytes(48)
+        versionCheck = (premasterSecret[0], premasterSecret[1])
+        if not premasterSecret:
+            premasterSecret = randomPreMasterSecret
+        elif len(premasterSecret)!=48:
+            premasterSecret = randomPreMasterSecret
+        elif versionCheck != self.clientHello.client_version:
+            #Tolerate buggy IE clients

[wtc, 2014/04/01 22:00:01]

This is a very old bug of IE. I suspect this bug has been fixed.

[davidben, 2014/04/01 23:25:18]

Yeah, this block was just moved over. I can also just drop it if that's
preferable.

[wtc, 2014/04/02 03:34:20]

It's fine to keep this. We should only drop this if we have verified this. But
when you submit this patch to tlslite upstream, you can mention that this IE bug
may have been fixed.

+            if versionCheck != self.serverHello.server_version:

[wtc, 2014/04/01 22:00:01]

Is self.serverHello.server_version correct? The original code uses self.version.
I think it is correct. Just wanted to doublecheck.

[davidben, 2014/04/01 23:25:18]

Should be. I made sure it passed all tlslite's original tests and also that the
RSA key exchange still worked in our tests.

+                premasterSecret = randomPreMasterSecret
+        return premasterSecret
+
+class DHE_RSAKeyExchange(KeyExchange):
+    # Generated with openssl dhparam 1024 -text. Generating a new one each time
+    # in Python is far far far too slow.

[wtc, 2014/04/01 22:00:01]

IMPORTANT: please use one of the well-known DH group parameters recommended in
RFC 5246. The recommendation is not easy to find. It is in Appendix F.1.1.3, in
the [IKEALG] or [MODP] reference.

I would also suggest using a 2048-bit group.

[davidben, 2014/04/01 23:25:18]

Done.

+    dh_p = bytesToNumber(
+        bytearray(
+            "\x00\x88\xc4\xdc\x2a\xd9\xba\x8d\x24\x69\x7f\xe4\xd4\xeb\x62"
+            "\x04\x43\xe8\x31\x08\x30\xa8\xe4\x24\x0f\xa7\xc3\xff\x84\x52"
+            "\x44\x4a\x1b\x8c\x88\x16\xa6\x17\x90\xac\x21\x34\xfb\xc5\xff"
+            "\x90\x46\x32\x29\xfa\x9d\xd3\x36\x05\xc2\x86\xcf\xde\x77\x68"
+            "\x4c\xe3\xb0\x34\x55\xce\x27\x85\x46\x94\xad\x0c\xef\x11\x53"
+            "\x61\x50\x0c\x3e\x7b\x8a\x3c\xa0\x52\x6c\xb2\xa5\x84\xf7\xa9"
+            "\xdc\x9d\x57\x60\xd0\x89\xc6\x14\x39\x54\xce\x20\xcc\x2d\xe9"
+            "\x90\xeb\xe7\x42\xf1\x03\x60\x5b\x06\x2f\x51\x97\x01\xc2\x1d"
+            "\xfa\xb3\xfe\x02\x4b\xee\xd1\x65\xcb"))
+    dh_g = 2
+
+    def makeServerKeyExchange(self):
+        self.dh_Xs = bytesToNumber(getRandomBytes(1024 / 8))
+        dh_Ys = powMod(self.dh_g, self.dh_Xs, self.dh_p)
+
+        serverKeyExchange = ServerKeyExchange(self.cipherSuite)
+        serverKeyExchange.createDH(self.dh_p, self.dh_g, dh_Ys)
+        serverKeyExchange.signature = self.privateKey.sign(
+            serverKeyExchange.hash(self.clientHello.random,
+                                   self.serverHello.random))
+        return serverKeyExchange
+
+    def processClientKeyExchange(self, clientKeyExchange):
+        dh_Yc = clientKeyExchange.dh_Yc
+
+        if dh_Yc % self.dh_p == 0:

[wtc, 2014/04/01 22:00:01]

Is this check recommended by RFC 5246? I'm just curious where this check comes
from. This check differs from the public key validation in
https://tools.ietf.org/html/rfc2631#section-2.1.5. (We can't do the second step
of that algorithm unless we have the 'q' parameter.)

[davidben, 2014/04/01 23:25:18]

I just copied the DH code from _serverAnonKeyExchange. I've switched it to the
first half of the RFC2631 check.

+            raise TLSLocalAlert(AlertDescription.illegal_parameter,
+                                "Suspicious dh_Yc value")
+
+        S = powMod(dh_Yc, self.dh_Xs, self.dh_p)
+        return numberToByteArray(S)
+
 class TLSConnection(TLSRecordLayer):
     """
     This class wraps a socket and provides TLS handshaking and data
@@ -500,6 +572,8 @@ class TLSConnection(TLSRecordLayer):
             cipherSuites += CipherSuite.getSrpAllSuites(settings)
         elif certParams:
             cipherSuites += CipherSuite.getCertSuites(settings)
+            # Client DHE_RSA not supported.
+            # cipherSuites += CipherSuite.getDheCertSuites(settings)
         elif anonParams:
             cipherSuites += CipherSuite.getAnonSuites(settings)
         else:
@@ -1205,9 +1279,22 @@ class TLSConnection(TLSRecordLayer):
             premasterSecret = result
 
         # Perform the RSA key exchange

[wtc, 2014/04/01 22:00:01]

Nit: this comment should say "Perform the RSA or DHE_RSA key exchange".

[davidben, 2014/04/01 23:25:18]

Done.

-        elif cipherSuite in CipherSuite.certSuites:
+        elif (cipherSuite in CipherSuite.certSuites or
+              cipherSuite in CipherSuite.dheCertSuites):
+            if cipherSuite in CipherSuite.certSuites:
+                keyExchange = RSAKeyExchange(cipherSuite,
+                                             clientHello,
+                                             serverHello,
+                                             privateKey)
+            elif cipherSuite in CipherSuite.dheCertSuites:
+                keyExchange = DHE_RSAKeyExchange(cipherSuite,
+                                                 clientHello,
+                                                 serverHello,
+                                                 privateKey)
+            else:
+                assert(False)
             for result in self._serverCertKeyExchange(clientHello, serverHello, 
-                                        certChain, privateKey,
+                                        certChain, keyExchange,
                                         reqCert, reqCAs, cipherSuite,
                                         settings, ocspResponse):
                 if result in (0,1): yield result
@@ -1268,6 +1355,7 @@ class TLSConnection(TLSRecordLayer):
             cipherSuites += CipherSuite.getSrpSuites(settings)
         elif certChain:
             cipherSuites += CipherSuite.getCertSuites(settings)
+            cipherSuites += CipherSuite.getDheCertSuites(settings)
         elif anon:
             cipherSuites += CipherSuite.getAnonSuites(settings)
         else:
@@ -1483,7 +1571,7 @@ class TLSConnection(TLSRecordLayer):
 
 
     def _serverCertKeyExchange(self, clientHello, serverHello, 
-                                serverCertChain, privateKey,
+                                serverCertChain, keyExchange,
                                 reqCert, reqCAs, cipherSuite,
                                 settings, ocspResponse):
         #Send ServerHello, Certificate[, CertificateRequest],

[wtc, 2014/04/01 22:00:01]

Nit: add the optional ServerKeyExchange method to this comment.

[davidben, 2014/04/01 23:25:18]

Done.

@@ -1497,6 +1585,9 @@ class TLSConnection(TLSRecordLayer):
         msgs.append(Certificate(CertificateType.x509).create(serverCertChain))
         if serverHello.status_request:
             msgs.append(CertificateStatus().create(ocspResponse))
+        serverKeyExchange = keyExchange.makeServerKeyExchange()
+        if serverKeyExchange is not None:
+            msgs.append(serverKeyExchange)
         if reqCert and reqCAs:
             msgs.append(CertificateRequest().create(\
                 [ClientCertificateType.rsa_sign], reqCAs))
@@ -1555,21 +1646,13 @@ class TLSConnection(TLSRecordLayer):
             else: break
         clientKeyExchange = result
 
-        #Decrypt ClientKeyExchange
-        premasterSecret = privateKey.decrypt(\
-            clientKeyExchange.encryptedPreMasterSecret)
-
-        # On decryption failure randomize premaster secret to avoid
-        # Bleichenbacher's "million message" attack
-        randomPreMasterSecret = getRandomBytes(48)
-        versionCheck = (premasterSecret[0], premasterSecret[1])
-        if not premasterSecret:
-            premasterSecret = randomPreMasterSecret
-        elif len(premasterSecret)!=48:
-            premasterSecret = randomPreMasterSecret
-        elif versionCheck != clientHello.client_version:
-            if versionCheck != self.version: #Tolerate buggy IE clients
-                premasterSecret = randomPreMasterSecret
+        #Process ClientKeyExchange
+        try:
+            premasterSecret = \
+                keyExchange.processClientKeyExchange(clientKeyExchange)
+        except TLSLocalAlert, e:
+            for result in self._sendError(alert.description, alert.message):

[wtc, 2014/04/01 22:00:01]

IMPORTANT: Should |alert| be |e|?

[davidben, 2014/04/01 23:25:18]

Oops. Done.

+                yield result
 
         #Get and check CertificateVerify, if relevant
         if clientCertChain: