Re: [LTP] [PATCH] ssh-stress: disable resource penalties

[Petr Vorel <pvorel@suse.cz>]

Hi Vasileios,

first, our ML requires subscription (unlike kernel's lore). I subscribed you then.

> Our tests create a number of ssh sessions in the
> background which are immediately killed. Some of
> them haven't finished the authentication stage yet
> and they close the connection incurring penalties from
> the ssh daemon.

> debug1: srclimit_penalise: active penalty for ipv4 10.0.0.1/32
> already exists, 16 seconds remaining

> Then when we try to reconnect to the daemon we are bounced
> because of the active penalty which leads to a failed test.

> ssh-stress 1 TINFO: Killing all ssh sessions
> kex_exchange_identification: read: Connection reset by peer
> Connection reset by fd00:1:1:1::2 port 58373
> ssh-stress 1 TFAIL: SSH not reachable

> From the sshd logs we can see

> debug1: srclimit_penalise: active penalty for ipv4 10.0.0.1/32
> already exists, 16 seconds remaining

> This feature was added to OpenSSH 9.8 in the 2024 release in the
> 81c1099d2 commit. Lets disable penalties for the versions that
> support them.

> Signed-off-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
> ---
>  testcases/network/stress/ssh/ssh-stress.sh | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)

> diff --git a/testcases/network/stress/ssh/ssh-stress.sh b/testcases/network/stress/ssh/ssh-stress.sh
> index c27c27a28..cb6659ed5 100755
> --- a/testcases/network/stress/ssh/ssh-stress.sh
> +++ b/testcases/network/stress/ssh/ssh-stress.sh
> @@ -39,8 +39,12 @@ cleanup()

>  setup()
>  {
> -	local port rc
> +	local port rc version major minor

> +	version=$(sshd -V 2>&1 | sed -nE 's/^.*OpenSSH_([0-9]+)\.([0-9]+).*$/\1 \2/p' | head -n1)
> +	set -- $version
> +	major=$1
> +	minor=$2

Interesting, I never used set like this.  FYI we suppose POSIX shell
compatibility, i.e. it should work on dash and busybox sh. At least
checkbashisms does not complain therefore it looks to be valid. Testing just
this part locally on both dash and busybox sh shows it's working.

>  	port=$(tst_rhost_run -c "tst_get_unused_port ipv${TST_IPVER} stream")

> @@ -60,6 +64,13 @@ HostKey $TST_TMPDIR/ssh_host_ecdsa_key
>  HostKey $TST_TMPDIR/ssh_host_ed25519_key
>  EOF

> +	if ([ -n "$major" ] && [ -n "$minor" ]); then
	if [ -n "$major" ] && [ -n "$minor" ]; then
nit: IMHO this could be without curly brackets, right? ( ). Any reason to add
them?

> +		if ([ "$major" -gt 9 ] || ([ "$major" -eq 9 ] && [ "$minor" -ge 8 ])); then
I never tried ( ) to force evaluation. I hope it works on POSIX only shell
(dash, busybox sh). BTW IMHO it should work as (which is POSIX compatible):

		if [ "$major" -gt 9 ] || [ "$major" -eq 9 -a "$minor" -ge 8 ]; then

The rest LGTM.

Kind regards,
Petr

> +			cat << EOF >> sshd_config
> +PerSourcePenalties no
> +EOF
> +		fi
> +	fi
>  	ssh-keygen -q -N "" -t rsa -b 4096 -f $TST_TMPDIR/ssh_host_rsa_key
>  	ssh-keygen -q -N "" -t ecdsa -f $TST_TMPDIR/ssh_host_ecdsa_key
>  	ssh-keygen -q -N "" -t ed25519 -f $TST_TMPDIR/ssh_host_ed25519_key

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp