Disallow unsupported options for TLSv1.3

[Maria-12648430]

When 'tlsv1.3' is the only protocol version (either in the versions key in given socket options, in the protocol_version key in the ssl application environment, or in the supported key in the list returned from ssl:versions/0, whichever is set first, in that order), the following options are additionally disallowed:

• alpn_preferred_protocols
• next_protocols_advertised
• reuse_sessions
• secure_renegotiate

However, I have no idea how this could be tested 😓.

[Maria-12648430]

... and something is wrong with the Alpine agent I guess? 🙄

[Maria-12648430]

Wait a minute... I just checked the OTP ssl source, and it looks like the alpn_preferred_protocols option is not unsupported/rejected in conjunction with TLSv1.3 (and yes, I tried, it is accepted). But there are some other options not listed in #313 which are unsupported, like psk_identity or reuse_session (without the "s"). Search for calls to assert_option_dependency.

There are options that are allowed for TLSv1.3 only (naturally). We could go all the way and filter them out the same way if TLSv1.3 is not among the protocols to be used, but that may be dangerous? It could lead to a user accidentially relying on TLSv1.3 features which are then not used, and dropped with only a warning, I mean? @essen, what do you think?

In any case, further research is needed.

[essen]

Yes alpn_preferred_protocols should be supported. ALPN works, NPN shouldn't.

There are also some ciphers that work with TLS1.2 but not 1.3 and vice versa if I recall. So yes definitely needs some research.

[Maria-12648430]

And there is other stuff that works only in conjunction with yet other stuff, or needs to be set to this-and-that when some-and-such is set to whatever... Are you sure we should check all that? The crosslinks/interdependencies are mostly undocumented, so it will have to be either tried or ferretted out from the ssl source, plus we would have to follow it very closely to keep ranch up to date.

[essen]

https://rabbitmq.com/ssl.html#tls1.3 might help.

I think it's OK if bad combinations are not detected. We should only focus on options no longer working because they were explicitly removed from the TLS 1.3 spec.

https://datatracker.ietf.org/doc/html/rfc8446#section-1.2

So that's reuse_sessions and secure_renegotiate (see page 28) covered. Note that at least renegotiation is a bad practice in any case. I'm not sure about reuse_sessions, I suppose the psk_identity option is what replaces it?

NPN is not explicitly stated but it was replaced by ALPN many years ago and the spec only mentions ALPN so if it doesn't work with TLS 1.3 that's why.

A test for each of these can be written like this: try the option with plain ssl, confirm the option doesn't work (can't listen), then setup Ranch with that option and confirm Ranch still inits properly and can be connected to.

[Maria-12648430]

The options unsupported by TLSv1.3 that are checked by ssl are:

• beast_mitigation (v1 only)
• client_renegotiation
• next_protocols_advertised
• next_protocol_selector (not in the docs)
• padding_check (v1 only)
• psk_identity
• secure_renegotiate
• srp_identity (already disallowed by ranch)
• reuse_session and reuse_sessions
• user_lookup_fun

[essen]

OK I don't think we need to block next_protocol_selector. I am wondering what option replaces reuse_session(s) if not psk_identity? Do you know?

[Maria-12648430]

OK I don't think we need to block next_protocol_selector. I am wondering what option replaces reuse_session(s) if not psk_identity? Do you know?

No idea. I just grepped this from the ssl source.

[essen]

session_tickets / use_ticket https://erlang.org/doc/apps/ssl/using_ssl.html#session-tickets-and-session-resumption-in-tls-1.3

Sounds like it's all fine for the server but would need some special handling in Gun for the manual case.

OK so we can just block everything then on the list minus next_protocol_selector then.

[Maria-12648430]

Ok, updated. What do you think?

I'm not fully satisfied with the version/support check, I'll try to think of something better :) But you can take a look already ;)

[essen]

I don't know what's wrong with the tests but otherwise I'm OK with the PR.

[Maria-12648430]

I don't know what's wrong with the tests

The problem is the check that usage of non-TLSv1.3 options fail when used in "plain" ssl, but that does not happen (ie, they are accepted) in OTP <23. I'm at it.

but otherwise I'm OK with the PR.

I have some improvements coming up, will explain more when pushed.

[Maria-12648430]

Ok, last commit skips the test if ssl version is <10.0 (ie, OTP<23). Let's see if this passes.

Also, I improved the matching on the protocol version(s). They are usorted first ( usort because the same protocol could potentially be given more than once), then the match happens on the lowest protocol in the sorted list. If that is tlsv1, we disallow nothing more, if it is tlsv1.1 or tlsv1.2, we additionally disallow beat_mitigation and padding_check, if it is tlsv1.3, we disallow all the options listed above. This way, options from earlier protocols than the lowest requested one are removed with a warning, but options from later protocols won't be and cause a dependency error (which is good, removing them might pose a security risk). If ever TLSv1.4 comes around, we will have to add another clause, but everything up to TLSv1.3 will keep working.

[essen]

Merged, thanks!

[Maria-12648430]

Progress 🤩