7188657: There should be a way to reorder the JSSE ciphers

Reviewed-by: weijun, wetmore

src/share/classes/javax/net/ssl/SSLParameters.java

 /*
- * 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.
  * <p>
  * SSLParameters can be created via the constructors in this class.
  * Objects can also be obtained using the <code>getSSLParameters()</code>
@@ -73,13 +73,14 @@ public class SSLParameters {
     private AlgorithmConstraints algorithmConstraints;
     private Map<Integer, SNIServerName> sniNames = null;
     private Map<Integer, SNIMatcher> sniMatchers = null;
+    private boolean preferLocalCipherSuites;
 
     /**
      * Constructs SSLParameters.
      * <p>
      * The values of cipherSuites, protocols, cryptographic algorithm
      * constraints, endpoint identification algorithm, server names and
-     * server name matchers are set to <code>null</code>,
+     * server name matchers are set to <code>null</code>, useCipherSuitesOrder,
      * wantClientAuth and needClientAuth are set to <code>false</code>.
      */
     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;
+    }
 }
 

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();
     }
 
     /**

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<SNIServerName> 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);
             }

src/share/classes/sun/security/ssl/SSLServerSocketImpl.java

 /*
- * 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<SNIMatcher>      sniMatchers =
                                     Collections.<SNIMatcher>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<SNIMatcher> 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();

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<SNIMatcher> sniMatchers) throws IOException {
+            Collection<SNIMatcher> 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<SNIServerName> 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);
             }

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");
     }
 
     /**

test/sun/security/ssl/javax/net/ssl/SSLParameters/UseCipherSuitesOrder.java

+/*
+ * 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.<String>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;
+            }
+        }
+    }
+}