From 79e3c477f8d0d8ea397820847c576e0a0aaa5323 Mon Sep 17 00:00:00 2001 From: Hans Nilsson Date: Thu, 3 May 2018 13:48:39 +0200 Subject: ssh: Extend rekey_limit to also take an optional time --- lib/ssh/doc/src/ssh.xml | 12 ++- lib/ssh/src/ssh.hrl | 5 +- lib/ssh/src/ssh_connection_handler.erl | 17 ++-- lib/ssh/src/ssh_options.erl | 16 +++- lib/ssh/test/ssh_basic_SUITE.erl | 161 +++++++++++++++++++++++++++++++-- 5 files changed, 191 insertions(+), 20 deletions(-) diff --git a/lib/ssh/doc/src/ssh.xml b/lib/ssh/doc/src/ssh.xml index 0223831cb1..6aed525e8b 100644 --- a/lib/ssh/doc/src/ssh.xml +++ b/lib/ssh/doc/src/ssh.xml @@ -763,8 +763,16 @@ -

Sets a limit, in bytes, when rekeying is to be initiated. - Defaults to once per each GB and once per hour.

+

Sets the limit when rekeying is to be initiated. Both the max time and max amount of data + could be configured: +

+ + {Minutes, Bytes} initiate rekeying when any of the limits are reached. + Bytes initiate rekeying when Bytes number of bytes are transferred, + or at latest after one hour. + +

When a rekeying is done, both the timer and the byte counter are restarted. + Defaults to one hour and one GByte.

 diff --git a/lib/ssh/src/ssh.hrl b/lib/ssh/src/ssh.hrl index a3d9a1b1cb..fc0a3786ac 100644 --- a/lib/ssh/src/ssh.hrl +++ b/lib/ssh/src/ssh.hrl @@ -29,7 +29,6 @@ -define(SSH_DEFAULT_PORT, 22). -define(SSH_MAX_PACKET_SIZE, (256*1024)). --define(REKEY_TIMOUT, 3600000). -define(REKEY_DATA_TIMOUT, 60000). -define(DEFAULT_PROFILE, default). @@ -192,7 +191,9 @@ -type user_dir_common_option() :: {user_dir, false | string()}. -type profile_common_option() :: {profile, atom() }. -type max_idle_time_common_option() :: {idle_time, timeout()}. --type rekey_limit_common_option() :: {rekey_limit, non_neg_integer() }. +-type rekey_limit_common_option() :: {rekey_limit, Bytes::non_neg_integer() | + {Minutes::non_neg_integer(), Bytes::non_neg_integer()} + }. -type key_cb_common_option() :: {key_cb, Module::atom() | {Module::atom(),Opts::[term()]} } . -type disconnectfun_common_option() :: diff --git a/lib/ssh/src/ssh_connection_handler.erl b/lib/ssh/src/ssh_connection_handler.erl index 57641cf74c..b21c0337ad 100644 --- a/lib/ssh/src/ssh_connection_handler.erl +++ b/lib/ssh/src/ssh_connection_handler.erl @@ -429,9 +429,6 @@ init([Role,Socket,Opts]) -> }, D = case Role of client -> - %% Start the renegotiation timers - timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]), - timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]), cache_init_idle_timer(D0); server -> Sups = ?GET_INTERNAL_OPT(supervisors, Opts), @@ -444,6 +441,10 @@ init([Role,Socket,Opts]) -> connection_supervisor = proplists:get_value(connection_sup, Sups) }}) end, + %% Start the renegotiation timers + {RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts), + timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]), + timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]), {ok, {hello,Role}, D}; {error,Error} -> @@ -1066,7 +1067,8 @@ handle_event(internal, Msg=#ssh_msg_channel_failure{}, StateName, D) - handle_event(cast, renegotiate, {connected,Role}, D) -> {KeyInitMsg, SshPacket, Ssh} = ssh_transport:key_exchange_init_msg(D#data.ssh_params), send_bytes(SshPacket, D), - timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]), + {RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, Ssh#ssh.opts), + timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]), {next_state, {kexinit,Role,renegotiate}, D#data{ssh_params = Ssh, key_exchange_init_msg = KeyInitMsg}}; @@ -1074,9 +1076,10 @@ handle_event({call,From}, get_alg, _, D) -> #ssh{algorithms=Algs} = D#data.ssh_params, {keep_state_and_data, [{reply,From,Algs}]}; -handle_event(cast, renegotiate, _, _) -> +handle_event(cast, renegotiate, _, D) -> %% Already in key-exchange so safe to ignore - timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]), % FIXME: not here in original + {RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts), + timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]), keep_state_and_data; @@ -1084,7 +1087,7 @@ handle_event(cast, renegotiate, _, _) -> handle_event(cast, data_size, {connected,Role}, D) -> {ok, [{send_oct,Sent0}]} = inet:getstat(D#data.socket, [send_oct]), Sent = Sent0 - D#data.last_size_rekey, - MaxSent = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts), + {_RekeyTimeout,MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts), timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]), case Sent >= MaxSent of true -> diff --git a/lib/ssh/src/ssh_options.erl b/lib/ssh/src/ssh_options.erl index 4dd9082250..73287e464a 100644 --- a/lib/ssh/src/ssh_options.erl +++ b/lib/ssh/src/ssh_options.erl @@ -599,9 +599,19 @@ default(common) -> class => user_options }, - {rekey_limit, def} => % FIXME: Why not common? - #{default => 1024000000, - chk => fun check_non_neg_integer/1, + {rekey_limit, def} => + #{default => {3600000, 1024000000}, % {1 hour, 1 GB} + chk => fun({TimeMins, SizBytes}) when is_integer(TimeMins) andalso TimeMins>=0, + is_integer(SizBytes) andalso SizBytes>=0 -> + %% New (>= 21) format + {true, {TimeMins * 60*1000, % To ms + SizBytes}}; + (SizBytes) when is_integer(SizBytes) andalso SizBytes>=0 -> + %% Old (< 21) format + {true, {3600000, SizBytes}}; + (_) -> + false + end, class => user_options }, diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl index 1fa94bef11..603ac71d4b 100644 --- a/lib/ssh/test/ssh_basic_SUITE.erl +++ b/lib/ssh/test/ssh_basic_SUITE.erl @@ -77,7 +77,12 @@ groups() -> ]}, {ssh_renegotiate_SUITE, [parallel], [rekey, - rekey_limit, + rekey_limit_client, + rekey_limit_daemon, + rekey_time_limit_client, + rekey_time_limit_daemon, + norekey_limit_client, + norekey_limit_daemon, renegotiate1, renegotiate2]}, @@ -1349,9 +1354,9 @@ rekey(Config) -> %%% Test rekeying by data volume -rekey_limit() -> [{timetrap,{seconds,400}}]. - -rekey_limit(Config) -> +rekey_limit_client() -> [{timetrap,{seconds,400}}]. +rekey_limit_client(Config) -> + Limit = 6000, UserDir = proplists:get_value(priv_dir, Config), DataFile = filename:join(UserDir, "rekey.data"), @@ -1359,7 +1364,7 @@ rekey_limit(Config) -> {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0}, {preferred_algorithms,Algs}]), - ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, 6000}, + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, Limit}, {max_random_length_padding,0}]), {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), @@ -1368,7 +1373,7 @@ rekey_limit(Config) -> timer:sleep(?REKEY_DATA_TMO), Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), - Data = lists:duplicate(159000,1), + Data = lists:duplicate(Limit+10,1), ok = ssh_sftp:write_file(SftpPid, DataFile, Data), timer:sleep(?REKEY_DATA_TMO), @@ -1393,6 +1398,150 @@ rekey_limit(Config) -> ssh:close(ConnectionRef), ssh:stop_daemon(Pid). + + +rekey_limit_daemon() -> [{timetrap,{seconds,400}}]. +rekey_limit_daemon(Config) -> + Limit = 6000, + UserDir = proplists:get_value(priv_dir, Config), + DataFile1 = filename:join(UserDir, "rekey1.data"), + DataFile2 = filename:join(UserDir, "rekey2.data"), + file:write_file(DataFile1, lists:duplicate(Limit+10,1)), + file:write_file(DataFile2, "hi\n"), + + Algs = proplists:get_value(preferred_algorithms, Config), + {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, Limit}, + {max_random_length_padding,0}, + {preferred_algorithms,Algs}]), + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]), + {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), + + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + timer:sleep(?REKEY_DATA_TMO), + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + + {ok,_} = ssh_sftp:read_file(SftpPid, DataFile1), + + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + false = (Kex2 == Kex1), + + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + {ok,_} = ssh_sftp:read_file(SftpPid, DataFile2), + + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + ssh_sftp:stop_channel(SftpPid), + ssh:close(ConnectionRef), + ssh:stop_daemon(Pid). + + +%% Check that datatransfer in the other direction does not trigger re-keying +norekey_limit_client() -> [{timetrap,{seconds,400}}]. +norekey_limit_client(Config) -> + Limit = 6000, + UserDir = proplists:get_value(priv_dir, Config), + DataFile = filename:join(UserDir, "rekey3.data"), + file:write_file(DataFile, lists:duplicate(Limit+10,1)), + + Algs = proplists:get_value(preferred_algorithms, Config), + {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0}, + {preferred_algorithms,Algs}]), + + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, Limit}, + {max_random_length_padding,0}]), + {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), + + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + timer:sleep(?REKEY_DATA_TMO), + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + + {ok,_} = ssh_sftp:read_file(SftpPid, DataFile), + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + Kex1 = Kex2, + ssh_sftp:stop_channel(SftpPid), + ssh:close(ConnectionRef), + ssh:stop_daemon(Pid). + +%% Check that datatransfer in the other direction does not trigger re-keying +norekey_limit_daemon() -> [{timetrap,{seconds,400}}]. +norekey_limit_daemon(Config) -> + Limit = 6000, + UserDir = proplists:get_value(priv_dir, Config), + DataFile = filename:join(UserDir, "rekey4.data"), + + Algs = proplists:get_value(preferred_algorithms, Config), + {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, Limit}, + {max_random_length_padding,0}, + {preferred_algorithms,Algs}]), + + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]), + {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), + + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + timer:sleep(?REKEY_DATA_TMO), + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + + ok = ssh_sftp:write_file(SftpPid, DataFile, lists:duplicate(Limit+10,1)), + timer:sleep(?REKEY_DATA_TMO), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + Kex1 = Kex2, + ssh_sftp:stop_channel(SftpPid), + ssh:close(ConnectionRef), + ssh:stop_daemon(Pid). + +%%-------------------------------------------------------------------- +%%% Test rekeying by time + +rekey_time_limit_client() -> [{timetrap,{seconds,400}}]. +rekey_time_limit_client(Config) -> + Minutes = 1, + GB = 1024*1000*1000, + Algs = proplists:get_value(preferred_algorithms, Config), + {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0}, + {preferred_algorithms,Algs}]), + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, {Minutes, GB}}, + {max_random_length_padding,0}]), + {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), + rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid). + +rekey_time_limit_daemon() -> [{timetrap,{seconds,400}}]. +rekey_time_limit_daemon(Config) -> + Minutes = 1, + GB = 1024*1000*1000, + Algs = proplists:get_value(preferred_algorithms, Config), + {Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, {Minutes, GB}}, + {max_random_length_padding,0}, + {preferred_algorithms,Algs}]), + ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]), + {ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef), + rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid). + + +rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid) -> + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + + timer:sleep(5000), + Kex1 = ssh_test_lib:get_kex_init(ConnectionRef), + + timer:sleep((Minutes*60 + 30) * 1000), + Kex2 = ssh_test_lib:get_kex_init(ConnectionRef), + + false = (Kex2 == Kex1), + + ssh_sftp:stop_channel(SftpPid), + ssh:close(ConnectionRef), + ssh:stop_daemon(Pid). + %%-------------------------------------------------------------------- %%% Test rekeying with simulataneous send request -- cgit v1.2.3