Merge branch 'dgud/ssl/default-fail-no-peercert/OTP-18567'

* dgud/ssl/default-fail-no-peercert/OTP-18567:
  ssl: Fix that users can send data during renegotiation
  ssl: Make fail_if_no_peer_cert default true if verify_peer is set

6 files changed, 30 insertions, 25 deletions

diff --git a/lib/ssl/src/dtls_connection.erl b/lib/ssl/src/dtls_connection.erl
index a37a72efdc..899e7d3305 100644
--- a/lib/ssl/src/dtls_connection.erl
+++ b/lib/ssl/src/dtls_connection.erl
@@ -479,10 +479,7 @@ wait_cert_verify(info, Event, State) ->
 wait_cert_verify(state_timeout, Event, State) ->
     handle_state_timeout(Event, ?FUNCTION_NAME, State);
 wait_cert_verify(Type, Event, State) ->
-    try tls_dtls_connection:gen_handshake(?FUNCTION_NAME, Type, Event, State)
-    catch throw:#alert{} = Alert ->
-            ssl_gen_statem:handle_own_alert(Alert, ?FUNCTION_NAME, State)
-    end.
+    gen_handshake(?FUNCTION_NAME, Type, Event, State).
 
 %%--------------------------------------------------------------------
 -spec cipher(gen_statem:event_type(), term(), #state{}) ->
@@ -506,7 +503,7 @@ cipher(internal = Type, #finished{} = Event, #state{connection_states = Connecti
 cipher(state_timeout, Event, State) ->
     handle_state_timeout(Event, ?FUNCTION_NAME, State);
 cipher(Type, Event, State) ->
-     gen_handshake(?FUNCTION_NAME, Type, Event, State).
+    gen_handshake(?FUNCTION_NAME, Type, Event, State).
 
 %%--------------------------------------------------------------------
 -spec connection(gen_statem:event_type(),
@@ -761,6 +758,8 @@ alert_or_reset_connection(Alert, StateName, #state{connection_states = Cs} = Sta
             {next_state, connection, NewState}
     end.
 
+gen_handshake(_, {call, _From}, {application_data, _Data}, _State) ->
+    {keep_state_and_data, [postpone]};
 gen_handshake(StateName, Type, Event, State) ->
     try tls_dtls_connection:StateName(Type, Event, State)
     catch
diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index 6e83e16e1e..c96173e98b 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -1703,18 +1703,22 @@ validate_versions(dtls, Vsns0) ->
 opt_verification(UserOpts, Opts0, #{role := Role} = Env) ->
     {Verify, Opts1} =
         case get_opt_of(verify, [verify_none, verify_peer], default_verify(Role), UserOpts, Opts0) of
+            {old, Val} ->
+                {Val, Opts0};
             {_, verify_none} ->
                 {verify_none, Opts0#{verify => verify_none, verify_fun => {none_verify_fun(), []}}};
             {_, verify_peer} ->
                 %% If 'verify' is changed from verify_none to verify_peer, (via update_options/3)
                 %% the 'verify_fun' must also be changed to undefined.
                 %% i.e remove verify_none fun
-                {verify_peer, Opts0#{verify => verify_peer, verify_fun => undefined}}
+                Temp = Opts0#{verify => verify_peer, verify_fun => undefined},
+                {verify_peer, maps:remove(fail_if_no_peer_cert, Temp)}
         end,
     Opts2 = opt_cacerts(UserOpts, Opts1, Env),
     {_, PartialChain} = get_opt_fun(partial_chain, 1, fun(_) -> unknown_ca end, UserOpts, Opts2),
 
-    {_, FailNoPeerCert} = get_opt_bool(fail_if_no_peer_cert, false, UserOpts, Opts2),
+    DefFailNoPeer = Role =:= server andalso Verify =:= verify_peer,
+    {_, FailNoPeerCert} = get_opt_bool(fail_if_no_peer_cert, DefFailNoPeer, UserOpts, Opts2),
     assert_server_only(Role, FailNoPeerCert, fail_if_no_peer_cert),
     option_incompatible(FailNoPeerCert andalso Verify =:= verify_none,
                         [{verify, verify_none}, {fail_if_no_peer_cert, true}]),
diff --git a/lib/ssl/test/openssl_alpn_SUITE.erl b/lib/ssl/test/openssl_alpn_SUITE.erl
index bb33165bcf..1d0bc82c4e 100644
--- a/lib/ssl/test/openssl_alpn_SUITE.erl
+++ b/lib/ssl/test/openssl_alpn_SUITE.erl
@@ -194,7 +194,7 @@ erlang_client_alpn_openssl_server_alpn(Config) when is_list(Config) ->
 
 erlang_server_alpn_openssl_client_alpn(Config) when is_list(Config) ->
     ClientOpts = proplists:get_value(client_rsa_opts, Config),
-    ServerOpts =  ssl_test_lib:ssl_options(server_rsa_verify_opts, Config),
+    ServerOpts =  ssl_test_lib:ssl_options(server_rsa_opts, Config),
     Protocol = <<"spdy/2">>,
     Server = ssl_test_lib:start_server(erlang, [{from, self()}],
                                        [{server_opts, [{alpn_preferred_protocols,
diff --git a/lib/ssl/test/openssl_renegotiate_SUITE.erl b/lib/ssl/test/openssl_renegotiate_SUITE.erl
index 01b5a7a8a9..5102730396 100644
--- a/lib/ssl/test/openssl_renegotiate_SUITE.erl
+++ b/lib/ssl/test/openssl_renegotiate_SUITE.erl
@@ -230,7 +230,7 @@ erlang_server_openssl_client_nowrap_seqnum() ->
 erlang_server_openssl_client_nowrap_seqnum(Config) when is_list(Config) ->
     process_flag(trap_exit, true),
     ServerOpts = ssl_test_lib:ssl_options(server_rsa_verify_opts, Config),
-    ClientOpts = ssl_test_lib:ssl_options(client_rsa_opts, Config),
+    ClientOpts = ssl_test_lib:ssl_options(client_rsa_verify_opts, Config),
 
     {_, ServerNode, _Hostname} = ssl_test_lib:run_where(Config),
 
diff --git a/lib/ssl/test/ssl_api_SUITE.erl b/lib/ssl/test/ssl_api_SUITE.erl
index a3bb2f6bdc..215097bd4d 100644
--- a/lib/ssl/test/ssl_api_SUITE.erl
+++ b/lib/ssl/test/ssl_api_SUITE.erl
@@ -2699,18 +2699,18 @@ options_verify(Config) ->  %% fail_if_no_peer_cert, verify, verify_fun, partial_
     ?OK(#{fail_if_no_peer_cert := true, verify := verify_peer, verify_fun := undefined, partial_chain := _},
         [{fail_if_no_peer_cert, true}, {verify, verify_peer}, {cacerts, [Cert]}],
         server),
-     ?OK(#{fail_if_no_peer_cert := false, verify := verify_peer, verify_fun := undefined, partial_chain := _},
-        [{verify, verify_peer}, {cacerts, [Cert]}], server),
+     ?OK(#{fail_if_no_peer_cert := true, verify := verify_peer, verify_fun := undefined, partial_chain := _},
+         [{verify, verify_peer}, {cacerts, [Cert]}], server),
 
     NewF3 = fun(_,_,_) -> ok end,
     NewF4 = fun(_,_,_,_) -> ok end,
     ?OK(#{}, [], client, [fail_if_no_peer_cert]),
     ?OK(#{verify := verify_none, verify_fun := {NewF3, foo}, partial_chain := _},
         [{verify_fun, {NewF3, foo}}], client),
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_peer, verify_fun := {NewF3, foo}, partial_chain := _},
+    ?OK(#{fail_if_no_peer_cert := true, verify := verify_peer, verify_fun := {NewF3, foo}, partial_chain := _},
         [{verify_fun, {NewF3, foo}}, {verify, verify_peer}, {cacerts, [Cert]}],
         server),
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_peer, verify_fun := {NewF4, foo}, partial_chain := _},
+    ?OK(#{fail_if_no_peer_cert := true, verify := verify_peer, verify_fun := {NewF4, foo}, partial_chain := _},
         [{verify_fun, {NewF4, foo}}, {verify, verify_peer}, {cacerts, [Cert]}],
         server),
 
diff --git a/lib/ssl/test/ssl_test_lib.erl b/lib/ssl/test/ssl_test_lib.erl
index 01235a1d74..64ee3c9d0d 100644
--- a/lib/ssl/test/ssl_test_lib.erl
+++ b/lib/ssl/test/ssl_test_lib.erl
@@ -973,15 +973,19 @@ openssl_server_loop(Pid, SslPort, Args) ->
 
 start_openssl_client(Args0, Config) ->
     {ClientNode, _, Hostname} = run_where(Config),
-    ClientOpts = get_client_opts(Config),
+
+    %% io:format("~p:~p: ~p~n",[?MODULE, ?LINE, Args0]),
+    %% io:format("~p:~p: ~p~n",[?MODULE, ?LINE, Config]),
+
+    ClientOpts0 = get_client_opts(Config),
+    ClientOpts = proplists:get_value(options, Args0, []) ++ ClientOpts0,
     DefaultVersions = default_tls_version(ClientOpts),
     [Version | _] = proplists:get_value(versions, ClientOpts, DefaultVersions),
     Node = proplists:get_value(node, Args0, ClientNode),
-    Args = [{from, self()},
-            {host, Hostname},
-            {options, ClientOpts} | Args0],
+    Args = [{from, self()}, {host, Hostname} | ClientOpts ++ Args0],
 
-    Result = spawn_link(Node, ?MODULE, init_openssl_client, [[{version, Version} | lists:delete(return_port, Args)]]),
+    Result = spawn_link(Node, ?MODULE, init_openssl_client,
+                        [[{version, Version} | lists:delete(return_port, Args)]]),
     receive
 	{connected, OpenSSLPort} ->
 	    case lists:member(return_port, Args) of
@@ -1636,9 +1640,7 @@ make_ec_cert_chains(UserConf, ClientChainType, ServerChainType, Config, Curve) -
     [{server_config, ServerConf}, 
      {client_config, ClientConf}] = 
         x509_test:gen_pem_config_files(GenCertData, ClientFileBase, ServerFileBase),               
-    {[{verify, verify_peer} | ClientConf],
-     [{reuseaddr, true}, {verify, verify_peer} | ServerConf]
-    }.
+    {ClientConf, [{reuseaddr, true} | ServerConf]}.
 
 default_cert_chain_conf() ->
     %% Use only default options
@@ -1654,8 +1656,8 @@ make_rsa_pss_pem(Alg, _UserConf, Config, Suffix) ->
     Conf = x509_test:gen_pem_config_files(GenCertData, ClientFileBase, ServerFileBase),               
     CConf = proplists:get_value(client_config, Conf),
     SConf = proplists:get_value(server_config, Conf),
-    #{server_config => [{verify, verify_peer} | SConf],
-      client_config => [{verify, verify_peer} | CConf]}.
+    #{server_config => SConf,
+      client_config => CConf}.
 
 make_rsa_sni_configs() ->
     Sha = appropriate_sha(crypto:supports()),
@@ -2032,7 +2034,7 @@ make_rsa_ecdsa_cert(Config, Curve) ->
 	    [{server_rsa_ecdsa_opts, [{reuseaddr, true} | ServerConf]},
 	     {server_rsa_ecdsa_verify_opts, [{ssl_imp, new},{reuseaddr, true},
                                             {verify, verify_peer} | ServerConf]},
-	     {client_rsa_ecdsa_opts, [{verify, cerify_none} | ClientConf]} | Config];
+	     {client_rsa_ecdsa_opts, [{verify, verify_none} | ClientConf]} | Config];
 	_ ->
 	    Config
     end.
@@ -2449,7 +2451,7 @@ start_server(erlang, _, ServerOpts, Config) ->
                            {mfa, {ssl_test_lib,
                                   check_key_exchange_send_active,
                                   [KeyEx]}},
-                           {options, [{verify, verify_peer} | ServerOpts]}]),
+                           {options, ServerOpts}]),
     {Server, inet_port(Server)}.
  
 sig_algs(undefined) ->