openssh unix dev - Dec 2012 - [PATCH] hostfile: list known names (if any) for new hostkeys

When connecting to a host for which there's no known hostkey, check if the
relevant key has been accepted for other hostnames.  This is useful when
connecting to a host with a dymamic IP address or multiple names.
---
 auth.c        |  4 ++--
 hostfile.c    | 42 ++++++++++++++++++++++++++++--------------
 hostfile.h    |  8 ++++++--
 sshconnect.c  | 39 +++++++++++++++++++++++++++++++++------
 sshconnect2.c |  4 ++--
 5 files changed, 71 insertions(+), 26 deletions(-)

diff --git a/auth.c b/auth.c
index 7bc6f40..1ca07e1 100644
--- a/auth.c
+++ b/auth.c
@@ -379,7 +379,7 @@ check_key_in_hostfiles(struct passwd *pw, Key *key, const
char *host,
 	const struct hostkey_entry *found;
 
 	hostkeys = init_hostkeys();
-	load_hostkeys(hostkeys, host, sysfile);
+	load_hostkeys(hostkeys, host, NULL, sysfile);
 	if (userfile != NULL) {
 		user_hostfile = tilde_expand_filename(userfile, pw->pw_uid);
 		if (options.strict_modes &&
@@ -393,7 +393,7 @@ check_key_in_hostfiles(struct passwd *pw, Key *key, const
char *host,
 			    user_hostfile);
 		} else {
 			temporarily_use_uid(pw);
-			load_hostkeys(hostkeys, host, user_hostfile);
+			load_hostkeys(hostkeys, host, NULL, user_hostfile);
 			restore_uid();
 		}
 		xfree(user_hostfile);
diff --git a/hostfile.c b/hostfile.c
index b6f924b..e493c91 100644
--- a/hostfile.c
+++ b/hostfile.c
@@ -58,11 +58,6 @@
 #include "log.h"
 #include "misc.h"
 
-struct hostkeys {
-	struct hostkey_entry *entries;
-	u_int num_entries;
-};
-
 static int
 extract_salt(const char *s, u_int l, char *salt, size_t salt_len)
 {
@@ -236,20 +231,22 @@ init_hostkeys(void)
 }
 
 void
-load_hostkeys(struct hostkeys *hostkeys, const char *host, const char *path)
+load_hostkeys(struct hostkeys *hostkeys, const char *lookup_host,
+    const Key *lookup_key, const char *path)
 {
 	FILE *f;
 	char line[8192];
 	u_long linenum = 0, num_loaded = 0;
 	char *cp, *cp2, *hashed_host;
+	const char *current_host;
 	HostkeyMarker marker;
 	Key *key;
 	int kbits;
 
 	if ((f = fopen(path, "r")) == NULL)
 		return;
-	debug3("%s: loading entries for host \"%.100s\" from file
\"%s\"",
-	    __func__, host, path);
+	debug3("%s: loading entries for host \"%.100s\"%s from file
\"%s\"",
+	    __func__, lookup_host, (lookup_key ? " and key" : ""),
path);
 	while (read_keyfile_line(f, path, line, sizeof(line), &linenum) == 0) {
 		cp = line;
 
@@ -269,11 +266,11 @@ load_hostkeys(struct hostkeys *hostkeys, const char *host,
const char *path)
 		for (cp2 = cp; *cp2 && *cp2 != ' ' && *cp2 !=
'\t'; cp2++)
 			;
 
-		/* Check if the host name matches. */
-		if (match_hostname(host, cp, (u_int) (cp2 - cp)) != 1) {
+		/* Check if the host name matches if we're looking for a host. */
+		if (lookup_host && match_hostname(lookup_host, cp, (u_int) (cp2 -
cp)) != 1) {
 			if (*cp != HASH_DELIM)
 				continue;
-			hashed_host = host_hash(host, cp, (u_int) (cp2 - cp));
+			hashed_host = host_hash(lookup_host, cp, (u_int) (cp2 - cp));
 			if (hashed_host == NULL) {
 				debug("Invalid hashed host line %lu of %s",
 				    linenum, path);
@@ -283,7 +280,17 @@ load_hostkeys(struct hostkeys *hostkeys, const char *host,
const char *path)
 				continue;
 		}
 
-		/* Got a match.  Skip host name. */
+		/* If we're looking for a key grab the hostname and ignore hashed
entries. */
+		if (lookup_key) {
+			if (*cp == HASH_DELIM)
+				continue;
+			*cp2++ = 0;
+			current_host = cp;
+		} else {
+			current_host = lookup_host;
+		}
+
+		/* Move pointer past the hostname. */
 		cp = cp2;
 
 		/*
@@ -299,7 +306,14 @@ load_hostkeys(struct hostkeys *hostkeys, const char *host,
const char *path)
 				continue;
 			}
 		}
-		if (!hostfile_check_key(kbits, key, host, path, linenum))
+
+		/* Check if the key matches if we're looking for a key. */
+		if (lookup_key) {
+		        if (!key_equal(lookup_key, key))
+		                continue;
+		}
+
+		if (!hostfile_check_key(kbits, key, current_host, path, linenum))
 			continue;
 
 		debug3("%s: found %skey type %s in file %s:%lu", __func__,
@@ -308,7 +322,7 @@ load_hostkeys(struct hostkeys *hostkeys, const char *host,
const char *path)
 		    key_type(key), path, linenum);
 		hostkeys->entries = xrealloc(hostkeys->entries,
 		    hostkeys->num_entries + 1, sizeof(*hostkeys->entries));
-		hostkeys->entries[hostkeys->num_entries].host = xstrdup(host);
+		hostkeys->entries[hostkeys->num_entries].host = xstrdup(current_host);
 		hostkeys->entries[hostkeys->num_entries].file = xstrdup(path);
 		hostkeys->entries[hostkeys->num_entries].line = linenum;
 		hostkeys->entries[hostkeys->num_entries].key = key;
diff --git a/hostfile.h b/hostfile.h
index d84d422..c2965f9 100644
--- a/hostfile.h
+++ b/hostfile.h
@@ -29,10 +29,14 @@ struct hostkey_entry {
 	Key *key;
 	HostkeyMarker marker;
 };
-struct hostkeys;
+
+struct hostkeys {
+	struct hostkey_entry *entries;
+	u_int num_entries;
+};
 
 struct hostkeys *init_hostkeys(void);
-void	 load_hostkeys(struct hostkeys *, const char *, const char *);
+void	 load_hostkeys(struct hostkeys *, const char *, const Key *, const char
*);
 void	 free_hostkeys(struct hostkeys *);
 
 HostStatus check_key_in_hostkeys(struct hostkeys *, Key *,
diff --git a/sshconnect.c b/sshconnect.c
index 07800a6..62306ac 100644
--- a/sshconnect.c
+++ b/sshconnect.c
@@ -718,13 +718,13 @@ check_host_key(char *hostname, struct sockaddr *hostaddr,
u_short port,
 	Key *raw_key = NULL;
 	char *ip = NULL, *host = NULL;
 	char hostline[1000], *hostp, *fp, *ra;
-	char msg[1024];
+	char msg[2048];
 	const char *type;
 	const struct hostkey_entry *host_found, *ip_found;
 	int len, cancelled_forwarding = 0;
 	int local = sockaddr_is_local(hostaddr);
 	int r, want_cert = key_is_cert(host_key), host_ip_differ = 0;
-	struct hostkeys *host_hostkeys, *ip_hostkeys;
+	struct hostkeys *host_hostkeys, *ip_hostkeys, *key_hostkeys = NULL;
 	u_int i;
 
 	/*
@@ -758,17 +758,17 @@ check_host_key(char *hostname, struct sockaddr *hostaddr,
u_short port,
 
 	host_hostkeys = init_hostkeys();
 	for (i = 0; i < num_user_hostfiles; i++)
-		load_hostkeys(host_hostkeys, host, user_hostfiles[i]);
+		load_hostkeys(host_hostkeys, host, NULL, user_hostfiles[i]);
 	for (i = 0; i < num_system_hostfiles; i++)
-		load_hostkeys(host_hostkeys, host, system_hostfiles[i]);
+		load_hostkeys(host_hostkeys, host, NULL, system_hostfiles[i]);
 
 	ip_hostkeys = NULL;
 	if (!want_cert && options.check_host_ip) {
 		ip_hostkeys = init_hostkeys();
 		for (i = 0; i < num_user_hostfiles; i++)
-			load_hostkeys(ip_hostkeys, ip, user_hostfiles[i]);
+			load_hostkeys(ip_hostkeys, ip, NULL, user_hostfiles[i]);
 		for (i = 0; i < num_system_hostfiles; i++)
-			load_hostkeys(ip_hostkeys, ip, system_hostfiles[i]);
+			load_hostkeys(ip_hostkeys, ip, NULL, system_hostfiles[i]);
 	}
 
  retry:
@@ -879,6 +879,29 @@ check_host_key(char *hostname, struct sockaddr *hostaddr,
u_short port,
 					    "No matching host key fingerprint"
 					    " found in DNS.\n");
 			}
+			/* Has this key been accepted for other hostnames? */
+			key_hostkeys = init_hostkeys();
+			for (i = 0; i < num_user_hostfiles; i++)
+				load_hostkeys(key_hostkeys, NULL, host_key,
+				    user_hostfiles[i]);
+			for (i = 0; i < num_system_hostfiles; i++)
+				load_hostkeys(key_hostkeys, NULL, host_key,
+				    system_hostfiles[i]);
+			if (key_hostkeys->num_entries > 0) {
+				strlcat(msg2, "You have previously accepted "
+				    "this key for the following hostnames:",
+				    sizeof(msg2));
+				for (i = 0; i < key_hostkeys->num_entries; i++) {
+					strlcat(msg2, "\n\t", sizeof(msg2));
+					strlcat(msg2, key_hostkeys->entries[i].host,
+					    sizeof(msg2));
+				}
+				if (strlcat(msg2, "\n", sizeof(msg2)) >+				    sizeof(msg2))
{
+					/* truncate at last newline. */
+					*(strrchr(msg2, '\n') + 1) = 0;
+				}
+			}
 			snprintf(msg, sizeof(msg),
 			    "The authenticity of host '%.200s (%s)' can't be "
 			    "established%s\n"
@@ -1097,6 +1120,8 @@ check_host_key(char *hostname, struct sockaddr *hostaddr,
u_short port,
 		free_hostkeys(host_hostkeys);
 	if (ip_hostkeys != NULL)
 		free_hostkeys(ip_hostkeys);
+	if (key_hostkeys != NULL)
+		free_hostkeys(key_hostkeys);
 	return 0;
 
 fail:
@@ -1120,6 +1145,8 @@ fail:
 		free_hostkeys(host_hostkeys);
 	if (ip_hostkeys != NULL)
 		free_hostkeys(ip_hostkeys);
+	if (key_hostkeys != NULL)
+		free_hostkeys(key_hostkeys);
 	return -1;
 }
 
diff --git a/sshconnect2.c b/sshconnect2.c
index 6791ea3..a3ed37a 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -115,9 +115,9 @@ order_hostkeyalgs(char *host, struct sockaddr *hostaddr,
u_short port)
 	get_hostfile_hostname_ipaddr(host, hostaddr, port, &hostname, NULL);
 	hostkeys = init_hostkeys();
 	for (i = 0; i < options.num_user_hostfiles; i++)
-		load_hostkeys(hostkeys, hostname, options.user_hostfiles[i]);
+		load_hostkeys(hostkeys, hostname, NULL, options.user_hostfiles[i]);
 	for (i = 0; i < options.num_system_hostfiles; i++)
-		load_hostkeys(hostkeys, hostname, options.system_hostfiles[i]);
+		load_hostkeys(hostkeys, hostname, NULL, options.system_hostfiles[i]);
 
 	oavail = avail = xstrdup(KEX_DEFAULT_PK_ALG);
 	maxlen = strlen(avail) + 1;
-- 
1.8.0.2

27.12.2012 17:15, Oskari Saarenmaa kirjoitti:
> When connecting to a host for which there''s no known hostkey,
check if the
> relevant key has been accepted for other hostnames.  This is useful when
> connecting to a host with a dymamic IP address or multiple names.
Ping, anyone had a chance to look at this patch yet?  I''ve also
attached
it to bugzilla,
https://bugzilla.mindrot.org/show_bug.cgi?id=2131

Thanks,
Oskari

Doesn''t this play in the same space as StrictHostKeyChecking ?   
Doesn''t it also sort of expose MITM if a known hostkey arrives from a
different IP/named host?
Cheers, -Kurt

-----Original Message-----
From: openssh-unix-dev-bounces+kurt.w.heberlein=hp.com@mindrot.org
[mailto:openssh-unix-dev-bounces+kurt.w.heberlein=hp.com@mindrot.org] On Behalf
Of Oskari Saarenmaa
Sent: Thursday, November 07, 2013 2:48 PM
To: openssh-unix-dev@mindrot.org
Subject: Re: [PATCH] hostfile: list known names (if any) for new hostkeys

27.12.2012 17:15, Oskari Saarenmaa kirjoitti:
> When connecting to a host for which there''s no known hostkey,
check if the
> relevant key has been accepted for other hostnames.  This is useful when
> connecting to a host with a dymamic IP address or multiple names.
Ping, anyone had a chance to look at this patch yet?  I''ve also
attached
it to bugzilla,
https://bugzilla.mindrot.org/show_bug.cgi?id=2131

Thanks,
Oskari

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev

On Thu, Nov 07, 2013 at 09:09:50PM +0000, Heberlein, Kurt William
wrote:
> Doesn''t this play in the same space as StrictHostKeyChecking ?   
Doesn''t
> it also sort of expose MITM if a known hostkey arrives from a different
> IP/named host?
This is related, but different.  This patch matches hostkeys for new hosts
against hostkeys of already known hosts but doesn''t accept or reject
them,
it just prints out the matching, already known keys.

I don''t think there''s much chance for MITM, the only thing
this patch does
is listing all previously accepted keys that match the new key presented by
the remote host.  The idea is to make it easier for the user to verify that
the new host (new hostname or ip address) they''re connecting to is the
same
host they''ve already accessed previously, if two hosts share the same
key
the chances are that the administrator has knowingly installed the same key
on multiple hosts (or they''ve been compromised, or they''re
running Debian''s
broken OpenSSL).

This patch would''ve probably exposed the issues in Debian''s
key generation a
bit sooner as a number of unrelated hosts would''ve been found to use
identical keys.
> -----Original Message-----
> 27.12.2012 17:15, Oskari Saarenmaa kirjoitti:
> > When connecting to a host for which there''s no known hostkey,
check if the
> > relevant key has been accepted for other hostnames.  This is useful
when
> > connecting to a host with a dymamic IP address or multiple names.
> 
> Ping, anyone had a chance to look at this patch yet?  I''ve also
attached
> it to bugzilla,
> https://bugzilla.mindrot.org/show_bug.cgi?id=2131
/ Oskari