Merge branch 'ingela/ssl/cache-refresh/OTP-19698' into maint

* ingela/ssl/cache-refresh/OTP-19698:
  ssl: files_to_certificates should return empty content if file ceased to exist

Author: IngelaAndin

lib/ssl/src/ssl_certificate.erl

@@ -186,18 +186,24 @@ certificate_chain(#cert{} = Cert, CertDbHandle, CertsDbRef, Candidates, Type) ->
 %% Description: Return list of DER encoded certificates.
 %%--------------------------------------------------------------------
 file_to_certificats(File, DbHandle) ->
-    {ok, List} = ssl_manager:cache_pem_file(File, DbHandle),
-    [Bin || {'Certificate', Bin, not_encrypted} <- List].
-
+    case ssl_manager:cache_pem_file(File, DbHandle) of
+        {ok, List} ->
+            [Bin || {'Certificate', Bin, not_encrypted} <- List];
+        _ ->
+            [] % If file no longer exists return empty content
+    end.
 %%--------------------------------------------------------------------
 -spec file_to_crls(binary(), term()) -> [public_key:der_encoded()].
 %%
 %% Description: Return list of DER encoded certificates.
 %%--------------------------------------------------------------------
 file_to_crls(File, DbHandle) ->
-    {ok, List} = ssl_manager:cache_pem_file(File, DbHandle),
-    [Bin || {'CertificateList', Bin, not_encrypted} <- List].
-
+    case ssl_manager:cache_pem_file(File, DbHandle) of
+        {ok, List} ->
+            [Bin || {'CertificateList', Bin, not_encrypted} <- List];
+        _ ->
+            [] % If file no longer exists return empty content
+    end.
 %%--------------------------------------------------------------------
 -spec validate(term(), {extension, #'Extension'{}} | {bad_cert, atom()} | valid | valid_peer,
 	       term(), logger:level() | none | all) -> {valid, term()} | {fail, tuple()} | {unknown, term()}.

lib/ssl/src/ssl_config.erl

@@ -60,7 +60,7 @@ init(SslOpts, Role) ->
     init_manager_name(maps:get(erl_dist, SslOpts, false)),
     #{pem_cache := PemCache} = Config = init_cacerts(SslOpts, Role),
     DHParams = init_diffie_hellman(PemCache, SslOpts, Role),
-    CertKeyAlts = init_certs_keys(SslOpts, Role, PemCache),
+    CertKeyAlts = init_certs_keys(SslOpts, PemCache),
     {ok, Config#{cert_key_alts => CertKeyAlts, dh_params => DHParams}}.
 
 new_emulated([], EmOpts) ->
@@ -142,24 +142,17 @@ get_internal_active_n(true) ->
 get_internal_active_n(false) ->
     application_int(internal_active_n, ?INTERNAL_ACTIVE_N).
 
-%%====================================================================
-%% Internal functions 
-%%====================================================================	     
-
 %%====================================================================
 %% Certificate and  Key configuration
 %%====================================================================
-init_certs_keys(#{certs_keys := CertsKeys} = Opts, Role, PemCache) ->
-    Pairs = lists:map(fun(CertKey) -> 
-                              init_cert_key_pair(CertKey, Role, PemCache) 
-                      end, CertsKeys),
+init_certs_keys(#{certs_keys := CertsKeys} = Opts, PemCache) ->
+    Pairs = lists:map(fun(CertKey) -> init_cert_key_pair(CertKey, PemCache) end, CertsKeys),
     CertKeyGroups = group_pairs(Pairs),
     prioritize_groups(CertKeyGroups, Opts).
 
-init_cert_key_pair(CertKey, Role, PemCache) ->
-    Certs = init_certificates(CertKey, PemCache, Role),
-    PrivateKey = init_private_key(maps:get(key, CertKey, undefined), 
-                                  CertKey, PemCache),
+init_cert_key_pair(CertKey, PemCache) ->
+    Certs = init_certificates(CertKey, PemCache),
+    PrivateKey = init_private_key(maps:get(key, CertKey, undefined), CertKey, PemCache),
     #{private_key => PrivateKey, certs => Certs}.
 
 group_pairs([#{certs := []}]) ->
@@ -304,26 +297,25 @@ init_cacerts(#{cacerts := CaCerts, crl_cache := CRLCache} = Opts, Role) ->
 	end,
     Config.
 
-init_certificates(CertKey, PemCache, Role) ->
+init_certificates(CertKey, PemCache) ->
     case maps:get(cert, CertKey, undefined) of
         undefined ->
-            init_certificate_file(maps:get(certfile, CertKey, <<>>), PemCache, Role);
+            init_certificate_file(maps:get(certfile, CertKey, <<>>), PemCache);
         Bin when is_binary(Bin) ->
             [Bin];
         Certs when is_list(Certs) ->
             Certs
     end.
 
-init_certificate_file(<<>>, _PemCache, _Role) ->
+init_certificate_file(<<>>, _PemCache) ->
     [];
-init_certificate_file(CertFile, PemCache, Role) ->
-    try %% OwnCert | [OwnCert | Chain]
-        ssl_certificate:file_to_certificats(CertFile, PemCache)
-    catch
-        _Error:_Reason when Role =:= client ->
-            [];
-        _Error:Reason ->
-            file_error(CertFile, {certfile, Reason})
+init_certificate_file(CertFile, PemCache) ->
+    case ssl_certificate:file_to_certificats(CertFile, PemCache) of
+        [] ->
+            Reason = cert_file_error(CertFile),
+            file_error(CertFile, Reason);
+        Certs ->
+            Certs
     end.
 
 init_private_key(#{algorithm := _, sign_fun := _SignFun} = Key, _, _) ->
@@ -2040,6 +2032,21 @@ ciphers_for_version([AtomVersion | _], CurrentSuites, Record) ->
             [Suite || Suite <- CurrentSuites, lists:member(Suite, Suites)]
     end.
 
+cert_file_error(CertFile) ->
+    %% ssl_certificate:file_to_certificats
+    %% ignores file errors to be efficient
+    %% and works correctly for most executed
+    %% code paths.
+    Reason =
+        case file:read_file_info(CertFile) of
+            {error, _} = Error ->
+                Error;
+            _ ->
+                %% A file existed but included no certs
+                no_certs
+        end,
+ {options, {certfile, binary_to_list(CertFile), Reason}}.
+
 %%%--------------------------------------------------------------
 %%% Tracing
 %%%--------------------------------------------------------------------

lib/ssl/test/ssl_pem_cache_SUITE.erl

@@ -69,9 +69,12 @@
          alternative_path_noabspath/1,
          alternative_path_symlink_relative/0,
          alternative_path_symlink_relative/1,
-         check_cert/3
+         cache_file_does_not_exist/0,
+         cache_file_does_not_exist/1
         ]).
 
+-export([check_cert/3]).
+
 -define(CLEANUP_INTERVAL, 5000).
 -define(BIG_CLEANUP_INTERVAL, 600000).
 -define(SLEEP_AMOUNT, 1000).
@@ -93,7 +96,8 @@ all() ->
      alternative_path_noabspath,
      alternative_path_hardlink,
      alternative_path_symlink,
-     alternative_path_symlink_relative].
+     alternative_path_symlink_relative,
+     cache_file_does_not_exist].
 
 groups() -> [].
 
@@ -431,6 +435,18 @@ alternative_path_symlink_relative(Config) when is_list(Config) ->
                  disconnected => [8, 0, 0, 0]},
     alternative_path_helper(Config, fun make_symlink_noabspath/1, Expected).
 
+cache_file_does_not_exist() ->
+    [{doc, "White box test, that a none existing file will provide an empty content. "
+      "This will have the effect that the ssl manager process "
+      "will not crash but previous content will be invalidated."}].
+
+cache_file_does_not_exist(Config) when is_list(Config)->
+    PrivDir = proplists:get_value(priv_dir, Config),
+    Id = ets:new(dummy_test, []),
+    NonExistingFile = filename:join(PrivDir, "foo.pem"),
+    [] = ssl_certificate:file_to_certificats(NonExistingFile, Id),
+    [] = ssl_certificate:file_to_crls(NonExistingFile, Id).
+
 %%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------