9001039?: DHKeyAgreement calculates wrong TlsPremasterSecret 1 out of 256 times
Andrew Hughes
gnu.andrew at redhat.com
Thu May 23 10:03:49 UTC 2013
----- Original Message -----
> 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?
>
It's been attached to the bug report: https://bugs.openjdk.java.net/show_bug.cgi?id=100316
Is there any reason the original patch can't be committed? I haven't seen any mentioned.
> 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---
> > >
> >
> >
>
>
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
More information about the security-dev
mailing list