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....@oracle.com > <mailto:xuelei....@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--- > > > >
