Fixed (ICE-5758) - Disable SSLv3 by default

6 files changed, 249 insertions, 37 deletions

diff --git a/cpp/src/IceSSL/OpenSSLEngine.cpp b/cpp/src/IceSSL/OpenSSLEngine.cpp
index c53ddbda7ca..75c58f50cf5 100644
--- a/cpp/src/IceSSL/OpenSSLEngine.cpp
+++ b/cpp/src/IceSSL/OpenSSLEngine.cpp
@@ -352,9 +352,16 @@ OpenSSLEngine::initialize()
         PropertiesPtr properties = communicator()->getProperties();
 
         //
-        // Protocols selects which protocols to enable.
+        // Protocols selects which protocols to enable, by default we only enable TLS1.0
+        // TLS1.1 and TLS1.2 to avoid security issues with SSLv3
         //
-        const int protocols = parseProtocols(properties->getPropertyAsList(propPrefix + "Protocols"));
+        vector<string> defaultProtocols;
+        defaultProtocols.push_back("tls1_0");
+        defaultProtocols.push_back("tls1_1");
+        defaultProtocols.push_back("tls1_2");
+
+        const int protocols = 
+                parseProtocols(properties->getPropertyAsListWithDefault(propPrefix + "Protocols", defaultProtocols));
 
         //
         // Create an SSL context if the application hasn't supplied one.
@@ -901,27 +908,26 @@ OpenSSLEngine::parseProtocols(const StringSeq& protocols) const
 
     for(Ice::StringSeq::const_iterator p = protocols.begin(); p != protocols.end(); ++p)
     {
-        string prot = *p;
-
-        if(prot == "ssl3" || prot == "sslv3")
+        string prot = IceUtilInternal::toUpper(*p);
+        if(prot == "SSL3" || prot == "SSLV3")
         {
             v |= SSLv3;
         }
-        else if(prot == "tls" || prot == "tls1" || prot == "tlsv1" || prot == "tls1_0" || prot == "tlsv1_0")
+        else if(prot == "TLS" || prot == "TLS1" || prot == "TLSV1" || prot == "TLS1_0" || prot == "TLSV1_0")
         {
             v |= TLSv1_0;
         }
-        else if(prot == "tls1_1" || prot == "tlsv1_1")
+        else if(prot == "TLS1_1" || prot == "TLSV1_1")
         {
             v |= TLSv1_1;
         }
-        else if(prot == "tls1_2" || prot == "tlsv1_2")
+        else if(prot == "TLS1_2" || prot == "TLSV1_2")
         {
             v |= TLSv1_2;
         }
         else
         {
-            throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + prot + "'");
+            throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + *p + "'");
         }
     }
 
diff --git a/cpp/src/IceSSL/SChannelEngine.cpp b/cpp/src/IceSSL/SChannelEngine.cpp
index 595a85aa220..bef93424650 100644
--- a/cpp/src/IceSSL/SChannelEngine.cpp
+++ b/cpp/src/IceSSL/SChannelEngine.cpp
@@ -93,31 +93,31 @@ parseProtocols(const StringSeq& protocols)
 
     for(Ice::StringSeq::const_iterator p = protocols.begin(); p != protocols.end(); ++p)
     {
-        string prot = *p;
+        string prot = IceUtilInternal::toUpper(*p);
 
-        if(prot == "ssl3" || prot == "sslv3")
+        if(prot == "SSL3" || prot == "SSLV3")
         {
             v |= SP_PROT_SSL3_SERVER;
             v |= SP_PROT_SSL3_CLIENT;
         }
-        else if(prot == "tls" || prot == "tls1" || prot == "tlsv1" || prot == "tls1_0" || prot == "tlsv1_0")
+        else if(prot == "TLS" || prot == "TLS1" || prot == "TLSV1" || prot == "TLS1_0" || prot == "TLSV1_0")
         {
             v |= SP_PROT_TLS1_SERVER;
             v |= SP_PROT_TLS1_CLIENT;
         }
-        else if(prot == "tls1_1" || prot == "tlsv1_1")
+        else if(prot == "TLS1_1" || prot == "TLSV1_1")
         {
             v |= SP_PROT_TLS1_1_SERVER;
             v |= SP_PROT_TLS1_1_CLIENT;
         }
-        else if(prot == "tls1_2" || prot == "tlsv1_2")
+        else if(prot == "TLS1_2" || prot == "TLSV1_2")
         {
             v |= SP_PROT_TLS1_2_SERVER;
             v |= SP_PROT_TLS1_2_CLIENT;
         }
         else
         {
-            throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + prot + "'");
+            throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + *p + "'");
         }
     }
 
@@ -182,9 +182,15 @@ SChannelEngine::initialize()
     const PropertiesPtr properties = communicator()->getProperties();
 
     //
-    // Protocols selects which protocols to enable.
+    // Protocols selects which protocols to enable, by default we only enable TLS1.0
+    // TLS1.1 and TLS1.2 to avoid security issues with SSLv3
     //
-    const_cast<DWORD&>(_protocols) = parseProtocols(properties->getPropertyAsList(prefix + "Protocols"));
+    vector<string> defaultProtocols;
+    defaultProtocols.push_back("tls1_0");
+    defaultProtocols.push_back("tls1_1");
+    defaultProtocols.push_back("tls1_2");
+    const_cast<DWORD&>(_protocols) = 
+                    parseProtocols(properties->getPropertyAsListWithDefault(prefix + "Protocols", defaultProtocols));
 
     //
     // Check for a default directory. We look in this directory for
diff --git a/cpp/src/IceSSL/SecureTransportEngine.cpp b/cpp/src/IceSSL/SecureTransportEngine.cpp
index 743347feb67..8d255f24124 100644
--- a/cpp/src/IceSSL/SecureTransportEngine.cpp
+++ b/cpp/src/IceSSL/SecureTransportEngine.cpp
@@ -751,27 +751,28 @@ CiphersHelper::ciphers()
 }
 
 SSLProtocol
-parseProtocol(const string& prot)
+parseProtocol(const string& p)
 {
-    if(prot == "ssl3" || prot == "sslv3")
+    const string prot = IceUtilInternal::toUpper(p);
+    if(prot == "SSL3" || prot == "SSLV3")
     {
         return kSSLProtocol3;
     }
-    else if(prot == "tls" || prot == "tls1" || prot == "tlsv1" || prot == "tls1_0" || prot == "tlsv1_0")
+    else if(prot == "TLS" || prot == "TLS1" || prot == "TLSV1" || prot == "TLS1_0" || prot == "TLSV1_0")
     {
         return kTLSProtocol1;
     }
-    else if(prot == "tls1_1" || prot == "tlsv1_1")
+    else if(prot == "TLS1_1" || prot == "TLSV1_1")
     {
         return kTLSProtocol11;
     }
-    else if(prot == "tls1_2" || prot == "tlsv1_2")
+    else if(prot == "TLS1_2" || prot == "TLSV1_2")
     {
         return kTLSProtocol12;
     }
     else
     {
-        throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + prot + "'");
+        throw PluginInitializationException(__FILE__, __LINE__, "IceSSL: unrecognized protocol `" + p + "'");
     }
 }
 
@@ -1212,7 +1213,10 @@ IceSSL::SecureTransportEngine::initialize()
         _protocolVersionMax = parseProtocol(protocolVersionMax);
     }
     
-    const string protocolVersionMin = properties->getProperty(propPrefix + "ProtocolVersionMin");
+    //
+    // The default min protocol version is set to TLS1.0 to avoid security issues with SSLv3
+    // 
+    const string protocolVersionMin = properties->getPropertyWithDefault(propPrefix + "ProtocolVersionMin", "tls1_0");
     if(!protocolVersionMin.empty())
     {
         _protocolVersionMin = parseProtocol(protocolVersionMin);
diff --git a/cpp/test/IceSSL/configuration/.depend.mak b/cpp/test/IceSSL/configuration/.depend.mak
index a09fc686430..0b7721e0916 100644
--- a/cpp/test/IceSSL/configuration/.depend.mak
+++ b/cpp/test/IceSSL/configuration/.depend.mak
@@ -320,9 +320,6 @@ AllTests.obj: \
     "$(includedir)\IceUtil\RecMutex.h" \
     "$(includedir)\IceUtil\UUID.h" \
     "Test.h" \
-    "Util.h" \
-    "$(includedir)\IceSSL\IceSSL.h" \
-    "$(includedir)\IceSSL\EndpointInfo.h" \
 
 TestI.obj: \
 	TestI.cpp \
@@ -450,9 +447,6 @@ TestI.obj: \
     "$(includedir)\IceSSL\Plugin.h" \
     "$(includedir)\IceSSL\Config.h" \
     "$(includedir)\IceSSL\ConnectionInfo.h" \
-    "Util.h" \
-    "$(includedir)\IceSSL\IceSSL.h" \
-    "$(includedir)\IceSSL\EndpointInfo.h" \
 
 Server.obj: \
 	Server.cpp \
diff --git a/cpp/test/IceSSL/configuration/AllTests.cpp b/cpp/test/IceSSL/configuration/AllTests.cpp
index 2a6f922d385..64cbda75975 100644
--- a/cpp/test/IceSSL/configuration/AllTests.cpp
+++ b/cpp/test/IceSSL/configuration/AllTests.cpp
@@ -11,7 +11,6 @@
 #include <IceSSL/Plugin.h>
 #include <TestCommon.h>
 #include <Test.h>
-#include <Util.h>
 #include <fstream>
 
 using namespace std;
@@ -1137,15 +1136,15 @@ allTests(const CommunicatorPtr& communicator, const string& testDir, bool pfx, b
         Test::ServerFactoryPrx fact = Test::ServerFactoryPrx::checkedCast(comm->stringToProxy(factoryRef));
         test(fact);
         Test::Properties d = createServerProps(defaultProperties, defaultDir, defaultHost, pfx);
-        initData.properties->setProperty("IceSSL.CertAuthFile", "cacert1.pem");
+        d["IceSSL.CertAuthFile"] = "cacert1.pem";
         if(pfx)
         {
-            initData.properties->setProperty("IceSSL.CertFile", "s_rsa_ca1.pfx");
+            d["IceSSL.CertFile"] = "s_rsa_ca1.pfx";
         }
         else
         {
-            initData.properties->setProperty("IceSSL.CertFile", "s_rsa_nopass_ca1_pub.pem");
-            initData.properties->setProperty("IceSSL.KeyFile", "s_rsa_nopass_ca1_priv.pem");
+            d["IceSSL.CertFile"] = "s_rsa_nopass_ca1_pub.pem";
+            d["IceSSL.KeyFile"] = "s_rsa_nopass_ca1_priv.pem";
         }
         d["IceSSL.VerifyPeer"] = "0";
         d["IceSSL.Protocols"] = "tls";
@@ -1201,6 +1200,102 @@ allTests(const CommunicatorPtr& communicator, const string& testDir, bool pfx, b
         }
         fact->destroyServer(server);
         comm->destroy();
+
+        //
+        // This should fail because the client only accept SSLv3 and the server
+        // use the default protocol set that disables SSLv3
+        //
+        {
+            InitializationData initData;
+            initData.properties = createClientProps(defaultProperties, defaultDir, defaultHost, pfx);
+            initData.properties->setProperty("IceSSL.CertAuthFile", "cacert1.pem");
+            if(pfx)
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_ca1.pfx");
+            }
+            else
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_nopass_ca1_pub.pem");
+                initData.properties->setProperty("IceSSL.KeyFile", "c_rsa_nopass_ca1_priv.pem");
+            }
+            initData.properties->setProperty("IceSSL.VerifyPeer", "0");
+            initData.properties->setProperty("IceSSL.Protocols", "ssl3");        
+            CommunicatorPtr comm = initialize(initData);
+            
+            Test::ServerFactoryPrx fact = Test::ServerFactoryPrx::checkedCast(comm->stringToProxy(factoryRef));
+            test(fact);
+            Test::Properties d = createServerProps(defaultProperties, defaultDir, defaultHost, pfx);
+            d["IceSSL.CertAuthFile"] = "cacert1.pem";
+            if(pfx)
+            {
+                d["IceSSL.CertFile"] = "s_rsa_ca1.pfx";
+            }
+            else
+            {
+                d["IceSSL.CertFile"] = "s_rsa_nopass_ca1_pub.pem";
+                d["IceSSL.KeyFile"] = "s_rsa_nopass_ca1_priv.pem";
+            }
+            d["IceSSL.VerifyPeer"] = "0";
+            Test::ServerPrx server = fact->createServer(d);
+            try
+            {
+                server->ice_ping();
+                test(false);
+            }
+            catch(const ProtocolException&)
+            {
+                // Expected on some platforms.
+            }
+            catch(const ConnectionLostException&)
+            {
+                // Expected on some platforms.
+            }
+            catch(const LocalException&)
+            {
+                test(false);
+            }
+            fact->destroyServer(server);
+            comm->destroy();
+        }
+
+        //
+        // This should success because both have SSLv3 enabled
+        //
+        {
+            InitializationData initData;
+            initData.properties = createClientProps(defaultProperties, defaultDir, defaultHost, pfx);
+            initData.properties->setProperty("IceSSL.CertAuthFile", "cacert1.pem");
+            initData.properties->setProperty("IceSSL.Protocols", "ssl3");        
+            CommunicatorPtr comm = initialize(initData);
+            
+            Test::ServerFactoryPrx fact = Test::ServerFactoryPrx::checkedCast(comm->stringToProxy(factoryRef));
+            test(fact);
+            Test::Properties d = createServerProps(defaultProperties, defaultDir, defaultHost, pfx);
+            d["IceSSL.CertAuthFile"] = "cacert1.pem";
+            if(pfx)
+            {
+                d["IceSSL.CertFile"] = "s_rsa_ca1.pfx";
+            }
+            else
+            {
+                d["IceSSL.CertFile"] = "s_rsa_nopass_ca1_pub.pem";
+                d["IceSSL.KeyFile"] = "s_rsa_nopass_ca1_priv.pem";
+            }
+            d["IceSSL.VerifyPeer"] = "0";
+            d["IceSSL.Protocols"] = "ssl3, tls, tls1_1, tls1_2";
+            Test::ServerPrx server = fact->createServer(d);
+            try
+            {
+                server->ice_ping();
+            }
+            catch(const LocalException& ex)
+            {
+                cerr << ex << endl;
+                test(false);
+            }
+            fact->destroyServer(server);
+            comm->destroy();
+        }
 #else
         //
         // This should fail because the client and server have no protocol
@@ -1263,6 +1358,113 @@ allTests(const CommunicatorPtr& communicator, const string& testDir, bool pfx, b
         }
         fact->destroyServer(server);
         comm->destroy();
+
+        //
+        // This should fail because the client only accept SSLv3 and the server
+        // use the default protocol set that disables SSLv3
+        //
+        {
+            InitializationData initData;
+            initData.properties = createClientProps(defaultProperties, defaultDir, defaultHost, pfx);
+            initData.properties->setProperty("IceSSL.CertAuthFile", "cacert1.pem");
+            if(pfx)
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_ca1.pfx");
+            }
+            else
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_nopass_ca1_pub.pem");
+                initData.properties->setProperty("IceSSL.KeyFile", "c_rsa_nopass_ca1_priv.pem");
+            }
+            initData.properties->setProperty("IceSSL.VerifyPeer", "0");
+            initData.properties->setProperty("IceSSL.ProtocolVersionMin", "ssl3");
+            initData.properties->setProperty("IceSSL.ProtocolVersionMax", "ssl3");     
+            CommunicatorPtr comm = initialize(initData);
+            
+            Test::ServerFactoryPrx fact = Test::ServerFactoryPrx::checkedCast(comm->stringToProxy(factoryRef));
+            test(fact);
+            Test::Properties d = createServerProps(defaultProperties, defaultDir, defaultHost, pfx);
+            d["IceSSL.CertAuthFile"] = "cacert1.pem";
+            if(pfx)
+            {
+                d["IceSSL.CertFile"] = "s_rsa_ca1.pfx";
+            }
+            else
+            {
+                d["IceSSL.CertFile"] = "s_rsa_nopass_ca1_pub.pem";
+                d["IceSSL.KeyFile"] = "s_rsa_nopass_ca1_priv.pem";
+            }
+            d["IceSSL.VerifyPeer"] = "0";
+            Test::ServerPrx server = fact->createServer(d);
+            try
+            {
+                server->ice_ping();
+                test(false);
+            }
+            catch(const ProtocolException&)
+            {
+                // Expected on some platforms.
+            }
+            catch(const ConnectionLostException&)
+            {
+                // Expected on some platforms.
+            }
+            catch(const LocalException&)
+            {
+                test(false);
+            }
+            fact->destroyServer(server);
+            comm->destroy();
+        }
+
+        //
+        // This should success because both have SSLv3 enabled
+        //
+        {
+            InitializationData initData;
+            initData.properties = createClientProps(defaultProperties, defaultDir, defaultHost, pfx);
+            initData.properties->setProperty("IceSSL.CertAuthFile", "cacert1.pem");
+            if(pfx)
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_ca1.pfx");
+            }
+            else
+            {
+                initData.properties->setProperty("IceSSL.CertFile", "c_rsa_nopass_ca1_pub.pem");
+                initData.properties->setProperty("IceSSL.KeyFile", "c_rsa_nopass_ca1_priv.pem");
+            }
+            initData.properties->setProperty("IceSSL.VerifyPeer", "0");
+            initData.properties->setProperty("IceSSL.ProtocolVersionMin", "ssl3");
+            initData.properties->setProperty("IceSSL.ProtocolVersionMax", "ssl3");        
+            CommunicatorPtr comm = initialize(initData);
+            
+            Test::ServerFactoryPrx fact = Test::ServerFactoryPrx::checkedCast(comm->stringToProxy(factoryRef));
+            test(fact);
+            Test::Properties d = createServerProps(defaultProperties, defaultDir, defaultHost, pfx);
+            d["IceSSL.CertAuthFile"] = "cacert1.pem";
+            if(pfx)
+            {
+                d["IceSSL.CertFile"] = "s_rsa_ca1.pfx";
+            }
+            else
+            {
+                d["IceSSL.CertFile"] = "s_rsa_nopass_ca1_pub.pem";
+                d["IceSSL.KeyFile"] = "s_rsa_nopass_ca1_priv.pem";
+            }
+            d["IceSSL.VerifyPeer"] = "0";
+            d["IceSSL.ProtocolVersionMin"] = "ssl3";
+            Test::ServerPrx server = fact->createServer(d);
+            try
+            {
+                server->ice_ping();
+            }
+            catch(const LocalException&)
+            {
+                test(false);
+            }
+            fact->destroyServer(server);
+            comm->destroy();
+        }
 #endif
     }
     cout << "ok" << endl;
@@ -1574,7 +1776,7 @@ allTests(const CommunicatorPtr& communicator, const string& testDir, bool pfx, b
                 IceSSL::NativeConnectionInfoPtr::dynamicCast(server->ice_getConnection()->getInfo());
             test(info->cipher.compare(0, cipherSub.size(), cipherSub) == 0);
         }
-        catch(const LocalException&)
+        catch(const LocalException& ex)
         {
 //
 // OS X 10.10 bug the handshake fails attempting client auth
diff --git a/cpp/test/IceSSL/configuration/run.py b/cpp/test/IceSSL/configuration/run.py
index 61da2e86b04..37e92428317 100755
--- a/cpp/test/IceSSL/configuration/run.py
+++ b/cpp/test/IceSSL/configuration/run.py
@@ -27,8 +27,8 @@ keychainPath = os.path.abspath(os.path.join(certsPath, "Find.keychain"))
 def keychainCleanup():
     os.system("rm -rf %s ../certs/keychain" % keychainPath)
 
-atexit.register(keychainCleanup)
 if TestUtil.isDarwin():
+    atexit.register(keychainCleanup)
     keychainCleanup()
     os.system("mkdir -p ../certs/keychain")