Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for TLS 1.3 #63

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adrigonzo
Copy link

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
Copy link
Author

Please find similar fix on Plug elixir-plug/plug#1011

@josevalim
Copy link
Member

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

@adrigonzo
Copy link
Author

adrigonzo commented Feb 8, 2021

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
Copy link
Member

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
Copy link
Author

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

  • secure_renegotiate
  • reuse_sessions
  • next_protocols_advertised
  • alpn_preferred_protocols
    This is why I ended up needing to remove them directly from Plug and Plug Cowboy. I actually started by removing them in the ranch package first, but I think since they're inserted in Plug and Plug Cowboy, then that's the more correct place for the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants