Jakub, can you please look at this?

btw. I've verified that:

# egrep -v '^#|^$' < /etc/ssh/ssh_config.d/05-redhat.conf
Match canonical all
Include /etc/crypto-policies/back-ends/openssh.config
Match canonical all
        GSSAPIAuthentication yes
        ForwardX11Trusted yes
        SendEnv LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES
        SendEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
        SendEnv LC_IDENTIFICATION LC_ALL LANGUAGE
        SendEnv XMODIFIERS

fixes the problem if canonicalization is turned on.
(not clear if it should be 'Match canonical' or 'Match canonical all')

But I'm not sure if it triggers correctly, and I'm not sure if it triggers at all if canonicalization is off.

Perhaps this is less a crypto-policies problem and more an openssh config parsing problem...

Perhaps the real problem is how it handles:
  keyword +opt
and
  keyword -opt
which you would expect to append/remove even after prior matches...

Obviously I can fix this via:

# cat ~/.ssh/config 
...
Host router router.foo.org
  KexAlgorithms +diffie-hellman-group1-sha1

but then I'm probably no longer getting the crypto-policy + diffie-hellman-group1-sha1 and am instead probably getting ssh's builtin + diffie-hellman-group1-sha1 instead...

Or maybe there should be a keyword to do canonicalization right now and not parse any more files?

ie. perhaps we should have:

# cat /etc/ssh/ssh_config:
IfFirstPassOfCanonicalizationThenStartFromScratch
Include /etc/ssh/ssh_config.d/*.conf


or perhaps we need a

Match canonical_or_not_canonicalizing

instead of 'Host *'

> Perhaps the real problem is how it handles:
>   keyword +opt
> and
>   keyword -opt
> which you would expect to append/remove even after prior matches...

Unfortunately, this is a limitation of the ssh_config parser in openssh, which works for most common cases, but there are some corner cases.

Personally, I do not use the canonicalization, but I know it is very cumbersome in many cases

Your proposed solution with `Match canonical all`, will probably not work, since this will avoid matching with canonicalization turned off:

> The canonical keyword matches only when the configuration file is being re-parsed after hostname canonicalization [...]

And we really do not want to turn the canonicalization by default, since it brings many unexpected issues out of the box.

> Obviously I can fix this via:

> # cat ~/.ssh/config 
> ...
> Host router router.foo.org
>   KexAlgorithms +diffie-hellman-group1-sha1

> but then I'm probably no longer getting the crypto-policy + diffie-hellman-group1-sha1 and am instead probably getting ssh's builtin + diffie-hellman-group1-sha1 instead...

I would say that this is preferred way -- you basically create an exception from crypto policy for this device, since the per-user configuration file has higher priority.

Introducing different canonicalization options would make it even more complicated than it is now.

Let me know if there is anything I can do for you in this way.

Seems like there really should be a 'Match finalpass' to replace the 'Host *'.  But I guess that would be an upstream openssh feature request?

Yes. If you can summarize the issue about the canonization in the upstream bugzilla https://bugzilla.mindrot.org/ it would be best since you have the first-hand experience with the canonicalization.

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

openssh-7.9p1-1.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2018-5be740cab4

openssh-7.9p1-1.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2018-5be740cab4

oh .. this did not make it to the release. Reverting the state back.

https://bugzilla.mindrot.org/show_bug.cgi?id=2906 is marked resolved.

Damien Miller 2018-11-23 16:09:45 EST
This has been committed and, barring catastrophe, will be in OpenSSH 8.0.

commit 9e34e0c59ab04514f9de9934a772283f7f372afe (HEAD -> master, origin/master, origin/HEAD)
Author: djm <djm>
Date:   Fri Nov 23 05:08:07 2018 +0000

    upstream: add a ssh_config "Match final" predicate
    
    Matches in same pass as "Match canonical" but doesn't require
    hostname canonicalisation be enabled. bz#2906 ok markus
    
    OpenBSD-Commit-ID: fba1dfe9f6e0cabcd0e2b3be13f7a434199beffa

Did you verify that the applied patch solves your problem? If so, I can try to backport it to the current version.

Yes, I confirm it works.

Now running:
  openssh.x86_64 7.9p1-2mz.fc29
  openssh-clients.x86_64 7.9p1-2mz.fc29
  openssh-server.x86_64 7.9p1-2mz.fc29

Built via:
  mkdir -p /root/rpmbuild/openssh
  cd /root/rpmbuild/openssh

  git clone https://github.com/openssh/openssh-portable.git
  cd openssh-portable/
  git show 9e34e0c59ab04514f9de9934a772283f7f372afe > ../openssh-7.9p1-final-pass.patch
  cd ..
  rm -rf openssh-portable/

# apply the spec file diff below, and manually remove the $Id$ chunks out of the patch

  cd /root/rpmbuild/SPECS
  rpmbuild -ba openssh.spec

# Finally fixup /etc/crypto-policies/back-ends/openssh.config to look like:

Match final
  Ciphers ...  
  MACs ...
  ...etc...

[root@sky SPECS]# diff -Naur openssh.spec_ openssh.spec
--- openssh.spec_       2018-11-23 23:28:32.365065119 -0800
+++ openssh.spec        2018-11-23 23:33:05.296161170 -0800
@@ -66,7 +66,7 @@
 
 # Do not forget to bump pam_ssh_agent_auth release if you rewind the main package release to 1
 %global openssh_ver 7.9p1
-%global openssh_rel 2
+%global openssh_rel 2mz
 %global pam_ssh_agent_ver 0.10.3
 %global pam_ssh_agent_rel 6
 
@@ -226,6 +226,10 @@
 # Allow to disable RSA signatures with SHA-1 in server
 # https://bugzilla.mindrot.org/show_bug.cgi?id=2746
 Patch954: openssh-7.9p1-disable-sha1.patch
+# Add support for 'match final' predicate
+# https://bugzilla.redhat.com/show_bug.cgi?id=1630166
+# https://bugzilla.mindrot.org/show_bug.cgi?id=2906
+Patch999: openssh-7.9p1-final-pass.patch
 
 License: BSD
 Group: Applications/Internet
@@ -450,6 +454,7 @@
 %patch953 -p1 -b .scp-ipv6
 %patch808 -p1 -b .gsskex-method
 %patch954 -p1 -b .disable-sha1
+%patch999 -p1 -b .patch-final
 
 %patch200 -p1 -b .audit
 %patch201 -p1 -b .audit-race

*** Bug 1438326 has been marked as a duplicate of this bug. ***

openssh-7.9p1-3.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-f6ff819834

openssh-7.9p1-3.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-f6ff819834

Unfortunately:

  dnf --enablerepo updates-testing upgrade openssh\*

doesn't find anything to upgrade (and I'm thus still at openssh-clients-7.9p1-2.fc29.x86_64).

Do note though that at *least* the /etc/ssh/ssh_config.d/05-redhat.conf file needs to be changed from 'Host *' to 'Match final'.

It was not clear to me that the /etc/ssh/ssh_config.d/05-redhat.conf needs to be changed to Match final too. I will do that with the next update. I will drop this bug from the bodhi update for now. Sorry for a delay.

Changing the "Host *" in /etc/ssh/ssh_config.d/05-redhat.conf alone won't help with the separate issue of bare directives in /etc/crypto-policies/back-ends/openssh.config as currently used.  Two separate changes are necessary to solve the two separate problems in this bug and bug 1438326.

To do this without changes to crypto-policies, the 05-redhat.conf file needs to be updated like this, in its entirety:

# Uncomment this if you want to use .local domain
# Host *.local
#   CheckHostIP no

Match final all
	GSSAPIAuthentication yes

# If this option is set to yes then remote X11 clients will have full access
# to the original X11 display. As virtually no X11 client supports the untrusted
# mode correctly we set this to yes.
	ForwardX11Trusted yes

# Send locale-related environment variables
	SendEnv LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES
	SendEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
	SendEnv LC_IDENTIFICATION LC_ALL LANGUAGE
	SendEnv XMODIFIERS

# Follow system-wide Crypto Policy, if defined:
	Include /etc/crypto-policies/back-ends/openssh.config



The two important changes are replacing "Host *" with "Match final all" (which is not the same thing as "Match final", please stop conflating the two) and moving the crypto policy include to within that group of directives.  See ssh_config(5) for descriptions of the order in which directives apply, and for use of Include following Host/Match directives.

Care to explain how 'Match final' and 'Match final all' differ? I'm confused.

Rob, I would be also interested in hearing what is difference between `Match final` and `Match final all`. The manual page is not very clear in this way, but from my testing so far, it looks like they both behave the same way (you can run it with -vvv to get more information what is going on in the background):

$ cat /tmp/ssh_config 
Match final all
  GSSAPIAuthentication yes
$ ssh -F /dev/null -G test@test | grep gssapiauthentication
gssapiauthentication no
$ ssh -F /tmp/ssh_config -G test@test | grep gssapiauthentication
gssapiauthentication yes
$ vim /tmp/ssh_config
$ cat /tmp/ssh_config 
Match final
  GSSAPIAuthentication yes
$ ssh -F /tmp/ssh_config -G test@test | grep gssapiauthentication
gssapiauthentication yes

If I understand it right, the "final" keyword invokes the second pass through the configuration file and the "all" after that is already implicit (automatically matches in case there is no other option following).

openssh-7.9p1-4.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-0576ad8060

openssh-7.9p1-4.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-0576ad8060

openssh-7.9p1-4.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.

$ rpm -q --whatprovides /etc/ssh/ssh_config
openssh-clients-7.9p1-4.fc29.x86_64

$ cat /etc/ssh/ssh_config | egrep 'Host '

# Host *

Think this comment should perhaps be changed to '# Match final all' ?

(btw. still not clear on what if any diff there is between Match final & Match final all)

(btw. afaict this works perfectly now.  thank you!)

Yes, that is the default example in the configuration file. I don't think it needs a change

From my understanding, the "final" itself is just a prefix defining when the matching should be done (in the initial pass or in the final pass). After that, any other match condition follows that should be a valid argument to match. In our case we want to "match all". But from my reading of the code, omitting the "all" does not hurt since the result is set to 1 and after the final, no argument is needed. But clearly, the "match final all" is more correct according to the description in manual page.