Bug 57116 - Bio connector: Do not fallback to default protocol list if "sslEnabledProtocols" has no matches
Summary: Bio connector: Do not fallback to default protocol list if "sslEnabledProtoco...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Connectors (show other bugs)
Version: 6.0.41
Hardware: PC All
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-19 14:12 UTC by Konstantin Kolinko
Modified: 2014-11-06 11:39 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2014-10-19 14:12:59 UTC
I noted this issue while reviewing Mark's patch to disable SSLv3 proposed for Tomcat 6, but it is present in the code before that patch and is reproducible with 6.0.42.

Steps to reproduce:
1. Configure an HTTPS connector in Tomcat using BIO protocol
2. Set sslEnabledProtocols property on that connector to some random value.

1.+2. may be done by the following steps:
a) Create a public key pair for localhost with
$JAVA_HOME/bin/keytool -genkey -alias tomcat -keyalg RSA
and specify a password value of "changeit".
"Your name" field is "localhost", the other fields are "Unknown".
b) Paste the following into server.xml:
    <Connector port="8443" protocol="org.apache.coyote.http11.Http11Protocol"
               SSLEnabled="true"
               maxThreads="150" scheme="https" secure="true"
               clientAuth="false" sslProtocol="TLS"
               sslEnabledProtocols="foo" />

3. Start Tomcat
4. Expected: Connector start failure due to broken configuration.
Actual: Tomcat starts successfully and can be accessed via https://localhost:8443/

Cause:
======
In JSSESocketFactory.initServerSocket(ServerSocket ssocket) there is the following code:

        String requestedProtocols = (String) attributes.get("protocols");
        setEnabledProtocols(socket, getEnabledProtocols(socket,
                                                         requestedProtocols));

The "getEnabledProtocols(..)" method filters requested protocols vs. socket.getSupportedProtocols(). If there is no match, it returns null.

The "setEnabledProtocols(..)" does
        if (protocols != null) {
            socket.setEnabledProtocols(protocols);
        }

Thus instead of failing with a message, it silently falls back to using the default list of enabled protocols on a socket.

The problem is that the default protocol list may be less secure, that server administrator has tried to configure.

Improve error checking
=========================
I think that silent filtering of sslEnabledProtocols by socket.getSupportedProtocols() is wrong. In case of a typo, e.g. sslEnabledProtocols="TLSv1,foo" the "foo" value shall cause a configuration error instead of being silently filtered.

The Nio connector uses different implementation, via NioEndpoint.createSSLEngine() -> engine.setEnabledProtocols(sslEnabledProtocolsarr)
and that method is documented to throw IllegalArgumentException on unsupported protocol names. [1]

[1] http://docs.oracle.com/javase/7/docs/api/javax/net/ssl/SSLEngine.html#setEnabledProtocols%28java.lang.String[]%29


The behaviour of Nio connector cannot be tested in 6.0.41/6.0.42, as it does not handle "sslEnabledProtocols" attribute due to bug 57102. Testing it with the current tc6.0.x @1632897 it starts successfully, but a web browser cannot connect to it. The following is logged for each connection attempt via HTTPS (using JDK 5u20):

[[[
19.10.2014 18:09:26 org.apache.tomcat.util.net.NioEndpoint setSocketOptions
SEVERE: 
java.lang.IllegalArgumentException: foo
	at com.sun.net.ssl.internal.ssl.ProtocolVersion.valueOf(ProtocolVersion.java:120)
	at com.sun.net.ssl.internal.ssl.ProtocolList.<init>(ProtocolList.java:36)
	at com.sun.net.ssl.internal.ssl.SSLEngineImpl.setEnabledProtocols(SSLEngineImpl.java:1723)
	at org.apache.tomcat.util.net.NioEndpoint.createSSLEngine(NioEndpoint.java:1145)
	at org.apache.tomcat.util.net.NioEndpoint.setSocketOptions(NioEndpoint.java:1098)
	at org.apache.tomcat.util.net.NioEndpoint$Acceptor.run(NioEndpoint.java:1331)
	at java.lang.Thread.run(Thread.java:595)
]]]
Comment 1 Konstantin Kolinko 2014-10-19 14:37:13 UTC
> Testing it with the current tc6.0.x @1632897 ...

It actually was @1632897 + Mark's 2014-10-17-poodle-tc6-v1.patch, so the line numbers of NioEndpoint.java in the stacktrace differ by a small offset.
Comment 2 Mark Thomas 2014-10-21 14:21:41 UTC
Updated patch proposed for 6.0.x to address this.
Comment 3 Mark Thomas 2014-11-06 11:39:30 UTC
This has been fixed in 6.0.x for 6.0.43 onwards.