Client fails attempts to re-use session ticket after ServerHello rejects it once

[wreuven]

Summary

In some rare circumstances, AWS NLB will do a 1-time reject of a previously working ticket by returning a session ID
not equal to the session ID provided by ClientHello. A full handshake will be done instead.

The client then continues to try using the old ticket which results in a key mismatch and MAC errors on subsequent requests as the client fails to authenticated the encrypted handshake finished record. It seems the client keys have become corrupted. Perhaps the newly negotiated keys are being used by the client instead of the keys associated with the old ticket

System information

Mbed TLS version (number or commit id): mbedtls-2.28.1
Operating system and version:
Configuration (if not default, please attach mbedtls_config.h):
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):
Additional environment information:

Expected behavior

After the server rejects the ticket, I would expect the client to discard it and not try to use it again. Instead it seems to corrupt the ticket for future use. As a secondary matter, I would think the client would discard the ticket if using it is resulting in MAC errors.

Actual behavior

The client continues to re-use the ticket even after the server has rejected it.

Steps to reproduce

I do not have a way to reproduce this server-side. It happens about once a week when AWS is draining the NLBs.

Additional information

[yanesca]

@wreuven thank you very much for your report. Could you please set the debug level to 3 (mbedtls_debug_set_threshold(3)) and post the client log about a run with the undesired behaviour?

[wreuven]

will try to reproduce this in a test app.

@wreuven thank you very much for your report. Could you please set the debug level to 3 (mbedtls_debug_set_threshold(3)) and post the client log about a run with the undesired behaviour?

[wreuven]

bad_mac_issue.zip

Attached:

1. client and server logs at level 3
2. server.c which rejects a ticket on the third session (simulate the NLB behaviour) in order to cause the client problem on subsequent sessions
3. client.c which results in failed sessions after session 3 with MAC auth failures
4. client.c also includes an ifdef WORKAROUND that can be defined which implements an ugly workaround to the problem

Here is another summary and a proposal for direction of what should be fixed within mbedtls.

Notes! mbedtls_ssl_get_session and set_session are used to get/set current session state for TLS re-use
but in the case when server chooses to ignore a ticket and force a new key exchange
mbedtls_ssl_get_session returns values that do not properly reflect state.
The session shows the old ticket, and the newly exchanged key which does NOT match this ticket.
The key SHOULD be the old one that matches the old ticket unless a new ticket is issued in the handshake
Dirty Workaround! If session state shows same ticket but different key, we keep prev state.

[wreuven]

I had another thought re: how to best maintain backwards compatibility in fixing the above.

As mentioned, most app developers assume they can call mbedtls_ssl_get_session to get the current session parameters, and then call mbedtls_ssl_set_session on the next connection to attempt TLS session reuse. For example, the popular libwebsockets does this. However, in the above described case, the bug is triggered that mbedtls_ssl_get_session returns a ticket and a master key that do not match. This results is all subsequent sessions failing with authentication errors.

It is conceivable that some program somewhere is using mbedtls_ssl_get_session to get the master key of the previous session. So instead of changing the key returned by mbedtls_ssl_get_session, perhaps mbedtls_ssl_get_session should add another new optional field called ticket_master_key which is only set if different than the master key. It is reasonable to assume that nobody calling mbedtls_ssl_set_session would purposefully want to use a ticket with the wrong key - so this function should be changed to use the ticket_master_key if that key is set.

[yanesca]

Thank you very much for the detailed information and the analysis. I think your original suggestion is the best way to go: we should remove the ticket from the session structure as soon as the client realises that the ticket won't be used. Otherwise the session structure state becomes inconsistent, which ultimately causes the unintended behaviour. Regarding backwards compatibility, this behaviour is clearly a bug, it is unlikely that a lot of users rely in this and should be justifiable to fix without offering the old behaviour. As far as I can tell the bug is present in later versions as well.