Conversation
|
Please find similar changes required on Plug Cowboy elixir-plug/plug_cowboy#63 |
|
/cc @voltone |
| options | ||
| |> Keyword.put_new(:secure_renegotiate, true) | ||
| |> Keyword.put_new(:reuse_sessions, true) | ||
| if options[:versions] == [:"tlsv1.3"] do |
There was a problem hiding this comment.
I think that it should check whether the options contain earlier versions instead, as I assume that if there will ever be TLSv1.4 it will not support these options as well.
There was a problem hiding this comment.
I agree, this should probably be something like:
| if options[:versions] == [:"tlsv1.3"] do | |
| if !Enum.any?([:tlsv1, :"tlsv1.1", :"tlsv1.2"], &(&1 in options[:versions])) do |
(Or rather, swap the do and else parts, and remove the negation)
No point in doing a MapSet dance here, is there? This is typically checked just once on server startup...
There was a problem hiding this comment.
Does :versions have to be set? I think that may not be the case.
| if options[:versions] == [:"tlsv1.3"] do | |
| versions = options[:versions] | |
| if versions && not Enum.any?(...) do |
WDYT?
There was a problem hiding this comment.
I was wondering the same thing. But if it's not set, then we don't know whether these options should be set or not. For that I think we'd have to pick up the default with :ssl.versions()[:supported]...
| if options[:versions] == [:"tlsv1.3"] do | |
| versions = Keyword.get(options, :versions, :ssl.versions()[:supported]) | |
| if not Enum.any?(...) do |
Another thing: technically :ssl options are not a proplist, so not sure we're supposed to use Keyword on it?
|
Ping @adrigonzo! We only need to address @hauleth’s comment and we are good to go! |
|
We haven't tested with any other version combinations. You're right though, if future versions come along, this would need another update. Will take a look at the option to revert the check for 1, 1.1 and 1.2. I think the oh, sorry, just realised you did another PR for that. Thank you! |
4 participants
secure_renegotiate and reuse_sessions 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.