[PATCH] X11MaxDisplays for multi-factor X11 gateways

[AG]

Hello,

Please accept this as my quarterly nag regarding the possibility of
merging the patch submitted back in June to the mindrot Bugzilla @
https://bugzilla.mindrot.org/show_bug.cgi?id=2580 upstream. It has
already been reviewed and cleaned up slightly by Jakub Jelen, and
documentation has been added.

To summarize, this removes the arbitrary limit in channels.c:

channels.c:155:#define MAX_DISPLAYS  1000

It instead makes it an option in sshd_config, X11MaxDisplays, which
keeps the defaults at 1000, in order to avoid any impact for users not
needing to tune it. This feature allows a sysadmin to make an educated
decision on what is required for their particular system, whether it
is higher or lower than 1000. Additionally, style-wise, it takes a
completely arbitrary number out of the source code. Admittedly, 1000
is plenty for the majority of cases- but it is still completely
arbitrary. It might be too high for some cases (I hadn't thought about
that) but it is certainly too low for my use case.

The use case for the patch: systems that are using OpenSSH as a
mulit-factor authenticated X11 gateway into a trusted network.
Currently, the hardcoded value causes there to be an effective maximum
of (1000-X11DisplayOffset) displays forwarded, assuming no other
applications are binding the loopback TCP ports between
6000+X11DisplayOffset and 6000+MAX_DISPLAYS on the system. We aren't
talking about a box that provides interactive shells here- this
infrastructure is purely an X11 gateway providing a strong PAM stack-
so it can handle well beyond 1000 displays (I hand compile it today
with this patch) and I support well over 5000 users, who utilize
multiple forwards simultaneously- so it's a must for my environment.

The patch also "corrects" the logic a bit by changing the logic to try
bind() across the range from (6000+X11DisplayOffset) through
(6000+X11DisplayOffset+MAX_DISPLAYS[1]) as well as changing the
integer 6000 (which is the minimum X11 display offset port, well known
to technical users) from a magic number in the loop to a #define that
is a bit more descriptive than the integer value alone in the source
code, for readability.

In any event, I'd appreciate an eyeball on this before the next
OpenSSH release if someone has the time. The patch is not large, but
it is not tiny. It is however VERY boilerplate.

It seems to be one of the few _arbitrary_ magic numbers in the OpenSSH
code and seems like it should be configurable- and for my own selfish
reasons (use in my own infrastructure) I would love to see this merged
sooner rather than later. It's been in use in a production environment
for 3+ years, and could be a quick review and merge for someone will
to take the time.

Thanks folks, sorry for the nudge, I know everyone's busy.

AG

(No patch attached, please see patch @
https://bugzilla.mindrot.org/show_bug.cgi?id=2580)