am e2fdfbde: Merge "OpenSSLSocketImpl should tolerate X509KeyManager returning null values"

* commit 'e2fdfbde569a4cc284590c92bc57dc15dcc29a9c':
  OpenSSLSocketImpl should tolerate X509KeyManager returning null values

7 files changed, 173 insertions, 37 deletions

diff --git a/luni/src/main/java/javax/net/ssl/KeyStoreBuilderParameters.java b/luni/src/main/java/javax/net/ssl/KeyStoreBuilderParameters.java
index f7fe7e8..cd3c1fa 100644
--- a/luni/src/main/java/javax/net/ssl/KeyStoreBuilderParameters.java
+++ b/luni/src/main/java/javax/net/ssl/KeyStoreBuilderParameters.java
@@ -41,6 +41,9 @@ public class KeyStoreBuilderParameters implements ManagerFactoryParameters {
      *            the key store builder.
      */
     public KeyStoreBuilderParameters(KeyStore.Builder builder) {
+        if (builder == null) {
+            throw new NullPointerException("builder == null");
+        }
         ksbuilders = Collections.singletonList(builder);
     }
 
@@ -55,10 +58,10 @@ public class KeyStoreBuilderParameters implements ManagerFactoryParameters {
      */
     public KeyStoreBuilderParameters(List<KeyStore.Builder> parameters) {
         if (parameters == null) {
-            throw new NullPointerException("Builders list is null");
+            throw new NullPointerException("parameters == null");
         }
         if (parameters.isEmpty()) {
-            throw new IllegalArgumentException("Builders list is empty");
+            throw new IllegalArgumentException("parameters.isEmpty()");
         }
         ksbuilders = Collections.unmodifiableList(new ArrayList<KeyStore.Builder>(parameters));
     }
diff --git a/luni/src/main/java/javax/net/ssl/SSLHandshakeException.java b/luni/src/main/java/javax/net/ssl/SSLHandshakeException.java
index 1c17ae7..1939433 100644
--- a/luni/src/main/java/javax/net/ssl/SSLHandshakeException.java
+++ b/luni/src/main/java/javax/net/ssl/SSLHandshakeException.java
@@ -26,12 +26,25 @@ public class SSLHandshakeException extends SSLException {
     private static final long serialVersionUID = -5045881315018326890L;
 
     /**
-     * Creates a new {@code SSLHandshakeException} with the specified message.
-     *
-     * @param reason
-     *            the detail message for the exception.
+     * Constructs a new instance with the given detail message.
      */
     public SSLHandshakeException(String reason) {
         super(reason);
     }
+
+    /**
+     * Constructs a new instance with given cause.
+     * @hide internal use only
+     */
+    public SSLHandshakeException(Throwable cause) {
+        super(cause);
+    }
+
+    /**
+     * Constructs a new instance with given detail message and cause.
+     * @hide internal use only
+     */
+    public SSLHandshakeException(String reason, Throwable cause) {
+        super(reason, cause);
+    }
 }
diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java
index 68ab952..eaaa678 100644
--- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java
+++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java
@@ -18,7 +18,6 @@ package org.apache.harmony.xnet.provider.jsse;
 
 import dalvik.system.BlockGuard;
 import dalvik.system.CloseGuard;
-import java.io.FileDescriptor;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -34,12 +33,12 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Logger;
 import javax.net.ssl.HandshakeCompletedEvent;
 import javax.net.ssl.HandshakeCompletedListener;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLProtocolException;
 import javax.net.ssl.SSLSession;
 import javax.net.ssl.X509TrustManager;
 import javax.security.auth.x500.X500Principal;
@@ -520,6 +519,8 @@ public class OpenSSLSocketImpl
             }
 
             exception = false;
+        } catch (SSLProtocolException e) {
+            throw new SSLHandshakeException(e);
         } finally {
             // on exceptional exit, treat the socket as closed
             if (exception) {
@@ -551,13 +552,18 @@ public class OpenSSLSocketImpl
         if (alias == null) {
             return;
         }
-
         PrivateKey privateKey = sslParameters.getKeyManager().getPrivateKey(alias);
-        byte[] privateKeyBytes = privateKey.getEncoded();
-        NativeCrypto.SSL_use_PrivateKey(sslNativePointer, privateKeyBytes);
-
+        if (privateKey == null) {
+            return;
+        }
         X509Certificate[] certificates = sslParameters.getKeyManager().getCertificateChain(alias);
+        if (certificates == null) {
+            return;
+        }
+
+        byte[] privateKeyBytes = privateKey.getEncoded();
         byte[][] certificateBytes = NativeCrypto.encodeCertificates(certificates);
+        NativeCrypto.SSL_use_PrivateKey(sslNativePointer, privateKeyBytes);
         NativeCrypto.SSL_use_certificate(sslNativePointer, certificateBytes);
 
         // checks the last installed private key and certificate,
diff --git a/luni/src/test/java/libcore/javax/net/ssl/KeyStoreBuilderParametersTest.java b/luni/src/test/java/libcore/javax/net/ssl/KeyStoreBuilderParametersTest.java
index 1e17a79..5b8f6e6 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/KeyStoreBuilderParametersTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/KeyStoreBuilderParametersTest.java
@@ -26,19 +26,17 @@ import libcore.java.security.TestKeyStore;
 
 public class KeyStoreBuilderParametersTest extends TestCase {
     public void test_init_Builder_null() {
-        // RI document claims they throw NullPointerException but they do not
-        test_init_Builder(null);
+        try {
+            new KeyStoreBuilderParameters((Builder) null);
+            fail();
+        } catch (NullPointerException expected) {
+        }
     }
 
     public void test_init_Builder() {
         TestKeyStore testKeyStore = TestKeyStore.getClient();
         Builder builder = Builder.newInstance(testKeyStore.keyStore,
                                               new PasswordProtection(testKeyStore.storePassword));
-        test_init_Builder(builder);
-    }
-
-    private void test_init_Builder(Builder builder) {
-
         KeyStoreBuilderParameters ksbp = new KeyStoreBuilderParameters(builder);
         assertNotNull(ksbp);
         assertNotNull(ksbp.getParameters());
diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLContextTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLContextTest.java
index 51570ff..900d950 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/SSLContextTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/SSLContextTest.java
@@ -216,7 +216,8 @@ public class SSLContextTest extends TestCase {
             SSLSessionContext sessionContext = sslContext.getServerSessionContext();
             assertNotNull(sessionContext);
 
-            if (protocol.equals(StandardNames.SSL_CONTEXT_PROTOCOLS_DEFAULT)) {
+            if (!StandardNames.IS_RI &&
+                    protocol.equals(StandardNames.SSL_CONTEXT_PROTOCOLS_DEFAULT)) {
                 assertSame(SSLContext.getInstance(protocol).getServerSessionContext(),
                            sessionContext);
             } else {
@@ -232,7 +233,8 @@ public class SSLContextTest extends TestCase {
             SSLSessionContext sessionContext = sslContext.getClientSessionContext();
             assertNotNull(sessionContext);
 
-            if (protocol.equals(StandardNames.SSL_CONTEXT_PROTOCOLS_DEFAULT)) {
+            if (!StandardNames.IS_RI &&
+                    protocol.equals(StandardNames.SSL_CONTEXT_PROTOCOLS_DEFAULT)) {
                 assertSame(SSLContext.getInstance(protocol).getClientSessionContext(),
                            sessionContext);
             } else {
diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
index 16a3359..5e91dc1 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
@@ -85,6 +85,7 @@ public class SSLEngineTest extends TestCase {
         TestSSLContext c = TestSSLContext.create(testKeyStore, testKeyStore);
         String[] cipherSuites = c.clientContext.createSSLEngine().getSupportedCipherSuites();
         for (String cipherSuite : cipherSuites) {
+            boolean errorExpected = StandardNames.IS_RI && cipherSuite.endsWith("_SHA256");
             try {
                 /*
                  * TLS_EMPTY_RENEGOTIATION_INFO_SCSV cannot be used on
@@ -115,8 +116,12 @@ public class SSLEngineTest extends TestCase {
                             server.setEnabledCipherSuites(cipherSuiteArray);
                         }
                     }));
-            } catch (Exception e) {
-                throw new Exception("Problem trying to connect cipher suite " + cipherSuite, e);
+                assertFalse(errorExpected);
+            } catch (Exception maybeExpected) {
+                if (!errorExpected) {
+                    throw new Exception("Problem trying to connect cipher suite " + cipherSuite,
+                                        maybeExpected);
+                }
             }
         }
         c.close();
diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
index 7ca5a63..90cdeb9 100644
--- a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
+++ b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
@@ -23,22 +23,28 @@ import java.net.Socket;
 import java.net.SocketException;
 import java.net.SocketTimeoutException;
 import java.security.Principal;
+import java.security.PrivateKey;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import javax.net.ssl.HandshakeCompletedEvent;
 import javax.net.ssl.HandshakeCompletedListener;
+import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLPeerUnverifiedException;
+import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLProtocolException;
 import javax.net.ssl.SSLSession;
 import javax.net.ssl.SSLSocket;
 import javax.net.ssl.SSLSocketFactory;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509KeyManager;
 import junit.framework.TestCase;
 import libcore.java.security.StandardNames;
 import libcore.java.security.TestKeyStore;
@@ -118,6 +124,7 @@ public class SSLSocketTest extends TestCase {
         }
 
         for (String cipherSuite : cipherSuites) {
+            boolean errorExpected = StandardNames.IS_RI && cipherSuite.endsWith("_SHA256");
             try {
                 /*
                  * TLS_EMPTY_RENEGOTIATION_INFO_SCSV cannot be used on
@@ -165,11 +172,14 @@ public class SSLSocketTest extends TestCase {
                 assertEquals(serverToClientString, new String(clientFromServer, 0, readFromServer));
                 client.close();
                 server.close();
-            } catch (Exception e) {
-                throw new Exception("Problem trying to connect cipher suite " + cipherSuite
-                                    + " client=" + clientProvider
-                                    + " server=" + serverProvider,
-                                    e);
+                assertFalse(errorExpected);
+            } catch (Exception maybeExpected) {
+                if (!errorExpected) {
+                    throw new Exception("Problem trying to connect cipher suite " + cipherSuite
+                                        + " client=" + clientProvider
+                                        + " server=" + serverProvider,
+                                        maybeExpected);
+                }
             }
         }
         c.close();
@@ -308,10 +318,35 @@ public class SSLSocketTest extends TestCase {
                                                  SSLContext.getDefault(), SSLContext.getDefault());
         SSLSocket client = (SSLSocket) c.clientContext.getSocketFactory().createSocket(c.host,
                                                                                        c.port);
-        try {
-            c.serverSocket.accept();
-            fail();
-        } catch (SSLException expected) {
+        // RI used to throw SSLException on accept, now throws on startHandshake
+        if (StandardNames.IS_RI) {
+            final SSLSocket server = (SSLSocket) c.serverSocket.accept();
+            Thread thread = new Thread(new Runnable () {
+                public void run() {
+                    try {
+                        server.startHandshake();
+                    } catch (SSLHandshakeException expected) {
+                    } catch (RuntimeException e) {
+                        throw e;
+                    } catch (Exception e) {
+                        throw new RuntimeException(e);
+                    }
+                }
+            });
+            thread.start();
+            try {
+                client.startHandshake();
+                fail();
+            } catch (SSLHandshakeException expected) {
+            }
+            thread.join();
+            server.close();
+        } else {
+            try {
+                c.serverSocket.accept();
+                fail();
+            } catch (SSLException expected) {
+            }
         }
         client.close();
         c.close();
@@ -503,6 +538,9 @@ public class SSLSocketTest extends TestCase {
             test_SSLSocket_setUseClientMode(true, true);
             fail();
         } catch (SSLProtocolException expected) {
+            assertTrue(StandardNames.IS_RI);
+        } catch (SSLHandshakeException expected) {
+            assertFalse(StandardNames.IS_RI);
         }
 
         // both are server
@@ -537,7 +575,7 @@ public class SSLSocketTest extends TestCase {
                                                                                        c.port);
         final SSLSocket server = (SSLSocket) c.serverSocket.accept();
 
-        final SSLProtocolException[] sslProtocolException = new SSLProtocolException[1];
+        final SSLHandshakeException[] sslHandshakeException = new SSLHandshakeException[1];
         final SocketTimeoutException[] socketTimeoutException = new SocketTimeoutException[1];
         Thread thread = new Thread(new Runnable () {
             public void run() {
@@ -547,8 +585,8 @@ public class SSLSocketTest extends TestCase {
                     }
                     server.setUseClientMode(serverClientMode);
                     server.startHandshake();
-                } catch (SSLProtocolException e) {
-                    sslProtocolException[0] = e;
+                } catch (SSLHandshakeException e) {
+                    sslHandshakeException[0] = e;
                 } catch (SocketTimeoutException e) {
                     socketTimeoutException[0] = e;
                 } catch (RuntimeException e) {
@@ -565,8 +603,8 @@ public class SSLSocketTest extends TestCase {
         client.setUseClientMode(clientClientMode);
         client.startHandshake();
         thread.join();
-        if (sslProtocolException[0] != null) {
-            throw sslProtocolException[0];
+        if (sslHandshakeException[0] != null) {
+            throw sslHandshakeException[0];
         }
         if (socketTimeoutException[0] != null) {
             throw socketTimeoutException[0];
@@ -586,6 +624,7 @@ public class SSLSocketTest extends TestCase {
             public void run() {
                 try {
                     server.startHandshake();
+                } catch (SSLHandshakeException expected) {
                 } catch (RuntimeException e) {
                     throw e;
                 } catch (Exception e) {
@@ -653,6 +692,70 @@ public class SSLSocketTest extends TestCase {
         c.close();
     }
 
+    public void test_SSLSocket_clientAuth_bogusAlias() throws Exception {
+        TestSSLContext c = TestSSLContext.create();
+        SSLContext clientContext = SSLContext.getInstance("TLS");
+        X509KeyManager keyManager = new X509KeyManager() {
+            @Override public String chooseClientAlias(String[] keyType,
+                                                      Principal[] issuers,
+                                                      Socket socket) {
+                return "bogus";
+            }
+            @Override public String chooseServerAlias(String keyType,
+                                                      Principal[] issuers,
+                                                      Socket socket) {
+                throw new AssertionError();
+            }
+            @Override public X509Certificate[] getCertificateChain(String alias) {
+                // return null for "bogus" alias
+                return null;
+            }
+            @Override public String[] getClientAliases(String keyType, Principal[] issuers) {
+                throw new AssertionError();
+            }
+            @Override public String[] getServerAliases(String keyType, Principal[] issuers) {
+                throw new AssertionError();
+            }
+            @Override public PrivateKey getPrivateKey(String alias) {
+                // return null for "bogus" alias
+                return null;
+            }
+        };
+        clientContext.init(new KeyManager[] { keyManager },
+                           new TrustManager[] { c.clientTrustManager },
+                           null);
+        SSLSocket client = (SSLSocket) clientContext.getSocketFactory().createSocket(c.host,
+                                                                                     c.port);
+        final SSLSocket server = (SSLSocket) c.serverSocket.accept();
+        Thread thread = new Thread(new Runnable () {
+            public void run() {
+                try {
+                    server.setNeedClientAuth(true);
+                    server.startHandshake();
+                    fail();
+                } catch (SSLHandshakeException expected) {
+                } catch (RuntimeException e) {
+                    throw e;
+                } catch (Exception e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        });
+
+        thread.start();
+        try {
+            client.startHandshake();
+            fail();
+        } catch (SSLHandshakeException expected) {
+            // before we would get a NullPointerException from passing
+            // due to the null PrivateKey return by the X509KeyManager.
+        }
+        thread.join();
+        client.close();
+        server.close();
+        c.close();
+    }
+
     public void test_SSLSocket_getEnableSessionCreation() throws Exception {
         TestSSLContext c = TestSSLContext.create();
         SSLSocket client = (SSLSocket) c.clientContext.getSocketFactory().createSocket(c.host,
@@ -867,7 +970,10 @@ public class SSLSocketTest extends TestCase {
         try {
             input.read(null, -1, -1);
             fail();
+        } catch (NullPointerException expected) {
+            assertTrue(StandardNames.IS_RI);
         } catch (SocketException expected) {
+            assertFalse(StandardNames.IS_RI);
         }
         try {
             output.write(-1);
@@ -877,7 +983,10 @@ public class SSLSocketTest extends TestCase {
         try {
             output.write(null, -1, -1);
             fail();
+        } catch (NullPointerException expected) {
+            assertTrue(StandardNames.IS_RI);
         } catch (SocketException expected) {
+            assertFalse(StandardNames.IS_RI);
         }
 
         // ... and one gives IllegalArgumentException