Jetty.project: TLS close behaviour breaking session resumption

[mbechler]

Mainly for documentation/information right now as this propably is better fixed on the JDK side.

SslConnection$DecryptedEndPoint calls SSLEngine.closeInbound whenever the socket input is closed. If that SSLEngine has not recieved a close_notify alert this will trigger the invalidation of the active TLS session.

There are clients out there which do not properly respond to or even send a close_notify at all (pointing a finger at Microsoft here). These clients currently perform an unnecessary amount of handshaking.

While the implemented behaviour used to be mandated by the TLS spec this is no longer the case starting with TLS 1.1: (RFC 5246 7.2.1):

Note that as of TLS 1.1, failure to properly close a connection no longer requires that a session not be resumed. This is a change from TLS 1.0 to conform with widespread implementation practice.

Unfortunately SSLEngine with it's current API does not allow setting the correct state (without reflectively calling a private method) while not invalidating the session so the only way to mitigate this issue seens to not call closeInbound() at all.

SSLEngine.closeInbound tells us:

If the application initiated the closing process by calling closeOutbound(), under some circumstances it is not required that the initiator wait for the peer's corresponding close message. (See section 7.2.1 of the TLS specification for more information on waiting for closure alerts.) In such cases, this method need not be called.

and RFC 5246 elaborates

If the application protocol will not transfer any additional data, but will only close the underlying transport connection, then the implementation MAY choose to close the transport without waiting for the responding close_notify.

So at least in some cases (e.g. idle timeout) this could be worked-around - but there would be some potential side-effects of doing so.

[gregw]

SSL is fraught with unintended consequences, so I'm very cautious about making changes. I'd much rather be held to account for over-rigorously apply the RFCs than being under-rigorous.

if an inbound socket close is OK without a close_notify, then I'd like to see a precise definition of the cases and how we can detect them....better yet, raise it as an issue on the JDK and let SSLEngine ignore or otherwise handle closeInbound() without needing us to logic about the state of the SSL connection.

So yes, let's keep this as informative for now.

[gemehendrix]

After some analysis using Wireshark on the TLS traffic served from Jetty (9.4.5), I've noticed the SSL session ids are changing quite a lot. After some further analysis it's because of the 'closeInBound' call. This is having a detrimental effect on our embedded platform's performance. The handshake is happening repeatably. It's commonly reproducible with standard Jetty; I've tried it on Windows and QNX.

[sbordet]

I have verified that the call to closeInbound(), when performed without having received the TLS Close Message, throws an exception that invalidates the session ("truncation attack").

There are 2 cases I can see:

1. client reads TLS Close Message from server, then raw closes (without replying with TLS Close Message)
2. client raw closes without sending the TLS Close Message and without having received it from the server

@gemehendrix, what case is yours ?

I think we can handle better case 1., where we know if we have closed already and we can skip the call to closeInbound().
I don't think it is correct to skip the call to closeInbound() for case 2., because it can really be a truncation attack.

[gemehendrix]

From what I understand it's probably the second use case. At the time this happens, the browser simply opens a connection and the connection is immediately closed. When the browser attempts to open a second connection with its original session id, the server hello responds with a different session id because the first message invalidated the session. Please note, this isn't an attempted attack but just standard browser usage using Chrome or Firefox. Again simply inspecting the traffic via Wireshark highlights the problem quite clearly.

I have proven that commenting out the first call to closing the inbound connection on a buffer underflow does seem to fix the problem. I can provide more details on this tomorrow.

[sbordet]

So there is no TLS traffic at all ? Just a TCP open with immediate close ?

[gregw]

On 15 May 2017 at 22:31, Simone Bordet [e-mail] wrote:

There are 2 cases I can see:

1. client reads TLS Close Message from server, then raw closes
   (without replying with TLS Close Message)
2. client raw closes without sending the TLS Close Message and without
   having received it from the server

I don't see how it could be case 2 as that indicates that the client
already wants the connection to go away. Plus a browser should not be
doing a raw close without a TLS close.

@gemehendrix indicates that the client is opening a new connection, so it
must be that the server has closed the connection when the client still
thinks it needs one?

--
Greg Wilkins [e-mail] CTO http://webtide.com

[gregw]

Hmm maybe I'm wrong. The closeInbound call i the underflown case is
protected by a a net_filled < 0, so perhaps we are receiving a half close
from the client and then sending a close message on the remaining down pipe

• causeing the browser to get confused and forget it is trying to close the
  connection?

I think we need a reproduction.... Trying to reproduce....

On 16 May 2017 at 06:15, Greg Wilkins [e-mail] wrote:

>

On 15 May 2017 at 22:31, Simone Bordet [e-mail] wrote:

There are 2 cases I can see:

1. client reads TLS Close Message from server, then raw closes
   (without replying with TLS Close Message)
2. client raw closes without sending the TLS Close Message and
   without having received it from the server

I don't see how it could be case 2 as that indicates that the client
already wants the connection to go away. Plus a browser should not be
doing a raw close without a TLS close.

@gemehendrix indicates that the client is opening a new connection, so it
must be that the server has closed the connection when the client still
thinks it needs one?

--
Greg Wilkins [e-mail] CTO http://webtide.com

--
Greg Wilkins [e-mail] CTO http://webtide.com

[gregw]

I can't reproduce.

Using a non secure local token I see Chrome open 3 connections - 1 for my
request, 1 for favicon.ico and a spare, but then immediately close them
with raw closes?

removing the closeInbound does not change the behavior for me.

So strange as that is, it's not the reported problem.

On 16 May 2017 at 06:18, Greg Wilkins [e-mail] wrote:

Hmm maybe I'm wrong. The closeInbound call i the underflown case is
protected by a a net_filled < 0, so perhaps we are receiving a half close
from the client and then sending a close message on the remaining down pipe

• causeing the browser to get confused and forget it is trying to close the
  connection?

I think we need a reproduction.... Trying to reproduce....

On 16 May 2017 at 06:15, Greg Wilkins [e-mail] wrote:

>

On 15 May 2017 at 22:31, Simone Bordet [e-mail] wrote:

There are 2 cases I can see:

1. client reads TLS Close Message from server, then raw closes
   (without replying with TLS Close Message)
2. client raw closes without sending the TLS Close Message and
   without having received it from the server

I don't see how it could be case 2 as that indicates that the client
already wants the connection to go away. Plus a browser should not be
doing a raw close without a TLS close.

@gemehendrix indicates that the client is opening a new connection, so it
must be that the server has closed the connection when the client still
thinks it needs one?

--
Greg Wilkins [e-mail] CTO http://webtide.com

--
Greg Wilkins [e-mail] CTO http://webtide.com

--
Greg Wilkins [e-mail] CTO http://webtide.com

[mbechler]

Out of interest I had a look at this, too. Unfortunately I somehow lost track and never reported that issue upstream - will do.

Testing with Chrome 58 I can see that it does not send close-notify at all. This is the same issue, sessions are getting invalidated by closeInbound. The fact that Chrome seems to close an connection almost immediately (and without ever using them for application data) just makes this way worse.

Maybe you have to test with a resource that has some more embedded resources to see this more clearly.

[mbechler]

RFE opened, unfortunately there does not seem to be a way to properly track this.

[sbordet]

@gregw I can easily reproduce. It's enough for a client to close without sending the TLS Close Message, we hit closeInbound(), which throws an exception (which we ignore), which invalidates the session, so that session ID is not reusable now.

IMO we can fix case 1. very easily, but not 2.

[sbordet]

@mbechler if Chrome is closing without sending the TLS Close Message and without having received it, then it's a violation of the TLS protocol. What RFE have you opened ?

[gemehendrix]

I've created a video that walks through how to easily reproduce the issue...

https://youtu.be/HJPuExEj05k

[gemehendrix]

Please note, as I mentioned in my video, the exact same problem occurs in Firefox. It can also be reproduced easily using a PC running Windows.

[sbordet]

@gemehendrix can you please make the following modification and report how it goes:

if (_underFlown)
{
    if (net_filled < 0 && !_closedOutbound)
        closeInbound();
    if (net_filled <= 0)
        return net_filled;
}

[mbechler]

The JSSE RFE is about adding an option to SSLEngine to avoid invalidating the session on closeInbound (but just leave the rest of it, including throwing) which would be permitted by the TLS specification and fixes the problem.

Not calling closeInbound is a bit problematic as it does not properly set the engine state, and does not release associated resources (until finalization).

Yes, this is a protocol violation by the client, but a very common one. This is probably done because it saves a couple of bytes and one round trip and does not really have consequences for HTTPS if the client always indicates the body length (through Content-Length or chunked encoding) as truncation can then be detected on the application protocol level.

[gemehendrix]

@sbordet unfortunately your suggestion doesn't fix the problem.

[gregw]

@gemehendrix, just to be crystal clear.... if you remove
the closeInbound(), you don't see the problem, but you do with Simone's
suggested change?

On 16 May 2017 at 12:34, gemehendrix [e-mail] wrote:

@sbordet https://github.com/sbordet unfortunately your suggestion this
doesn't fix the problem.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/eclipse/jetty.project/issues/523#issuecomment-301742343,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEUrYJWCTM6fAQG64pwxQALxlhpLVQNks5r6XuqgaJpZM4IOetd
.

--
Greg Wilkins [e-mail] CTO http://webtide.com

[gemehendrix]

@gregw yes that's correct. If I remove closeInbound() it fixes the problem. Simone's change does not fix the problem. Therefore this fixes the problem...

if (_underFlown) { if (net_filled <= 0) return net_filled; }

[sbordet]

To add more context, I'm inclined to agree with this answer.
The answer refers to this section of RFC 5246.

The problem is that SslConnection does not know, in general, if the upper protocol is delimited by connection termination. This may happen in HTTP/1.1 as well.

[sbordet]

1. Server is ok because request content can never be connection delimited per RFC.
2. Need to test the client in case of truncation attack and report exception to application; actually it will be better to make this behavior configurable and have the default to accept raw TCP closes from servers.

[gregw]

The section where it says that requests cannot be connection delimited: https://tools.ietf.org/html/rfc7230#section-3.3.3

[sbordet]

I believe this is fixed now, from 9.3.x onwards.

We made the call to closeInbound() only for clients (that are the only ones suffering from truncation attacks), and added an option in the client (disabled by default) to check for truncation attacks.

@gemehendrix can you please check the latest code and see if it works for you ?

[gemehendrix]

@sbordet thanks! I'll give it a try and will get back to you.

[gemehendrix]

@sbordet the fix appears to work. I still have more tests to run on other hardware platforms (especially embedded) but my first test looks good. The TLS session id is being retained. I'll post back if I find anymore issues regarding this fix.

As a note, I'm using the 9.3.x to test this with.

[gemehendrix]

@sbordet I've done further testing on the 9.3.x branch and can confirm the issue is resolved.

[sbordet]

@gemehendrix great, thanks for feedback.

[mbechler]

For future reference (maybe this can lead to a better fix some time), looks like the RFE was accepted: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8180584