MaxDisplays configuration option

AG openssh at mzpqnxow.com
Thu Jun 2 11:51:27 AEST 2016


Hello,

I manage OpenSSH on a dozen or so servers that act as gateways for a large
amount of developers and system administrators. On these servers it is
common for there to be more than 1000 active X11 forwards active at peak
usage. Beyond ~1000 active X11 forwards, sshd will fail to bind additional
ports due to a hard coded range check in channels.c that limits the port
range that sshd will attempt to bind. Today this is set at 1000:

channels.c:152:#define MAX_DISPLAYS  1000

I have made changes to OpenSSH portable that allow this setting to be
configured via an option in sshd_config named MaxDisplays. If not
explicitly set, it maintains the default value of 1000.

It seems to me that this setting should be configurable by the user similar
to how X11DisplayOffset is configurable. I've read the code carefully and
am currently using this patch in my production environment without any
issues. I don't see any reason this change would cause any issues for users
that do not need to explicitly set it. I also don't envision this being a
maintenance burden as it's a very simple feature.

I'd appreciate this being considered for acceptance into OpenSSH.

Also, I'm curious if this issue has ever come up before? Is it really that
strange of a case?

I understand that you don't utilize GitHub for development, but for
convenience you can see the changes in a web browser:

https://github.com/openssh/openssh-portable/pull/41

I've attached the patch to this message as well.

Thanks,
Adam
-------------- next part --------------
diff -Naur openssh-portable/channels.c openssh-portable-maxdisplays/channels.c
--- openssh-portable/channels.c	2016-06-01 21:14:01.772052924 -0400
+++ openssh-portable-maxdisplays/channels.c	2016-06-01 21:14:22.924053856 -0400
@@ -148,9 +148,6 @@
 
 /* -- X11 forwarding */
 
-/* Maximum number of fake X11 displays to try. */
-#define MAX_DISPLAYS  1000
-
 /* Saved X11 local (client) display. */
 static char *x11_saved_display = NULL;
 
@@ -3890,7 +3887,8 @@
  */
 int
 x11_create_display_inet(int x11_display_offset, int x11_use_localhost,
-    int single_connection, u_int *display_numberp, int **chanids)
+	int max_displays, int single_connection, u_int *display_numberp, 
+	int **chanids)
 {
 	Channel *nc = NULL;
 	int display_number, sock;
@@ -3902,8 +3900,11 @@
 	if (chanids == NULL)
 		return -1;
 
+	/* Try max_displays ports starting at the range 6000+X11DisplayOffset */
+	max_displays = max_displays + x11_display_offset;
+
 	for (display_number = x11_display_offset;
-	    display_number < MAX_DISPLAYS;
+	    display_number < max_displays;
 	    display_number++) {
 		port = 6000 + display_number;
 		memset(&hints, 0, sizeof(hints));
@@ -3957,7 +3958,7 @@
 		if (num_socks > 0)
 			break;
 	}
-	if (display_number >= MAX_DISPLAYS) {
+	if (display_number >= max_displays) {
 		error("Failed to allocate internet-domain X11 display socket.");
 		return -1;
 	}
diff -Naur openssh-portable/channels.h openssh-portable-maxdisplays/channels.h
--- openssh-portable/channels.h	2016-06-01 21:14:01.772052924 -0400
+++ openssh-portable-maxdisplays/channels.h	2016-06-01 21:14:22.924053856 -0400
@@ -286,7 +286,7 @@
 
 void	 channel_set_x11_refuse_time(u_int);
 int	 x11_connect_display(void);
-int	 x11_create_display_inet(int, int, int, u_int *, int **);
+int	 x11_create_display_inet(int, int, int, int, u_int *, int **);
 int      x11_input_open(int, u_int32_t, void *);
 void	 x11_request_forwarding_with_spoofing(int, const char *, const char *,
 	     const char *, int);
diff -Naur openssh-portable/servconf.c openssh-portable-maxdisplays/servconf.c
--- openssh-portable/servconf.c	2016-06-01 21:14:01.820052926 -0400
+++ openssh-portable-maxdisplays/servconf.c	2016-06-01 21:14:22.976053858 -0400
@@ -96,6 +96,7 @@
 	options->print_lastlog = -1;
 	options->x11_forwarding = -1;
 	options->x11_display_offset = -1;
+	options->max_displays = -1;
 	options->x11_use_localhost = -1;
 	options->permit_tty = -1;
 	options->permit_user_rc = -1;
@@ -327,6 +328,8 @@
 		options->max_authtries = DEFAULT_AUTH_FAIL_MAX;
 	if (options->max_sessions == -1)
 		options->max_sessions = DEFAULT_SESSIONS_MAX;
+	if (options->max_displays == -1)
+		options->max_displays = MAX_DISPLAYS;
 	if (options->use_dns == -1)
 		options->use_dns = 0;
 	if (options->client_alive_interval == -1)
@@ -429,7 +432,7 @@
 	sAuthorizedKeysCommand, sAuthorizedKeysCommandUser,
 	sAuthenticationMethods, sHostKeyAgent, sPermitUserRC,
 	sStreamLocalBindMask, sStreamLocalBindUnlink,
-	sAllowStreamLocalForwarding, sFingerprintHash,
+	sAllowStreamLocalForwarding, sFingerprintHash, sMaxDisplays,
 	sDeprecated, sUnsupported
 } ServerOpCodes;
 
@@ -572,6 +575,7 @@
 	{ "streamlocalbindunlink", sStreamLocalBindUnlink, SSHCFG_ALL },
 	{ "allowstreamlocalforwarding", sAllowStreamLocalForwarding, SSHCFG_ALL },
 	{ "fingerprinthash", sFingerprintHash, SSHCFG_GLOBAL },
+	{ "maxdisplays", sMaxDisplays, SSHCFG_GLOBAL },
 	{ NULL, sBadOption, 0 }
 };
 
@@ -1031,7 +1035,15 @@
 			fatal("%s line %d: Badly formatted port number.",
 			    filename, linenum);
 		break;
-
+    case sMaxDisplays:
+		arg = strdelim(&cp);
+	    if (!arg || *arg == '\0')
+	        fatal("%s line %d: missing value.",filename, linenum);
+	    if ((options->max_displays = a2port(arg)) == -1) {
+	        error("Invalid MaxDisplays '%s'", arg);
+	       	return -1;
+	    }
+	  	break;
 	case sServerKeyBits:
 		intptr = &options->server_key_bits;
  parse_int:
@@ -2001,6 +2013,7 @@
 	M_CP_INTOPT(permit_tty);
 	M_CP_INTOPT(permit_user_rc);
 	M_CP_INTOPT(max_sessions);
+	M_CP_INTOPT(max_displays);
 	M_CP_INTOPT(max_authtries);
 	M_CP_INTOPT(ip_qos_interactive);
 	M_CP_INTOPT(ip_qos_bulk);
@@ -2254,6 +2267,7 @@
 	dump_cfg_int(sX11DisplayOffset, o->x11_display_offset);
 	dump_cfg_int(sMaxAuthTries, o->max_authtries);
 	dump_cfg_int(sMaxSessions, o->max_sessions);
+	dump_cfg_int(sMaxDisplays, o->max_displays);
 	dump_cfg_int(sClientAliveInterval, o->client_alive_interval);
 	dump_cfg_int(sClientAliveCountMax, o->client_alive_count_max);
 	dump_cfg_oct(sStreamLocalBindMask, o->fwd_opts.streamlocal_bind_mask);
diff -Naur openssh-portable/servconf.h openssh-portable-maxdisplays/servconf.h
--- openssh-portable/servconf.h	2016-06-01 21:14:01.820052926 -0400
+++ openssh-portable-maxdisplays/servconf.h	2016-06-01 21:14:22.976053858 -0400
@@ -29,6 +29,7 @@
 #define MAX_MATCH_GROUPS	256	/* Max # of groups for Match. */
 #define MAX_AUTHKEYS_FILES	256	/* Max # of authorized_keys files. */
 #define MAX_AUTH_METHODS	256	/* Max # of AuthenticationMethods. */
+#define MAX_DISPLAYS  		1000 /* Maximum number of fake X11 displays to try. */
 
 /* permit_root_login */
 #define	PERMIT_NOT_SET		-1
@@ -154,6 +155,7 @@
 	int	max_startups;
 	int	max_authtries;
 	int	max_sessions;
+	int max_displays;
 	char   *banner;			/* SSH-2 banner message */
 	int	use_dns;
 	int	client_alive_interval;	/*
diff -Naur openssh-portable/session.c openssh-portable-maxdisplays/session.c
--- openssh-portable/session.c	2016-06-01 21:14:01.820052926 -0400
+++ openssh-portable-maxdisplays/session.c	2016-06-01 21:14:22.980053858 -0400
@@ -2701,8 +2701,9 @@
 		return 0;
 	}
 	if (x11_create_display_inet(options.x11_display_offset,
-	    options.x11_use_localhost, s->single_connection,
-	    &s->display_number, &s->x11_chanids) == -1) {
+	    options.x11_use_localhost, options.max_displays,
+	    s->single_connection, &s->display_number, 
+	    &s->x11_chanids) == -1) {
 		debug("x11_create_display_inet failed.");
 		return 0;
 	}


More information about the openssh-unix-dev mailing list