Author: markt Date: Thu Nov 6 11:38:59 2014 New Revision: 1637083 URL: http://svn.apache.org/r1637083 Log: Mitigate POODLE by disabling SSLv3 by default for JSSE Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57116 Do not fallback to default protocol list for HTTPS BIO connector if sslEnabledProtocols has no matches. I'll look at a separate patch for BZ 56780
Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1637083&r1=1637082&r2=1637083&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Thu Nov 6 11:38:59 2014 @@ -48,34 +48,6 @@ PATCHES PROPOSED TO BACKPORT: +1: kkolinko, markt, remm -1: -* Mitigate POODLE by disabling SSLv3 by default for JSSE - http://people.apache.org/~markt/patches/2014-10-21-poodle-tc6-v2.patch - +1: markt, schultz - +1: kkolinko (several comments below) - -1: - kkolinko: - Good. - I think this makes BZ 57116 fixed as well. - Several notes: - 1) From BZ 56780 the static{} block in JSSESocketFactory - needs try/catch(IllegalArgumentException), - like it is already done in Tomcat 7 in r1615951 - - 2) In getEnabledProtocols() the - "if (requestedProtocols == null) { return DEFAULT_SERVER_PROTOCOLS; }" - block can be moved several lines earlier. - - 3) From BZ 56780 the DEFAULT_SERVER_PROTOCOLS value might result as - null. I am afraid that passing that null to Java APIs will result in - some cryptic messages. This question may be addressed later via BZ 56780. - https://issues.apache.org/bugzilla/show_bug.cgi?id=56780#c9 - - schultz: it's not clear from the code what will happen if - DEFAULT_SERVER_PROTOCOLS remains null. Would it be more clear - to use an empty string array instead of null? I seem to recall - slightly different "null" behavior in Oracle/OpenJDK versus - IBM JVMs. - PATCHES/ISSUES THAT ARE STALLED: Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1637083&r1=1637082&r2=1637083&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Thu Nov 6 11:38:59 2014 @@ -59,6 +59,7 @@ import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.util.IntrospectionUtils; import org.apache.tomcat.util.net.SecureNioChannel.ApplicationBufferHandler; +import org.apache.tomcat.util.net.jsse.JSSESocketFactory; import org.apache.tomcat.util.net.jsse.NioX509KeyManager; import org.apache.tomcat.util.res.StringManager; @@ -1139,7 +1140,13 @@ public class NioEndpoint extends Abstrac } engine.setUseClientMode(false); if ( ciphersarr.length > 0 ) engine.setEnabledCipherSuites(ciphersarr); - if ( sslEnabledProtocolsarr.length > 0 ) engine.setEnabledProtocols(sslEnabledProtocolsarr); + if (sslEnabledProtocolsarr.length > 0) { + engine.setEnabledProtocols(sslEnabledProtocolsarr); + } else { + // Filter out the insecure protocols from the defaults + engine.setEnabledProtocols(JSSESocketFactory.filterInsecureProcotols( + engine.getEnabledProtocols())); + } return engine; } Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java?rev=1637083&r1=1637082&r2=1637083&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/JSSESocketFactory.java Thu Nov 6 11:38:59 2014 @@ -40,7 +40,11 @@ import java.security.cert.CertificateFac import java.security.cert.CollectionCertStoreParameters; import java.security.cert.PKIXBuilderParameters; import java.security.cert.X509CertSelector; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.Vector; import javax.net.ssl.CertPathTrustManagerParameters; @@ -86,6 +90,8 @@ public class JSSESocketFactory private static final boolean RFC_5746_SUPPORTED; + public static final String[] DEFAULT_SERVER_PROTOCOLS; + // defaults static String defaultProtocol = "TLS"; static boolean defaultClientAuth = false; @@ -102,6 +108,7 @@ public class JSSESocketFactory static { boolean result = false; SSLContext context; + String[] protocols = null; try { context = SSLContext.getInstance("TLS"); context.init(null, null, new SecureRandom()); @@ -113,12 +120,26 @@ public class JSSESocketFactory break; } } + + // There is no API to obtain the default server protocols and cipher + // suites. Having inspected the OpenJDK code there the same results + // can be achieved via the standard API but there is no guarantee + // that every JVM implementation determines the defaults the same + // way. Therefore the defaults are determined by creating a server + // socket and requested the configured values. + + SSLServerSocket socket = (SSLServerSocket) ssf.createServerSocket(); + // Filter out all the insecure protocols + protocols = filterInsecureProcotols(socket.getEnabledProtocols()); } catch (NoSuchAlgorithmException e) { // Assume no RFC 5746 support } catch (KeyManagementException e) { // Assume no RFC 5746 support + } catch (IOException e) { + // Unable to determine default ciphers/protocols so use none } RFC_5746_SUPPORTED = result; + DEFAULT_SERVER_PROTOCOLS = protocols; } protected boolean initialized; @@ -698,7 +719,9 @@ public class JSSESocketFactory * @param protocols the protocols to use. */ protected void setEnabledProtocols(SSLServerSocket socket, String []protocols){ - if (protocols != null) { + if (protocols == null) { + socket.setEnabledProtocols(DEFAULT_SERVER_PROTOCOLS); + } else { socket.setEnabledProtocols(protocols); } } @@ -715,67 +738,28 @@ public class JSSESocketFactory */ protected String[] getEnabledProtocols(SSLServerSocket socket, String requestedProtocols){ - String[] supportedProtocols = socket.getSupportedProtocols(); - - String[] enabledProtocols = null; + Set<String> supportedProtocols = new HashSet<String>(); + for (String supportedProtocol : socket.getSupportedProtocols()) { + supportedProtocols.add(supportedProtocol); + } - if (requestedProtocols != null) { - Vector vec = null; - String protocol = requestedProtocols; - int index = requestedProtocols.indexOf(','); - if (index != -1) { - int fromIndex = 0; - while (index != -1) { - protocol = requestedProtocols.substring(fromIndex, index).trim(); - if (protocol.length() > 0) { - /* - * Check to see if the requested protocol is among the - * supported protocols, i.e., may be enabled - */ - for (int i=0; supportedProtocols != null - && i<supportedProtocols.length; i++) { - if (supportedProtocols[i].equals(protocol)) { - if (vec == null) { - vec = new Vector(); - } - vec.addElement(protocol); - break; - } - } - } - fromIndex = index+1; - index = requestedProtocols.indexOf(',', fromIndex); - } // while - protocol = requestedProtocols.substring(fromIndex); - } + if (requestedProtocols == null) { + return DEFAULT_SERVER_PROTOCOLS; + } - if (protocol != null) { - protocol = protocol.trim(); - if (protocol.length() > 0) { - /* - * Check to see if the requested protocol is among the - * supported protocols, i.e., may be enabled - */ - for (int i=0; supportedProtocols != null - && i<supportedProtocols.length; i++) { - if (supportedProtocols[i].equals(protocol)) { - if (vec == null) { - vec = new Vector(); - } - vec.addElement(protocol); - break; - } - } - } - } + String[] requestedProtocolsArr = requestedProtocols.split(","); + List<String> enabledProtocols = new ArrayList<String>(requestedProtocolsArr.length); - if (vec != null) { - enabledProtocols = new String[vec.size()]; - vec.copyInto(enabledProtocols); + for (String requestedProtocol : requestedProtocolsArr) { + String requestedProtocolTrim = requestedProtocol.trim(); + if (supportedProtocols.contains(requestedProtocolTrim)) { + enabledProtocols.add(requestedProtocolTrim); + } else { + log.warn(sm.getString("jsse.unsupportedProtocol", requestedProtocolTrim)); } } - return enabledProtocols; + return enabledProtocols.toArray(new String[enabledProtocols.size()]); } /** @@ -816,8 +800,7 @@ public class JSSESocketFactory } String requestedProtocols = (String) attributes.get("protocols"); - setEnabledProtocols(socket, getEnabledProtocols(socket, - requestedProtocols)); + socket.setEnabledProtocols(getEnabledProtocols(socket, requestedProtocols)); // we don't know if client auth is needed - // after parsing the request we may re-handshake @@ -864,6 +847,22 @@ public class JSSESocketFactory socket.close(); } } + } + + + public static String[] filterInsecureProcotols(String[] protocols) { + if (protocols == null) { + return null; + } + List<String> result = new ArrayList<String>(protocols.length); + for (String protocol : protocols) { + if (protocol == null || protocol.contains("SSL")) { + log.debug(sm.getString("jsse.excludeDefaultProtocol", protocol)); + } else { + result.add(protocol); + } + } + return result.toArray(new String[result.size()]); } } Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties?rev=1637083&r1=1637082&r2=1637083&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/jsse/res/LocalStrings.properties Thu Nov 6 11:38:59 2014 @@ -17,6 +17,8 @@ jsse.alias_no_key_entry=Alias name {0} d jsse.keystore_load_failed=Failed to load keystore type {0} with path {1} due to {2} jsse.invalid_truststore_password=The provided trust store password could not be used to unlock and/or validate the trust store. Retrying to access the trust store with a null password which will skip validation. jsse.invalidTrustManagerClassName=The trustManagerClassName provided [{0}] does not implement javax.net.ssl.TrustManager +jsse.excludeDefaultProtocol=The SSL protocol [{0}] which is enabled by default in this JRE was excluded from the defaults used by Tomcat +jsse.unsupportedProtocol=The specified SSL protocol [{0}] is not supported jsseSupport.clientCertError=Error trying to obtain a certificate from the client jseeSupport.certTranslationError=Error translating certificate [{0}] jsseSupport.noCertWant=No client certificate sent for want Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1637083&r1=1637082&r2=1637083&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Thu Nov 6 11:38:59 2014 @@ -62,6 +62,15 @@ Do not increase remaining counter at end of stream in IdentityInputFilter. (kkolinko) </fix> + <fix> + Disable SSLv3 by default (along with SSLv2 which was already disabled + by default) in light of the recently announced POODLE vulnerability + (CVE-2014-3566). (markt) + </fix> + <fix> + <bug>57116</bug>: Do not fallback to default protocol list for HTTPS BIO + connector if <code>sslEnabledProtocols</code> has no matches. (markt) + </fix> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org