9001039?: DHKeyAgreement calculates wrong TlsPremasterSecret 1 out of 256 times

Xuelei Fan xuelei.fan at oracle.com
Wed May 22 02:34:53 UTC 2013


On 5/20/2013 5:28 PM, Pasi Eronen wrote:
> Hi Xuelei,
> 
> It seems the PKSC11 doesn't actually have this bug.
> 
> P11KeyAgreement has a separate code path for the "TlsPremasterSecret" 
> algorithm, which strips leading zeroes if the key can be extracted from 
> the token. (And if the key cannot be extracted, then the token is doing 
> the premaster secret->master secret computation, and has to do the 
> stripping -- it can't be done from the Java PKSC11 provider.) 
> 
It makes sense to me.

> To make sure this behavior doesn't change, I added a test case 
> for the PKSC11 provider to the Bugzilla (which passes with the 
> "SunPKCS11-NSS" provider without any changes).
> 
That's great.  Would you mind to contribute the regression test for
PKCS11 provider?

Thanks,
Xuelei

> Best regards,
> Pasi
> 
> 
> 
> On Thu, May 16, 2013 at 10:56 AM, Xuelei Fan <xuelei.fan at oracle.com
> <mailto:xuelei.fan at oracle.com>> wrote:
> 
>     Hi Pasi,
> 
>     Thank you for your patience, and contribution to OpenJDK.  The bug is
>     accepted, and you should be able to review it at:
> 
>        http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014618
> 
>     Let's use the above bug ID to track the issue.
> 
>     Your patch looks fine in general (I may have some very minor comments
>     later).  We also have similar problems in PKCS11 provider because of the
>     update of P11KeyAgreement.java in changeset:
> 
>         http://hg.openjdk.java.net/jdk7u/jdk7u-gate/jdk/rev/e574e475c8a6
> 
>     Would you like to also fix it in your patch?
> 
>     Thanks again for your nice work.
> 
>     Regards,
>     Xuelei
> 
> 
>     On 5/10/2013 5:00 PM, Pasi Eronen wrote:
>     > AKA "1 out of 256 SSL/TLS handshakes fails with DHE cipher suites"
>     >
>     > I reported this bug over a month of ago, but for some reason, it's not
>     > yet visible at bugs.sun.com <http://bugs.sun.com>
>     <http://bugs.sun.com>. I've included the bug
>     > report below just in
>     > case.
>     >
>     > It seems this commit from March 2012 inadvertently broke SSL/TLS DHE
>     > cipher suites, causing the SSL/TLS handshake to fail approximately
>     > 1 out of 256 times:
>     >
>     > http://hg.openjdk.java.net/jdk7u/jdk7u-gate/jdk/rev/e574e475c8a6
>     >
>     > The commit was done to fix this bug:
>     >
>     > http://bugs.sun.com/view_bug.do?bug_id=7146728
>     >
>     > While generating a secret of the same length as modulus may be the
>     right
>     > choice generally speaking (and it's what e.g. IPsec uses), SSL/TLS
>     uses
>     > a different convention: leading zeroes must be stripped.
>     >
>     > This is currently blocking us from updating our production systems to
>     > Java 7, so although I have not contributed to OpenJDK before, I'd like
>     > to submit a patch and a test case for this (I've signed the OCA
>     > already). But before I do this, I'd like to check that the approach is
>     > agreeable.
>     >
>     > We have a separate "algorithm" value "TlsPremasterSecret", so
>     > behavior for other cases could stay the same. Would a patch
>     > like this:
>     >
>     >     } else if (algorithm.equals("TlsPremasterSecret")) {
>     >         // remove leading zero bytes per RFC 5246 Section 8.1.2
>     >         int i = 0;
>     >         while ((i < secret.length - 1) && (secret[i] == 0)) {
>     >             i++;
>     >         }
>     >         if (i == 0) {
>     >             return new SecretKeySpec(secret, "TlsPremasterSecret");
>     >         } else {
>     >             byte[] secret2 = new byte[secret.length - i];
>     >             System.arraycopy(secret, i, secret2, 0, secret2.length);
>     >             return new SecretKeySpec(secret2, "TlsPremasterSecret");
>     >         }
>     >     }
>     >
>     > Plus a test case (with fixed keys) that checks that leading zero is
>     > stripped
>     > for TlsPremasterSecret and is not stripped otherwise, be sufficient?
>     >
>     > Best regards,
>     > Pasi
>     >
>     > ---snip---
>     >
>     > Synopsis:
>     > DHKeyAgreement calculates wrong TlsPremasterSecret 1 out of 256 times
>     >
>     > Full OS version:
>     > Tested on Windows 7 (Microsoft Windows [Version 6.1.7601]), but
>     occurs in
>     > e..g OpenJDK 7 as well.
>     >
>     > Development Kit or Runtime version:
>     > java version "1.7.0_17"
>     > Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
>     > Java HotSpot(TM) Client VM (build 23.7-b01, mixed mode, sharing)
>     >
>     > Description:
>     > When performing Diffie-Hellman key agreement for SSL/TLS, the TLS
>     > specification (RFC 5246) says that "Leading bytes of Z that
>     contain all zero
>     > bits are stripped before it is used as the pre_master_secret."
>     >
>     > However, com.sun.crypto.provider.DHKeyAgreement.java does not
>     strip leading
>     > zero bytes. This causes approximately 1 out 256 SSL/TLS handshakes
>     with
>     > DH/DHE cipher suites to fail (when the leading byte happens, by
>     chance, to
>     > be zero).
>     >
>     > Steps to Reproduce:
>     > 1. Start a simple JSSE socket server with -Djavax.net.debug=all.
>     >
>     > 2. Connect to the server with e.g. OpenSSL command line tool,
>     ensuring that
>     > DHE cipher suite gets selected (e.g. "openssl s_client -cipher
>     > DHE-RSA-AES128-SHA -connect 192.168.81.1:9999
>     <http://192.168.81.1:9999>
>     > <http://192.168.81.1:9999>") repeatedly. Other SSL
>     > clients can be used -- this is not an OpenSSL bug (see below).
>     >
>     > 3. Repeat the connection. After a couple of hundred successful
>     connections,
>     > the connection will fail with handshake_failure alert.
>     >
>     > 4. Examine the JSSE debug logs produced by the server: the failed
>     connection
>     > will have a PreMaster secret that begins with zero byte
>     > (while all other connections have non-zero byte here). For example:
>     >
>     > SESSION KEYGEN:
>     > PreMaster Secret:
>     > 0000: 00 70 C5 7E 91 38 C8 DE   ED 75 3D 76 8A B5 44 69
>      .p...8...u=v..Di
>     > 0010: E7 32 1C EE 80 77 50 C7   A9 51 24 2E E3 15 11 30
>      .2...wP..Q$....0
>     > 0020: 9D F6 9F BC 9D EB 5C 18   F7 A4 19 ED 1A AC 2E 0C
>      ......\.........
>     > 0030: E3 18 C5 11 B1 80 07 7D   B1 C6 70 A8 D7 EB CF DD
>      ..........p.....
>     > 0040: 2D B5 1D BC 01 3E 28 2A   2B 5B 38 8F EB 20 F2 A2
>      -....>(*+[8.. ..
>     > 0050: 00 07 47 F7 87 B8 99 CB   EF B4 13 04 C8 8B 82 FB
>      ..G.............
>     >
>     > Expected Result:
>     > Expected result is that every connection succeed.
>     >
>     > Actual Result:
>     > Roughly one out of 256 connections fail.
>     >
>     > Source code for an executable test case:
>     >
>     > Java server:
>     >
>     > import javax.net.ssl.SSLServerSocket;
>     > import javax.net.ssl.SSLServerSocketFactory;
>     > import javax.net.ssl.SSLSocket;
>     >
>     > public class TestServer {
>     >     public static void main(String args[]) throws Exception {
>     >         SSLServerSocketFactory ssf = (SSLServerSocketFactory)
>     > SSLServerSocketFactory.getDefault();
>     >         SSLServerSocket ss = (SSLServerSocket)
>     ssf.createServerSocket(9999);
>     >         System.out.println("Listening on port 9999");
>     >         for (String cs : ss.getEnabledCipherSuites()) {
>     >             System.out.println(cs);
>     >         }
>     >         while (true) {
>     >             SSLSocket s = (SSLSocket) ss.accept();
>     >             System.out.println("Connected with
>     > "+s.getSession().getCipherSuite());
>     >             s.close();
>     >         }
>     >     }
>     > }
>     >
>     > Run as as follows:
>     >
>     > keytool -storepass "password" -keypass "password" -genkey -keyalg RSA
>     > -keystore test_keystore.jks -dname CN=test
>     > javac TestServer.java
>     > java -Djavax.net.debug=all
>     -Djavax.net.ssl.keyStore=./test_keystore.jks
>     > -Djavax.net.ssl.keyStorePassword=password TestServer
>     >
>     > OpenSSL client:
>     >
>     > set -e
>     > while true; do
>     >   openssl s_client -cipher DHE-RSA-AES128-SHA -connect
>     127.0.0.1:9999 <http://127.0.0.1:9999>
>     > <http://127.0.0.1:9999> -quiet -no_ign_eof < /dev/null
>     > done
>     >
>     > Workaround:
>     > Disable Diffie-Hellman cipher suites.
>     >
>     > ---snip---
>     >
> 
> 




More information about the security-dev mailing list