ssl: Remove double assert and fix test cases to handle client defaults to {verify, verify_peer}

6 files changed, 69 insertions, 67 deletions

diff --git a/lib/diameter/test/diameter_tls_SUITE.erl b/lib/diameter/test/diameter_tls_SUITE.erl
index 1e7f7ed50a..e576da0f6a 100644
--- a/lib/diameter/test/diameter_tls_SUITE.erl
+++ b/lib/diameter/test/diameter_tls_SUITE.erl
@@ -280,7 +280,7 @@ inband_security(Ids) ->
 
 ssl_options(Dir, Base) ->
     Root = filename:join([Dir, Base]),
-    [{ssl_options, [{certfile, Root ++ "_ca.pem"},
+    [{ssl_options, [{verify, verify_none},{certfile, Root ++ "_ca.pem"},
                     {keyfile,  Root ++ "_key.pem"}]}].
 
 make_cert(Dir, Base) ->
@@ -290,7 +290,7 @@ make_cert(Dir, Keyfile, Certfile) ->
     [KP,CP] = [filename:join([Dir, F]) || F <- [Keyfile, Certfile]],
 
     KC = join(["openssl genrsa -out", KP, "2048"]),
-    CC = join(["openssl req -new -x509 -key", KP, "-out", CP, "-days 7",
+    CC = join(["openssl req -new -sha256 -x509 -key", KP, "-out", CP, "-days 7",
                "-subj /C=SE/ST=./L=Stockholm/CN=www.erlang.org"]),
 
     %% Hope for the best and only check that files are written.
diff --git a/lib/eldap/test/eldap_basic_SUITE.erl b/lib/eldap/test/eldap_basic_SUITE.erl
index 8bc041f609..881b46ec41 100644
--- a/lib/eldap/test/eldap_basic_SUITE.erl
+++ b/lib/eldap/test/eldap_basic_SUITE.erl
@@ -296,7 +296,7 @@ init_per_testcase(TC, Config) when TC == ssl_connection; TC == ssl_conn_socket_i
 		    ct:log("SSL listening to port ~p (process ~p)",[SSL_Port, Listener]),
 		    [{ssl_listener,Listener},
 		     {ssl_listen_port,SSL_Port},
-		     {ssl_connect_opts,[]}
+		     {ssl_connect_opts,[{verify, verify_none}]}
 		     | Config];
 		{no_ok,SSL_Other,Listener} ->
 		    ct:log("ssl:listen on port ~p failed: ~p",[SSL_Port,SSL_Other]),
diff --git a/lib/inets/test/inets_test_lib.erl b/lib/inets/test/inets_test_lib.erl
index 5ea7a6f833..c5c292cf4f 100644
--- a/lib/inets/test/inets_test_lib.erl
+++ b/lib/inets/test/inets_test_lib.erl
@@ -457,7 +457,7 @@ connect_bin(SockType, Host, Port) ->
     connect_bin(SockType, Host, Port, []).
 
 connect_bin(ssl, Host, Port, Opts0) ->
-    Opts = [binary, {packet,0} | Opts0], 
+    Opts = [binary, {packet,0}, {verify, verify_none} | Opts0],
     connect(ssl, Host, Port, Opts);
 connect_bin(ip_comm, Host, Port, Opts0) ->
     Opts = [binary, {packet, 0} | Opts0],
@@ -469,7 +469,7 @@ connect_byte(SockType, Host, Port) ->
     connect_byte(SockType, Host, Port, []).
     
 connect_byte(ssl, Host, Port, Opts0) ->
-    Opts = [list, {packet,0} | Opts0], 
+    Opts = [list, {packet,0}, {verify, verify_none} | Opts0],
     connect(ssl, Host, Port, Opts);
 connect_byte(ip_comm, Host, Port, Opts0) ->
     Opts = [list, {packet,0} | Opts0],
@@ -681,7 +681,7 @@ gen_pem_config_files(#{server_config := ServerConf,
                             ClientCaCertFile),
     [{server_config, [{certfile, ServerCertFile}, 
                       {keyfile, ServerKeyFile}, {cacertfile, ServerCaCertFile}]}, 
-     {client_config, [{certfile, ClientCertFile}, 
+     {client_config, [{certfile, ClientCertFile},
                       {keyfile, ClientKeyFile}, {cacertfile, ClientCaCertFile}]}].
 extensions(Exts) ->
     [extension(Ext) || Ext <- Exts].
diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index 59fc55b64e..6c845cbd86 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -1694,7 +1694,7 @@ validate_versions(dtls, Vsns0) ->
     lists:sort(fun dtls_record:is_higher/2, Vsns).
 
 opt_verification(UserOpts, Opts0, #{role := Role} = Env) ->
-    {Verify, Opts} =
+    {Verify, Opts1} =
         case get_opt_of(verify, [verify_none, verify_peer], default_verify(Role), UserOpts, Opts0) of
             {_, verify_none} ->
                 {verify_none, Opts0#{verify => verify_none, verify_fun => {none_verify_fun(), []}}};
@@ -1704,19 +1704,25 @@ opt_verification(UserOpts, Opts0, #{role := Role} = Env) ->
                 %% i.e remove verify_none fun
                 {verify_peer, Opts0#{verify => verify_peer, verify_fun => undefined}}
         end,
-    assert_cacerts(Verify, maps:merge(UserOpts, Opts0)),
-    {_, PartialChain} = get_opt_fun(partial_chain, 1, fun(_) -> unknown_ca end, UserOpts, Opts),
+    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, Opts),
+    {_, FailNoPeerCert} = get_opt_bool(fail_if_no_peer_cert, false, 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}]),
 
-    Opts1 = set_opt_int(depth, 0, 255, ?DEFAULT_DEPTH, UserOpts, Opts),
+    Opts = set_opt_int(depth, 0, 255, ?DEFAULT_DEPTH, UserOpts, Opts2),
 
-    opt_verify_fun(UserOpts, Opts1#{partial_chain => PartialChain,
-                                    fail_if_no_peer_cert => FailNoPeerCert},
-                   Env).
+    case Role of
+        client ->
+            opt_verify_fun(UserOpts, Opts#{partial_chain => PartialChain},
+                           Env);
+        server ->
+            opt_verify_fun(UserOpts, Opts#{partial_chain => PartialChain,
+                                           fail_if_no_peer_cert => FailNoPeerCert},
+                           Env)
+    end.
 
 default_verify(client) ->
     %% Server authenication is by default requiered
@@ -1771,16 +1777,15 @@ convert_verify_fun() ->
     end.
 
 opt_certs(UserOpts, #{log_level := LogLevel} = Opts0, Env) ->
-    Opts = case get_opt_list(certs_keys, [], UserOpts, Opts0) of
-               {Where, []} when Where =/= new ->
-                   opt_old_certs(UserOpts, #{}, Opts0, Env);
-               {old, [CertKey]} ->
-                   opt_old_certs(UserOpts, CertKey, Opts0, Env);
-               {Where, CKs} when is_list(CKs) ->
-                   warn_override(Where, UserOpts, certs_keys, [cert,certfile,key,keyfile,password], LogLevel),
-                   Opts0#{certs_keys => [check_cert_key(CK, #{}, LogLevel) || CK <- CKs]}
-           end,
-    opt_cacerts(UserOpts, Opts, Env).
+    case get_opt_list(certs_keys, [], UserOpts, Opts0) of
+        {Where, []} when Where =/= new ->
+            opt_old_certs(UserOpts, #{}, Opts0, Env);
+        {old, [CertKey]} ->
+            opt_old_certs(UserOpts, CertKey, Opts0, Env);
+        {Where, CKs} when is_list(CKs) ->
+            warn_override(Where, UserOpts, certs_keys, [cert,certfile,key,keyfile,password], LogLevel),
+            Opts0#{certs_keys => [check_cert_key(CK, #{}, LogLevel) || CK <- CKs]}
+    end.
 
 opt_old_certs(UserOpts, CertKeys, #{log_level := LogLevel}=SSLOpts, _Env) ->
     CK = check_cert_key(UserOpts, CertKeys, LogLevel),
@@ -2454,13 +2459,6 @@ role_error(true, ErrorDesc, Option)
   when ErrorDesc =:= client_only; ErrorDesc =:= server_only ->
     throw_error({option, ErrorDesc, Option}).
 
-assert_cacerts(verify_peer, Options) ->
-    CaCerts = maps:get(cacerts, Options, undefined),
-    CaCertsFile = maps:get(cacertfile, Options, undefined),
-    option_error((CaCerts == undefined) andalso (CaCertsFile == undefined), verify, {missing_dep_cacertfile_or_cacerts});
-assert_cacerts(verify_none,_) ->
-    ok.
-
 option_incompatible(false, _Options) -> ok;
 option_incompatible(true, Options) -> option_incompatible(Options).
 
diff --git a/lib/ssl/test/ssl_api_SUITE.erl b/lib/ssl/test/ssl_api_SUITE.erl
index ea7043b5bc..5cd25ae352 100644
--- a/lib/ssl/test/ssl_api_SUITE.erl
+++ b/lib/ssl/test/ssl_api_SUITE.erl
@@ -2182,29 +2182,37 @@ options_whitebox() ->
     [{doc,"Whitebox tests of option handling"}].
 
 
-patch_version(Opts, Role, Host) ->
+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,
     case proplists:get_value(protocol, Opts, tls) of
         dtls ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options([{protocol, dtls}], Role, Host),
-            {DOpts, Opts};
+            {ok, #config{ssl=DOpts}} = ssl:handle_options([{verify, verify_none}, {protocol, dtls}], Role, Host),
+            {DOpts, ClientIgnorDef ++ Opts};
         tls ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options([], Role, Host),
+            {ok, #config{ssl=DOpts}} = ssl:handle_options([{verify, verify_none}], Role, Host),
             case proplists:get_value(versions, Opts) of
                 undefined ->
-                    {DOpts, [{versions, ['tlsv1.2','tlsv1.3']}|Opts]};
+                    {DOpts, ClientIgnorDef ++ [{versions, ['tlsv1.2','tlsv1.3']}|Opts]};
                 _ ->
-                    {DOpts, Opts}
+                    {DOpts, ClientIgnorDef ++ Opts}
             end;
         _ ->
-            {ok, #config{ssl=DOpts}} = ssl:handle_options([], Role, Host),
-            {DOpts, Opts}
+            {ok, #config{ssl=DOpts}} = ssl:handle_options(ClientIgnorDef, Role, Host),
+            {DOpts, ClientIgnorDef ++ Opts}
     end.
 
 -define(OK(EXP, Opts, Role), ?OK(EXP,Opts, Role, [])).
 -define(OK(EXP, Opts, Role, ShouldBeMissing),
         fun() ->
                 Host = "dummy.host.org",
-                {__DefOpts, __Opts} = patch_version(Opts, Role, Host),
+                {__DefOpts, __Opts} = customize_defaults(Opts, Role, Host),
                 try ssl:handle_options(__Opts, Role, Host) of
                     {ok, #config{ssl=EXP = __ALL}} ->
                         ShouldBeMissing = ShouldBeMissing -- maps:keys(__ALL);
@@ -2238,7 +2246,7 @@ patch_version(Opts, Role, Host) ->
 -define(ERR(EXP, Opts, Role),
         fun() ->
                 Host = "dummy.host.org",
-                {__DefOpts, __Opts} = patch_version(Opts, Role, Host),
+                {__DefOpts, __Opts} = customize_defaults(Opts, Role, Host),
                 try ssl:handle_options(__Opts, Role, Host) of
                     Other ->
                         ct:pal("ssl:handle_options(~0p,~0p,~0p).",[__Opts,Role,Host]),
@@ -2361,7 +2369,7 @@ options_version(_Config) ->
          client),
     ok.
 
-options_alpn(_Config) -> %% alpn & next_protocols 
+options_alpn(_Config) -> %% alpn & next_protocols
     Http = <<"HTTP/2">>,
     ?OK(#{alpn_advertised_protocols := undefined}, [], client,
         [alpn_preferred_protocols, next_protocol_selector, next_protocols_advertised]),
@@ -2435,7 +2443,7 @@ options_anti_replay(_Config) ->
          server),
     ok.
 
-options_beast_mitigation(_Config) -> %% Beast mitigation
+options_beast_mitigation(_Config) -> %% Beast mitigation TLS-1.0 option only
     ?OK(#{beast_mitigation := one_n_minus_one}, [{versions, [tlsv1,'tlsv1.1']}], client),
     ?OK(#{}, [{versions, ['tlsv1.1']}], client, [beast_mitigation]),
     ?OK(#{}, [{beast_mitigation, disabled}, {versions, [tlsv1]}], client,
@@ -2446,7 +2454,7 @@ options_beast_mitigation(_Config) -> %% Beast mitigation
     %% Errors
     ?ERR({beast_mitigation, enabled},
          [{beast_mitigation, enabled}, {versions, [tlsv1]}], client),
-    ?ERR({options, incompatible, [beast_mitigation, {versions, _}]}, %% ok?
+    ?ERR({options, incompatible, [beast_mitigation, {versions, _}]},
          [{beast_mitigation, disabled}], client),
     ok.
 
@@ -2471,13 +2479,11 @@ options_cacerts(Config) ->  %% cacert[s]file
     ?ERR({cacerts, Cert}, [{cacerts, Cert}], client),
     ?ERR({cacertfile, cert}, [{cacertfile, cert}], client),
 
-    begin %% depth
-        ?OK(#{}, [], client, [depth]),
-        ?OK(#{depth := 5}, [{depth, 5}], client),
-        %% Error
-        ?ERR({depth, 256}, [{depth, 256}], client),
-        ?ERR({depth, not_an_int}, [{depth, not_an_int}], client)
-    end,
+    ?OK(#{}, [], client, [depth]),
+    ?OK(#{depth := 5}, [{depth, 5}], client),
+    %% Error
+    ?ERR({depth, 256}, [{depth, 256}], client),
+    ?ERR({depth, not_an_int}, [{depth, not_an_int}], client),
     ok.
 
 options_cert(Config) -> %% cert[file] cert_keys keys password
@@ -2685,28 +2691,20 @@ options_eccs(_Config) ->
 
 options_verify(Config) ->  %% fail_if_no_peer_cert, verify, verify_fun, partial_chain
     Cert = proplists:get_value(cert, ssl_test_lib:ssl_options(server_rsa_der_opts, Config)),
-    {ok, #config{ssl = DefOpts = #{verify_fun := {DefVerify,_}}}} = ssl:handle_options([], client, "dummy.host.org"),
+    {ok, #config{ssl = DefOpts = #{verify_fun := {DefVerify,_}}}} = ssl:handle_options([{verify, verify_none}], client, "dummy.host.org"),
 
     ?OK(#{fail_if_no_peer_cert := false, verify := verify_none, verify_fun := {DefVerify, []}, partial_chain := _},
-        [], client),
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_none, verify_fun := {DefVerify, []}, partial_chain := _},
         [], server),
     ?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_none, verify_fun := {DefVerify, []}, partial_chain := _},
-        [{verify, verify_none}], client),
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_peer, verify_fun := undefined, partial_chain := _},
+     ?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 := false, verify := verify_none, verify_fun := {_, []}, partial_chain := _},
-        [{partial_chain, fun(_) -> ok end}], client),
 
-    OldF1 = fun(_) -> ok end,
     NewF3 = fun(_,_,_) -> ok end,
     NewF4 = fun(_,_,_,_) -> ok end,
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_none, verify_fun := {_, OldF1}, partial_chain := _},
-        [{verify_fun, OldF1}], client),
-    ?OK(#{fail_if_no_peer_cert := false, verify := verify_none, verify_fun := {NewF3, foo}, partial_chain := _},
+    ?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 := _},
         [{verify_fun, {NewF3, foo}}, {verify, verify_peer}, {cacerts, [Cert]}],
@@ -2726,10 +2724,11 @@ options_verify(Config) ->  %% fail_if_no_peer_cert, verify, verify_fun, partial_
     ?ERR({partial_chain, undefined}, [{partial_chain, undefined}], client),
     ?ERR({options, incompatible, [{verify, verify_none}, {fail_if_no_peer_cert, true}]},
          [{fail_if_no_peer_cert, true}], server),
-    ?ERR({verify, verify}, [{verify, verify}], client),
+    ?ERR({options, incompatible, [{verify, _}, {cacerts, undefined}]}, [{verify, verify_peer}], client),
     ?ERR({option, server_only, fail_if_no_peer_cert},
          [{fail_if_no_peer_cert, true}, {verify, verify_peer}, {cacerts, [Cert]}],
          client),
+    ?ERR({verify, verify}, [{verify, verify}], client),
     ?ERR({options, incompatible, [{verify, _}, {cacerts, undefined}]}, [{verify, verify_peer}], server),
     ?ERR({partial_chain, not_a_fun}, [{partial_chain, not_a_fun}], client),
     ?ERR({verify_fun, not_a_fun}, [{verify_fun, not_a_fun}], client),
@@ -2761,8 +2760,12 @@ options_handshake(_Config) -> %% handshake
 
 options_process(_Config) -> % hibernate_after, spawn_opts
     ?OK(#{}, [], client, [hibernate_after, receiver_spawn_opts, sender_spawn_opts]),
-    ?OK(#{hibernate_after := 10000, receiver_spawn_opts := [foo], sender_spawn_opts := [bar]},
-        [{hibernate_after, 10000}, {receiver_spawn_opts, [foo]}, {sender_spawn_opts, [bar]}],
+    ?OK(#{hibernate_after := 10000,
+          receiver_spawn_opts := [{fullsweep_after, 500}],
+          sender_spawn_opts := [{fullsweep_after, 500}]},
+        [{hibernate_after, 10000},
+         {receiver_spawn_opts,[{fullsweep_after, 500}]},
+         {sender_spawn_opts, [{fullsweep_after, 500}]}],
         client),
     %% Errors
     ?ERR({hibernate_after, -1}, [{hibernate_after, -1}], server),
diff --git a/lib/ssl/test/ssl_cert_SUITE.erl b/lib/ssl/test/ssl_cert_SUITE.erl
index dcdc0ed634..d445072d7b 100644
--- a/lib/ssl/test/ssl_cert_SUITE.erl
+++ b/lib/ssl/test/ssl_cert_SUITE.erl
@@ -519,7 +519,8 @@ missing_root_cert_auth(Config) when is_list(Config) ->
 					      {options, no_reuse(Version) ++ [{verify, verify_peer}
                                                                              | ServerOpts]}]),
 
-    Error = {error, {options, {verify, {missing_dep_cacertfile_or_cacerts}}}},
+    Error = {error, {options, incompatible,
+                     [{verify,verify_peer},{cacerts,undefined}]}},
     ssl_test_lib:check_result(Server, Error),
     
     ClientOpts =  proplists:delete(cacertfile, ssl_test_lib:ssl_options(extra_client, client_cert_opts, Config)),