From 7a58f19ed8169a69b1d8649b21f64748307ee3fc Mon Sep 17 00:00:00 2001 From: Thomas Lamprecht Date: Thu, 21 Sep 2017 13:31:22 +0200 Subject: [PATCH] ssh_merge_known_hosts: address auth failure problem MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit On node addition we create two entries in the cluster-wide known_host file with our public host key, one with the pve-localhost bound IP address and one with our nodename. SSH always lower cases hostnames or their aliases before comparing them to the known host entry. This is allowed as per RFC 1035, Section "2.3.3 Character Case" [1]. No problems are caused by this, if known_host entries are not hashed, as both, the original value and the now specified value can be compared canonically in an case insensitive matter. But, if a known_host entry is hashed we have no access to its original plain text value – and we cannot do a case insensitive comparison anymore. SSH thus expects that the original value was transformed to lowercase before hashing. We did not follow this convention when we added node keys to the clusters known_host file as we kept the case. This resulted in problems when a user set up nodes with names containing uppercase letters.[2] Instead of transforming everything to lowercase on hashing lets omit hashing known_host entries completely. To explain why this can be done safely – without security implications - we need to state the reason why hashing those entries would gain some security in the first place. It wants to prevent information leakage for the case an local account gets taken over by an attacker. If not hashed, the attacker could use the known_host file to see which other host the user connected to. This could "just" leak information on what a user does but could also make it easier to attacked the listed hosts too - e.g. if the user had an unprotected SSH key which the hosts trust. As there are other ways to get a list of hosts where an user connected too (.bash_history, monitoring outgoing traffic, ...) hashing known_host entries itself provides just a small hurdle of obfuscation in the case an account got already taken over. And this is the case for an normal, unprivileged user account. In the case of PVE hashing the used known_host file brings absolutely *no* advantage. First, the affected known_host file is located under /etc/pve/priv where only root has read access. Thus, an attacker would need to take over root to get the known_hosts in the first place. If he did take over root all hope is lost one way or another. Even if known_host was world readable, hashing would not do much. As and attacker would know that the nodes IPs are entries he could use /etc/network/interfaces to get the subnet of interest and just bruteforce all entries until we got all node IPs - he normally would only need to iterate through 8-16 bit in an IPv4 network. Even this could be simplified by just port scanning the range for an open port 8006, to get all PVE nodes in a subnet. Further /etc/hosts (world readable) often provides the information which hashing known_hosts tries to hide, as does /etc/pve/.members (readable by: www-data,root) So, to summarize, while for an unprivileged user it may add a slight defense against a information leak it really doesn't for a PVE systems root/cluster members - all information which it tries to hide is accessible in various other ways. Add new entries in plain text, add checks if entries are already there for the plain text case too. Further use lowercase comparison as openssh does. If hashed entries are already there allow them still, but ensure that a lowercase'd version is saved to avoid authentication failed problems. Ensure that our current valid SSH keys gets added even if another key on the same hostname exists already for some reasons. The code path which handles hashed host names has this behavior already since the beginning, so let the new non-hashed code act the same way. [1]: https://tools.ietf.org/html/rfc1035#section-2.3.3 [2]: https://forum.proxmox.com/threads/35473 Signed-off-by: Thomas Lamprecht (cherry picked from commit e4f92a201dc457483da285b1055a2fe665750fd4) squashed with fixup from: (cherry picked from commit fe129500fc0017aa8de6f9eba244d33e0f94221e) Signed-off-by: Thomas Lamprecht --- data/PVE/Cluster.pm | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm index 39708c8..435c915 100644 --- a/data/PVE/Cluster.pm +++ b/data/PVE/Cluster.pm @@ -1211,6 +1211,10 @@ sub ssh_merge_known_hosts { die "no node name specified" if !$nodename; die "no ip address specified" if !$ip_address; + # ssh lowercases hostnames (aliases) before comparision, so we need too + $nodename = lc($nodename); + $ip_address = lc($ip_address); + mkdir $authdir; if (! -f $sshknownhosts) { @@ -1269,6 +1273,13 @@ sub ssh_merge_known_hosts { } return; } + } else { + $key = lc($key); # avoid duplicate entries, ssh compares lowercased + if ($key eq $ip_address) { + $found_local_ip = 1 if $rsakey eq $hostkey; + } elsif ($key eq $nodename) { + $found_nodename = 1 if $rsakey eq $hostkey; + } } $data .= $line; } @@ -1291,16 +1302,9 @@ sub ssh_merge_known_hosts { &$merge_line($line); } - my $addIndex = $$; - my $add_known_hosts_entry = sub { + my $add_known_hosts_entry = sub { my ($name, $hostkey) = @_; - $addIndex++; - my $hmac = Digest::HMAC_SHA1->new("$addIndex" . time()); - my $b64salt = $hmac->b64digest . '='; - $hmac = Digest::HMAC_SHA1->new(decode_base64($b64salt)); - $hmac->add($name); - my $digest = $hmac->b64digest . '='; - $data .= "|1|$b64salt|$digest $hostkey\n"; + $data .= "$name $hostkey\n"; }; if (!$found_nodename || !$found_local_ip) { -- 2.39.2