Created attachment 2619 [details] proposed patch Recently we got some report about sshd_config documentation and behaviour in corner cases. One of the problems found during the analysis was that when using Match blocks, we are unable to match negated addresses. In this example, the block is *never* matched: [root@r6 ~]# tail -n 3 /etc/ssh/sshd_config AuthenticationMethods password Match Address !1.2.3.4 AuthenticationMethods publickey,password [root@r6 build]# sshd -TC user=none,host=myhost,addr=1.2.3.4 | grep authenticationmethods authenticationmethods password [root@r6 build]# sshd -TC user=none,host=myhost,addr=1.2.3.5 | grep authenticationmethods authenticationmethods password ## should return "authenticationmethods publickey,password" From this issue I got to function addr_match_list, that is not handling properly negated addresses. I put together few assertions that should apply from my point of view: assert(addr_match_list("1.2.3.4", "1.2.3.4") == 1); assert(addr_match_list("1.2.3.4", "1.2.3.5") == 0); assert(addr_match_list("1.2.3.4", "!1.2.3.5") == 1); // current version returns 0 assert(addr_match_list("1.2.3.4", "!1.2.3.4") == -1); assert(addr_match_list("1.2.3.4", "1.2.3.4,1.2.3.5") == 1); assert(addr_match_list("1.2.3.4", "1.2.3.5,1.2.3.6") == 0); assert(addr_match_list("1.2.3.4", "!1.2.3.5,!1.2.3.6") == 1); // current version returns 0 assert(addr_match_list("1.2.3.4", "!1.2.3.4,!1.2.3.5") == -1); assert(addr_match_list("1.2.3.4", "1.2.3.5,1.2.3.4") == 1); assert(addr_match_list("1.2.3.4", "!1.2.3.5,!1.2.3.4") == -1); I believe that this change can be potentially regression, but I would like you to review this issue and attached patch. If you wish, I can also create some unit test or ellaborate on this topic more.
match_pattern_list() is similarly broken, see bug 1918 IMO we should fix them both at once and have a unit test for both. And do it in the 7.0 release, where we are a bit more willing to break stuff.
Retarget to openssh-7.3
retarget unfinished bugs to next release
This is fixed and will be in the OpenSSH 7.4 release. Thanks! commit cc182d01cef8ca35a1d25ea9bf4e2ff72e588208 Author: djm@openbsd.org <djm@openbsd.org> Date: Tue Aug 23 03:24:10 2016 +0000 upstream commit fix negated address matching where the address list consists of a single negated match, e.g. "Match addr !192.20.0.1" Report and patch from Jakub Jelen. bz#2397 ok dtucker@ Upstream-ID: 01dcac3f3e6ca47518cf293e31c73597a4bb40d8
I've reverted this fix because it yields surprising behaviour, e.g. Match address 2002::/16,!::1 will also match 10.0.0.1 (I'm using the example from bug #1918, but the same logic applies here too). I'll look at a better fix, probably a combination of special-casing match strings that consist solely of negated matches and better documentation.
OpenSSH 7.4 release is closing; punt the bugs to 7.5
Move incomplete bugs to openssh-7.6 target since 7.5 shipped a while back. To calibrate expectations, there's little chance all of these are going to make 7.6.
remove 7.5 target
Merge this with the other match bug because I'm going to tackle them together *** This bug has been marked as a duplicate of bug 1918 ***
Close all resolved bugs after release of OpenSSH 7.7.