ssl: Error server options when no certs

dgud · dgud · commit 51ad60f731b2 · 2023-11-30T16:22:37.000+01:00
When running a ssl server the user must provide cert and key or
use an anonymous cipher in tls1.2. Otherwise no connection will
succeed.

Add an option check so that this is dectected earlier, and gives
the user an appropriate error instead of just failing each connection
attempt.

To keep backwards compatibility the check is only done in handshake,
since it is allowed to use an empty (or minimal) option list in
ssl:listen and provide the options in handshake later.
diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
@@ -1621,7 +1621,8 @@ ssl_options() ->
 -spec update_options([any()], client | server, map()) -> map().
 update_options(Opts, Role, InheritedSslOpts) when is_map(InheritedSslOpts) ->
     {UserSslOpts, _} = split_options(Opts, ssl_options()),
-    process_options(UserSslOpts, InheritedSslOpts, #{role => Role}).
+    Env = #{role => Role, validate_certs_or_anon_ciphers => Role == server},
+    process_options(UserSslOpts, InheritedSslOpts, Env).
 
 process_options(UserSslOpts, SslOpts0, Env) ->
     %% Reverse option list so we get the last set option if set twice,
@@ -1646,6 +1647,7 @@ process_options(UserSslOpts, SslOpts0, Env) ->
     SslOpts17 = opt_handshake(UserSslOptsMap, SslOpts16, Env),
     SslOpts18 = opt_use_srtp(UserSslOptsMap, SslOpts17, Env),
     SslOpts = opt_process(UserSslOptsMap, SslOpts18, Env),
+    validate_server_cert_opts(SslOpts, Env),
     SslOpts.
 
 -spec handle_options([any()], client | server, undefined|host()) -> {ok, #config{}}.
@@ -1655,8 +1657,10 @@ handle_options(Opts, Role, Host) ->
 %% Handle all options in listen, connect and handshake
 handle_options(Transport, Socket, Opts0, Role, Host) ->
     {UserSslOptsList, SockOpts0} = split_options(Opts0, ssl_options()),
-
-    Env = #{role => Role, host => Host},
+    NeedValidate = not (Socket == undefined) andalso Role =:= server, %% handshake options
+    Env = #{role => Role, host => Host,
+            validate_certs_or_anon_ciphers => NeedValidate
+           },
     SslOpts = process_options(UserSslOptsList, #{}, Env),
 
     %% Handle special options
@@ -2614,6 +2618,36 @@ validate_filename([_|_] = FN, _Option) ->
 validate_filename(FN, Option) ->
     option_error(Option, FN).
 
+validate_server_cert_opts(_Opts, #{validate_certs_or_anon_ciphers := false}) ->
+    ok;
+validate_server_cert_opts(#{certs_keys := [_|_]=CertsKeys, ciphers := CPHS, versions := Versions}, _) ->
+    validate_certs_or_anon_ciphers(CertsKeys, CPHS, Versions);
+validate_server_cert_opts(#{ciphers := CPHS, versions := Versions}, _) ->
+    validate_anon_ciphers(CPHS, Versions).
+
+validate_certs_or_anon_ciphers(CertsKeys, Ciphers, Versions) ->
+    CheckCertsAndKeys =
+        fun(Map) ->
+                (maps:is_key(cert, Map) orelse maps:is_key(certfile, Map))
+                    andalso (maps:is_key(key, Map) orelse maps:is_key(keyfile, Map))
+        end,
+    case lists:any(CheckCertsAndKeys, CertsKeys) of
+        true -> ok;
+        false -> validate_anon_ciphers(Ciphers, Versions)
+    end.
+
+validate_anon_ciphers(Ciphers, Versions) ->
+    MakeSet = fun(Version, Acc) ->
+                      Set = sets:from_list(ssl_cipher:anonymous_suites(Version), [{version, 2}]),
+                      sets:union(Set, Acc)
+              end,
+    Anonymous = lists:foldl(MakeSet, sets:new([{version, 2}]), Versions),
+    CiphersSet = sets:from_list(Ciphers, [{version,2}]),
+    case sets:is_disjoint(Anonymous, CiphersSet) of
+        false -> ok;
+        true -> option_error(certs_keys, no_cert_or_key_given)
+    end.
+
 %% Do not allow configuration of TLS 1.3 with a gap where TLS 1.2 is not supported
 %% as that configuration can trigger the built in version downgrade protection
 %% mechanism and the handshake can fail with an Illegal Parameter alert.
diff --git a/lib/ssl/src/ssl_cipher.erl b/lib/ssl/src/ssl_cipher.erl
@@ -344,15 +344,15 @@ tls_legacy_suites(Version) ->
 %%--------------------------------------------------------------------
 
 anonymous_suites(Version) when ?TLS_1_X(Version) ->
-    SuitesToTest = anonymous_suite_to_test(Version),
-    lists:flatmap(fun tls_v1:exclusive_anonymous_suites/1, SuitesToTest);
+    Versions = versions_included(Version),
+    lists:flatmap(fun tls_v1:exclusive_anonymous_suites/1, Versions);
 anonymous_suites(Version) when ?DTLS_1_X(Version) ->
     dtls_v1:anonymous_suites(Version).
 
-anonymous_suite_to_test(?TLS_1_0) -> [?TLS_1_0];
-anonymous_suite_to_test(?TLS_1_1) -> [?TLS_1_1, ?TLS_1_0];
-anonymous_suite_to_test(?TLS_1_2) -> [?TLS_1_2, ?TLS_1_1, ?TLS_1_0];
-anonymous_suite_to_test(?TLS_1_3) -> [?TLS_1_3].
+versions_included(?TLS_1_0) -> [?TLS_1_0];
+versions_included(?TLS_1_1) -> [?TLS_1_1, ?TLS_1_0];
+versions_included(?TLS_1_2) -> [?TLS_1_2, ?TLS_1_1, ?TLS_1_0];
+versions_included(?TLS_1_3) -> [?TLS_1_3].
 
 %%--------------------------------------------------------------------
 -spec filter(undefined | binary(), [ssl_cipher_format:cipher_suite()],
diff --git a/lib/ssl/test/ssl_api_SUITE.erl b/lib/ssl/test/ssl_api_SUITE.erl
@@ -1684,9 +1684,10 @@ close_with_timeout(Config) when is_list(Config) ->
 close_in_error_state() ->
     [{doc,"Special case of closing socket in error state"}].
 close_in_error_state(Config) when is_list(Config) ->
-    ServerOpts0 = ssl_test_lib:ssl_options(server_opts, Config),
+    ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config),
     ServerOpts = [{cacertfile, "foo.pem"} | proplists:delete(cacertfile, ServerOpts0)],
     ClientOpts = ssl_test_lib:ssl_options(client_opts, Config),
+
     _ = spawn(?MODULE, run_error_server_close, [[self() | ServerOpts]]),
     receive
         {_Pid, Port} ->
@@ -1703,7 +1704,7 @@ close_in_error_state(Config) when is_list(Config) ->
 call_in_error_state() ->
     [{doc,"Special case of call error handling"}].
 call_in_error_state(Config) when is_list(Config) ->
-    ServerOpts0 = ssl_test_lib:ssl_options(server_opts, Config),
+    ServerOpts0 = ssl_test_lib:ssl_options(server_rsa_opts, Config),
     ClientOpts = ssl_test_lib:ssl_options(client_opts, Config),
     ServerOpts = [{cacertfile, "foo.pem"} | proplists:delete(cacertfile, ServerOpts0)],
     Pid = spawn(?MODULE, run_error_server, [[self() | ServerOpts]]),
@@ -2187,27 +2188,44 @@ options_whitebox() ->
 customize_defaults(Opts, Role, Host) ->
     %% In many options test scenarios we do not care about verifcation options
     %% but the client now requiers verification options by default.
-    ClientIgnorDef = case proplists:get_value(verify, Opts, undefined) of
-                         undefined when Role == client ->
-                             [{verify, verify_none}];
-                         _ ->
-                             []
-                     end,
+    DefOpts = case Role of
+                  client ->
+                      case proplists:get_value(verify, Opts, undefined) of
+                          undefined -> [{verify, verify_none}];
+                          _ -> []
+                      end;
+                  server ->
+                      Ciphers = proplists:get_value(ciphers, Opts, undefined),
+                      Cert = proplists:get_value(cert, Opts, undefined),
+                      Key = proplists:get_value(key, Opts, undefined),
+                      CertsKeys = proplists:get_value(certs_keys, Opts, undefined),
+                      NoCertOrKeys = Cert == undefined orelse Key == undefined andalso
+                          CertsKeys == undefined,
+                      if Ciphers == undefined andalso NoCertOrKeys ->
+                              [{certs_keys, [#{cert => <<>>, key => {rsa, <<>>}}]}];
+                         true ->
+                              []
+                      end
+              end,
+    NoVerify = case Role of
+                   client -> [{verify, verify_none}|DefOpts];
+                   server -> DefOpts
+               end,
     case proplists:get_value(protocol, Opts, tls) of
         dtls ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options([{verify, verify_none}, {protocol, dtls}], Role, Host),
-            {DOpts, ClientIgnorDef ++ Opts};
+            {ok, #config{ssl=DOpts}} = ssl:handle_options([{protocol, dtls}|NoVerify], Role, Host),
+            {DOpts, DefOpts ++ Opts};
         tls ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options([{verify, verify_none}], Role, Host),
+            {ok, #config{ssl=DOpts}} = ssl:handle_options(NoVerify, Role, Host),
             case proplists:get_value(versions, Opts) of
                 undefined ->
-                    {DOpts, ClientIgnorDef ++ [{versions, ['tlsv1.2','tlsv1.3']}|Opts]};
+                    {DOpts, DefOpts ++ [{versions, ['tlsv1.2','tlsv1.3']}|Opts]};
                 _ ->
-                    {DOpts, ClientIgnorDef ++ Opts}
+                    {DOpts, DefOpts ++ Opts}
             end;
         _ ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options(ClientIgnorDef, Role, Host),
-            {DOpts, ClientIgnorDef ++ Opts}
+            {ok, #config{ssl=DOpts}} = ssl:handle_options(NoVerify, Role, Host),
+            {DOpts, DefOpts ++ Opts}
     end.
 
 -define(OK(EXP, Opts, Role), ?OK(EXP,Opts, Role, [])).
@@ -2279,6 +2297,41 @@ customize_defaults(Opts, Role, Host) ->
                 end
         end()).
 
+-define(ERR_UPD(EXP, Opts, Role),
+        fun() ->
+                Host = "dummy.host.org",
+                {__DefOpts, __Opts} = customize_defaults(Opts, Role, Host),
+                try ssl:handle_options(__Opts, Role, Host) of
+                    {ok, #config{}} ->
+                        ok;
+                    Other ->
+                        ?CT_PAL("ssl:handle_options(~0p,~0p,~0p).",[__Opts,Role,Host]),
+                        error({unexpected, Other})
+                catch
+                    throw:{error,{options,{insufficient_crypto_support,{'tlsv1.3',_}}}} -> ignored;
+                    C:Other:ST ->
+                        ?CT_PAL("ssl:handle_options(~0p,~0p,~0p).",[__Opts,Role,Host]),
+                        error({unexpected, C, Other,ST})
+                end,
+                try ssl:update_options(__Opts, Role, __DefOpts) of
+                    Other2 ->
+                        ?CT_PAL("{ok,Cfg} = ssl:handle_options([],~p,~p),"
+                                "ssl:update_options(~p,~p, element(2,Cfg)).",
+                                [Role,Host,__Opts,Role]),
+                        error({unexpected, Other2})
+                catch
+                    throw:{error,{options,{insufficient_crypto_support,{'tlsv1.3',_}}}} -> ignored;
+                    throw:{error, {options, EXP}} -> ok;
+                    throw:{error, EXP} -> ok;
+                    C2:Other2:ST2 ->
+                        ?CT_PAL("{ok,Cfg} = ssl:handle_options([],~p,~p),"
+                                "ssl:update_options(~p,~p, element(2,Cfg)).",
+                                [Role,Host,__Opts,Role]),
+                        error({unexpected, C2, Other2,ST2})
+                end
+        end()).
+
+
 options_whitebox(Config) when is_list(Config) ->
     Cert = proplists:get_value(cert, ssl_test_lib:ssl_options(server_rsa_der_opts, Config)),
     true = is_binary(Cert),
@@ -2520,6 +2573,7 @@ options_cert(Config) -> %% cert[file] cert_keys keys password
     ?ERR({cert, #{}}, [{cert, #{}}], client),
     ?ERR({certfile, cert}, [{certfile, cert}], client),
     ?ERR({certs_keys, #{}}, [{certs_keys, #{}}], client),
+    ?ERR_UPD({certs_keys, no_cert_or_key_given}, [{certs_keys, []}], server),
     ?ERR({keyfile, #{}}, [{keyfile, #{}}], client),
     ?ERR({key, <<>>}, [{key, <<>>}], client),
     ?ERR({password, _}, [{password, fun(Arg) -> Arg end}], client),
@@ -2548,7 +2602,11 @@ options_ciphers(_Config) ->
     ?OK(#{ciphers := [_|_]}, [{ciphers, "RC4-SHA:RC4-MD5"}], client),
     ?OK(#{ciphers := [_|_]}, [{ciphers, ["RC4-SHA", "RC4-MD5"]}], client),
 
-    %% FIXME extend this
+    ?OK(#{ciphers := [_|_]}, [{ciphers, ["TLS_DH_anon_WITH_AES_256_CBC_SHA256"]}], server),
+    %% Errors
+    ?ERR({ciphers, _}, [{ciphers, "foobar:RC4-MD5"}], client),
+    ?ERR({ciphers, _}, [{ciphers, ["RC4-SHA:RC4-MD5", "RC4-SHA:RC4-MD5"]}], client),
+    ?ERR_UPD({certs_keys, no_cert_or_key_given}, [{ciphers, "RC4-SHA:RC4-MD5"}], server),
     ok.
 
 options_client_renegotiation(_Config) ->
diff --git a/lib/ssl/test/tls_api_SUITE.erl b/lib/ssl/test/tls_api_SUITE.erl
@@ -193,8 +193,9 @@ init_per_suite(Config0) ->
     try crypto:start() of
 	ok ->
 	    ssl_test_lib:clean_start(),
-	    ssl_test_lib:make_rsa_cert_with_protected_keyfile(Config0,
-                                                              ?CORRECT_PASSWORD)
+	    Config1 = ssl_test_lib:make_rsa_cert_with_protected_keyfile(Config0,
+                                                                        ?CORRECT_PASSWORD),
+            ssl_test_lib:make_dsa_cert(Config1)
     catch _:_ ->
 	    {skip, "Crypto did not start"}
     end.
@@ -299,6 +300,7 @@ tls_upgrade_new_opts_with_sni_fun() ->
 tls_upgrade_new_opts_with_sni_fun(Config) when is_list(Config) ->
     ClientOpts = ssl_test_lib:ssl_options(client_rsa_verify_opts, Config),
     ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config),
+    ServerDsaOpts = ssl_test_lib:ssl_options(server_dsa_opts, Config),
     {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
     TcpOpts = [binary, {reuseaddr, true}],
     Version = ssl_test_lib:protocol_version(Config),
@@ -309,23 +311,26 @@ tls_upgrade_new_opts_with_sni_fun(Config) when is_list(Config) ->
                {ciphers, Ciphers},
                {verify, verify_peer}],
 
-    Server = ssl_test_lib:start_upgrade_server([{node, ServerNode}, {port, 0},
-						{from, self()},
-						{mfa, {?MODULE,
-						       upgrade_result, []}},
-						{tcp_options,
-						 [{active, false} | TcpOpts]},
-						{ssl_options, [{versions,  [Version |NewVersions]}, {sni_fun, fun(_SNI) -> ServerOpts ++ NewOpts end}]}]),
+    Server = ssl_test_lib:start_upgrade_server(
+               [{node, ServerNode}, {port, 0},
+                {from, self()},
+                {mfa, {?MODULE, upgrade_result, []}},
+                {tcp_options,
+                 [{active, false} | TcpOpts]},
+                {ssl_options, [{versions, [Version |NewVersions]},
+                               {sni_fun, fun(_SNI) -> ServerOpts ++ NewOpts end}
+                              | ServerDsaOpts]}]),
     Port = ssl_test_lib:inet_port(Server),
-    Client = ssl_test_lib:start_upgrade_client([{node, ClientNode},
-						{port, Port},
-				   {host, Hostname},
-				   {from, self()},
-				   {mfa, {?MODULE, upgrade_result, []}},
-				   {tcp_options, [binary]},
-				   {ssl_options,  [{versions,  [Version |NewVersions]},
-                                                   {ciphers, Ciphers},
-                                                   {server_name_indication, Hostname} | ClientOpts]}]),
+    Client = ssl_test_lib:start_upgrade_client(
+               [{node, ClientNode},
+                {port, Port},
+                {host, Hostname},
+                {from, self()},
+                {mfa, {?MODULE, upgrade_result, []}},
+                {tcp_options, [binary]},
+                {ssl_options,  [{versions,  [Version |NewVersions]},
+                                {ciphers, Ciphers},
+                                {server_name_indication, Hostname} | ClientOpts]}]),
 
     ?CT_LOG("Client ~p  Server ~p ~n", [Client, Server]),
 
@@ -515,7 +520,7 @@ tls_client_closes_socket() ->
     [{doc,"Test what happens when client closes socket before handshake is completed"}].
 
 tls_client_closes_socket(Config) when is_list(Config) ->
-    ServerOpts = ssl_test_lib:ssl_options(server_opts, Config),
+    ServerOpts = ssl_test_lib:ssl_options(server_rsa_opts, Config),
     {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
     TcpOpts = [binary, {reuseaddr, true}],
 
