openssh bugs - Mar 2017 - [Bug 2685] New: Case sensitive hostname matching

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

            Bug ID: 2685
           Summary: Case sensitive hostname matching
           Product: Portable OpenSSH
           Version: 7.4p1
          Hardware: Other
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: pcerny at suse.cz

Created attachment 2951
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2951&action=edit
make host matching in ssh case insensitive

Currently ssh (and possibly sshd) match hostnames in a case sensitive
manner. While recognise there are reasons this can be desired
behaviour, the comments in code (and the generally accepted property of
hostnames being case-instensitive - RFC 4343) suggest otherwise. 

Comment for match_hostname() (in match.c) even mentions explicitly,
that the hostname is expected to be lowercased, which it has been not,
since the d56b44d2dfa093883a5c4e91be3f72d99946b170 +
eb6d870a0ea8661299bb2ea8f013d3ace04e2024 commit combo.

I have no clear position on what to do with originalhost matching,
though I would suggest handling it in the same way to prevent gotcha
(maybe by lowercasing the argument passed to match_host() and not
expecting the caller to do it), so the attached patch only handles
(possibly expanded) hostname matching.

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

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

Richard Birkett <openssh at richard.birkett.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |openssh at richard.birkett.com

--- Comment #1 from Richard Birkett <openssh at richard.birkett.com> ---
Historically, matching in ssh_config was always case-sensitive.  This
stopped happening sometime in 2013/2014, which I guess is why the
commits you mention were considered to be a bug-fix, not a new feature.

I'm not sure the argument of the "Host" keyword should necessarily
be
expected to behave exactly like a DNS hostname, including all the
equivalence rules like RFC 4343.  Despite the keyword's name, in
practice it's just a pattern to match against whatever text was
supplied on the command-line.  That's *usually* a hostname, but there
are valid use-cases which treat it as an arbitrary label, with the real
hostname specified by HostName.

Maybe the best of both worlds (though not entirely backward-compatible)
would be for matching of the Host keyword to be case-insensitive, but
add an optional modifier on the Match keyword so that people who
specifically want case-sensitive matching can have it, eg.

    Match caseful originalhost WeIrDlAbEl

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

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

--- Comment #2 from Petr Cerny [:hrosik] <pcerny at suse.cz> ---
(In reply to Richard Birkett from comment #1)
> Historically, matching in ssh_config was always case-sensitive. 
> This stopped happening sometime in 2013/2014, which I guess is why
> the commits you mention were considered to be a bug-fix, not a new
> feature.
If I remember correctly, the Match Host was actually introduced as case
insensitive at first.
> I'm not sure the argument of the "Host" keyword should
necessarily
> be expected to behave exactly like a DNS hostname, including all the
> equivalence rules like RFC 4343.  Despite the keyword's name, in
> practice it's just a pattern to match against whatever text was
> supplied on the command-line.  That's *usually* a hostname, but
> there are valid use-cases which treat it as an arbitrary label, with
> the real hostname specified by HostName.
That was my thought when I saw that first, yet reading ssh_config(5):
    ...
    The criteria for the host keyword are matched against the target
    hostname, after any substitution by the Hostname option.
    originalhost keyword matches against the hostname as it was
    specified on the command-line
    ...

That is why the proposed patch leaves _originalhost_ alone, so that it
can be used for this purpose. One more hint that it should be
case-insensitive is the comment preceding match_hostname().
> Maybe the best of both worlds (though not entirely
> backward-compatible) would be for matching of the Host keyword to be
> case-insensitive, but add an optional modifier on the Match keyword
> so that people who specifically want case-sensitive matching can
> have it, eg.
> 
>     Match caseful originalhost WeIrDlAbEl
Or making this a global option (e.g. MatchHostCaseSensitive) - would be
less flexible, yet much simpler and "more" backwards compatible.

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

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

--- Comment #3 from Richard Birkett <openssh at richard.birkett.com> ---
(In reply to Petr Cerny [:hrosik] from comment #2)
> (In reply to Richard Birkett from comment #1)
> > Historically, matching in ssh_config was always case-sensitive. 
> > This stopped happening sometime in 2013/2014, which I guess is why
> > the commits you mention were considered to be a bug-fix, not a new
> > feature.
> 
> If I remember correctly, the Match Host was actually introduced as
> case insensitive at first.
Match was first introduced in 2013, which was around the same time as
Host (briefly) became case-insensitive.  It looks as though e9fc72e
introduced the case-insensitive matching: it was committed on the same
day as the introduction of the Match keyword into portable OpenSSH
(194fd90), though the comment suggests it might have previously been in
the upstream OpenBSD version for a while before that.  So when Match
was introduced, that was the current behaviour - but the behaviour was
later reported and acknowledged as a regression.
> That is why the proposed patch leaves _originalhost_ alone, so that
> it can be used for this purpose. One more hint that it should be
> case-insensitive is the comment preceding match_hostname().
That's interesting - the comment "which must be in all lowercase"
dates
back to 2001, but wasn't true until 2013 (and then hasn't been true
again since 2014)!

For least surprise, I think "Host" should always behave exactly like
"Match originalhost", including case (in)sensitivity.

CanonicalizeHostname (and the double-parsing it triggers) makes it
behave more like "Match host", and also has the consequence that
HostName could affect Host (and "Match host") directives that appear
*before* it in the config file.  I think both of those changes are
unintuitive generally, but they are mentioned in the documentation, and
at least they were supposed to apply only if canonicalisation was on. 
Commit 13f97b2 made them happen even when canonicalisation is off (bug
2267).  I'm still hoping to come up with a better solution for all of
this, btw!
> > Maybe the best of both worlds (though not entirely
> > backward-compatible) would be for matching of the Host keyword to be
> > case-insensitive, but add an optional modifier on the Match keyword
> > so that people who specifically want case-sensitive matching can
> > have it, eg.
> > 
> >     Match caseful originalhost WeIrDlAbEl
> 
> Or making this a global option (e.g. MatchHostCaseSensitive) - would
> be less flexible, yet much simpler and "more" backwards
compatible.
I think we should go with flexibility, but we could have a
"MatchHostCaseSensitive" option *as well*, which effectively makes
"caseful" the default.  For symmetry, perhaps have a
"caseless"
modifier to allow it to be overridden both ways around.

I'm torn about whether caseful or caseless should be the default. 
Historically it has been, but the number of people who actually rely on
caseful matching is probably very small, so I'd tentatively support a
change to make the common usage less surprising - provided there's a
way of overriding it.

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

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

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2951|0                           |1
        is obsolete|                            |
                 CC|                            |djm at mindrot.org,
                   |                            |dtucker at zip.com.au
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
   Attachment #2960|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Created attachment 2960
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2960&action=edit
make hostname matching really case-insensitive

I think this patch is a bit simpler and more clearly makes
match_hostname() case insensitive. That was the intent to begin with
AFAIK, since it calls match_pattern_list() with the dolower argument
set. Unfortunately that only applies to the pattern and not the name
being matched.

I've checked the existing callers to verify that they will be tolerant
of this change, esp. that we weren't using match_hostname() to compare
a hashed hostname. All looks fine.

-- 
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=2685

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2960|ok?(dtucker at zip.com.au)     |ok+
              Flags|                            |

-- 
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=2685

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Patch applied. This will be in OpenSSH 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=2685

Damien Miller <djm at mindrot.org> changed:

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

--- Comment #6 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 someone on the CC list of the bug.
You are watching the assignee of the bug.