[PATCH] Add config option disabling drop_connection() behavior

[Ben Lindstrom]

I'd rather see the following applied before yours.  Mainly because
I don't want 'Yet another Fine Option' floating arounding.  Plus
it touchs less code and acts the way 90% of what people expect.

The more options you provide the more chances someone will fuck up.
Besides this follows the KISS concept. =)

- Ben

[Against -current portable]

Index: sshd.c
===================================================================
RCS file: /var/cvs/openssh/sshd.c,v
retrieving revision 1.207
diff -u -r1.207 sshd.c
--- sshd.c      21 May 2002 17:59:13 -0000      1.207
+++ sshd.c      29 May 2002 05:12:07 -0000
@@ -721,6 +721,10 @@
 {
        double p, r;

+       /* If Max Startup is zero, then the feature is disabled */
+       if (options->max_startups == 0)
+               return 0;
+
        if (startups < options.max_startups_begin)
                return 0;
        if (startups >= options.max_startups)



On Wed, 29 May 2002, Garry Zacheiss wrote:

> 	The patch below (against openssh 3.2.3p1) adds a
> CheckMaxStartups option, defaulting to yes, to determine whether sshd
> calls drop_connection().
>
> 	The motivation behind this is twofold.  In our environment, our
> timesharing machines get enough incoming connections that will trigger
> spuriously with the default value (10 forked unauthenticated
> connections) as well as some significantly higher values, and I'd rather
> disable this feature than just configure it to some ridiculously high
> value.
>
> 	 A secondary motivation is that this code is sometimes triggered
> when the machine's AFS client has gotten into a broken state (forked
> sshd tries to touch AFS for user homedir, loses), and I've already had
> at least one coworker get dragged down the wrong debugging path and "try
> to debug why sshd is accepting new connections and immediately dropping
> them" when the real problem the machine is experiencing is different.
>
> 	 It didn't seem like being able to selectively disable this
> feature would be a bad thing, so please consider this patch for
> inclusion in a future version of OpenSSH.
>
> 	 I'm not currently subscribed to this list, so please cc me on
> any replies.  Thanks in advance for your consideration.
>
> Garry
>
> --- servconf.h	2002/05/29 03:50:01	1.1
> +++ servconf.h	2002/05/29 03:50:53
> @@ -112,6 +112,7 @@
>  	char   *subsystem_name[MAX_SUBSYSTEMS];
>  	char   *subsystem_command[MAX_SUBSYSTEMS];
>
> + 	int     check_max_startups;
>  	int	max_startups_begin;
>  	int	max_startups_rate;
>  	int	max_startups;
> --- servconf.c	2002/05/29 03:49:54	1.1
> +++ servconf.c	2002/05/29 03:54:09
> @@ -112,6 +112,7 @@
>  	options->protocol = SSH_PROTO_UNKNOWN;
>  	options->gateway_ports = -1;
>  	options->num_subsystems = 0;
> +	options->check_max_startups = -1;
>  	options->max_startups_begin = -1;
>  	options->max_startups_rate = -1;
>  	options->max_startups = -1;
> @@ -228,6 +229,8 @@
>  		options->allow_tcp_forwarding = 1;
>  	if (options->gateway_ports == -1)
>  		options->gateway_ports = 0;
> +	if (options->check_max_startups == -1)
> +	        options->check_max_startups = 1;
>  	if (options->max_startups == -1)
>  		options->max_startups = 10;
>  	if (options->max_startups_rate == -1)
> @@ -281,7 +284,8 @@
>  	sUseLogin, sAllowTcpForwarding,
>  	sAllowUsers, sDenyUsers, sAllowGroups, sDenyGroups,
>  	sIgnoreUserKnownHosts, sCiphers, sMacs, sProtocol, sPidFile,
> -	sGatewayPorts, sPubkeyAuthentication, sXAuthLocation, sSubsystem, sMaxStartups,
> +	sGatewayPorts, sPubkeyAuthentication, sXAuthLocation, sSubsystem,
> +	sCheckMaxStartups, sMaxStartups,
>  	sBanner, sVerifyReverseMapping, sHostbasedAuthentication,
>  	sHostbasedUsesNameFromPacketOnly, sClientAliveInterval,
>  	sClientAliveCountMax, sAuthorizedKeysFile, sAuthorizedKeysFile2,
> @@ -353,6 +357,7 @@
>  	{ "protocol", sProtocol },
>  	{ "gatewayports", sGatewayPorts },
>  	{ "subsystem", sSubsystem },
> +	{ "checkmaxstartups", sCheckMaxStartups },
>  	{ "maxstartups", sMaxStartups },
>  	{ "banner", sBanner },
>  	{ "verifyreversemapping", sVerifyReverseMapping },
> @@ -835,6 +840,10 @@
>  		options->num_subsystems++;
>  		break;
>
> +	case sCheckMaxStartups:
> +	  	intptr = &options->check_max_startups;
> +		goto_parse_flag;
> +
>  	case sMaxStartups:
>  		arg = strdelim(&cp);
>  		if (!arg || *arg == '\0')
> --- sshd.8	2002/05/29 03:50:10	1.1
> +++ sshd.8	2002/05/29 03:54:38
> @@ -656,6 +656,11 @@
>  Multiple algorithms must be comma-separated.
>  The default is
>  .Dq hmac-md5,hmac-sha1,hmac-ripemd160,hmac-sha1-96,hmac-md5-96 .
> +.It Cm CheckMaxStartups
> +Specifies whether the server should check the number of concurrent
> +unauthenticated connections to the daemon, and drop new incoming
> +connections if this number exceeds some threshold.  See the
> +"MaxStartups" configuration option for more information.
>  .It Cm MaxStartups
>  Specifies the maximum number of concurrent unauthenticated connections to the
>  .Nm
> --- sshd.c	2002/05/29 03:50:13	1.1
> +++ sshd.c	2002/05/29 03:55:59
> @@ -1243,7 +1243,8 @@
>  					close(newsock);
>  					continue;
>  				}
> -				if (drop_connection(startups) == 1) {
> +				if (options.check_max_startups &&
> +				    drop_connection(startups) == 1) {
>  					debug("drop connection #%d", startups);
>  					close(newsock);
>  					continue;
> _______________________________________________
> openssh-unix-dev at mindrot.org mailing list
> http://www.mindrot.org/mailman/listinfo/openssh-unix-dev
>