ssl: File handling optimization

Avoid cach validation with file:file_info/2 as this i too expensive and
causes a bottleneck in the file server. Instead we expose a new API function
ssl:clear_pem_cache/0 to deal with the problem. As we think it will be
of occasional use and the normal case is that the cache will be valid we think
it is the right thing to do.

Convert file paths to binary representation in the ssl API module to
avoid uncessarry calls in file later on.

Also add sanity checks for openssl versions in testsuite due to new
openssl bugs.

6 files changed, 134 insertions, 95 deletions

diff --git a/lib/ssl/src/ssl.erl b/lib/ssl/src/ssl.erl
index 0bcdffbeff..5e3ced144a 100644
--- a/lib/ssl/src/ssl.erl
+++ b/lib/ssl/src/ssl.erl
@@ -30,7 +30,7 @@
 	 controlling_process/2, listen/2, pid/1, peername/1, peercert/1,
 	 recv/2, recv/3, send/2, getopts/2, setopts/2, sockname/1,
 	 versions/0, session_info/1, format_error/1,
-	 renegotiate/1, prf/5]).
+	 renegotiate/1, prf/5, clear_pem_cache/0]).
 
 -deprecated({pid, 1, next_major_release}).
 
@@ -431,6 +431,15 @@ prf(#sslsocket{pid = Pid, fd = new_ssl},
     Secret, Label, Seed, WantedLength) ->
     ssl_connection:prf(Pid, Secret, Label, Seed, WantedLength).
 
+
+%%--------------------------------------------------------------------
+-spec clear_pem_cache() -> ok.
+%%
+%% Description: Clear the PEM cache
+%%--------------------------------------------------------------------
+clear_pem_cache() ->
+    ssl_manager:clear_pem_cache().
+
 %%---------------------------------------------------------------
 -spec format_error({error, term()}) -> list().
 %%
@@ -532,7 +541,7 @@ handle_options(Opts0, _Role) ->
 		throw({error, {eoptions, {verify, Value}}})
 	end,
 
-    CertFile = handle_option(certfile, Opts, ""),
+    CertFile = handle_option(certfile, Opts, <<>>),
     
     SSLOptions = #ssl_options{
       versions   = handle_option(versions, Opts, []),
@@ -619,8 +628,12 @@ validate_option(depth, Value) when is_integer(Value),
 validate_option(cert, Value) when Value == undefined;
                                  is_binary(Value) ->
     Value;
-validate_option(certfile, Value) when Value == undefined; is_list(Value) ->
+validate_option(certfile, undefined = Value) ->
     Value;
+validate_option(certfile, Value) when is_binary(Value) ->
+    Value;
+validate_option(certfile, Value) when is_list(Value) ->
+    list_to_binary(Value);
 
 validate_option(key, undefined) ->
     undefined;
@@ -631,8 +644,13 @@ validate_option(key, {KeyType, Value}) when is_binary(Value),
 					    KeyType == 'DSAPrivateKey';
 					    KeyType == 'PrivateKeyInfo' ->
     {KeyType, Value};
-validate_option(keyfile, Value) when is_list(Value) ->
+
+validate_option(keyfile, undefined) ->
+   <<>>;
+validate_option(keyfile, Value) when is_binary(Value) ->
     Value;
+validate_option(keyfile, Value) when is_list(Value), Value =/= "" ->
+    list_to_binary(Value);
 validate_option(password, Value) when is_list(Value) ->
     Value;
 
@@ -642,16 +660,20 @@ validate_option(cacerts, Value) when Value == undefined;
 %% certfile must be present in some cases otherwhise it can be set
 %% to the empty string.
 validate_option(cacertfile, undefined) ->
-    "";
-validate_option(cacertfile, Value) when is_list(Value), Value =/= "" ->
+   <<>>;
+validate_option(cacertfile, Value) when is_binary(Value) ->
     Value;
+validate_option(cacertfile, Value) when is_list(Value), Value =/= ""->
+    list_to_binary(Value);
 validate_option(dh, Value) when Value == undefined;
 				is_binary(Value) ->
     Value;
 validate_option(dhfile, undefined = Value)  ->
     Value;
-validate_option(dhfile, Value) when is_list(Value), Value =/= "" ->
+validate_option(dhfile, Value) when is_binary(Value) ->
     Value;
+validate_option(dhfile, Value) when is_list(Value), Value =/= "" ->
+    list_to_binary(Value);
 validate_option(ciphers, Value)  when is_list(Value) ->
     Version = ssl_record:highest_protocol_version([]),
     try cipher_suites(Version, Value)
diff --git a/lib/ssl/src/ssl_certificate_db.erl b/lib/ssl/src/ssl_certificate_db.erl
index 80fdc5201a..01ddf056c9 100644
--- a/lib/ssl/src/ssl_certificate_db.erl
+++ b/lib/ssl/src/ssl_certificate_db.erl
@@ -27,15 +27,11 @@
 -include_lib("kernel/include/file.hrl").
 
 -export([create/0, remove/1, add_trusted_certs/3, 
-	 remove_trusted_certs/2, insert/3, remove/2, clean/1,
+	 remove_trusted_certs/2, insert/3, remove/2, clear/1, db_size/1,
 	 ref_count/3, lookup_trusted_cert/4, foldl/3,
-	 lookup_cached_pem/2, cache_pem_file/3, cache_pem_file/4,
+	 lookup_cached_pem/2, cache_pem_file/2, cache_pem_file/3,
 	 lookup/2]).
 
--type time()      :: {non_neg_integer(), non_neg_integer(), non_neg_integer()}.
-
--define(NOT_TO_BIG, 10).
-
 %%====================================================================
 %% Internal application API
 %%====================================================================
@@ -90,7 +86,8 @@ lookup_cached_pem(PemChache, MD5) ->
     lookup(MD5, PemChache).
 
 %%--------------------------------------------------------------------
--spec add_trusted_certs(pid(), string() | {der, list()}, [db_handle()]) -> {ok, [db_handle()]}.
+-spec add_trusted_certs(pid(), {erlang:timestamp(), string()} |
+			{der, list()}, [db_handle()]) -> {ok, [db_handle()]}.
 %%
 %% Description: Adds the trusted certificates from file <File> to the
 %% runtime database. Returns Ref that should be handed to lookup_trusted_cert
@@ -102,49 +99,36 @@ add_trusted_certs(_Pid, {der, DerList}, [CerDb, _,_]) ->
     {ok, NewRef};
 
 add_trusted_certs(_Pid, File, [CertsDb, RefDb, PemChache] = Db) ->
-    {ok, #file_info{mtime = LastWrite}} = file:read_file_info(File),
     MD5 = crypto:md5(File),
     case lookup_cached_pem(Db, MD5) of
-	[{Mtime, Content}] ->
-	    case LastWrite of
-		Mtime ->
-		    Ref0 = make_ref(),
-		    insert(Ref0, [], 1, RefDb),
-		    insert(MD5, {Mtime, Content, Ref0}, PemChache),
-		    add_certs_from_pem(Content, Ref0, CertsDb),
-		    {ok, Ref0};
-		_ ->
-		    new_trusted_cert_entry(File, LastWrite, Db)
-	    end;
-	[{Mtime, _Content, Ref0}] ->
-	    case LastWrite of
-		Mtime ->
-		    ref_count(Ref0, RefDb, 1),
-		    {ok, Ref0};
-		_ ->
-		    new_trusted_cert_entry(File, LastWrite, Db)
-	    end;
+	[{_Content, Ref}] ->
+	    ref_count(Ref, RefDb, 1),
+	    {ok, Ref};
+	[Content] ->
+	    Ref = make_ref(),
+	    insert(Ref, [], 1, RefDb),
+	    insert(MD5, {Content, Ref}, PemChache),
+	    add_certs_from_pem(Content, Ref, CertsDb),
+	    {ok, Ref};
 	undefined ->
-	    new_trusted_cert_entry(File, LastWrite, Db)
+	    new_trusted_cert_entry({MD5, File}, Db)
     end.
 %%--------------------------------------------------------------------
--spec cache_pem_file(string(), time(), [db_handle()]) -> term().
--spec cache_pem_file(reference(), string(), time(), [db_handle()]) -> term().
+-spec cache_pem_file(string(), [db_handle()]) -> term().
+-spec cache_pem_file(reference(), string(), [db_handle()]) -> term().
 %%
 %% Description: Cache file as binary in DB
 %%--------------------------------------------------------------------
-cache_pem_file(File, Time, [_CertsDb, _RefDb, PemChache]) ->
-    {ok, PemBin} = file:read_file(File), 
+cache_pem_file({MD5, File}, [_CertsDb, _RefDb, PemChache]) ->
+    {ok, PemBin} = file:read_file(File),
     Content = public_key:pem_decode(PemBin),
-    MD5 = crypto:md5(File),
-    insert(MD5, {Time, Content}, PemChache),
+    insert(MD5, Content, PemChache),
     {ok, Content}.
 
-cache_pem_file(Ref, File, Time, [_CertsDb, _RefDb, PemChache]) ->
+cache_pem_file(Ref, {MD5, File}, [_CertsDb, _RefDb, PemChache]) ->
     {ok, PemBin} = file:read_file(File),
     Content = public_key:pem_decode(PemBin),
-    MD5 = crypto:md5(File),
-    insert(MD5, {Time, Content, Ref}, PemChache),
+    insert(MD5, {Content, Ref}, PemChache),
     {ok, Content}.
 
 remove_trusted_certs(Ref, CertsDb) ->
@@ -194,17 +178,21 @@ ref_count(Key, Db, N) ->
     ets:update_counter(Db,Key,N).
 
 %%--------------------------------------------------------------------
--spec clean(db_handle()) -> term().
+-spec clear(db_handle()) -> term().
 %%
-%% Description: Updates a reference counter in a <Db>.
+%% Description: Clears the cache
 %%--------------------------------------------------------------------
-clean(Db) ->
-    case ets:info(Db, size) of
-	N  when N < ?NOT_TO_BIG ->
-	    ok;
-	_ ->
-	    ets:delete_all_objects(Db)
-    end.
+clear(Db) ->
+    ets:delete_all_objects(Db).
+
+%%--------------------------------------------------------------------
+-spec db_size(db_handle()) -> integer().
+%%
+%% Description: Returns the size of the db
+%%--------------------------------------------------------------------
+db_size(Db) ->
+    ets:info(Db, size).
+
 %%--------------------------------------------------------------------
 %%-spec insert(Key::term(), Data::term(), Db::db_handle()) -> no_return().
 %%
@@ -246,9 +234,9 @@ add_certs(Cert, Ref, CertsDb) ->
 	    error_logger:info_report(Report)
     end.
 
-new_trusted_cert_entry(File, LastWrite, [CertsDb,RefDb,PemChache] = Db) ->
+new_trusted_cert_entry(FileRef, [CertsDb, RefDb, _] = Db) ->
     Ref = make_ref(),
     insert(Ref, [], 1, RefDb),
-    {ok, Content} = cache_pem_file(Ref, File, LastWrite, Db),
+    {ok, Content} = cache_pem_file(Ref, FileRef, Db),
     add_certs_from_pem(Content, Ref, CertsDb),
     {ok, Ref}.
diff --git a/lib/ssl/src/ssl_connection.erl b/lib/ssl/src/ssl_connection.erl
index 2abe2d3a6c..ce20e72524 100644
--- a/lib/ssl/src/ssl_connection.erl
+++ b/lib/ssl/src/ssl_connection.erl
@@ -1094,7 +1094,7 @@ init_certificates(#ssl_options{cacerts = CaCerts,
 	end,
     init_certificates(Cert, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, CertFile, Role).
 
-init_certificates(undefined, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, "", _) ->
+init_certificates(undefined, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, <<>>, _) ->
     {ok, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, undefined};
 
 init_certificates(undefined, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheHandle, CertFile, client) ->
@@ -1117,7 +1117,7 @@ init_certificates(undefined, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHan
 init_certificates(Cert, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheRef, _, _) ->
     {ok, CertDbRef, CertDbHandle, FileRefHandle, PemCacheHandle, CacheRef, Cert}.
 
-init_private_key(_, undefined, "", _Password, _Client) ->
+init_private_key(_, undefined, <<>>, _Password, _Client) ->
     undefined;
 init_private_key(DbHandle, undefined, KeyFile, Password, _) ->
     try
@@ -2373,7 +2373,7 @@ get_timeout(#state{ssl_options=#ssl_options{hibernate_after = undefined}}) ->
 get_timeout(#state{ssl_options=#ssl_options{hibernate_after = HibernateAfter}}) ->
     HibernateAfter.
 
-handle_trusted_certs_db(#state{ssl_options = #ssl_options{cacertfile = ""}}) ->
+handle_trusted_certs_db(#state{ssl_options = #ssl_options{cacertfile = <<>>}}) ->
     %% No trusted certs specified
     ok;
 handle_trusted_certs_db(#state{cert_db_ref = Ref,
diff --git a/lib/ssl/src/ssl_manager.erl b/lib/ssl/src/ssl_manager.erl
index e3bf0a1f92..a18cb70e2d 100644
--- a/lib/ssl/src/ssl_manager.erl
+++ b/lib/ssl/src/ssl_manager.erl
@@ -32,7 +32,7 @@
 	 lookup_trusted_cert/4,
 	 new_session_id/1, clean_cert_db/2,
 	 register_session/2, register_session/3, invalidate_session/2,
-	 invalidate_session/3]).
+	 invalidate_session/3, clear_pem_cache/0]).
 
 % Spawn export
 -export([init_session_validator/1]).
@@ -58,9 +58,10 @@
 -define('24H_in_sec', 8640).
 -define(GEN_UNIQUE_ID_MAX_TRIES, 10).
 -define(SESSION_VALIDATION_INTERVAL, 60000).
--define(PEM_CACHE_CLEANUP, 120000).
+-define(CLEAR_PEM_CACHE, 120000).
 -define(CLEAN_SESSION_DB, 60000).
 -define(CLEAN_CERT_DB, 500).
+-define(NOT_TO_BIG, 10).
 
 %%====================================================================
 %% API
@@ -84,24 +85,46 @@ start_link_dist(Opts) ->
     gen_server:start_link({local, ssl_manager_dist}, ?MODULE, [ssl_manager_dist, Opts], []).
 
 %%--------------------------------------------------------------------
--spec connection_init(string()| {der, list()}, client | server) ->
+-spec connection_init(binary()| {der, list()}, client | server) ->
 			     {ok, certdb_ref(), db_handle(), db_handle()}.
 %%			     
 %% Description: Do necessary initializations for a new connection.
 %%--------------------------------------------------------------------
+connection_init({der, _} = Trustedcerts, Role) ->
+    call({connection_init, Trustedcerts, Role});
+
+connection_init(<<>> = Trustedcerts, Role) ->
+    call({connection_init, Trustedcerts, Role});
+
 connection_init(Trustedcerts, Role) ->
     call({connection_init, Trustedcerts, Role}).
+
 %%--------------------------------------------------------------------
--spec cache_pem_file(string(), term()) -> {ok, term()} | {error, reason()}.
+-spec cache_pem_file(binary(), term()) -> {ok, term()} | {error, reason()}.
 %%		    
 %% Description: Cach a pem file and return its content.
 %%--------------------------------------------------------------------
 cache_pem_file(File, DbHandle) ->
-    case file:read_file_info(File) of
-	{ok, #file_info{mtime = LastWrite}} ->
-	    cache_pem_file(File, LastWrite, DbHandle);
-	Error -> Error
+    MD5 = crypto:md5(File),
+    case ssl_certificate_db:lookup_cached_pem(DbHandle, MD5) of
+	[Content] ->
+	   {ok, Content};
+	[{Content,_}] ->
+	    {ok, Content};
+	undefined ->
+	    call({cache_pem, {MD5, File}})
     end.
+
+%%--------------------------------------------------------------------
+-spec clear_pem_cache() -> ok.
+%%
+%% Description: Clear the PEM cache
+%%--------------------------------------------------------------------
+clear_pem_cache() ->
+    %% Not supported for distribution at the moement, should it be?
+    put(ssl_manager, ssl_manager),
+    call(unconditionally_clear_pem_cache).
+
 %%--------------------------------------------------------------------
 -spec lookup_trusted_cert(term(), reference(), serialnumber(), issuer()) ->
 				 undefined | 
@@ -170,7 +193,7 @@ init([Name, Opts]) ->
     SessionCache = CacheCb:init(proplists:get_value(session_cb_init_args, Opts, [])),
     Timer = erlang:send_after(SessionLifeTime * 1000, 
 			      self(), validate_sessions),
-    erlang:send_after(?PEM_CACHE_CLEANUP, self(), clean_pem_cache),
+    erlang:send_after(?CLEAR_PEM_CACHE, self(), clear_pem_cache),
     {ok, #state{certificate_db = CertDb,
 		session_cache = SessionCache,
 		session_cache_cb = CacheCb,
@@ -188,7 +211,7 @@ init([Name, Opts]) ->
 %%
 %% Description: Handling call messages
 %%--------------------------------------------------------------------
-handle_call({{connection_init, "", _Role}, _Pid}, _From,
+handle_call({{connection_init, <<>>, _Role}, _Pid}, _From,
 	    #state{certificate_db = [CertDb, FileRefDb, PemChace],
 		   session_cache = Cache} = State) ->
     Result = {ok, make_ref(),CertDb, FileRefDb, PemChace, Cache},
@@ -214,15 +237,18 @@ handle_call({{new_session_id,Port}, _},
     {reply, Id, State};
 
 
-handle_call({{cache_pem, File, LastWrite}, _Pid}, _,
+handle_call({{cache_pem, File}, _Pid}, _,
 	    #state{certificate_db = Db} = State) ->
-    try ssl_certificate_db:cache_pem_file(File, LastWrite, Db) of
+    try ssl_certificate_db:cache_pem_file(File, Db) of
 	Result ->
 	    {reply, Result, State}
     catch 
 	_:Reason ->
 	    {reply, {error, Reason}, State}
-    end.
+    end;
+handle_call({unconditionally_clear_pem_cache, _},_, #state{certificate_db = [_,_,PemChace]} = State) ->
+    ssl_certificate_db:clear(PemChace),
+    {reply, ok,  State}.
 
 %%--------------------------------------------------------------------
 -spec  handle_cast(msg(), #state{}) -> {noreply, #state{}}.
@@ -283,19 +309,25 @@ handle_info({delayed_clean_session, Key}, #state{session_cache = Cache,
     CacheCb:delete(Cache, Key),
     {noreply, State};
 
-handle_info(clean_pem_cache, #state{certificate_db = [_,_,PemChace]} = State) ->
-    ssl_certificate_db:clean(PemChace),
-    erlang:send_after(?PEM_CACHE_CLEANUP, self(), clean_pem_cache),
+handle_info(clear_pem_cache, #state{certificate_db = [_,_,PemChace]} = State) ->
+    case ssl_certificate_db:db_size(PemChace) of
+	N  when N < ?NOT_TO_BIG ->
+	    ok;
+	_ ->
+	    ssl_certificate_db:clear(PemChace)
+    end,
+    erlang:send_after(?CLEAR_PEM_CACHE, self(), clear_pem_cache),
     {noreply, State};
 
+
 handle_info({clean_cert_db, Ref, File},
 	    #state{certificate_db = [CertDb,RefDb, PemCache]} = State) ->
     case ssl_certificate_db:ref_count(Ref, RefDb, 0) of
 	0 ->
 	    MD5 = crypto:md5(File),
 	    case ssl_certificate_db:lookup_cached_pem(MD5, PemCache) of
-		[{Mtime, Content, Ref}] ->
-		    ssl_certificate_db:insert(MD5, {Mtime, Content}, PemCache);
+		[{Content, Ref}] ->
+		    ssl_certificate_db:insert(MD5, Content, PemCache);
 		undefined ->
 		    ok
 	    end,
@@ -380,24 +412,6 @@ session_validation({{Port, _}, Session}, LifeTime) ->
     validate_session(Port, Session, LifeTime),
     LifeTime.
 
-cache_pem_file(File, LastWrite, DbHandle) ->
-    case ssl_certificate_db:lookup_cached_pem(DbHandle,crypto:md5(File)) of
-	[{Mtime, Content}] ->
-	    handle_cached_entry(File, LastWrite, Mtime, Content);
-	[{Mtime, Content,_}] ->
-	     handle_cached_entry(File, LastWrite, Mtime, Content);
-	undefined ->
-	    call({cache_pem, File, LastWrite})
-    end.
-
-handle_cached_entry(File, LastWrite, Time, Content) ->
-    case LastWrite of
-	Time ->
-	    {ok, Content};
-	_ ->
-	    call({cache_pem, File, LastWrite})
-    end.
-
 delay_time() ->
     case application:get_env(ssl, session_delay_cleanup_time) of
 	{ok, Time} when is_integer(Time) ->
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index 590ecf33ca..0618628df2 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -3649,6 +3649,8 @@ no_reuses_session_server_restart_new_cert_file(Config) when is_list(Config) ->
     ssl_test_lib:close(Server),
     ssl_test_lib:close(Client0),
 
+    ssl:clear_pem_cache(),
+
     NewServerOpts = new_config(PrivDir, DsaServerOpts),
 
     Server1 =
diff --git a/lib/ssl/test/ssl_to_openssl_SUITE.erl b/lib/ssl/test/ssl_to_openssl_SUITE.erl
index 01fca1f166..8197e14b64 100644
--- a/lib/ssl/test/ssl_to_openssl_SUITE.erl
+++ b/lib/ssl/test/ssl_to_openssl_SUITE.erl
@@ -112,6 +112,9 @@ special_init(TestCase, Config)
 special_init(ssl2_erlang_server_openssl_client, Config) ->
     check_sane_openssl_sslv2(Config);
 
+special_init(ciphers_dsa_signed_certs, Config) ->
+    check_sane_openssl_dsa(Config);
+
 special_init(_, Config) ->
     Config.
     
@@ -1440,14 +1443,24 @@ check_sane_openssl_renegotaite(Config) ->
 	    {skip, "Known renegotiation bug in OppenSSL"};
 	"OpenSSL 0.9.7" ++ _ ->
 	    {skip, "Known renegotiation bug in OppenSSL"};
+	"OpenSSL 1.0.1c" ++ _ ->
+	    {skip, "Known renegotiation bug in OppenSSL"};
 	_ ->
 	    Config
     end.
 
 check_sane_openssl_sslv2(Config) ->
     case os:cmd("openssl version") of
-	"OpenSSL 1.0.0" ++ _ ->
+	"OpenSSL 1." ++ _ ->
 	    {skip, "sslv2 by default turned of in 1.*"};
 	_ ->
 	    Config
     end.
+
+check_sane_openssl_dsa(Config) ->
+    case os:cmd("openssl version") of
+	"OpenSSL 1.0.1" ++ _ ->
+	    {skip, "known dsa bug in openssl"};
+	_ ->
+	    Config
+    end.