Skip to content

Commit 5a4112b

Browse files
HansNHansN
committed
Merge branch 'hans/ssh/rekey_limit/OTP-15069'
* hans/ssh/rekey_limit/OTP-15069: ssh: Extend rekey_limit to also take an optional time
2 parents a8ede33 + 79e3c47 commit 5a4112b

5 files changed

Lines changed: 191 additions & 20 deletions

File tree

‎lib/ssh/doc/src/ssh.xml‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -763,8 +763,16 @@
763763
<datatype>
764764
<name name="rekey_limit_common_option"/>
765765
<desc>
766-
<p>Sets a limit, in bytes, when rekeying is to be initiated.
767-
Defaults to once per each GB and once per hour.</p>
766+
<p>Sets the limit when rekeying is to be initiated. Both the max time and max amount of data
767+
could be configured:
768+
</p>
769+
<list>
770+
<item><c>{Minutes, Bytes}</c> initiate rekeying when any of the limits are reached.</item>
771+
<item><c>Bytes</c> initiate rekeying when <c>Bytes</c> number of bytes are transferred,
772+
or at latest after one hour.</item>
773+
</list>
774+
<p>When a rekeying is done, both the timer and the byte counter are restarted.
775+
Defaults to one hour and one GByte.</p>
768776
</desc>
769777
</datatype>
770778

‎lib/ssh/src/ssh.hrl‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929

3030
-define(SSH_DEFAULT_PORT, 22).
3131
-define(SSH_MAX_PACKET_SIZE, (256*1024)).
32-
-define(REKEY_TIMOUT, 3600000).
3332
-define(REKEY_DATA_TIMOUT, 60000).
3433
-define(DEFAULT_PROFILE, default).
3534

@@ -192,7 +191,9 @@
192191
-type user_dir_common_option() :: {user_dir, false | string()}.
193192
-type profile_common_option() :: {profile, atom() }.
194193
-type max_idle_time_common_option() :: {idle_time, timeout()}.
195-
-type rekey_limit_common_option() :: {rekey_limit, non_neg_integer() }.
194+
-type rekey_limit_common_option() :: {rekey_limit, Bytes::non_neg_integer() |
195+
{Minutes::non_neg_integer(), Bytes::non_neg_integer()}
196+
}.
196197

197198
-type key_cb_common_option() :: {key_cb, Module::atom() | {Module::atom(),Opts::[term()]} } .
198199
-type disconnectfun_common_option() ::

‎lib/ssh/src/ssh_connection_handler.erl‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,6 @@ init([Role,Socket,Opts]) ->
429429
},
430430
D = case Role of
431431
client ->
432-
%% Start the renegotiation timers
433-
timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]),
434-
timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]),
435432
cache_init_idle_timer(D0);
436433
server ->
437434
Sups = ?GET_INTERNAL_OPT(supervisors, Opts),
@@ -444,6 +441,10 @@ init([Role,Socket,Opts]) ->
444441
connection_supervisor = proplists:get_value(connection_sup, Sups)
445442
}})
446443
end,
444+
%% Start the renegotiation timers
445+
{RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts),
446+
timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]),
447+
timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]),
447448
{ok, {hello,Role}, D};
448449

449450
{error,Error} ->
@@ -1066,25 +1067,27 @@ handle_event(internal, Msg=#ssh_msg_channel_failure{}, StateName, D) -
10661067
handle_event(cast, renegotiate, {connected,Role}, D) ->
10671068
{KeyInitMsg, SshPacket, Ssh} = ssh_transport:key_exchange_init_msg(D#data.ssh_params),
10681069
send_bytes(SshPacket, D),
1069-
timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]),
1070+
{RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, Ssh#ssh.opts),
1071+
timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]),
10701072
{next_state, {kexinit,Role,renegotiate}, D#data{ssh_params = Ssh,
10711073
key_exchange_init_msg = KeyInitMsg}};
10721074

10731075
handle_event({call,From}, get_alg, _, D) ->
10741076
#ssh{algorithms=Algs} = D#data.ssh_params,
10751077
{keep_state_and_data, [{reply,From,Algs}]};
10761078

1077-
handle_event(cast, renegotiate, _, _) ->
1079+
handle_event(cast, renegotiate, _, D) ->
10781080
%% Already in key-exchange so safe to ignore
1079-
timer:apply_after(?REKEY_TIMOUT, gen_statem, cast, [self(), renegotiate]), % FIXME: not here in original
1081+
{RekeyTimeout,_MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts),
1082+
timer:apply_after(RekeyTimeout, gen_statem, cast, [self(), renegotiate]),
10801083
keep_state_and_data;
10811084

10821085

10831086
%% Rekey due to sent data limit reached?
10841087
handle_event(cast, data_size, {connected,Role}, D) ->
10851088
{ok, [{send_oct,Sent0}]} = inet:getstat(D#data.socket, [send_oct]),
10861089
Sent = Sent0 - D#data.last_size_rekey,
1087-
MaxSent = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts),
1090+
{_RekeyTimeout,MaxSent} = ?GET_OPT(rekey_limit, (D#data.ssh_params)#ssh.opts),
10881091
timer:apply_after(?REKEY_DATA_TIMOUT, gen_statem, cast, [self(), data_size]),
10891092
case Sent >= MaxSent of
10901093
true ->

‎lib/ssh/src/ssh_options.erl‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -599,9 +599,19 @@ default(common) ->
599599
class => user_options
600600
},
601601

602-
{rekey_limit, def} => % FIXME: Why not common?
603-
#{default => 1024000000,
604-
chk => fun check_non_neg_integer/1,
602+
{rekey_limit, def} =>
603+
#{default => {3600000, 1024000000}, % {1 hour, 1 GB}
604+
chk => fun({TimeMins, SizBytes}) when is_integer(TimeMins) andalso TimeMins>=0,
605+
is_integer(SizBytes) andalso SizBytes>=0 ->
606+
%% New (>= 21) format
607+
{true, {TimeMins * 60*1000, % To ms
608+
SizBytes}};
609+
(SizBytes) when is_integer(SizBytes) andalso SizBytes>=0 ->
610+
%% Old (< 21) format
611+
{true, {3600000, SizBytes}};
612+
(_) ->
613+
false
614+
end,
605615
class => user_options
606616
},
607617

‎lib/ssh/test/ssh_basic_SUITE.erl‎

Lines changed: 155 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,12 @@ groups() ->
7777
]},
7878

7979
{ssh_renegotiate_SUITE, [parallel], [rekey,
80-
rekey_limit,
80+
rekey_limit_client,
81+
rekey_limit_daemon,
82+
rekey_time_limit_client,
83+
rekey_time_limit_daemon,
84+
norekey_limit_client,
85+
norekey_limit_daemon,
8186
renegotiate1,
8287
renegotiate2]},
8388

@@ -1349,17 +1354,17 @@ rekey(Config) ->
13491354

13501355
%%% Test rekeying by data volume
13511356

1352-
rekey_limit() -> [{timetrap,{seconds,400}}].
1353-
1354-
rekey_limit(Config) ->
1357+
rekey_limit_client() -> [{timetrap,{seconds,400}}].
1358+
rekey_limit_client(Config) ->
1359+
Limit = 6000,
13551360
UserDir = proplists:get_value(priv_dir, Config),
13561361
DataFile = filename:join(UserDir, "rekey.data"),
13571362

13581363
Algs = proplists:get_value(preferred_algorithms, Config),
13591364
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0},
13601365
{preferred_algorithms,Algs}]),
13611366

1362-
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, 6000},
1367+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, Limit},
13631368
{max_random_length_padding,0}]),
13641369
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
13651370

@@ -1368,7 +1373,7 @@ rekey_limit(Config) ->
13681373
timer:sleep(?REKEY_DATA_TMO),
13691374
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
13701375

1371-
Data = lists:duplicate(159000,1),
1376+
Data = lists:duplicate(Limit+10,1),
13721377
ok = ssh_sftp:write_file(SftpPid, DataFile, Data),
13731378

13741379
timer:sleep(?REKEY_DATA_TMO),
@@ -1393,6 +1398,150 @@ rekey_limit(Config) ->
13931398
ssh:close(ConnectionRef),
13941399
ssh:stop_daemon(Pid).
13951400

1401+
1402+
1403+
rekey_limit_daemon() -> [{timetrap,{seconds,400}}].
1404+
rekey_limit_daemon(Config) ->
1405+
Limit = 6000,
1406+
UserDir = proplists:get_value(priv_dir, Config),
1407+
DataFile1 = filename:join(UserDir, "rekey1.data"),
1408+
DataFile2 = filename:join(UserDir, "rekey2.data"),
1409+
file:write_file(DataFile1, lists:duplicate(Limit+10,1)),
1410+
file:write_file(DataFile2, "hi\n"),
1411+
1412+
Algs = proplists:get_value(preferred_algorithms, Config),
1413+
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, Limit},
1414+
{max_random_length_padding,0},
1415+
{preferred_algorithms,Algs}]),
1416+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]),
1417+
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
1418+
1419+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1420+
timer:sleep(?REKEY_DATA_TMO),
1421+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1422+
1423+
{ok,_} = ssh_sftp:read_file(SftpPid, DataFile1),
1424+
1425+
timer:sleep(?REKEY_DATA_TMO),
1426+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1427+
false = (Kex2 == Kex1),
1428+
1429+
timer:sleep(?REKEY_DATA_TMO),
1430+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1431+
1432+
{ok,_} = ssh_sftp:read_file(SftpPid, DataFile2),
1433+
1434+
timer:sleep(?REKEY_DATA_TMO),
1435+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1436+
1437+
timer:sleep(?REKEY_DATA_TMO),
1438+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1439+
1440+
ssh_sftp:stop_channel(SftpPid),
1441+
ssh:close(ConnectionRef),
1442+
ssh:stop_daemon(Pid).
1443+
1444+
1445+
%% Check that datatransfer in the other direction does not trigger re-keying
1446+
norekey_limit_client() -> [{timetrap,{seconds,400}}].
1447+
norekey_limit_client(Config) ->
1448+
Limit = 6000,
1449+
UserDir = proplists:get_value(priv_dir, Config),
1450+
DataFile = filename:join(UserDir, "rekey3.data"),
1451+
file:write_file(DataFile, lists:duplicate(Limit+10,1)),
1452+
1453+
Algs = proplists:get_value(preferred_algorithms, Config),
1454+
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0},
1455+
{preferred_algorithms,Algs}]),
1456+
1457+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, Limit},
1458+
{max_random_length_padding,0}]),
1459+
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
1460+
1461+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1462+
timer:sleep(?REKEY_DATA_TMO),
1463+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1464+
1465+
{ok,_} = ssh_sftp:read_file(SftpPid, DataFile),
1466+
timer:sleep(?REKEY_DATA_TMO),
1467+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1468+
1469+
Kex1 = Kex2,
1470+
ssh_sftp:stop_channel(SftpPid),
1471+
ssh:close(ConnectionRef),
1472+
ssh:stop_daemon(Pid).
1473+
1474+
%% Check that datatransfer in the other direction does not trigger re-keying
1475+
norekey_limit_daemon() -> [{timetrap,{seconds,400}}].
1476+
norekey_limit_daemon(Config) ->
1477+
Limit = 6000,
1478+
UserDir = proplists:get_value(priv_dir, Config),
1479+
DataFile = filename:join(UserDir, "rekey4.data"),
1480+
1481+
Algs = proplists:get_value(preferred_algorithms, Config),
1482+
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, Limit},
1483+
{max_random_length_padding,0},
1484+
{preferred_algorithms,Algs}]),
1485+
1486+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]),
1487+
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
1488+
1489+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1490+
timer:sleep(?REKEY_DATA_TMO),
1491+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1492+
1493+
ok = ssh_sftp:write_file(SftpPid, DataFile, lists:duplicate(Limit+10,1)),
1494+
timer:sleep(?REKEY_DATA_TMO),
1495+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1496+
1497+
Kex1 = Kex2,
1498+
ssh_sftp:stop_channel(SftpPid),
1499+
ssh:close(ConnectionRef),
1500+
ssh:stop_daemon(Pid).
1501+
1502+
%%--------------------------------------------------------------------
1503+
%%% Test rekeying by time
1504+
1505+
rekey_time_limit_client() -> [{timetrap,{seconds,400}}].
1506+
rekey_time_limit_client(Config) ->
1507+
Minutes = 1,
1508+
GB = 1024*1000*1000,
1509+
Algs = proplists:get_value(preferred_algorithms, Config),
1510+
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{max_random_length_padding,0},
1511+
{preferred_algorithms,Algs}]),
1512+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{rekey_limit, {Minutes, GB}},
1513+
{max_random_length_padding,0}]),
1514+
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
1515+
rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid).
1516+
1517+
rekey_time_limit_daemon() -> [{timetrap,{seconds,400}}].
1518+
rekey_time_limit_daemon(Config) ->
1519+
Minutes = 1,
1520+
GB = 1024*1000*1000,
1521+
Algs = proplists:get_value(preferred_algorithms, Config),
1522+
{Pid, Host, Port} = ssh_test_lib:std_daemon(Config,[{rekey_limit, {Minutes, GB}},
1523+
{max_random_length_padding,0},
1524+
{preferred_algorithms,Algs}]),
1525+
ConnectionRef = ssh_test_lib:std_connect(Config, Host, Port, [{max_random_length_padding,0}]),
1526+
{ok, SftpPid} = ssh_sftp:start_channel(ConnectionRef),
1527+
rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid).
1528+
1529+
1530+
rekey_time_limit(Pid, Minutes, ConnectionRef, SftpPid) ->
1531+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1532+
1533+
timer:sleep(5000),
1534+
Kex1 = ssh_test_lib:get_kex_init(ConnectionRef),
1535+
1536+
timer:sleep((Minutes*60 + 30) * 1000),
1537+
Kex2 = ssh_test_lib:get_kex_init(ConnectionRef),
1538+
1539+
false = (Kex2 == Kex1),
1540+
1541+
ssh_sftp:stop_channel(SftpPid),
1542+
ssh:close(ConnectionRef),
1543+
ssh:stop_daemon(Pid).
1544+
13961545
%%--------------------------------------------------------------------
13971546

13981547
%%% Test rekeying with simulataneous send request

0 commit comments

Comments
 (0)