[pve-devel] [PATCH cluster 1/2] ssh_merge_known_hosts: address auth failure problem

[Thomas Lamprecht]

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.

[1]: https://tools.ietf.org/html/rfc1035#section-2.3.3
[2]: https://forum.proxmox.com/threads/35473

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---
 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 9c8ce02..2bda5d9 100644
--- a/data/PVE/Cluster.pm
+++ b/data/PVE/Cluster.pm
@@ -1193,6 +1193,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) {
@@ -1251,6 +1255,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;
+		    } elsif ($key eq $nodename) {
+			$found_nodename = 1;
+		    }
 		}
 		$data .= $line;
 	    }
@@ -1273,16 +1284,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.11.0