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;
}