diff options
author | djm@openbsd.org <djm@openbsd.org> | 2020-10-14 00:55:17 +0000 |
---|---|---|
committer | Damien Miller <djm@mindrot.org> | 2020-10-14 11:57:13 +1100 |
commit | 95b0bcfd1531d59e056ae8af27bb741391f26ab0 (patch) | |
tree | 472e89788df20eb6f607ae9368243c3ad763b735 /clientloop.c | |
parent | a336ce8c2c55547cc00e0070a18c55f30bb53fb6 (diff) |
upstream: make UpdateHostkeys still more conservative: refuse to
proceed if one of the keys offered by the server is already in known_hosts
under another name. This avoid collisions between address entries for
different host aliases when CheckHostIP=yes
Also, do not attempt to fix known_hosts with incomplete host/ip matches
when there are no new or deprecated hostkeys.
OpenBSD-Commit-ID: 95c19842f7c41f9bd9c92aa6441a278c0fd0c4a3
Diffstat (limited to 'clientloop.c')
-rw-r--r-- | clientloop.c | 114 |
1 files changed, 76 insertions, 38 deletions
diff --git a/clientloop.c b/clientloop.c index 2d401923..8cd6f710 100644 --- a/clientloop.c +++ b/clientloop.c @@ -1,4 +1,4 @@ -/* $OpenBSD: clientloop.c,v 1.352 2020/10/11 22:14:38 djm Exp $ */ +/* $OpenBSD: clientloop.c,v 1.353 2020/10/14 00:55:17 djm Exp $ */ /* * Author: Tatu Ylonen <ylo@cs.hut.fi> * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland @@ -1834,6 +1834,7 @@ struct hostkeys_update_ctx { int complex_hostspec; /* wildcard or manual pattern-list host name */ int ca_available; /* saw CA key for this host */ int old_key_seen; /* saw old key with other name/addr */ + int other_name_seen; /* saw key with other name/addr */ }; static void @@ -1887,9 +1888,39 @@ hostkeys_find(struct hostkey_foreach_line *l, void *_ctx) size_t i; struct sshkey **tmp; - if (l->status != HKF_STATUS_MATCHED || l->key == NULL || - l->marker != MRK_NONE) + if (l->key == NULL) return 0; + if (l->status != HKF_STATUS_MATCHED) { + /* Record if one of the keys appears on a non-matching line */ + for (i = 0; i < ctx->nkeys; i++) { + if (sshkey_equal(l->key, ctx->keys[i])) { + ctx->other_name_seen = 1; + debug3("%s: found %s key under different " + "name/addr at %s:%ld", __func__, + sshkey_ssh_name(ctx->keys[i]), + l->path, l->linenum); + return 0; + } + } + return 0; + } + /* Don't proceed if revocation or CA markers are present */ + /* XXX relax this */ + if (l->marker != MRK_NONE) { + debug3("%s: hostkeys file %s:%ld has CA/revocation marker", + __func__, l->path, l->linenum); + ctx->complex_hostspec = 1; + return 0; + } + + /* Record if address matched against a different hostname. */ + if (ctx->ip_str != NULL && (l->match & HKF_MATCH_HOST) == 0 && + strchr(l->hosts, ',') != NULL) { + ctx->other_name_seen = 1; + debug3("%s: found address %s against different hostname at " + "%s:%ld", __func__, ctx->ip_str, l->path, l->linenum); + return 0; + } /* * UpdateHostkeys is skipped for wildcard host names and hostnames @@ -2307,19 +2338,28 @@ client_input_hostkeys(struct ssh *ssh) ctx->nincomplete++; } - debug3("%s: %zu server keys: %zu new, %zu retained, " "%zu incomplete match. %zu to remove", __func__, ctx->nkeys, ctx->nnew, ctx->nkeys - ctx->nnew - ctx->nincomplete, ctx->nincomplete, ctx->nold); - if (ctx->complex_hostspec && - (ctx->nnew != 0 || ctx->nold != 0 || ctx->nincomplete != 0)) { - debug("%s: manual list or wildcard host pattern found, " - "skipping UserKnownHostsFile update", __func__); + if (ctx->nnew == 0 && ctx->nold == 0) { + debug("%s: no new or deprecated keys from server", __func__); goto out; } + /* Various reasons why we cannot proceed with the update */ + if (ctx->complex_hostspec) { + debug("%s: CA/revocation marker, manual host list or wildcard " + "host pattern found, skipping UserKnownHostsFile update", + __func__); + goto out; + } + if (ctx->other_name_seen) { + debug("%s: host key found matching a different name/address, " + "skipping UserKnownHostsFile update", __func__); + goto out; + } /* * If removing keys, check whether they appear under different * names/addresses and refuse to proceed if they do. This avoids @@ -2339,45 +2379,43 @@ client_input_hostkeys(struct ssh *ssh) } } - if (ctx->nnew == 0 && (ctx->nold != 0 || ctx->nincomplete != 0)) { + if (ctx->nnew == 0) { /* * We have some keys to remove or fix matching for. * We can proceed to do this without requiring a fresh proof * from the server. */ update_known_hosts(ctx); - } else if (ctx->nnew != 0) { - /* - * We have received hitherto-unseen keys from the server. - * Ask the server to confirm ownership of the private halves. - */ - debug3("%s: asking server to prove ownership for %zu keys", - __func__, ctx->nnew); - if ((r = sshpkt_start(ssh, SSH2_MSG_GLOBAL_REQUEST)) != 0 || - (r = sshpkt_put_cstring(ssh, - "hostkeys-prove-00@openssh.com")) != 0 || - (r = sshpkt_put_u8(ssh, 1)) != 0) /* bool: want reply */ - fatal("%s: cannot prepare packet: %s", + goto out; + } + /* + * We have received previously-unseen keys from the server. + * Ask the server to confirm ownership of the private halves. + */ + debug3("%s: asking server to prove ownership for %zu keys", + __func__, ctx->nnew); + if ((r = sshpkt_start(ssh, SSH2_MSG_GLOBAL_REQUEST)) != 0 || + (r = sshpkt_put_cstring(ssh, + "hostkeys-prove-00@openssh.com")) != 0 || + (r = sshpkt_put_u8(ssh, 1)) != 0) /* bool: want reply */ + fatal("%s: prepare hostkeys-prove: %s", __func__, ssh_err(r)); + if ((buf = sshbuf_new()) == NULL) + fatal("%s: sshbuf_new", __func__); + for (i = 0; i < ctx->nkeys; i++) { + if (ctx->keys_match[i]) + continue; + sshbuf_reset(buf); + if ((r = sshkey_putb(ctx->keys[i], buf)) != 0 || + (r = sshpkt_put_stringb(ssh, buf)) != 0) { + fatal("%s: assemble hostkeys-prove: %s", __func__, ssh_err(r)); - if ((buf = sshbuf_new()) == NULL) - fatal("%s: sshbuf_new", __func__); - for (i = 0; i < ctx->nkeys; i++) { - if (ctx->keys_match[i]) - continue; - sshbuf_reset(buf); - if ((r = sshkey_putb(ctx->keys[i], buf)) != 0) - fatal("%s: sshkey_putb: %s", - __func__, ssh_err(r)); - if ((r = sshpkt_put_stringb(ssh, buf)) != 0) - fatal("%s: sshpkt_put_string: %s", - __func__, ssh_err(r)); } - if ((r = sshpkt_send(ssh)) != 0) - fatal("%s: sshpkt_send: %s", __func__, ssh_err(r)); - client_register_global_confirm( - client_global_hostkeys_private_confirm, ctx); - ctx = NULL; /* will be freed in callback */ } + if ((r = sshpkt_send(ssh)) != 0) + fatal("%s: sshpkt_send: %s", __func__, ssh_err(r)); + client_register_global_confirm( + client_global_hostkeys_private_confirm, ctx); + ctx = NULL; /* will be freed in callback */ /* Success */ out: |