zero pad DHE public key in ServerKeyExchange message for interop
authorrussor <russor@whatsapp.com>
Mon, 25 Jul 2016 17:30:13 +0000 (10:30 -0700)
committerRich Salz <rsalz@openssl.org>
Mon, 25 Jul 2016 17:41:33 +0000 (13:41 -0400)
Some versions of the Microsoft TLS stack have problems when the DHE public key
is encoded with fewer bytes than the DHE prime.

There's some public acknowledgement of the bug at these links:

https://connect.microsoft.com/IE/feedback/details/1253526/tls-serverkeyexchange-with-1024-dhe-may-encode-dh-y-as-127-bytes-breaking-internet-explorer-11
https://connect.microsoft.com/IE/feedback/details/1104905/wininet-calculation-of-mac-in-tls-handshake-intermittently-fails-for-dhe-rsa-key-exchange

This encoding issue also causes the same errors with 2048-bit DHE, if the
public key is encoded in fewer than 256 bytes and includes the TLS stack on
Windows Phone 8.x.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1320)

ssl/statem/statem_srvr.c

index b7f2a0fe2d3c34877896ffa8aba6686f3b3a851b..dc43095a1d47baf1bb2f369280ad68e66a9b4057 100644 (file)
@@ -1608,6 +1608,7 @@ int tls_construct_server_key_exchange(SSL *s)
 {
 #ifndef OPENSSL_NO_DH
     EVP_PKEY *pkdh = NULL;
+    int j;
 #endif
 #ifndef OPENSSL_NO_EC
     unsigned char *encodedPoint = NULL;
@@ -1798,6 +1799,16 @@ int tls_construct_server_key_exchange(SSL *s)
         if ((i == 2) && (type & SSL_kSRP))
             n += 1 + nr[i];
         else
+#endif
+#ifndef OPENSSL_NO_DH
+        /*
+         * for interoperability with some versions of the Microsoft TLS
+         * stack, we need to zero pad the DHE pub key to the same length
+         * as the prime, so use the length of the prime here
+         */
+        if ((i == 2) && (type & (SSL_kDHE | SSL_kDHEPSK)))
+            n += 2 + nr[0];
+        else
 #endif
             n += 2 + nr[i];
     }
@@ -1855,6 +1866,20 @@ int tls_construct_server_key_exchange(SSL *s)
             *p = nr[i];
             p++;
         } else
+#endif
+#ifndef OPENSSL_NO_DH
+        /*
+         * for interoperability with some versions of the Microsoft TLS
+         * stack, we need to zero pad the DHE pub key to the same length
+         * as the prime
+         */
+        if ((i == 2) && (type & (SSL_kDHE | SSL_kDHEPSK))) {
+            s2n(nr[0], p);
+            for (j = 0; j < (nr[0] - nr[2]); ++j) {
+                *p = 0;
+                ++p;
+            }
+        } else
 #endif
             s2n(nr[i], p);
         BN_bn2bin(r[i], p);