Oskari Saarenmaa
2012-Dec-27 15:15 UTC
[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
Oskari Saarenmaa
2013-Nov-07 20:48 UTC
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
Heberlein, Kurt William
2013-Nov-07 21:09 UTC
RE: [PATCH] hostfile: list known names (if any) for new hostkeys
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
Oskari Saarenmaa
2013-Nov-07 21:49 UTC
Re: [PATCH] hostfile: list known names (if any) for new hostkeys
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