aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorIngela Anderton Andin <[email protected]>2013-01-15 11:32:19 +0100
committerIngela Anderton Andin <[email protected]>2013-01-17 09:46:51 +0100
commit7fa4c654f29a3231f707dcac7fffdf60140cf1b3 (patch)
tree0eda89c10ac23b3b993cd637c8f009d90cace3c1
parentfa6771380931c7ded0ad4d36e6cd2554bb932dfc (diff)
downloadotp-7fa4c654f29a3231f707dcac7fffdf60140cf1b3.tar.gz
otp-7fa4c654f29a3231f707dcac7fffdf60140cf1b3.tar.bz2
otp-7fa4c654f29a3231f707dcac7fffdf60140cf1b3.zip
ssl: Certificates and PEM-cache cleaning fixed to avoid memory leak
Certificate db cleaning messages where sent to the wrong process after restructuring to avoid bottlenecks. It is possible that the ssl manager process gets two cleaning messages for the same entry. E.i. first cleaning message is sent and before it is processed a new reference is allocated and again released for the entry, generating a second cleaning message. Also in ssl_manger:handle_info/2 it is possible that there exists a new reference to an "old" file name with a potential new content.
-rw-r--r--lib/ssl/src/ssl_manager.erl43
-rw-r--r--lib/ssl/test/ssl_basic_SUITE.erl51
2 files changed, 78 insertions, 16 deletions
diff --git a/lib/ssl/src/ssl_manager.erl b/lib/ssl/src/ssl_manager.erl
index 13689ce7d8..14fba72d86 100644
--- a/lib/ssl/src/ssl_manager.erl
+++ b/lib/ssl/src/ssl_manager.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2007-2012. All Rights Reserved.
+%% Copyright Ericsson AB 2007-2013. All Rights Reserved.
%%
%% The contents of this file are subject to the Erlang Public License,
%% Version 1.1, (the "License"); you may not use this file except in
@@ -142,8 +142,14 @@ lookup_trusted_cert(DbHandle, Ref, SerialNumber, Issuer) ->
new_session_id(Port) ->
call({new_session_id, Port}).
+%%--------------------------------------------------------------------
+-spec clean_cert_db(reference(), binary()) -> term().
+%%
+%% Description: Send clean request of cert db to ssl_manager process should
+%% be called by ssl-connection processes.
+%%--------------------------------------------------------------------
clean_cert_db(Ref, File) ->
- erlang:send_after(?CLEAN_CERT_DB, self(), {clean_cert_db, Ref, File}).
+ erlang:send_after(?CLEAN_CERT_DB, get(ssl_manager), {clean_cert_db, Ref, File}).
%%--------------------------------------------------------------------
-spec register_session(inet:port_number(), #session{}) -> ok.
@@ -320,19 +326,12 @@ handle_info(clear_pem_cache, #state{certificate_db = [_,_,PemChace]} = 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(PemCache, MD5) of
- [{Content, Ref}] ->
- ssl_certificate_db:insert(MD5, Content, PemCache);
- undefined ->
- ok
- end,
- ssl_certificate_db:remove(Ref, RefDb),
- ssl_certificate_db:remove_trusted_certs(Ref, CertDb);
+
+ case ssl_certificate_db:lookup(Ref, RefDb) of
+ undefined -> %% Alredy cleaned
+ ok;
_ ->
- ok
+ clean_cert_db(Ref, CertDb, RefDb, PemCache, File)
end,
{noreply, State};
@@ -464,3 +463,19 @@ new_id(Port, Tries, Cache, CacheCb) ->
_ ->
new_id(Port, Tries - 1, Cache, CacheCb)
end.
+
+clean_cert_db(Ref, CertDb, RefDb, PemCache, File) ->
+ case ssl_certificate_db:ref_count(Ref, RefDb, 0) of
+ 0 ->
+ MD5 = crypto:md5(File),
+ case ssl_certificate_db:lookup_cached_pem(PemCache, MD5) of
+ [{Content, Ref}] ->
+ ssl_certificate_db:insert(MD5, Content, PemCache);
+ _ ->
+ ok
+ end,
+ ssl_certificate_db:remove(Ref, RefDb),
+ ssl_certificate_db:remove_trusted_certs(Ref, CertDb);
+ _ ->
+ ok
+ end.
diff --git a/lib/ssl/test/ssl_basic_SUITE.erl b/lib/ssl/test/ssl_basic_SUITE.erl
index 5ba71f9218..a313380ece 100644
--- a/lib/ssl/test/ssl_basic_SUITE.erl
+++ b/lib/ssl/test/ssl_basic_SUITE.erl
@@ -84,7 +84,8 @@ basic_tests() ->
alerts,
send_close,
connect_twice,
- connect_dist
+ connect_dist,
+ clear_pem_cache
].
options_tests() ->
@@ -536,6 +537,33 @@ connect_dist(Config) when is_list(Config) ->
ssl_test_lib:close(Client).
%%--------------------------------------------------------------------
+
+clear_pem_cache() ->
+ [{doc,"Test that internal reference tabel is cleaned properly even when "
+ " the PEM cache is cleared" }].
+clear_pem_cache(Config) when is_list(Config) ->
+ {status, _, _, StatusInfo} = sys:get_status(whereis(ssl_manager)),
+ [_, _,_, _, Prop] = StatusInfo,
+ State = ssl_test_lib:state(Prop),
+ [_,FilRefDb, _] = element(5, State),
+ {Server, Client} = basic_verify_test_no_close(Config),
+ 2 = ets:info(FilRefDb, size),
+ ssl:clear_pem_cache(),
+ _ = sys:get_status(whereis(ssl_manager)),
+ {Server1, Client1} = basic_verify_test_no_close(Config),
+ 4 = ets:info(FilRefDb, size),
+ ssl_test_lib:close(Server),
+ ssl_test_lib:close(Client),
+ ct:sleep(5000),
+ _ = sys:get_status(whereis(ssl_manager)),
+ 2 = ets:info(FilRefDb, size),
+ ssl_test_lib:close(Server1),
+ ssl_test_lib:close(Client1),
+ ct:sleep(5000),
+ _ = sys:get_status(whereis(ssl_manager)),
+ 0 = ets:info(FilRefDb, size).
+
+%%--------------------------------------------------------------------
peername() ->
[{doc,"Test API function peername/1"}].
@@ -2641,6 +2669,26 @@ tcp_send_recv_result(Socket) ->
{ok,"Hello world"} = gen_tcp:recv(Socket, 11),
ok.
+basic_verify_test_no_close(Config) ->
+ ClientOpts = ?config(client_verification_opts, Config),
+ ServerOpts = ?config(server_verification_opts, Config),
+
+ {ClientNode, ServerNode, Hostname} = ssl_test_lib:run_where(Config),
+
+ Server = ssl_test_lib:start_server([{node, ServerNode}, {port, 0},
+ {from, self()},
+ {mfa, {ssl_test_lib, send_recv_result_active, []}},
+ {options, ServerOpts}]),
+ Port = ssl_test_lib:inet_port(Server),
+ Client = ssl_test_lib:start_client([{node, ClientNode}, {port, Port},
+ {host, Hostname},
+ {from, self()},
+ {mfa, {ssl_test_lib, send_recv_result_active, []}},
+ {options, ClientOpts}]),
+
+ ssl_test_lib:check_result(Server, ok, Client, ok),
+ {Server, Client}.
+
basic_test(Config) ->
ClientOpts = ?config(client_opts, Config),
ServerOpts = ?config(server_opts, Config),
@@ -2659,7 +2707,6 @@ basic_test(Config) ->
{options, ClientOpts}]),
ssl_test_lib:check_result(Server, ok, Client, ok),
-
ssl_test_lib:close(Server),
ssl_test_lib:close(Client).