ssl: Error handling improvment

4 files changed, 66 insertions, 16 deletions

diff --git a/lib/ssl/src/ssl_manager.erl b/lib/ssl/src/ssl_manager.erl
index f44fe6a2bf..52aa164420 100644
--- a/lib/ssl/src/ssl_manager.erl
+++ b/lib/ssl/src/ssl_manager.erl
@@ -127,7 +127,13 @@ cache_pem_file(File, DbHandle) ->
 	[Content]  ->
 	    {ok, Content};
 	undefined ->
-            ssl_pem_cache:insert(File)
+            case ssl_pkix_db:decode_pem_file(File) of
+                {ok, Content} ->
+                    ssl_pem_cache:insert(File, Content),
+                    {ok, Content};
+                Error ->
+                    Error
+            end
     end.
 
 %%--------------------------------------------------------------------
diff --git a/lib/ssl/src/ssl_pem_cache.erl b/lib/ssl/src/ssl_pem_cache.erl
index 115ab4451d..a952e20133 100644
--- a/lib/ssl/src/ssl_pem_cache.erl
+++ b/lib/ssl/src/ssl_pem_cache.erl
@@ -29,7 +29,7 @@
 -export([start_link/1, 
 	 start_link_dist/1,
 	 name/1,
-	 insert/1,
+	 insert/2,
 	 clear/0]).
 
 % Spawn export
@@ -90,19 +90,17 @@ start_link_dist(_) ->
 
 
 %%--------------------------------------------------------------------
--spec insert(binary()) -> {ok, term()} | {error, reason()}.
+-spec insert(binary(), term()) -> ok | {error, reason()}.
 %%		    
 %% Description: Cache a pem file and return its content.
 %%--------------------------------------------------------------------
-insert(File) ->    
-    {ok, PemBin} = file:read_file(File),
-    Content = public_key:pem_decode(PemBin),
+insert(File, Content) ->    
     case bypass_cache() of
 	true ->
-	    {ok, Content};
+	    ok;
 	false ->
 	    cast({cache_pem, File, Content}),
-	    {ok, Content}
+            ok
     end.
 
 %%--------------------------------------------------------------------
diff --git a/lib/ssl/src/ssl_pkix_db.erl b/lib/ssl/src/ssl_pkix_db.erl
index 8828c3a0d8..b6fae36ff9 100644
--- a/lib/ssl/src/ssl_pkix_db.erl
+++ b/lib/ssl/src/ssl_pkix_db.erl
@@ -157,7 +157,7 @@ extract_trusted_certs(File) ->
             {error, {badmatch, Error}}
     end.
 
--spec decode_pem_file(binary()) -> {ok, term()}.
+-spec decode_pem_file(binary()) -> {ok, term()} | {error, term()}.
 decode_pem_file(File) ->
     case file:read_file(File) of
         {ok, PemBin} ->
@@ -316,11 +316,16 @@ decode_certs(Ref, Cert) ->
     end.
 
 new_trusted_cert_entry(File, [CertsDb, RefsDb, _ | _]) ->
-    Ref = make_ref(),
-    init_ref_db(Ref, File, RefsDb),
-    {ok, Content} = ssl_pem_cache:insert(File),
-    add_certs_from_pem(Content, Ref, CertsDb),
-    {ok, Ref}.
+    case decode_pem_file(File) of
+        {ok, Content} ->
+            Ref = make_ref(),
+            init_ref_db(Ref, File, RefsDb),
+            ok = ssl_pem_cache:insert(File, Content),
+            add_certs_from_pem(Content, Ref, CertsDb),
+            {ok, Ref};
+        Error ->
+            Error
+    end.
 
 add_crls([_,_,_, {_, Mapping} | _], ?NO_DIST_POINT, CRLs) ->
     [add_crls(CRL, Mapping) || CRL <- CRLs];
diff --git a/lib/ssl/test/ssl_pem_cache_SUITE.erl b/lib/ssl/test/ssl_pem_cache_SUITE.erl
index 96b15d9b51..3b79780974 100644
--- a/lib/ssl/test/ssl_pem_cache_SUITE.erl
+++ b/lib/ssl/test/ssl_pem_cache_SUITE.erl
@@ -34,7 +34,7 @@
 %% Common Test interface functions -----------------------------------
 %%--------------------------------------------------------------------
 all() ->
-    [pem_cleanup].
+    [pem_cleanup, invalid_insert].
 
 groups() ->
     [].
@@ -68,6 +68,10 @@ init_per_testcase(pem_cleanup = Case, Config) ->
     application:set_env(ssl, ssl_pem_cache_clean, ?CLEANUP_INTERVAL),
     ssl:start(),
     ct:timetrap({minutes, 1}),
+    Config;
+init_per_testcase(_, Config) ->
+    ssl:start(),
+    ct:timetrap({seconds, 5}),
     Config.
 
 end_per_testcase(_TestCase, Config) ->
@@ -108,7 +112,34 @@ pem_cleanup(Config)when is_list(Config) ->
     ssl_test_lib:close(Server),
     ssl_test_lib:close(Client),
     false = Size == Size1.
-       
+
+invalid_insert() ->
+    [{doc, "Test that insert of invalid pem does not cause empty cache entry"}].
+invalid_insert(Config)when is_list(Config) ->      
+    process_flag(trap_exit, true),
+    
+    ClientOpts = proplists:get_value(client_verification_opts, Config),
+    ServerOpts = proplists:get_value(server_verification_opts, Config),
+    {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+    BadClientOpts = [{cacertfile, "tmp/does_not_exist.pem"} | proplists:delete(cacertfile, ClientOpts)],
+    Server =
+	ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+				   {from, self()},
+				   {mfa, {ssl_test_lib, no_result, []}},
+				   {options, ServerOpts}]),
+    Port = ssl_test_lib:inet_port(Server),
+    ssl_test_lib:start_client_error([{node, ClientNode},
+                               {port, Port}, {host, Hostname},
+                               {from, self()}, {options, BadClientOpts}]),
+    ssl_test_lib:close(Server),
+    1 = ssl_pkix_db:db_size(get_fileref_db()).
+
+
+
+%%--------------------------------------------------------------------
+%% Internal funcations 
+%%--------------------------------------------------------------------
+
 get_pem_cache() ->
     {status, _, _, StatusInfo} = sys:get_status(whereis(ssl_manager)),
     [_, _,_, _, Prop] = StatusInfo,
@@ -120,6 +151,16 @@ get_pem_cache() ->
 	    undefined
     end.
 
+get_fileref_db() ->
+    {status, _, _, StatusInfo} = sys:get_status(whereis(ssl_manager)),
+    [_, _,_, _, Prop] = StatusInfo,
+    State = ssl_test_lib:state(Prop),
+    case element(6, State) of
+	[_CertDb, {FileRefDb,_} | _] ->
+	    FileRefDb;
+	_ ->
+	    undefined
+    end.
 later()->
     DateTime = calendar:now_to_local_time(os:timestamp()), 
     Gregorian = calendar:datetime_to_gregorian_seconds(DateTime),