Fixed ICE-6438, ICE-6439 and various inconsistencies between IceSSL implementations

3 files changed, 77 insertions, 31 deletions

diff --git a/java/src/Ice/src/main/java/IceSSL/SSLEngine.java b/java/src/Ice/src/main/java/IceSSL/SSLEngine.java
index 354473a0d9b..bacb4a9cf26 100644
--- a/java/src/Ice/src/main/java/IceSSL/SSLEngine.java
+++ b/java/src/Ice/src/main/java/IceSSL/SSLEngine.java
@@ -12,6 +12,7 @@ package IceSSL;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.List;
+import java.security.cert.*;
 
 class SSLEngine
 {
@@ -90,7 +91,7 @@ class SSLEngine
         // chain, including the peer's certificate. A value of 0 means there is
         // no maximum.
         //
-        _verifyDepthMax = properties.getPropertyAsIntWithDefault(prefix + "VerifyDepthMax", 2);
+        _verifyDepthMax = properties.getPropertyAsIntWithDefault(prefix + "VerifyDepthMax", 3);
 
         //
         // VerifyPeer determines whether certificate validation failures abort a connection.
@@ -524,6 +525,25 @@ class SSLEngine
                 }
 
                 //
+                // Create a certificate path validator to build the full
+                // certificate chain of the peer certificate. NOTE: this must
+                // be done before wrapping the trust manager since our wrappers
+                // return an empty list of accepted issuers.
+                //
+                _validator = CertPathValidator.getInstance("PKIX");
+                java.util.Set<TrustAnchor> anchors = new java.util.HashSet<TrustAnchor>();
+                for(javax.net.ssl.TrustManager tm : trustManagers)
+                {
+                    X509Certificate[] certs = ((javax.net.ssl.X509TrustManager)tm).getAcceptedIssuers();
+                    for(X509Certificate cert : certs)
+                    {
+                        anchors.add(new TrustAnchor(cert, null));
+                    }
+                }
+                _validatorParams = new PKIXParameters(anchors);
+                _validatorParams.setRevocationEnabled(false);
+
+                //
                 // Wrap each trust manager.
                 //
                 for(int i = 0; i < trustManagers.length; ++i)
@@ -553,6 +573,26 @@ class SSLEngine
         _initialized = true;
     }
 
+    Certificate[] getVerifiedCertificateChain(Certificate[] chain)
+    {
+        List<Certificate> certs = new ArrayList<Certificate>(java.util.Arrays.asList(chain));
+        try
+        {
+            CertPath path = CertificateFactory.getInstance("X.509").generateCertPath(certs);
+            CertPathValidatorResult result = _validator.validate(path, _validatorParams);
+            Certificate root = ((PKIXCertPathValidatorResult)result).getTrustAnchor().getTrustedCert();
+            if(!root.equals(chain[chain.length - 1])) // Only add the root certificate if it's not already in the chain
+            {
+                certs.add(root);
+            }
+            return certs.toArray(new Certificate[certs.size()]);
+        }
+        catch(Exception ex)
+        {
+            return null; // Couldn't validate the given certificate chain.
+        }
+    }
+
     void context(javax.net.ssl.SSLContext context)
     {
         if(_initialized)
@@ -817,12 +857,24 @@ class SSLEngine
     void verifyPeer(NativeConnectionInfo info, java.nio.channels.SelectableChannel fd, String address)
     {
         //
+        // IceSSL.VerifyPeer is translated into the proper SSLEngine configuration
+        // for a server, but we have to do it ourselves for a client.
+        //
+        if(!info.incoming)
+        {
+            if(_verifyPeer > 0 && !info.verified)
+            {
+                throw new Ice.SecurityException("IceSSL: server did not supply a certificate");
+            }
+        }
+
+        //
         // For an outgoing connection, we compare the proxy address (if any) against
         // fields in the server's certificate (if any).
         //
         if(info.nativeCerts != null && info.nativeCerts.length > 0 && address.length() > 0)
         {
-            java.security.cert.X509Certificate cert = (java.security.cert.X509Certificate)info.nativeCerts[0];
+            X509Certificate cert = (X509Certificate)info.nativeCerts[0];
 
             //
             // Extract the IP addresses and the DNS names from the subject
@@ -850,7 +902,7 @@ class SSLEngine
                     }
                 }
             }
-            catch(java.security.cert.CertificateParsingException ex)
+            catch(CertificateParsingException ex)
             {
                 assert(false);
             }
@@ -992,8 +1044,8 @@ class SSLEngine
         }
     }
 
-    void trustManagerFailure(boolean incoming, java.security.cert.CertificateException ex)
-        throws java.security.cert.CertificateException
+    void trustManagerFailure(boolean incoming, CertificateException ex)
+        throws CertificateException
     {
         if(_verifyPeer == 0)
         {
@@ -1157,4 +1209,7 @@ class SSLEngine
     private InputStream _keystoreStream;
     private InputStream _truststoreStream;
     private List<InputStream> _seeds = new ArrayList<InputStream>();
+
+    private CertPathValidator _validator;
+    private PKIXParameters _validatorParams;
 }
diff --git a/java/src/Ice/src/main/java/IceSSL/TransceiverI.java b/java/src/Ice/src/main/java/IceSSL/TransceiverI.java
index 4e0a9aaa5fc..c9de921c8f4 100644
--- a/java/src/Ice/src/main/java/IceSSL/TransceiverI.java
+++ b/java/src/Ice/src/main/java/IceSSL/TransceiverI.java
@@ -37,26 +37,6 @@ final class TransceiverI implements IceInternal.Transceiver
         }
 
         //
-        // IceSSL.VerifyPeer is translated into the proper SSLEngine configuration
-        // for a server, but we have to do it ourselves for a client.
-        //
-        if(!_incoming)
-        {
-            int verifyPeer = _instance.properties().getPropertyAsIntWithDefault("IceSSL.VerifyPeer", 2);
-            if(verifyPeer > 0)
-            {
-                try
-                {
-                    _engine.getSession().getPeerCertificates();
-                }
-                catch(javax.net.ssl.SSLPeerUnverifiedException ex)
-                {
-                    throw new Ice.SecurityException("IceSSL: server did not supply a certificate", ex);
-                }
-            }
-        }
-
-        //
         // Additional verification.
         //
         _instance.verifyPeer(getNativeConnectionInfo(), _stream.fd(), _host);
@@ -322,8 +302,12 @@ final class TransceiverI implements IceInternal.Transceiver
             info.cipher = session.getCipherSuite();
             try
             {
+                java.security.cert.Certificate[] pcerts = session.getPeerCertificates();
+                java.security.cert.Certificate[] vcerts = _instance.engine().getVerifiedCertificateChain(pcerts);
+                info.verified = vcerts != null;
+                info.nativeCerts = vcerts != null ? vcerts : pcerts;
+
                 java.util.ArrayList<String> certs = new java.util.ArrayList<String>();
-                info.nativeCerts = session.getPeerCertificates();
                 for(java.security.cert.Certificate c : info.nativeCerts)
                 {
                     StringBuilder s = new StringBuilder("-----BEGIN CERTIFICATE-----\n");
@@ -331,15 +315,15 @@ final class TransceiverI implements IceInternal.Transceiver
                     s.append("\n-----END CERTIFICATE-----");
                     certs.add(s.toString());
                 }
-                info.certs = certs.toArray(new String[0]);
-            }
-            catch(java.security.cert.CertificateEncodingException ex)
-            {
+                info.certs = certs.toArray(new String[certs.size()]);
             }
             catch(javax.net.ssl.SSLPeerUnverifiedException ex)
             {
                 // No peer certificates.
             }
+            catch(java.security.cert.CertificateEncodingException ex)
+            {
+            }
         }
         info.adapterName = _adapterName;
         info.incoming = _incoming;
diff --git a/java/src/Ice/src/main/java/IceSSL/X509TrustManagerI.java b/java/src/Ice/src/main/java/IceSSL/X509TrustManagerI.java
index 599112ec6a2..c39a143bc49 100644
--- a/java/src/Ice/src/main/java/IceSSL/X509TrustManagerI.java
+++ b/java/src/Ice/src/main/java/IceSSL/X509TrustManagerI.java
@@ -67,9 +67,16 @@ final class X509TrustManagerI implements javax.net.ssl.X509TrustManager
     public java.security.cert.X509Certificate[]
     getAcceptedIssuers()
     {
-        return _delegate.getAcceptedIssuers();
+        //
+        // This method is used to send CA names to the client as part of the CertificateRequest
+        // message sent by the server to request a client certificate. We want the client to always
+        // send its certificate so we don't provide any CAs here.
+        //
+        //return _delegate.getAcceptedIssuers();
+        return EMPTY;
     }
 
     private SSLEngine _engine;
     private javax.net.ssl.X509TrustManager _delegate;
+    private java.security.cert.X509Certificate[] EMPTY = new java.security.cert.X509Certificate[0];
 }