[openssh-commits] [openssh] 02/03: upstream: UpdateHostkeys: better CheckHostIP handling
git+noreply at mindrot.org
git+noreply at mindrot.org
Mon Oct 12 11:23:03 AEDT 2020
This is an automated email from the git hooks/post-receive script.
djm pushed a commit to branch master
in repository openssh.
commit d98f14b5328922ae3085e07007d820c4f655b57a
Author: djm at openbsd.org <djm at openbsd.org>
Date: Sun Oct 11 22:13:37 2020 +0000
upstream: UpdateHostkeys: better CheckHostIP handling
When preparing to update the known_hosts file, fully check both
entries for both the host and the address (if CheckHostIP enabled)
and ensure that, at the end of the operation, entries for both are
recorded.
Make sure this works with HashKnownHosts too, which requires maintaining
a list of entry-types seen across the whole file for each key.
ok markus@
OpenBSD-Commit-ID: 374dc263103f6b343d9671f87dbf81ffd0d6abdd
---
clientloop.c | 71 +++++++++++++++++++++++++++++++++++--------------------
hostfile.c | 77 ++++++++++++++++++++++++++++++++++++++----------------------
2 files changed, 95 insertions(+), 53 deletions(-)
diff --git a/clientloop.c b/clientloop.c
index 9daec13c..a4a53dd1 100644
--- a/clientloop.c
+++ b/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.350 2020/10/11 22:12:44 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.351 2020/10/11 22:13:37 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo at cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -1814,12 +1814,13 @@ struct hostkeys_update_ctx {
/*
* Keys received from the server and a flag for each indicating
* whether they already exist in known_hosts.
- * keys_seen is filled in by hostkeys_find() and later (for new
+ * keys_match is filled in by hostkeys_find() and later (for new
* keys) by client_global_hostkeys_private_confirm().
*/
struct sshkey **keys;
- int *keys_seen;
- size_t nkeys, nnew;
+ u_int *keys_match; /* mask of HKF_MATCH_* from hostfile.h */
+ int *keys_verified; /* flag for new keys verified by server */
+ size_t nkeys, nnew, nincomplete; /* total, new keys, incomplete match */
/*
* Keys that are in known_hosts, but were not present in the update
@@ -1844,7 +1845,8 @@ hostkeys_update_ctx_free(struct hostkeys_update_ctx *ctx)
for (i = 0; i < ctx->nkeys; i++)
sshkey_free(ctx->keys[i]);
free(ctx->keys);
- free(ctx->keys_seen);
+ free(ctx->keys_match);
+ free(ctx->keys_verified);
for (i = 0; i < ctx->nold; i++)
sshkey_free(ctx->old_keys[i]);
free(ctx->old_keys);
@@ -1900,12 +1902,12 @@ hostkeys_find(struct hostkey_foreach_line *l, void *_ctx)
/* Mark off keys we've already seen for this host */
for (i = 0; i < ctx->nkeys; i++) {
- if (sshkey_equal(l->key, ctx->keys[i])) {
- debug3("%s: found %s key at %s:%ld", __func__,
- sshkey_ssh_name(ctx->keys[i]), l->path, l->linenum);
- ctx->keys_seen[i] = 1;
- return 0;
- }
+ if (!sshkey_equal(l->key, ctx->keys[i]))
+ continue;
+ debug3("%s: found %s key at %s:%ld", __func__,
+ sshkey_ssh_name(ctx->keys[i]), l->path, l->linenum);
+ ctx->keys_match[i] |= l->match;
+ return 0;
}
/* This line contained a key that not offered by the server */
debug3("%s: deprecated %s key at %s:%ld", __func__,
@@ -1940,7 +1942,7 @@ update_known_hosts(struct hostkeys_update_ctx *ctx)
struct stat sb;
for (i = 0; i < ctx->nkeys; i++) {
- if (ctx->keys_seen[i] != 2)
+ if (!ctx->keys_verified[i])
continue;
if ((fp = sshkey_fingerprint(ctx->keys[i],
options.fingerprint_hash, SSH_FP_DEFAULT)) == NULL)
@@ -2053,10 +2055,10 @@ client_global_hostkeys_private_confirm(struct ssh *ssh, int type,
/*
* Expect a signature for each of the ctx->nnew private keys we
* haven't seen before. They will be in the same order as the
- * ctx->keys where the corresponding ctx->keys_seen[i] == 0.
+ * ctx->keys where the corresponding ctx->keys_match[i] == 0.
*/
for (ndone = i = 0; i < ctx->nkeys; i++) {
- if (ctx->keys_seen[i])
+ if (ctx->keys_match[i])
continue;
/* Prepare data to be signed: session ID, unique string, key */
sshbuf_reset(signdata);
@@ -2088,7 +2090,7 @@ client_global_hostkeys_private_confirm(struct ssh *ssh, int type,
goto out;
}
/* Key is good. Mark it as 'seen' */
- ctx->keys_seen[i] = 2;
+ ctx->keys_verified[i] = 1;
ndone++;
}
if (ndone != ctx->nnew)
@@ -2141,6 +2143,7 @@ client_input_hostkeys(struct ssh *ssh)
static int hostkeys_seen = 0; /* XXX use struct ssh */
extern struct sockaddr_storage hostaddr; /* XXX from ssh.c */
struct hostkeys_update_ctx *ctx = NULL;
+ u_int want;
if (hostkeys_seen)
fatal("%s: server already sent hostkeys", __func__);
@@ -2205,8 +2208,10 @@ client_input_hostkeys(struct ssh *ssh)
goto out;
}
- if ((ctx->keys_seen = calloc(ctx->nkeys,
- sizeof(*ctx->keys_seen))) == NULL)
+ if ((ctx->keys_match = calloc(ctx->nkeys,
+ sizeof(*ctx->keys_match))) == NULL ||
+ (ctx->keys_verified = calloc(ctx->nkeys,
+ sizeof(*ctx->keys_verified))) == NULL)
fatal("%s: calloc failed", __func__);
get_hostfile_hostname_ipaddr(host,
@@ -2234,21 +2239,37 @@ client_input_hostkeys(struct ssh *ssh)
}
/* Figure out if we have any new keys to add */
- ctx->nnew = 0;
+ ctx->nnew = ctx->nincomplete = 0;
+ want = HKF_MATCH_HOST | ( options.check_host_ip ? HKF_MATCH_IP : 0);
for (i = 0; i < ctx->nkeys; i++) {
- if (!ctx->keys_seen[i])
+ if (ctx->keys_match[i] == 0)
ctx->nnew++;
+ if ((ctx->keys_match[i] & want) != want)
+ ctx->nincomplete++;
}
- debug3("%s: %zu keys from server: %zu new, %zu retained. %zu to remove",
- __func__, ctx->nkeys, ctx->nnew, ctx->nkeys - ctx->nnew, ctx->nold);
+ /*
+ * XXX if removing keys, check whether they appear under different
+ * names/addresses and refuse to proceed if they do.
+ */
- if (ctx->complex_hostspec && (ctx->nnew != 0 || ctx->nold != 0)) {
+ 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__);
goto out;
- } else if (ctx->nnew == 0 && ctx->nold != 0) {
- /* We have some keys to remove. Just do it. */
+ } else if (ctx->nnew == 0 &&
+ (ctx->nold != 0 || ctx->nincomplete != 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) {
/*
@@ -2266,7 +2287,7 @@ client_input_hostkeys(struct ssh *ssh)
if ((buf = sshbuf_new()) == NULL)
fatal("%s: sshbuf_new", __func__);
for (i = 0; i < ctx->nkeys; i++) {
- if (ctx->keys_seen[i])
+ if (ctx->keys_match[i])
continue;
sshbuf_reset(buf);
if ((r = sshkey_putb(ctx->keys[i], buf)) != 0)
diff --git a/hostfile.c b/hostfile.c
index 650ad66f..373b9d8d 100644
--- a/hostfile.c
+++ b/hostfile.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: hostfile.c,v 1.84 2020/10/07 02:25:43 djm Exp $ */
+/* $OpenBSD: hostfile.c,v 1.85 2020/10/11 22:13:37 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo at cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo at cs.hut.fi>, Espoo, Finland
@@ -520,8 +520,8 @@ add_host_to_hostfile(const char *filename, const char *host,
struct host_delete_ctx {
FILE *out;
int quiet;
- const char *host;
- int *skip_keys; /* XXX split for host/ip? might want to ensure both */
+ const char *host, *ip;
+ u_int *match_keys; /* mask of HKF_MATCH_* for this key */
struct sshkey * const *keys;
size_t nkeys;
int modified;
@@ -534,26 +534,21 @@ host_delete(struct hostkey_foreach_line *l, void *_ctx)
int loglevel = ctx->quiet ? SYSLOG_LEVEL_DEBUG1 : SYSLOG_LEVEL_VERBOSE;
size_t i;
- if (l->status == HKF_STATUS_MATCHED) {
- if (l->marker != MRK_NONE) {
- /* Don't remove CA and revocation lines */
- fprintf(ctx->out, "%s\n", l->line);
- return 0;
- }
-
+ /* Don't remove CA and revocation lines */
+ if (l->status == HKF_STATUS_MATCHED && l->marker == MRK_NONE) {
/*
* If this line contains one of the keys that we will be
* adding later, then don't change it and mark the key for
* skipping.
*/
for (i = 0; i < ctx->nkeys; i++) {
- if (sshkey_equal(ctx->keys[i], l->key)) {
- ctx->skip_keys[i] = 1;
- fprintf(ctx->out, "%s\n", l->line);
- debug3("%s: %s key already at %s:%ld", __func__,
- sshkey_type(l->key), l->path, l->linenum);
- return 0;
- }
+ if (!sshkey_equal(ctx->keys[i], l->key))
+ continue;
+ ctx->match_keys[i] |= l->match;
+ fprintf(ctx->out, "%s\n", l->line);
+ debug3("%s: %s key already at %s:%ld", __func__,
+ sshkey_type(l->key), l->path, l->linenum);
+ return 0;
}
/*
@@ -584,15 +579,19 @@ hostfile_replace_entries(const char *filename, const char *host, const char *ip,
int loglevel = quiet ? SYSLOG_LEVEL_DEBUG1 : SYSLOG_LEVEL_VERBOSE;
struct host_delete_ctx ctx;
char *fp, *temp = NULL, *back = NULL;
+ const char *what;
mode_t omask;
size_t i;
+ u_int want;
omask = umask(077);
memset(&ctx, 0, sizeof(ctx));
ctx.host = host;
+ ctx.ip = ip;
ctx.quiet = quiet;
- if ((ctx.skip_keys = calloc(nkeys, sizeof(*ctx.skip_keys))) == NULL)
+
+ if ((ctx.match_keys = calloc(nkeys, sizeof(*ctx.match_keys))) == NULL)
return SSH_ERR_ALLOC_FAIL;
ctx.keys = keys;
ctx.nkeys = nkeys;
@@ -621,7 +620,7 @@ hostfile_replace_entries(const char *filename, const char *host, const char *ip,
goto fail;
}
- /* Remove all entries for the specified host from the file */
+ /* Remove stale/mismatching entries for the specified host */
if ((r = hostkeys_foreach(filename, host_delete, &ctx, host, ip,
HKF_WANT_PARSE_KEY)) != 0) {
oerrno = errno;
@@ -629,23 +628,45 @@ hostfile_replace_entries(const char *filename, const char *host, const char *ip,
goto fail;
}
- /* Add the requested keys */
+ /* Re-add the requested keys */
+ want = HKF_MATCH_HOST | (ip == NULL ? 0 : HKF_MATCH_IP);
for (i = 0; i < nkeys; i++) {
- if (ctx.skip_keys[i])
+ if ((want & ctx.match_keys[i]) == want)
continue;
if ((fp = sshkey_fingerprint(keys[i], hash_alg,
SSH_FP_DEFAULT)) == NULL) {
r = SSH_ERR_ALLOC_FAIL;
goto fail;
}
- do_log2(loglevel, "%s%sAdding new key for %s to %s: %s %s",
- quiet ? __func__ : "", quiet ? ": " : "", host, filename,
+ /* write host/ip */
+ what = "";
+ if (ctx.match_keys[i] == 0) {
+ what = "Adding new key";
+ if (!write_host_entry(ctx.out, host, ip,
+ keys[i], store_hash)) {
+ r = SSH_ERR_INTERNAL_ERROR;
+ goto fail;
+ }
+ } else if ((want & ~ctx.match_keys[i]) == HKF_MATCH_HOST) {
+ what = "Fixing match (hostname)";
+ if (!write_host_entry(ctx.out, host, NULL,
+ keys[i], store_hash)) {
+ r = SSH_ERR_INTERNAL_ERROR;
+ goto fail;
+ }
+ } else if ((want & ~ctx.match_keys[i]) == HKF_MATCH_IP) {
+ what = "Fixing match (address)";
+ if (!write_host_entry(ctx.out, ip, NULL,
+ keys[i], store_hash)) {
+ r = SSH_ERR_INTERNAL_ERROR;
+ goto fail;
+ }
+ }
+ do_log2(loglevel, "%s%s%s for %s%s%s to %s: %s %s",
+ quiet ? __func__ : "", quiet ? ": " : "", what,
+ host, ip == NULL ? "" : ",", ip == NULL ? "" : ip, filename,
sshkey_ssh_name(keys[i]), fp);
free(fp);
- if (!write_host_entry(ctx.out, host, ip, keys[i], store_hash)) {
- r = SSH_ERR_INTERNAL_ERROR;
- goto fail;
- }
ctx.modified = 1;
}
fclose(ctx.out);
@@ -690,7 +711,7 @@ hostfile_replace_entries(const char *filename, const char *host, const char *ip,
free(back);
if (ctx.out != NULL)
fclose(ctx.out);
- free(ctx.skip_keys);
+ free(ctx.match_keys);
umask(omask);
if (r == SSH_ERR_SYSTEM_ERROR)
errno = oerrno;
--
To stop receiving notification emails like this one, please contact
djm at mindrot.org.
More information about the openssh-commits
mailing list