TLS 1.3 middlebox incompatibilities

[dotsimon]

There are two related bugs - one server side, one client side.

Describe the server bug

OTP 25.? changes the SSL server behaviour in deciding if/when to send change_cipher_spec
The simplistic explanation of that being a client problem given in #6374 ignores the real world.

Although RFC 8446 D.4 describes a method for "partially negotiated" middlebox compatibility, it also states:

Either side can send change_cipher_spec at any time during the handshake

Furthermore, section 5 states an implementation receiving change_cipher_spec

MUST simply drop it without further processing

For this reason many servers send change_cipher_spec regardless - OTP 24 is an example, as is www.google.com

The bug is being too pedantic in interpreting D.4, introducing a breaking change for cases where the clients can't (easily) be changed.

Describe the client bug

OTP 25.2 and also 24.3.4.8 have the change introduced for #6241
The problem is that this change violates the handling of change_cipher_spec stated in section 5.

A TLS 1.3 client MUST drop change_cipher_spec

To Reproduce
Connect to an OTP 24 server or, you know, the internet in general

1> ssl:start(), ssl:connect("www.google.com", 443, [{verify,verify_none},{middlebox_comp_mode,false}], 1000).
=NOTICE REPORT==== 1-Feb-2023::17:29:17.067186 ===
TLS client: In state wait_ee at ssl_gen_statem.erl:738 generated CLIENT ALERT: Fatal - Unexpected Message
 - {unexpected_msg,{internal,{change_cipher_spec,<<1>>}}}
{error,{tls_alert,{unexpected_message,"TLS client: In state wait_ee at ssl_gen_statem.erl:738 generated CLIENT ALERT: Fatal - Unexpected Message\n {unexpected_msg,{internal,{change_cipher_spec,<<1>>}}}"}}}

Expected behavior

• Regardless of middlebox_comp_mode the client MUST ignore change_cipher_spec
• The server should send change_cipher_spec when middlebox_comp_mode := true for greater compatibility.
  In any case the behavioural change should have been clearly documented.

Affected versions
OTP 25.2 and OTP 24.4.3.8

[IngelaAndin]

Hum ... you might be right that the current implementation could be too strict, we will look into it. The change was not viewed as behavior change as the change_cipher_spec is not expected unless middlebox_comp_mode is indicated by the client.

[dotsimon]

I don't disagree that the server side change could be argued either way if it is a "bug" or not.
But, it was a behavioural change that did not improve security or conformance. Apparently a number of other big/well known implementations adopt the "looser" approach (sending change_cipher_spec regardless) that OTP used to. I would say then that the behaviour should be reverted when the server has {middlebox_comp_mode,true}

[IngelaAndin]

I do not think we need to send change_cipher_spec regardless, that I think would be a bug. But what we can do is to always disregard such a message regardless of if middle-box mode was negotiated (or rather mandated by the client) or not.

[timofey-barmin]

Thanks a lot @dotsimon and @IngelaAndin

As you mentioned in the ticket:

introducing a breaking change for cases where the clients can't (easily) be changed.

Exactly! and I still can see the problem with OTP-24.3.4.4 clients.
When such client connects to OTP-25.3 server, I see CLIENT ALERT with {unexpected_msg,{internal,{encrypted_extensions,#{}}}} (which is #6241).

From Ingela's explanation I understood that the bug is in client, but not everybody can change/upgrade the client (product that uses 24.3.4.4 client is already released). In our case in order to upgrade we need to establish a TLS connection. And that is a big, big, big problem.

Basically new erlang is still incompatible with previous version of erlang.

Could you please suggest a workaround that can be implemented on the server to make it possible to accept connections from 24.3.4.4 clients?

Setting server TLS version to 1.2 is not an option for us because we need support TLS 1.3. Setting TLS version to 1.3 only on the server does not help. Setting middlebox_comp_mode to false on the server doesn't help either.

[IngelaAndin]

@timofey-barmin If you want to work around the bug you need to be able configure the clients, even if you can not upgrade them. I can see two thing that the clients can do that could help. You could make the clients not try to reuse TLS-1.2 sessions or make the clients support only one of the versions TLS-1.2 or TLS-1.3. As you said you can not disable middlebox_comp_mode on the server as it is the client behavior that mandates the middlebox compatible mode and the server has to adhere.

[timofey-barmin]

@IngelaAndin
Thanks but unfortunately we can't reconfigure the client. Do I understand correctly that this incompatibility can be resolved if server starts sending change_cipher_spec? It seems like other servers send it (for example I see change_cipher_spec when connecting to "openssl s_server" and to google, "bad" erlang client can establish a TLS connection without any problems to those servers).
If so, I agree with @dotsimon on the following:

The server should send change_cipher_spec when middlebox_comp_mode := true for greater compatibility.

If you are absolutely sure you don't want to fix it in mainstream, could you please suggest a change for server that we can apply on our fork only? I guess this should be a pretty small change but still it would be a huge help.
In my opinion it should be fixed in mainstream though.
Thanks.

[dotsimon]

@timofey-barmin since the spec clearly states the client MUST ignore change_cipher_spec, my previous solution on 25.2 was to always send it - the same way you noticed that google and openssl's test server behave.

The code was something like this:

diff --git a/lib/ssl/src/tls_handshake_1_3.erl b/lib/ssl/src/tls_handshake_1_3.erl
index 176d659..32435e9 100644
--- a/lib/ssl/src/tls_handshake_1_3.erl
+++ b/lib/ssl/src/tls_handshake_1_3.erl
@@ -1204,7 +1204,8 @@ maybe_append_change_cipher_spec(#state{
                                    session = #session{session_id = Id},
                                     handshake_env =
                                         #handshake_env{
-                                           change_cipher_spec_sent = false} = HSEnv} = State, Bin) when Id =/= ?EMPTY_ID  ->
+                                           change_cipher_spec_sent = false} = HSEnv} = State, Bin)
+  when Id =/= ?EMPTY_ID orelse (State#state.static_env)#static_env.role == server ->
     CCSBin = create_change_cipher_spec(State),
     {State#state{handshake_env =
                      HSEnv#handshake_env{change_cipher_spec_sent = true}},

My interpretation of the change in OTP (25.2 & 24.3.4.8) was that it did not improve security or conformance
I suggested making this configurable via middlebox_comp_mode in order to coax the OTP team to consider changing the server behaviour to something more universally compatible.
Hopefully this diff helps you.

[timofey-barmin]

@dotsimon
Thank you so much! It's been working great so far.
I will report it here if I notice any issues in future.

[dave-finlay]

@dotsimon : just a word of thanks for your extremely valuable help with this issue. It is very much appreciated.

[IngelaAndin]

@timofey-barmin We will think about adding the server workaround option suggested by @dotsimon as it seems it can cause such awkward interop issues once we finished some pressing OTP-26 work.