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) _______________________________________________ openssh-unix-dev mailing list openssh-unix-dev@xxxxxxxxxxx https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
