ssl: Remove use of insecure legacy option

Time to adopt to always use secure renegotiation for TLS-1.2
specified in RFC 5746 from 2010 is overdue.

Also remove documentation of legacy options handling
of Erlang distribution over TLS previous to OTP 20 that
mentions secure_renegotiate option and is now scheduled
for removal.

Author: IngelaAndin

lib/ssl/doc/guides/ssl_distribution.md

@@ -271,11 +271,6 @@ handling decoding errors and directly verifying signatures.
 For more details see `{verify_fun, Verify}` [in common_option_cert](`t:ssl:common_option_cert/0`)
 
 
-> #### Note {: .info }
-> The legacy command line format for `verify_fun` cannot be used
-> in a `-ssl_dist_optfile` file as described below in
-> [Specifying TLS Options (Legacy)](#specifying-tls-options-legacy).
-
 ## Using TLS distribution over IPv6
 
 It is possible to use TLS distribution over IPv6 instead of IPv4. To do this,
@@ -295,99 +290,3 @@ A node started in this way will only be able to communicate with other nodes
 using TLS distribution over IPv6.
 
 
-## Specifying TLS Options (Legacy)
-
-> #### Note {: .info }
-> The following section describes TLS Option handling prior to OTP 20.2
-> and can only handle a small subset of the actual available options.
-> It is here only for the sake of backwards compatibility .
-
-As in the previous section the PEM file `"/home/me/ssl/erlserver.pem"` contains
-both the server certificate and its private key.
-
-On the `erl` command line you can specify options that the TLS distribution adds
-when creating a socket.
-
-The simplest TLS options in the following list can be specified by adding the
-prefix `server_` or `client_` to the option name:
-
-- `certfile`
-- `keyfile`
-- `password`
-- `cacertfile`
-- `verify`
-- `verify_fun` (write as `{Module, Function, InitialUserState}`)
-- `crl_check`
-- `crl_cache` (write as Erlang term)
-- `reuse_sessions`
-- `secure_renegotiate`
-- `depth`
-- `hibernate_after`
-- `ciphers` (use old string format)
-
-Note that `verify_fun` needs to be written in a different form than the
-corresponding TLS option, since funs are not accepted on the command line.
-
-The server can also take the options `dhfile` and `fail_if_no_peer_cert` (also
-prefixed).
-
-`client_`\-prefixed options are used when the distribution initiates a
-connection to another node. `server_`\-prefixed options are used when accepting
-a connection from a remote node.
-
-Raw socket options, such as `packet` and `size` must not be specified on the
-command line.
-
-The command-line argument for specifying the TLS options is named
-`-ssl_dist_opt` and is to be followed by pairs of SSL options and their values.
-Argument `-ssl_dist_opt` can be repeated any number of times.
-
-An example command line doing the same as the example in the previous section
-can now look as follows (line breaks in the command are for readability, and
-shall not be there when typed):
-
-```text
-$ erl -boot /home/me/ssl/start_ssl -proto_dist inet_tls
-  -ssl_dist_opt server_certfile "/home/me/ssl/erlserver.pem"
-  -ssl_dist_opt server_secure_renegotiate true client_secure_renegotiate true
-  -sname ssl_test
-Erlang (BEAM) emulator version 5.0 [source]
-
-Eshell V5.0  (abort with ^G)
-(ssl_test@myhost)1>
-```
-
-### Setting up Environment to Always Use TLS
-
-A convenient way to specify arguments to Erlang is to use environment variable
-`ERL_FLAGS`. All the flags needed to use the TLS distribution can be specified
-in that variable and are then interpreted as command-line arguments for all
-subsequent invocations of Erlang.
-
-In a Unix (Bourne) shell, it can look as follows (line breaks are for
-readability, they are not to be there when typed):
-
-```erlang
-$ ERL_FLAGS="-boot /home/me/ssl/start_ssl -proto_dist inet_tls
-  -ssl_dist_opt server_certfile /home/me/ssl/erlserver.pem
-  -ssl_dist_opt server_secure_renegotiate true client_secure_renegotiate true"
-$ export ERL_FLAGS
-$ erl -sname ssl_test
-Erlang (BEAM) emulator version 5.0 [source]
-
-Eshell V5.0  (abort with ^G)
-(ssl_test@myhost)1> init:get_arguments().
-[{root,["/usr/local/erlang"]},
- {progname,["erl "]},
- {sname,["ssl_test"]},
- {boot,["/home/me/ssl/start_ssl"]},
- {proto_dist,["inet_tls"]},
- {ssl_dist_opt,["server_certfile","/home/me/ssl/erlserver.pem"]},
- {ssl_dist_opt,["server_secure_renegotiate","true",
-                "client_secure_renegotiate","true"]
- {home,["/home/me"]}]
-```
-
-The `init:get_arguments()` call verifies that the correct arguments are supplied
-to the emulator.
-

lib/ssl/src/ssl.erl

@@ -953,15 +953,13 @@ Options common to client and server side prior to TLS-1.3.
 
   Elliptic curves that can be used in pre TLS-1.3 key exchange.
 
-- **`{secure_renegotiate, SecureRenegotiate}`** - Inter-operate trade-off option
+- **`{secure_renegotiate, SecureRenegotiate}`** - Previous interoperability option
 
-  Specifies whether to reject renegotiation attempt that does not live
-  up to [RFC 5746](http://www.ietf.org/rfc/rfc5746.txt). By default,
-  `SecureRenegotiate` is `true`, meaning that secure renegotiation is
-  enforced. If `SecureRenegotiate` is `false` secure renegotiation
-  will still be used if possible, but it falls back to insecure
-  renegotiation if the peer does not support if [RFC
-  5746](http://www.ietf.org/rfc/rfc5746.txt).
+ Since OTP 29.0 setting this option to false will fail, that is accepting
+ possible fallback to insecure behavior preceding implementation of
+ [RFC 5746](http://www.ietf.org/rfc/rfc5746.txt) is no longer supported.
+ Setting it to true will continue to work but is not necessary, as the default
+ of `true` will now always be enforced.
 
 - **`{user_lookup_fun, {LookupFun, UserState}}`** - PSK/SRP cipher suite option
 
@@ -988,7 +986,7 @@ Options common to client and server side prior to TLS-1.3.
 """.
 
 -type common_option_pre_tls13() :: {eccs, NamedCurves::[named_curve()]} |
-                                   {secure_renegotiate, SecureRenegotiate::boolean()} |
+                                   {secure_renegotiate, SecureRenegotiate::true} |
                                    {user_lookup_fun, {Lookupfun :: fun(), UserState :: any()}}.
 
 -doc(#{group => 

lib/ssl/src/ssl_config.erl

@@ -1353,10 +1353,12 @@ opt_renegotiate(UserOpts, #{versions := Versions} = Opts, _Env) ->
     {_, RA0} = get_opt_pos_int(renegotiate_at, ?DEFAULT_RENEGOTIATE_AT, UserOpts, Opts),
     RA = min(RA0, ?DEFAULT_RENEGOTIATE_AT),  %% Override users choice without notifying ??
 
-    {Where3, SR} = get_opt_bool(secure_renegotiate, true, UserOpts, Opts),
+    {Where3, _} = disable_insecure_fallback(UserOpts, Opts),
     assert_version_dep(Where3 =:= new, secure_renegotiate, Versions, ['tlsv1','tlsv1.1','tlsv1.2']),
 
-    Opts#{secure_renegotiate => SR, key_update_at => KUA, renegotiate_at => RA}.
+    %% Do not include secure_renegotiate as option is no longer needed,
+    %% that is, it is treated as always set to true
+    Opts#{key_update_at => KUA, renegotiate_at => RA}.
 
 opt_reuse_sessions(UserOpts, #{versions := Versions} = Opts, #{role := client}) ->
     {Where1, RUSS} = get_opt_of(reuse_sessions, [true, false, save], true, UserOpts, Opts),
@@ -1388,6 +1390,15 @@ opt_identity(UserOpts, Opts, _Env) ->
     Lookup = handle_user_lookup(UserOpts, Opts),
     Opts#{psk_identity => PSK, srp_identity => SRP, user_lookup_fun => Lookup}.
 
+disable_insecure_fallback(UserOpts, Opts) ->
+    case get_opt_bool(secure_renegotiate, true, UserOpts, Opts) of
+        {_, true} = Result ->
+            Result;
+        {_, What} ->
+            option_error(secure_renegotiate,
+                         {What,
+                          fallback_to_insecure_renegotiation_no_longer_supported})
+    end.
 
 handle_psk(UserOpts, #{versions := Versions} = Opts) ->
     case get_opt_list(psk_identity, undefined, UserOpts, Opts) of

lib/ssl/src/ssl_handshake.erl

@@ -753,9 +753,9 @@ encode_extensions([#sni{hostname = ""} | Rest], Acc) ->
 encode_extensions([#sni{hostname = Hostname} | Rest], Acc) ->
     HostLen = length(Hostname),
     HostnameBin = list_to_binary(Hostname),
-                                                % Hostname type (1 byte) + Hostname length (2 bytes) + Hostname (HostLen bytes)
+    %% Hostname type (1 byte) + Hostname length (2 bytes) + Hostname (HostLen bytes)
     ServerNameLength = 1 + 2 + HostLen,
-                                                % ServerNameListSize (2 bytes) + ServerNameLength
+    %% ServerNameListSize (2 bytes) + ServerNameLength
     ExtLength = 2 + ServerNameLength,
     encode_extensions(Rest, <<?UINT16(?SNI_EXT), ?UINT16(ExtLength),
                               ?UINT16(ServerNameLength),
@@ -1561,7 +1561,7 @@ handle_client_hello_extensions(RecordCB, Random, ClientCipherSuites,
                                                        Random, NegotiatedCipherSuite,
                                                        ClientCipherSuites,
                                                        ConnectionStates0,
-                                                       Renegotiation, SecureRenegotation),
+                                                       Renegotiation),
 
     Empty = empty_extensions(Version, server_hello),
     {ServerMaxFragEnum, ConnectionStates, Session} =
@@ -1595,7 +1595,7 @@ handle_client_hello_extensions(RecordCB, Random, ClientCipherSuites,
 
 handle_server_hello_extensions(RecordCB, Random, CipherSuite,
                                Exts, Version,
-			       SslOpts,
+                               SslOpts,
 			       ConnectionStates0, Renegotiation, IsNew) ->
     ConnectionStates = handle_renegotiation_extension(client, RecordCB, Version,
                                                       maps:get(renegotiation_info, Exts, undefined),
@@ -1618,7 +1618,8 @@ handle_server_hello_extensions(RecordCB, Random, CipherSuite,
         undefined ->
             NextProtocolNegotiation = maps:get(next_protocol_negotiation, Exts, undefined),
             NextProtocolSelector = maps:get(next_protocol_selector, SslOpts, undefined),
-            Protocol = handle_next_protocol(NextProtocolNegotiation, NextProtocolSelector, Renegotiation),
+            Protocol =
+                handle_next_protocol(NextProtocolNegotiation, NextProtocolSelector, Renegotiation),
             {ConnectionStates, npn, Protocol, StaplingState};
         {error, Reason} ->
             throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, Reason));
@@ -3918,12 +3919,12 @@ handle_renegotiation_info(_, _RecordCB, server, undefined, ConnectionStates, _,
 	    {ok, ssl_record:set_renegotiation_flag(false, ConnectionStates)}
     end;
 
-handle_renegotiation_info(_, _RecordCB, _, undefined, ConnectionStates, false, _, _) ->
+handle_renegotiation_info(_, _RecordCB, _, undefined, ConnectionStates, false,_) ->
     {ok, ssl_record:set_renegotiation_flag(false, ConnectionStates)};
 
 handle_renegotiation_info(_, _RecordCB, client,
                           #renegotiation_info{renegotiated_connection = ClientServerVerify},
-			  ConnectionStates, true, _, _) ->
+                          ConnectionStates, true, _) ->
     #{reneg := ReNeg} = ssl_record:current_connection_state(ConnectionStates, read),
     #{client_verify_data := CData, server_verify_data := SData} = ReNeg,
     case <<CData/binary, SData/binary>> == ClientServerVerify of
@@ -3934,10 +3935,11 @@ handle_renegotiation_info(_, _RecordCB, client,
     end;
 handle_renegotiation_info(_, _RecordCB, server,
                           #renegotiation_info{renegotiated_connection = ClientVerify},
-			  ConnectionStates, true, _, CipherSuites) ->
+                          ConnectionStates, true, CipherSuites) ->
       case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
 	  true ->
-              throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv}));
+              throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE,
+                               {server_renegotiation, empty_renegotiation_info_scsv}));
 	  false ->
 	      case ssl_record:current_connection_state(ConnectionStates, read) of
 		  #{reneg := #{client_verify_data := ClientVerify}} ->
@@ -3946,28 +3948,17 @@ handle_renegotiation_info(_, _RecordCB, server,
                       throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, server_renegotiation))
 	      end
       end;
-handle_renegotiation_info(_, RecordCB, client, undefined, ConnectionStates, true, SecureRenegotation, _) ->
-    handle_renegotiation_info(RecordCB, ConnectionStates, SecureRenegotation);
-
-handle_renegotiation_info(_, RecordCB, server, undefined, ConnectionStates, true, SecureRenegotation, CipherSuites) ->
+handle_renegotiation_info(_, _, client, undefined, _, true, _) ->
+    throw(?ALERT_REC(?FATAL, ?NO_RENEGOTIATION, only_allow_secure_renegotiation));
+handle_renegotiation_info(_, _, server, undefined, _, true, CipherSuites) ->
      case is_member(?TLS_EMPTY_RENEGOTIATION_INFO_SCSV, CipherSuites) of
 	  true ->
-             throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, {server_renegotiation, empty_renegotiation_info_scsv}));
+             throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE,
+                              {server_renegotiation, empty_renegotiation_info_scsv}));
 	 false ->
-	     handle_renegotiation_info(RecordCB, ConnectionStates, SecureRenegotation)
+             throw(?ALERT_REC(?FATAL, ?NO_RENEGOTIATION, only_allow_secure_renegotiation))
      end.
 
-handle_renegotiation_info(_RecordCB, ConnectionStates, SecureRenegotation) ->
-    #{reneg := #{secure_renegotiation := SR}} = ssl_record:current_connection_state(ConnectionStates, read),
-    case {SecureRenegotation, SR} of
-	{_, true} ->
-            throw(?ALERT_REC(?FATAL, ?HANDSHAKE_FAILURE, already_secure));
-	{true, false} ->
-	    throw(?ALERT_REC(?FATAL, ?NO_RENEGOTIATION));
-	{false, false} ->
-	    {ok, ConnectionStates}
-    end.
-
 cert_curve(_, _, no_suite) ->
     {no_curve, no_suite};
 cert_curve(Cert, ECCCurve0, CipherSuite) ->

lib/ssl/test/ssl_api_SUITE.erl

@@ -3019,17 +3019,23 @@ options_debug(_Config) -> %% debug  log_level keep_secrets
     ok.
 
 options_renegotiate(_Config) -> %% key_update_at   renegotiate_at  secure_renegotiate
-    ?OK(#{key_update_at := ?KEY_USAGE_LIMIT_AES_GCM, renegotiate_at := 268435456, secure_renegotiate := true},
+    ?OK(#{key_update_at := ?KEY_USAGE_LIMIT_AES_GCM, renegotiate_at := 268435456},
         [], server),
-    ?OK(#{key_update_at := 123456, renegotiate_at := 64000, secure_renegotiate := false},
-        [{key_update_at, 123456}, {renegotiate_at, 64000}, {secure_renegotiate, false}],
+    ?OK(#{key_update_at := 123456, renegotiate_at := 64000},
+        [{key_update_at, 123456}, {renegotiate_at, 64000}, {secure_renegotiate, true}],
         server),
 
     %% Errors
     ?ERR({options, incompatible, [key_update_at, {versions, _}]},
          [{key_update_at, 123456}, {versions, ['tlsv1.2']}], server),
-    ?ERR({options, incompatible, [secure_renegotiate, {versions, _}]},
-         [{secure_renegotiate, true}, {versions, ['tlsv1.3']}], server),
+    ?ERR({options,
+          {secure_renegotiate,
+           {false,
+            fallback_to_insecure_renegotiation_no_longer_supported}
+          }},
+         [{secure_renegotiate, false}], client),
+     ?ERR({options, {no_supported_algorithms, {signature_algs,[]}}},
+          [{signature_algs, []}], client),
 
     ?ERR({key_update_at, -1}, [{key_update_at, -1}], server),
     ?ERR({renegotiate_at, not_a_int}, [{renegotiate_at, not_a_int}], server),
@@ -3690,7 +3696,7 @@ invalid_options_tls13(Config) when is_list(Config) ->
           {options, incompatible, [reuse_sessions, {versions,['tlsv1.3']}]},
           common},
 
-         {{secure_renegotiate, false},
+         {{secure_renegotiate, true},
           {options, incompatible, [secure_renegotiate, {versions,['tlsv1.3']}]},
           common},
 

lib/ssl/test/ssl_renegotiate_SUITE.erl

@@ -49,8 +49,6 @@
          server_renegotiate/1,
          client_secure_renegotiate/0,
          client_secure_renegotiate/1,
-         client_secure_renegotiate_fallback/0,
-         client_secure_renegotiate_fallback/1,
          client_renegotiate_reused_session/0,
          client_renegotiate_reused_session/1,
          server_renegotiate_reused_session/0,
@@ -110,7 +108,6 @@ renegotiate_tests() ->
     [client_renegotiate,
      server_renegotiate,
      client_secure_renegotiate,
-     client_secure_renegotiate_fallback,
      client_renegotiate_reused_session,
      server_renegotiate_reused_session,
      client_no_wrap_sequence_number,
@@ -211,37 +208,6 @@ client_secure_renegotiate(Config) when is_list(Config) ->
     ssl_test_lib:close(Server),
     ssl_test_lib:close(Client).
 
-%%--------------------------------------------------------------------
-client_secure_renegotiate_fallback() ->
-    [{doc,"Test that we can set secure_renegotiate to false that is "
-      "fallback option, we however do not have a insecure server to test against!"}].
-client_secure_renegotiate_fallback(Config) when is_list(Config) ->
-    ServerOpts = ssl_test_lib:ssl_options(server_rsa_verify_opts, Config),
-    ClientOpts = ssl_test_lib:ssl_options(client_rsa_verify_opts, Config),
-
-    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
-
-    Data = "From erlang to erlang",
-
-    Server =
-	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
-				   {from, self()},
-				   {mfa, {?MODULE, erlang_ssl_receive, [Data]}},
-				   {options, [{secure_renegotiate, false} | ServerOpts]}]),
-    Port = ssl_test_lib:inet_port(Server),
-
-    Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
-					{host, Hostname},
-					{from, self()},
-					{mfa, {?MODULE,
-					       renegotiate, [Data]}},
-					{options, [{reuse_sessions, false},
-						   {secure_renegotiate, false}| ClientOpts]}]),
-    
-    ssl_test_lib:check_result(Client, ok, Server, ok),
-    ssl_test_lib:close(Server),
-    ssl_test_lib:close(Client).
-
 %%--------------------------------------------------------------------
 server_renegotiate() ->
     [{doc,"Test ssl:renegotiate/1 on server."}].