On Mon, 23 Nov 2020, Stuart Henderson wrote:
> I have a machine (OpenBSD -current) with the following known_hosts
> entries from various old connections:
>
> v4_address ecdsa-sha2-nistp256
> v6_address ecdsa-sha2-nistp256 (line 1515)
> hostname ecdsa-sha2-nistp256
> hostname,v4_address ssh-rsa
> hostname,v4_address ssh-ed25519 (line 1222)
>
> I've changed my resolver to hand out v6 addresses before v4 addresses
> (I have done this before - given the above entries it looks like it was
> at a time before ssh-ed25519 keys - then reverted it after an ISP problem
> at some point). Now I get hostkey warnings. It's easy enough to cope
with
> the change manually of course, but I wonder if the automated behaviour
> could be improved.
[snip]
> Should order_hostkeyalgs consider the address family and actually prefer
> ecdsa-sha2-nistp256 in this case even though it has a better algorithm
> for the host *name*?
ATM order_hostkeyalgs() doesn't consider the address at all - it only
uses the hostname. I'm not sure of a good heuristic to use here.
> Should this warning be skipped if the algorithm differs? (At least in the
> case where UpdateHostKeys is used?)
I have considered something like the following, that disregards the
CheckHostIP warnings if the key matched the name but didn't match the
address *and* the algorithm is a lower priority in HostkeyAlgorithms.
That being said, I'm not entirely clear on exactly what problem
CheckHostIP is trying to solve. It was added to OpenSSH before the
2.x protocol and thus was written prior to a server potentially having
multiple host keys of different types.
-d
diff --git a/sshconnect.c b/sshconnect.c
index f07d342..c5f63ad 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -827,6 +827,41 @@ other_hostkeys_message(const char *host, const char *ip,
return ret;
}
+/* Returns the offset of 's' in a comma-separated list */
+static int
+match_list_offset(const char *list, const char *s)
+{
+ const char *cp;
+ size_t l = strlen(s);
+
+ for (cp = list; cp != NULL; ) {
+ if (strncmp(cp, s, l) == 0 && (cp[l] == ',' || cp[l] ==
'\0'))
+ return (int)(cp - list);
+ if ((cp = strchr(cp, ',')) == NULL)
+ break;
+ cp++;
+ }
+ return -1;
+}
+
+/*
+ * returns nonzero if the type of key 'a' is lower (less preferred)
+ * than key 'b' on HostkeyAlgorithms.
+ */
+static int
+hostkey_priority_lower(const struct sshkey *a, const struct sshkey *b)
+{
+ int ao, bo;
+
+ if (a == NULL || b == NULL)
+ return 0;
+ ao = match_list_offset(options.hostkeyalgorithms, sshkey_ssh_name(a));
+ bo = match_list_offset(options.hostkeyalgorithms, sshkey_ssh_name(b));
+ if (ao == -1 || bo == -1)
+ return 0;
+ return ao > bo;
+}
+
/*
* check whether the supplied host key is valid, return -1 if the key
* is not valid. user_hostfile[0] will not be updated if 'readonly' is
true.
@@ -919,9 +954,15 @@ check_host_key(char *hostname, struct sockaddr *hostaddr,
u_short port,
if (!want_cert && ip_hostkeys != NULL) {
ip_status = check_key_in_hostkeys(ip_hostkeys, host_key,
&ip_found);
- if (host_status == HOST_CHANGED &&
- (ip_status != HOST_CHANGED ||
- (ip_found != NULL &&
+ if (ip_status == HOST_CHANGED && host_status == HOST_OK &&
+ hostkey_priority_lower(ip_found->key, host_found->key)) {
+ debug_f("matching lower-priority key type %s for "
+ "address %s found at %s:%lu",
+ sshkey_ssh_name(ip_found->key), ip,
+ ip_found->file, ip_found->line);
+ ip_status = HOST_NEW;
+ } else if (host_status == HOST_CHANGED &&
+ (ip_status != HOST_CHANGED || (ip_found != NULL &&
!sshkey_equal(ip_found->key, host_found->key))))
host_ip_differ = 1;
} else