[Bug 2267] Host matching uses modified hostname as well as original

bugzilla-daemon at mindrot.org bugzilla-daemon at mindrot.org
Mon Sep 1 13:32:30 EST 2014


--- Comment #3 from Damien Miller <djm at mindrot.org> ---
(In reply to Richard Birkett from comment #2)

> - ssh-keysign.c doesn't compile, as it's still calling
> read_config_file with the old function signature - adding an extra
> "" agument fixes the compilation, though I'm not certain whether
> that's actually correct!

serves me right for sending out a patch when I should have been asleep.

> - With canonicalisation enabled, "Match canonical" is giving a
> "Missing Match criteria for canonical" error during the second pass
> - the "continue" statement needs to apply to both branches of the
> "if (!post_canon)" test.

Thanks - I'll fix this in the next revision of the patch.

> Functionally, everything's much improved.  With
> "CanonicalizeHostname no", the second scan is not happening, which
> is good.  "Match host" and "Match originalhost" seem to do what
> they're supposed to do.  There are a few oddities, though...
> - "Host" is still matching the value of a preceding HostName option
> - ie. it's behaving like "Match host", instead of "Match
> originalhost", which is what it did pre-6.6.

That's intentional and only triggered when canonicalisation is enabled.
I don't think it is unreasonable for the configuration parsing
behaviour to change when hostname canonicalisation is enabled.

> - With canonicalisation enabled, the second pass is triggered, but
> all the tests (Host, Match canonical host, Match canonical
> originalhost) now seem to match only the *uncanonicalised* hostname
> - so canonicalisation has actually stopped working altogether.

AFAIK only "match host" was incorrect here. Fixed in next patch.

> But I really like the "canonical" keyword on Match.  In fact, this
> feels like a better solution all round than allowing one option
> (CanonicalizeHostname) to magically change the meaning of other
> options (Host and Match).
> A suggestion: can we deprecate the whole concept of "global"
> canonicalisation, and do it specifically when parsing "Match
> canonical [original]host"?  That would also avoid the
> double-parsing, which I think can still have unintended
> consequences, even with the extra checks you've added.
> Unfortunately the problem is then deciding how to grandfather the
> 6.6-style behaviour into the more flexible framework.  Could we
> perhaps make "CanonicalizeHostname yes" immediately abort parsing
> and start again, with a flag set to treat plain "Host" and "Match
> [original]host" as if they were "Match canonical [original]host"?

I considered that, but a) I don't want to add more flags
(canonicalisation added more than I wanted already) and b) I think it
would be even more difficult to reason about.

I also considered adding a "RereadConfiguration yes|no|if-canon" flag,
but I'd prefer to make something that doesn't need an extra flag and is
backwards compatible when canonicalisation is off (I don't mind
changing behaviour when it is enabled).

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.

More information about the openssh-bugs mailing list