openssh bugs - Jun 2016 - [Bug 2580] New: Support for MaxDisplays to replace artificial MAX_DISPLAYS limit

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

            Bug ID: 2580
           Summary: Support for MaxDisplays to replace artificial
                    MAX_DISPLAYS limit
           Product: Portable OpenSSH
           Version: 7.2p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: openssh at mzpqnxow.com

Created attachment 2821
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2821&action=edit
Patch for OpenSSH Portable GitHub master branch to add MaxDisplays
option

Please see the post at
http://lists.mindrot.org/pipermail/openssh-unix-dev/2016-June/035125.html

I have attached the patch,  it is also in-lined in the e-mail.

In short, this moves a hardcoded port range limit dealing with X11
forwards to a configuration option called MaxDisplays. The mailing list
post has more details.

This is a request to have this merged in as a feature.

The pull request against the GitHub openssh-portable is available at
https://github.com/openssh/openssh-portable/pull/41

Thanks.

A

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

AG <openssh at mzpqnxow.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |openssh at mzpqnxow.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

--- Comment #1 from AG <openssh at mzpqnxow.com> ---
I will implement changes to the patch based on feedback from
openssh-unix-dev and RHEL engineers and then report back. It seems that
others agree this is a reasonable feature to have.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

AG <openssh at mzpqnxow.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2821|0                           |1
        is obsolete|                            |

--- Comment #2 from AG <openssh at mzpqnxow.com> ---
Created attachment 2825
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2825&action=edit
Updated X11MaxDisplays patch w/documentation and fixed whitespace

This is an updated patch for anongit OpenSSH that provides
X11MaxDisplays. It includes documentation in sshd_config and a more
consistent coding style including whitespace and variable naming, etc..

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

Jakub Jelen <jjelen at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jjelen at redhat.com

--- Comment #3 from Jakub Jelen <jjelen at redhat.com> ---
Created attachment 2843
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2843&action=edit
cleaned up patch from Adam (whitespace, magic number)

Last thing I didn't like in the patch was the usage of magic number
6000 (also in other places in the same file) and few more tabs/spaces
mixture, which I fixed in the last attachment.

Damien, Darren, what do you think about that? Would you have a minute
to have a look if this would be acceptable for upstream to avoid future
incompatibility?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

AG <openssh at mzpqnxow.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

--- Comment #4 from AG <openssh at mzpqnxow.com> ---
Update:

I understand this didn't make it into 7.3 due to the size of the patch
relative to some of the smaller patches, which were easier to review.
I'm hoping someone will have the chance to review for 7.3px or 7.4. The
majority of the patch is boilerplate (a new integer option in
sshd_config and accompanying field in ServerOptions) and it doesn't
change any behavior unless explicitly used in sshd_config, it just
allows a default setting to be changed by the user, as opposed to
changing a #define and rebuilding.

I realize there haven't been many (any?) requests to the list for this
functionality, but it does seem like something best suited for a config
option by common sense- it's one of the only hard coded 'limits' in
this part of the code, aside from mandatory implementation details and
things having to do with security, which are obviously set and fixed at
specific values for very good reasons. 

One could argue that allowing the user to change the limit that is
currently set (MAX_DISPLAYS 1000) has potential stability (and thus
security) implications since it would allow authenticated users to
allocate N ports on the loopback device, but this risk is clearly
documented in the man page and I think it's fair to say that any
sysadmin messing with this setting will understand the risk. It isn't
really too far off from allowing sysadmins to set values like
MaxAuthTries, AllowTcpForwarding, and other variables when it comes to
protecting the user from shooting one's own foot.

As always, if there's anything I can do to help beyond using this patch
in my environment, let me know.

FWIW, this change has been live on (critical) production infrastructure
for at LEAST 2 years now, in an environment supporting > 5000 users,
with many many more concurrent active sessions. This sounds silly since
in retrospect, I should have cleaned up and submitted the patch much
sooner.

Thanks Jakub for the whitespace cleanup and the adjustment of the
'magic number' for the X11 base port and thanks to anyone who is
willing to help in reviewing this for the next release. I would love to
get this into RHEL 7.3 or 7.4 (and other distributions, for the sake of
other users who may need it now or down the line) but until it goes
upstream,. that is unlikely to happen.

Thanks

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2580

--- Comment #5 from AG <openssh at mzpqnxow.com> ---
Note, this went into RHEL 7.3 but does not appear to be in the official
OpenSSH source at this time.

It is verified functional in my environment (on RHEL 7.3)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.