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: [email protected]
For additional commands, e-mail: [email protected]
