elixir-plug / plug_cowboy

Plug adapter for the Cowboy web server
Other
243 stars 48 forks source link

fix for TLS 1.3 #63

Open adrigonzo opened 3 years ago

adrigonzo commented 3 years ago

next_protocols_advertised and alpn_preferred_protocols options are not supported by the OTP SSL module when earlier version of TLS are not being used. (i.e. when we specify only TSL1.3 version, without TLS1.2 or earlier versions). It seems TLS1.2 or earlier must ALSO be specified for this to work, since it's not supported in TLS1.3. Hence, adding a check whether TLS1.3 is the ONLY version being used.

adrigonzo commented 3 years ago

Please find similar fix on Plug https://github.com/elixir-plug/plug/pull/1011

josevalim commented 3 years ago

Thank you @adrigonzo! Do you know if those will never be supported or they just are not temporarily supported?

adrigonzo commented 3 years ago

Looking into it, it seems like it's more to do with the implementation in the version of SSL being used with OTP23. ALPN seems like an extension that should be supported on TLS1.3 too, so I guess that may change in the future. This PR may be redundant then, but may be helpful for anyone that comes across the same error we were seeing:

Failed to start Ranch listener MyEndpoint.HTTPS in :ranch_ssl:listen(%{max_connections: 16384, num_acceptors: 12, socket_opts: 
[cacerts: :..., key: :..., cert: :..., alpn_preferred_protocols: ["h2", "http/1.1"], next_protocols_advertised: ["h2", "http/1.1"], certfile: 
'/system/ssl/self_signed_ssl_cert.pem', keyfile: '/system/ssl/self_signed_ssl_key.pem', port: 443, ciphers: [{:dhe_rsa, ...}, {...}, ...], 
versions: [:"tlsv1.3"], ip: {0, 0, 0, 0, 0, 0, 0, 0}, honor_cipher_order: true]}) for reason {:options, :dependency, 
{:next_protocols_advertised, {:versions, [:tlsv1, :"tlsv1.1", :"tlsv1.2"]}}} (unknown POSIX error)

and similar for :alpn_preferred_protocols

josevalim commented 3 years ago

Thanks! So the issue with the current PR is that it will be insecure once the TLS 1.3 implementation catches up? I wonder if we should instead instruct the users to set those options to an empty list? And then we can remove them if an empty list or nil are set? 🤔

adrigonzo commented 3 years ago

I found that the additional options cause the above error whether set to empty list or to false: