[Bug 2397] New: Match block doesn't match negated addresses

[bugzilla-daemon at mindrot.org]

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

            Bug ID: 2397
           Summary: Match block doesn't match negated addresses
           Product: Portable OpenSSH
           Version: 6.8p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: minor
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 2619
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2619&action=edit
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 at r6 ~]# tail -n 3 /etc/ssh/sshd_config
AuthenticationMethods password
Match Address !1.2.3.4
    AuthenticationMethods publickey,password

[root at r6 build]# sshd -TC user=none,host=myhost,addr=1.2.3.4 | grep
authenticationmethods
authenticationmethods password
[root at 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.

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