From d6256fe9e0063e314cc401f19b4a72a5710189c9 Mon Sep 17 00:00:00 2001 From: xuelei Date: Sat, 7 Sep 2013 17:05:22 -0700 Subject: [PATCH] 7188657: There should be a way to reorder the JSSE ciphers Reviewed-by: weijun, wetmore --- .../classes/javax/net/ssl/SSLParameters.java | 36 +- .../classes/sun/security/ssl/Handshaker.java | 31 +- .../sun/security/ssl/SSLEngineImpl.java | 10 + .../sun/security/ssl/SSLServerSocketImpl.java | 13 +- .../sun/security/ssl/SSLSocketImpl.java | 14 +- .../sun/security/ssl/ServerHandshaker.java | 17 +- .../SSLParameters/UseCipherSuitesOrder.java | 357 ++++++++++++++++++ 7 files changed, 466 insertions(+), 12 deletions(-) create mode 100644 test/sun/security/ssl/javax/net/ssl/SSLParameters/UseCipherSuitesOrder.java diff --git a/src/share/classes/javax/net/ssl/SSLParameters.java b/src/share/classes/javax/net/ssl/SSLParameters.java index 1dc6f3315..5e0d403cd 100644 --- a/src/share/classes/javax/net/ssl/SSLParameters.java +++ b/src/share/classes/javax/net/ssl/SSLParameters.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -40,7 +40,7 @@ import java.util.LinkedHashMap; * the list of protocols to be allowed, the endpoint identification * algorithm during SSL/TLS handshaking, the Server Name Indication (SNI), * the algorithm constraints and whether SSL/TLS servers should request - * or require client authentication. + * or require client authentication, etc. *

* SSLParameters can be created via the constructors in this class. * Objects can also be obtained using the getSSLParameters() @@ -73,13 +73,14 @@ public class SSLParameters { private AlgorithmConstraints algorithmConstraints; private Map sniNames = null; private Map sniMatchers = null; + private boolean preferLocalCipherSuites; /** * Constructs SSLParameters. *

* The values of cipherSuites, protocols, cryptographic algorithm * constraints, endpoint identification algorithm, server names and - * server name matchers are set to null, + * server name matchers are set to null, useCipherSuitesOrder, * wantClientAuth and needClientAuth are set to false. */ public SSLParameters() { @@ -434,5 +435,34 @@ public class SSLParameters { return null; } + + /** + * Sets whether the local cipher suites preference should be honored. + * + * @param honorOrder whether local cipher suites order in + * {@code #getCipherSuites} should be honored during + * SSL/TLS handshaking. + * + * @see #getUseCipherSuitesOrder() + * + * @since 1.8 + */ + public final void setUseCipherSuitesOrder(boolean honorOrder) { + this.preferLocalCipherSuites = honorOrder; + } + + /** + * Returns whether the local cipher suites preference should be honored. + * + * @return whether local cipher suites order in {@code #getCipherSuites} + * should be honored during SSL/TLS handshaking. + * + * @see #setUseCipherSuitesOrder(boolean) + * + * @since 1.8 + */ + public final boolean getUseCipherSuitesOrder() { + return preferLocalCipherSuites; + } } diff --git a/src/share/classes/sun/security/ssl/Handshaker.java b/src/share/classes/sun/security/ssl/Handshaker.java index 17b1f92ac..a92320451 100644 --- a/src/share/classes/sun/security/ssl/Handshaker.java +++ b/src/share/classes/sun/security/ssl/Handshaker.java @@ -145,6 +145,14 @@ abstract class Handshaker { /* True if it's OK to start a new SSL session */ boolean enableNewSession; + // Whether local cipher suites preference should be honored during + // handshaking? + // + // Note that in this provider, this option only applies to server side. + // Local cipher suites preference is always honored in client side in + // this provider. + boolean preferLocalCipherSuites = false; + // Temporary storage for the individual keys. Set by // calculateConnectionKeys() and cleared once the ciphers are // activated. @@ -462,6 +470,13 @@ abstract class Handshaker { this.sniMatchers = sniMatchers; } + /** + * Sets the cipher suites preference. + */ + void setUseCipherSuitesOrder(boolean on) { + this.preferLocalCipherSuites = on; + } + /** * Prior to handshaking, activate the handshake and initialize the version, * input stream and output stream. @@ -533,7 +548,9 @@ abstract class Handshaker { } /** - * Check if the given ciphersuite is enabled and available. + * Check if the given ciphersuite is enabled and available within the + * current active cipher suites. + * * Does not check if the required server certificates are available. */ boolean isNegotiable(CipherSuite s) { @@ -541,7 +558,17 @@ abstract class Handshaker { activeCipherSuites = getActiveCipherSuites(); } - return activeCipherSuites.contains(s) && s.isNegotiable(); + return isNegotiable(activeCipherSuites, s); + } + + /** + * Check if the given ciphersuite is enabled and available within the + * proposed cipher suite list. + * + * Does not check if the required server certificates are available. + */ + final static boolean isNegotiable(CipherSuiteList proposed, CipherSuite s) { + return proposed.contains(s) && s.isNegotiable(); } /** diff --git a/src/share/classes/sun/security/ssl/SSLEngineImpl.java b/src/share/classes/sun/security/ssl/SSLEngineImpl.java index c2e94f3b4..5302b6147 100644 --- a/src/share/classes/sun/security/ssl/SSLEngineImpl.java +++ b/src/share/classes/sun/security/ssl/SSLEngineImpl.java @@ -319,6 +319,12 @@ final public class SSLEngineImpl extends SSLEngine { */ private boolean isFirstAppOutputRecord = true; + /* + * Whether local cipher suites preference in server side should be + * honored during handshaking? + */ + private boolean preferLocalCipherSuites = false; + /* * Class and subclass dynamic debugging support */ @@ -470,6 +476,7 @@ final public class SSLEngineImpl extends SSLEngine { protocolVersion, connectionState == cs_HANDSHAKE, secureRenegotiation, clientVerifyData, serverVerifyData); handshaker.setSNIMatchers(sniMatchers); + handshaker.setUseCipherSuitesOrder(preferLocalCipherSuites); } else { handshaker = new ClientHandshaker(this, sslContext, enabledProtocols, @@ -2074,6 +2081,7 @@ final public class SSLEngineImpl extends SSLEngine { params.setAlgorithmConstraints(algorithmConstraints); params.setSNIMatchers(sniMatchers); params.setServerNames(serverNames); + params.setUseCipherSuitesOrder(preferLocalCipherSuites); return params; } @@ -2088,6 +2096,7 @@ final public class SSLEngineImpl extends SSLEngine { // the super implementation does not handle the following parameters identificationProtocol = params.getEndpointIdentificationAlgorithm(); algorithmConstraints = params.getAlgorithmConstraints(); + preferLocalCipherSuites = params.getUseCipherSuitesOrder(); List sniNames = params.getServerNames(); if (sniNames != null) { @@ -2104,6 +2113,7 @@ final public class SSLEngineImpl extends SSLEngine { handshaker.setAlgorithmConstraints(algorithmConstraints); if (roleIsServer) { handshaker.setSNIMatchers(sniMatchers); + handshaker.setUseCipherSuitesOrder(preferLocalCipherSuites); } else { handshaker.setSNIServerNames(serverNames); } diff --git a/src/share/classes/sun/security/ssl/SSLServerSocketImpl.java b/src/share/classes/sun/security/ssl/SSLServerSocketImpl.java index 0e0edf18e..464bab23a 100644 --- a/src/share/classes/sun/security/ssl/SSLServerSocketImpl.java +++ b/src/share/classes/sun/security/ssl/SSLServerSocketImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -92,6 +92,12 @@ class SSLServerSocketImpl extends SSLServerSocket Collection sniMatchers = Collections.emptyList(); + /* + * Whether local cipher suites preference in server side should be + * honored during handshaking? + */ + private boolean preferLocalCipherSuites = false; + /** * Create an SSL server socket on a port, using a non-default * authentication context and a specified connection backlog. @@ -304,6 +310,8 @@ class SSLServerSocketImpl extends SSLServerSocket params.setEndpointIdentificationAlgorithm(identificationProtocol); params.setAlgorithmConstraints(algorithmConstraints); params.setSNIMatchers(sniMatchers); + params.setUseCipherSuitesOrder(preferLocalCipherSuites); + return params; } @@ -318,6 +326,7 @@ class SSLServerSocketImpl extends SSLServerSocket // the super implementation does not handle the following parameters identificationProtocol = params.getEndpointIdentificationAlgorithm(); algorithmConstraints = params.getAlgorithmConstraints(); + preferLocalCipherSuites = params.getUseCipherSuitesOrder(); Collection matchers = params.getSNIMatchers(); if (matchers != null) { sniMatchers = params.getSNIMatchers(); @@ -334,7 +343,7 @@ class SSLServerSocketImpl extends SSLServerSocket SSLSocketImpl s = new SSLSocketImpl(sslContext, useServerMode, enabledCipherSuites, doClientAuth, enableSessionCreation, enabledProtocols, identificationProtocol, algorithmConstraints, - sniMatchers); + sniMatchers, preferLocalCipherSuites); implAccept(s); s.doneConnect(); diff --git a/src/share/classes/sun/security/ssl/SSLSocketImpl.java b/src/share/classes/sun/security/ssl/SSLSocketImpl.java index dfe612966..4d591e3d7 100644 --- a/src/share/classes/sun/security/ssl/SSLSocketImpl.java +++ b/src/share/classes/sun/security/ssl/SSLSocketImpl.java @@ -377,6 +377,12 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { */ private ByteArrayOutputStream heldRecordBuffer = null; + /* + * Whether local cipher suites preference in server side should be + * honored during handshaking? + */ + private boolean preferLocalCipherSuites = false; + // // CONSTRUCTORS AND INITIALIZATION CODE // @@ -482,7 +488,8 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { boolean sessionCreation, ProtocolList protocols, String identificationProtocol, AlgorithmConstraints algorithmConstraints, - Collection sniMatchers) throws IOException { + Collection sniMatchers, + boolean preferLocalCipherSuites) throws IOException { super(); doClientAuth = clientAuth; @@ -490,6 +497,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { this.identificationProtocol = identificationProtocol; this.algorithmConstraints = algorithmConstraints; this.sniMatchers = sniMatchers; + this.preferLocalCipherSuites = preferLocalCipherSuites; init(context, serverMode); /* @@ -1284,6 +1292,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { protocolVersion, connectionState == cs_HANDSHAKE, secureRenegotiation, clientVerifyData, serverVerifyData); handshaker.setSNIMatchers(sniMatchers); + handshaker.setUseCipherSuitesOrder(preferLocalCipherSuites); } else { handshaker = new ClientHandshaker(this, sslContext, enabledProtocols, @@ -2502,6 +2511,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { params.setAlgorithmConstraints(algorithmConstraints); params.setSNIMatchers(sniMatchers); params.setServerNames(serverNames); + params.setUseCipherSuitesOrder(preferLocalCipherSuites); return params; } @@ -2516,6 +2526,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { // the super implementation does not handle the following parameters identificationProtocol = params.getEndpointIdentificationAlgorithm(); algorithmConstraints = params.getAlgorithmConstraints(); + preferLocalCipherSuites = params.getUseCipherSuitesOrder(); List sniNames = params.getServerNames(); if (sniNames != null) { @@ -2532,6 +2543,7 @@ final public class SSLSocketImpl extends BaseSSLSocketImpl { handshaker.setAlgorithmConstraints(algorithmConstraints); if (roleIsServer) { handshaker.setSNIMatchers(sniMatchers); + handshaker.setUseCipherSuitesOrder(preferLocalCipherSuites); } else { handshaker.setSNIServerNames(serverNames); } diff --git a/src/share/classes/sun/security/ssl/ServerHandshaker.java b/src/share/classes/sun/security/ssl/ServerHandshaker.java index e317e1f18..23b806f33 100644 --- a/src/share/classes/sun/security/ssl/ServerHandshaker.java +++ b/src/share/classes/sun/security/ssl/ServerHandshaker.java @@ -932,8 +932,18 @@ final class ServerHandshaker extends Handshaker { * the cipherSuite and keyExchange variables. */ private void chooseCipherSuite(ClientHello mesg) throws IOException { - for (CipherSuite suite : mesg.getCipherSuites().collection()) { - if (isNegotiable(suite) == false) { + CipherSuiteList prefered; + CipherSuiteList proposed; + if (preferLocalCipherSuites) { + prefered = getActiveCipherSuites(); + proposed = mesg.getCipherSuites(); + } else { + prefered = mesg.getCipherSuites(); + proposed = getActiveCipherSuites(); + } + + for (CipherSuite suite : prefered.collection()) { + if (isNegotiable(proposed, suite) == false) { continue; } @@ -948,8 +958,7 @@ final class ServerHandshaker extends Handshaker { } return; } - fatalSE(Alerts.alert_handshake_failure, - "no cipher suites in common"); + fatalSE(Alerts.alert_handshake_failure, "no cipher suites in common"); } /** diff --git a/test/sun/security/ssl/javax/net/ssl/SSLParameters/UseCipherSuitesOrder.java b/test/sun/security/ssl/javax/net/ssl/SSLParameters/UseCipherSuitesOrder.java new file mode 100644 index 000000000..c25d74c7d --- /dev/null +++ b/test/sun/security/ssl/javax/net/ssl/SSLParameters/UseCipherSuitesOrder.java @@ -0,0 +1,357 @@ +/* + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +// +// SunJSSE does not support dynamic system properties, no way to re-use +// system properties in samevm/agentvm mode. +// + +/* + * @test + * @bug 7188657 + * @summary There should be a way to reorder the JSSE ciphers + * @run main/othervm UseCipherSuitesOrder + * TLS_RSA_WITH_AES_128_CBC_SHA,SSL_RSA_WITH_RC4_128_SHA + */ + +import java.io.*; +import java.net.*; +import javax.net.ssl.*; +import java.util.Arrays; + +public class UseCipherSuitesOrder { + + /* + * ============================================================= + * Set the various variables needed for the tests, then + * specify what tests to run on each side. + */ + + /* + * Should we run the client or server in a separate thread? + * Both sides can throw exceptions, but do you have a preference + * as to which side should be the main thread. + */ + static boolean separateServerThread = false; + + /* + * Where do we find the keystores? + */ + static String pathToStores = "../../../../etc"; + static String keyStoreFile = "keystore"; + static String trustStoreFile = "truststore"; + static String passwd = "passphrase"; + + /* + * Is the server ready to serve? + */ + volatile static boolean serverReady = false; + + /* + * Turn on SSL debugging? + */ + static boolean debug = false; + + /* + * If the client or server is doing some kind of object creation + * that the other side depends on, and that thread prematurely + * exits, you may experience a hang. The test harness will + * terminate all hung threads after its timeout has expired, + * currently 3 minutes by default, but you might try to be + * smart about it.... + */ + + /* + * Define the server side of the test. + * + * If the server prematurely exits, serverReady will be set to true + * to avoid infinite hangs. + */ + void doServerSide() throws Exception { + SSLServerSocketFactory sslssf = + (SSLServerSocketFactory) SSLServerSocketFactory.getDefault(); + SSLServerSocket sslServerSocket = + (SSLServerSocket) sslssf.createServerSocket(serverPort); + serverPort = sslServerSocket.getLocalPort(); + + // use local cipher suites preference + SSLParameters params = sslServerSocket.getSSLParameters(); + params.setUseCipherSuitesOrder(true); + params.setCipherSuites(srvEnabledCipherSuites); + sslServerSocket.setSSLParameters(params); + + /* + * Signal Client, we're ready for his connect. + */ + serverReady = true; + + SSLSocket sslSocket = (SSLSocket) sslServerSocket.accept(); + InputStream sslIS = sslSocket.getInputStream(); + OutputStream sslOS = sslSocket.getOutputStream(); + + sslIS.read(); + sslOS.write(85); + sslOS.flush(); + + SSLSession session = sslSocket.getSession(); + if (!srvEnabledCipherSuites[0].equals(session.getCipherSuite())) { + throw new Exception( + "Expected to negotiate " + srvEnabledCipherSuites[0] + + " , but not " + session.getCipherSuite()); + } + + sslSocket.close(); + } + + /* + * Define the client side of the test. + * + * If the server prematurely exits, serverReady will be set to true + * to avoid infinite hangs. + */ + void doClientSide() throws Exception { + + /* + * Wait for server to get started. + */ + while (!serverReady) { + Thread.sleep(50); + } + + SSLSocketFactory sslsf = + (SSLSocketFactory) SSLSocketFactory.getDefault(); + SSLSocket sslSocket = (SSLSocket) + sslsf.createSocket("localhost", serverPort); + sslSocket.setEnabledCipherSuites(cliEnabledCipherSuites); + + InputStream sslIS = sslSocket.getInputStream(); + OutputStream sslOS = sslSocket.getOutputStream(); + + sslOS.write(280); + sslOS.flush(); + sslIS.read(); + + sslSocket.close(); + } + + // client enabled cipher suites + private static String[] cliEnabledCipherSuites; + + // server enabled cipher suites + private static String[] srvEnabledCipherSuites; + + private static void parseArguments(String[] args) throws Exception { + if (args.length != 1) { + System.out.println("Usage: java UseCipherSuitesOrder ciphersuites"); + System.out.println("\tciphersuites: " + + "a list of enabled cipher suites, separated with comma"); + throw new Exception("Incorrect usage"); + } + + cliEnabledCipherSuites = args[0].split(","); + + if (cliEnabledCipherSuites.length < 2) { + throw new Exception("Need to enable at least two cipher suites"); + } + + // Only need to use 2 cipher suites in server side. + srvEnabledCipherSuites = Arrays.copyOf( + cliEnabledCipherSuites, 2); + + // Reverse the cipher suite preference in server side. + srvEnabledCipherSuites[0] = cliEnabledCipherSuites[1]; + srvEnabledCipherSuites[1] = cliEnabledCipherSuites[0]; + } + + /* + * ============================================================= + * The remainder is just support stuff + */ + + // use any free port by default + volatile int serverPort = 0; + + volatile Exception serverException = null; + volatile Exception clientException = null; + + public static void main(String[] args) throws Exception { + // parse the arguments + parseArguments(args); + + String keyFilename = + System.getProperty("test.src", ".") + "/" + pathToStores + + "/" + keyStoreFile; + String trustFilename = + System.getProperty("test.src", ".") + "/" + pathToStores + + "/" + trustStoreFile; + + System.setProperty("javax.net.ssl.keyStore", keyFilename); + System.setProperty("javax.net.ssl.keyStorePassword", passwd); + System.setProperty("javax.net.ssl.trustStore", trustFilename); + System.setProperty("javax.net.ssl.trustStorePassword", passwd); + + if (debug) + System.setProperty("javax.net.debug", "all"); + + /* + * Start the tests. + */ + new UseCipherSuitesOrder(); + } + + Thread clientThread = null; + Thread serverThread = null; + + /* + * Primary constructor, used to drive remainder of the test. + * + * Fork off the other side, then do your work. + */ + UseCipherSuitesOrder() throws Exception { + Exception startException = null; + try { + if (separateServerThread) { + startServer(true); + startClient(false); + } else { + startClient(true); + startServer(false); + } + } catch (Exception e) { + startException = e; + } + + /* + * Wait for other side to close down. + */ + if (separateServerThread) { + if (serverThread != null) { + serverThread.join(); + } + } else { + if (clientThread != null) { + clientThread.join(); + } + } + + /* + * When we get here, the test is pretty much over. + * Which side threw the error? + */ + Exception local; + Exception remote; + + if (separateServerThread) { + remote = serverException; + local = clientException; + } else { + remote = clientException; + local = serverException; + } + + Exception exception = null; + + /* + * Check various exception conditions. + */ + if ((local != null) && (remote != null)) { + // If both failed, return the curthread's exception. + local.initCause(remote); + exception = local; + } else if (local != null) { + exception = local; + } else if (remote != null) { + exception = remote; + } else if (startException != null) { + exception = startException; + } + + /* + * If there was an exception *AND* a startException, + * output it. + */ + if (exception != null) { + if (exception != startException && startException != null) { + exception.addSuppressed(startException); + } + throw exception; + } + + // Fall-through: no exception to throw! + } + + void startServer(boolean newThread) throws Exception { + if (newThread) { + serverThread = new Thread() { + public void run() { + try { + doServerSide(); + } catch (Exception e) { + /* + * Our server thread just died. + * + * Release the client, if not active already... + */ + System.err.println("Server died..."); + serverReady = true; + serverException = e; + } + } + }; + serverThread.start(); + } else { + try { + doServerSide(); + } catch (Exception e) { + serverException = e; + } finally { + serverReady = true; + } + } + } + + void startClient(boolean newThread) throws Exception { + if (newThread) { + clientThread = new Thread() { + public void run() { + try { + doClientSide(); + } catch (Exception e) { + /* + * Our client thread just died. + */ + System.err.println("Client died..."); + clientException = e; + } + } + }; + clientThread.start(); + } else { + try { + doClientSide(); + } catch (Exception e) { + clientException = e; + } + } + } +} -- GitLab