openssh bugs - May 2015 - [Bug 2397] New: Match block doesn't match negated addresses

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.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
         Depends on|                            |1918

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2451


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.2
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
Retarget to openssh-7.3

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2543


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2451                        |

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Retarget to openssh-7.3


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.2
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2594

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
retarget unfinished bugs to next release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2543                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2397
Bug 2397 depends on bug 1918, which changed state.

Bug 1918 Summary: match_pattern_list fails for negated failure
https://bugzilla.mindrot.org/show_bug.cgi?id=1918

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Damien Miller <djm at mindrot.org> ---
This is fixed and will be in the OpenSSH 7.4 release. Thanks!

commit cc182d01cef8ca35a1d25ea9bf4e2ff72e588208
Author: djm at openbsd.org <djm at 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

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #9 from Damien Miller <djm at mindrot.org> ---
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.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2397
Bug 2397 depends on bug 1918, which changed state.

Bug 1918 Summary: match_pattern_list fails for negated failure
https://bugzilla.mindrot.org/show_bug.cgi?id=1918

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2647

--- Comment #10 from Damien Miller <djm at mindrot.org> ---
OpenSSH 7.4 release is closing; punt the bugs to 7.5


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2647
[Bug 2647] Tracking bug for OpenSSH 7.5 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2594                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2594
[Bug 2594] Tracking bug for OpenSSH 7.4 release
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2698

--- Comment #11 from Damien Miller <djm at mindrot.org> ---
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.


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2698
[Bug 2698] Tracking bug for OpenSSH 7.6 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

--- Comment #12 from Damien Miller <djm at mindrot.org> ---
remove 7.5 target

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2647                        |


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2647
[Bug 2647] Tracking bug for OpenSSH 7.5 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2698                        |
         Resolution|---                         |DUPLICATE
             Status|REOPENED                    |RESOLVED

--- Comment #13 from Damien Miller <djm at mindrot.org> ---
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 ***


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2698
[Bug 2698] Tracking bug for OpenSSH 7.6 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2397
Bug 2397 depends on bug 1918, which changed state.

Bug 1918 Summary: match_pattern_list fails for negated failure
https://bugzilla.mindrot.org/show_bug.cgi?id=1918

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #14 from Damien Miller <djm at mindrot.org> ---
Close all resolved bugs after release of OpenSSH 7.7.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.