Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSL-Handshake-problems don't lead to an SSLHandshakeException on client-side and server doesn't close the socket #120

Closed
jmcc0nn3ll opened this issue Feb 16, 2016 · 1 comment
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@jmcc0nn3ll
Copy link
Contributor

migrated from Bugzilla #412983
status ASSIGNED severity major in component server for 9.0.x
Reported in version 9.0.3 on platform PC
Assigned to: Simone Bordet

Original attachment names and IDs:

On 2013-07-15 10:50:44 -0400, Lothar Kimmeringer wrote:

Created attachment 233480
JUnit-Testcase to reproduce the problem

Before finding the register-button for Bugzilla I already described the problem in http://dev.eclipse.org/mhonarc/lists/jetty-users/msg03542.html

Here is the mail again:

I'm currenty doing a migration from Jetty 7 to Jetty 9 and
run into problems when setting up an SSL-server with and without
needed client certificates. The problems are similar to the
mail at http://dev.eclipse.org/mhonarc/lists/jetty-users/msg03102.html
but it goes farther.

[...]

The problem is that the server doesn't generate SSL-Alerts leaving
the client in a socket.read() until that client is running into a
timeout (which might be never in dependence on how the socket is
created). The closing of the endpoint in oej.io.ssl.SslConnection
doesn't seem to have any effect. Until there is a garbage collector
run the socket stays open. Sooner or later (on low-traffic systems
expect it to be the latter) the sockets are closed, so at least
you shouldn't be able to construct a DoS-attack with this.

To reproduce the problem I created a JUnit-testcase that allows
an easy reproduction of the problem. As well you can see another
problem occuring if you comment out lines 249-252 leading to the
effect that the server get no server-certificates for the connection.
In that case testSslStandardConnection will also end in a read
timeout (in that case after two minutes).

In addition to that I added the Wireshark-trace of the second
test showing that no SSL-Alert is sent to the client and that
the connection is only closed after the client ran into the timeout.
Don't get confused by the IPs being involved. I ran a TCP-gateway
on a second computer to be able to capture the packets, since
this doesn't work here on the loopback device.

If this bug-report is better sent to jetty-dev, just tell me
JVM being used for the test is JDK 1.7.0_21 on Windows 32 Bit.
The testcase uses BouncyCastle for the creation of the certificates
the server should use. The testcase sets its own KeyStore,
KeyManager and TrustManager, so you don't need to create
file-based KeyStores before starting the test.

This bug stopped our migration, so a fix is highly appreciated

Best regards,

Lothar Kimmeringer

On 2013-07-15 10:51:38 -0400, Lothar Kimmeringer wrote:

Created attachment 233481
Debug-Output when running the testcase

On 2013-07-15 10:52:47 -0400, Lothar Kimmeringer wrote:

Created attachment 233482
Wireshark-Trace showing the ssl-handshake with needClientAuth==true

On 2013-07-18 23:45:34 -0400, Greg Wilkins wrote:

Lothar,

I've made some SSL changes in the 9.1 branch and am wondering if these may have fixed it already.

If you get a chance to try 9.1 before I get a chance to try your unit test, that would be great!

cheers

On 2013-07-19 04:43:16 -0400, Lothar Kimmeringer wrote:

If you have a jar, containing the jetty-classes I can do it. Setting it up with direct access to the repository would take too much time here at the moment.

On 2013-07-22 09:40:13 -0400, Greg Wilkins wrote:

Lothar,

try this distro:

https://oss.sonatype.org/content/groups/jetty-with-staging/org/eclipse/jetty/jetty-distribution/9.1.0-SNAPSHOT/

On 2013-07-23 03:48:48 -0400, Lothar Kimmeringer wrote:

(In reply to comment # 5)

try this distro:
[...]

I tried and have the same effect as with 9.0.3. I will attach the debug-output of my test.

On 2013-07-23 03:50:05 -0400, Lothar Kimmeringer wrote:

Created attachment 233692
Debug-Output when running the testcase with Jetty jetty-9.1.0-SNAPSHOT

On 2013-07-29 10:46:19 -0400, Lothar Kimmeringer wrote:

When replacing the catch-blocks in SslConnection (line 632ff) to the following:

catch (Exception e)
{
    EndPoint ep = getEndPoint();
    try{
        if (e instanceof SSLHandshakeException){
            byte[] data = new byte[]{
                    0x15, 0x03, 0x01, 0x00, 0x02, 0x02, 40
            };
            ep.flush(ByteBuffer.wrap(data));
        }
        if (e instanceof SSLException){
            throw new EofException(e);
        }
    }
    finally{
        ep.close();
    }
    throw e;
}

the client gets an SSLHandshakeException as expected and is no longer running into a timeout. But I see that more as a hack than a fix so I don't add this as a patch, more as a base for discussion ;-)

On 2015-01-03 09:04:47 -0500, Thomas Lu�nig wrote:

Hi,

while i try to get an A+ rating at SSL Labs with TLS_FALLBACK_SCSV.
(https://fweimer.fedorapeople.org/openjdk/tls-fallback-scsv/jdk.patch)
This patch can also be used with JDK8 :-)
I found that jetty missed to transmit the ssl alert message.
The correct fix is not to send and fixed alert message but to check the handshakeStatus and send (flush)
the rest protocol data to the socket.

Package: org.eclipse.jetty.io.ssl
Class SslConnection
Method: int fill(final ByteBuffer buffer);

Change the catch block ad the method end to:

} catch (final Throwable e) {
SslConnection.this._encryptedInput.clear();
if(SslConnection.this._sslEngine.getHandshakeStatus()==HandshakeStatus.NEED_WRAP) { flush(__FILL_CALLED_FLUSH); }
getEndPoint().close();
throw e;
} finally {

@jmcc0nn3ll jmcc0nn3ll added the Bug For general bugs on Jetty side label Feb 16, 2016
@sbordet sbordet self-assigned this Feb 17, 2016
@sbordet
Copy link
Contributor

sbordet commented Sep 14, 2017

Closing as TLS has been improved and 9.4.x should not have the issues reported here.

@sbordet sbordet closed this as completed Sep 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

2 participants