nickva commented on a change in pull request #5:
URL: https://github.com/apache/couchdb-mochiweb/pull/5#discussion_r640036832



##########
File path: src/mochiweb_socket.erl
##########
@@ -29,11 +29,22 @@ listen(Ssl, Port, Opts, SslOpts) ->
             gen_tcp:listen(Port, Opts)
     end.
 
+-ifdef(new_ssl_unavailable).
 add_unbroken_ciphers_default(Opts) ->
     Default = filter_unsecure_cipher_suites(ssl:cipher_suites()),
     Ciphers = filter_broken_cipher_suites(proplists:get_value(ciphers, Opts, 
Default)),
     [{ciphers, Ciphers} | proplists:delete(ciphers, Opts)].
 
+-else.
+add_unbroken_ciphers_default(Opts) ->
+    CipherSuitesListMap = ssl:cipher_suites(default, 'tlsv1.3'),
+    CipherSuitesList = [{KeyExchange, Cipher, Mac, Prf} || #{cipher := Cipher, 
key_exchange := KeyExchange, mac := Mac, prf := Prf} <- CipherSuitesListMap],
+    Default = filter_unsecure_cipher_suites(CipherSuitesList),
+    Ciphers = filter_broken_cipher_suites(proplists:get_value(ciphers, Opts, 
Default)),
+    [{ciphers, Ciphers} | proplists:delete(ciphers, Opts)].

Review comment:
       This is a bit tricky. I wonder if connect method on 23 (where ` 
ssl:cipher_suites/0` returns tuples) would accept ciphers in a tuple format for 
the connect method, I think it should or mochiweb would break. They just don't 
document it I think. On 24 it might not. So if we always translate them to 
tuples, it might not work. 
   
   I wonder if we can filter both tuples and maps with a few changes and pass 
in maps on Erlang 20+ and handle tuples on Erlang < 20.
   
   I think for `case proplists:get_value(ssl_app, ssl:versions()) of "5.3" ++ _ 
...` we don't have to worry about as ssh app version 5.3 only accepts tuples. 
Erlang 20 has ssl app version as 8.2.x.x.
   
   `filter_unsecure_cipher_suites(Ciphers) ->` could be handled by adding extra 
clauses to the fun:
   
   ```
   (#{key_exchange := des_cbc}) -> false;
   (#{mac := md5}) -> false;
   ```
   
   With a comment saying we are filtering both old style tuple cipher suites 
and new map ones
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to