From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 232B7C1650 for ; Tue, 16 Jan 2024 10:00:13 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 03E1233846 for ; Tue, 16 Jan 2024 10:00:13 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 16 Jan 2024 10:00:12 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EE9AA4913E; Tue, 16 Jan 2024 10:00:11 +0100 (CET) Date: Tue, 16 Jan 2024 10:00:10 +0100 (CET) From: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= To: Proxmox VE development discussion Cc: Esi Y Message-ID: <685792483.3789.1705395610875@webmail.proxmox.com> In-Reply-To: References: <20240111105123.370028-1-f.gruenbichler@proxmox.com> <20240111105123.370028-3-f.gruenbichler@proxmox.com> <1169764233.3317.1705319508414@webmail.proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.6-Rev57 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.065 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [sshinfo.pm] Subject: Re: [pve-devel] [PATCH cluster 2/4] fix #4886: SSH: pin node's host key if available X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Jan 2024 09:00:13 -0000 > Esi Y via pve-devel hat am 15.01.2024 15:31= CET geschrieben: > On Mon, Jan 15, 2024 at 12:51:48PM +0100, Fabian Gr=C3=BCnbichler wrote: > > > On Thu, Jan 11, 2024 at 11:51:16AM +0100, Fabian Gr=C3=BCnbichler wro= te: > > > > if the target node has already stored their SSH host key on pmxcfs,= pin it and > > > > ignore the global known hosts information. > > > >=20 > > > > Signed-off-by: Fabian Gr=C3=BCnbichler > > > > --- > > > > src/PVE/SSHInfo.pm | 15 ++++++++++++++- > > > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > >=20 > > > > diff --git a/src/PVE/SSHInfo.pm b/src/PVE/SSHInfo.pm > > > > index c351148..fad23bf 100644 > > > > --- a/src/PVE/SSHInfo.pm > > > > +++ b/src/PVE/SSHInfo.pm > > > > @@ -49,11 +49,24 @@ sub get_ssh_info { > > > > =20 > > > > sub ssh_info_to_command_base { > > > > my ($info, @extra_options) =3D @_; > > > > + > > > > + my $nodename =3D $info->{name}; > > > > + > > > > + my $known_hosts_file =3D "/etc/pve/nodes/$nodename/ssh_known_h= osts"; > > > > + my $known_hosts_options =3D undef; > > > > + if (-f $known_hosts_file) { > > > > +=09$known_hosts_options =3D [ > > > > +=09 '-o', "UserKnownHostsFile=3D$known_hosts_file", > > > > +=09 '-o', 'GlobalKnownHostsFile=3Dnone', > > >=20 > > > why does Global need to be none, even as this only applies if the sni= ppet exists? > >=20 > > because we want to only let SSH look at our pinned file, not the regula= r one, which might contain bogus information. since our pinned file contain= s an entry for our host key alias which must match, the global file can nev= er improve the situation, but it can cause a verification failure. >=20 > This might not work as expected. >=20 > 1. There will not be any verification failure if there is at least some v= alid key present. If wrong keys are present alongside a good one, it's a pa= ss. If _only_ wrong keys are present, with StrictHostKeyChecking default (a= sk) it will outright stop. >=20 > 2. The Global none does not improve anything there. If no keys are presen= t it will try to ask (under SKHC default), but no use in BatchMode. technically true, but doesn't really matter for our use case. we only want = to use our own pinned key (or maybe, keys, at some point in the future) for= internal connections. > 3. Using -o UserKHF alongside default SKHC, e.g. if run by someone even m= anually after a failed script without BatchMode, will have it crash for the= m because the pinned file cannot be updated by ssh properly due to the same= issue as mentioned before regarding ssh-keygen -R. In this case the pmxcfs= will cause it to crash again on link-unlink-rename() again [1]. >=20 > [1] https://github.com/openssh/openssh-portable/blob/50080fa42f5f744b798e= e29400c0710f1b59f50e/hostfile.c#L695 it doesn't crash, it just fails to work. and this is not the same issue as = the original one at all, since previously running the suggest command would= break the PVE setup by removing our symlink, whereas now it creates an emp= ty temp file but preserves our setup. > 4. I suppose you did not like my suggestion re KnownHostsCommand [2] inst= ead of "pinning", but giving -o's to ssh code where the files reside on pmx= cfs is just creating the same problem (that e.g. keygen -R had) elsewhere d= epending if you plan e.g. multiline. the only advantage of a KnownHostsCommand would be to avoid the above (tiny= ) issue in interactive use cases. our use case is by definition not interac= tive. the only situation where this should arise in practice is if you manu= ally rotate the SSH host key of a node already in the cluster. even then, i= t will solve itself after a reboot (or manual invocation of pvecm updatecer= ts, which should definitely be noted in a yet-to-be-written "keys/secrets a= nd rotating them" section of the docs). the command approach has similar problems though: - if it outputs a non-matching host key line, the connection will be aborte= d (so this is stricter than the file based solution! which is especially pr= oblematic if we extend this to handle all key types, since then a rotation = of one of them would already trip it up) - it internally treats the command option as if it were a file, leading to = very nice output like this: Offending RSA key in KnownHostsCommand-HOSTNAME:2 remove with: ssh-keygen -f "KnownHostsCommand-HOSTNAME" -R "XXX" Host key for XXX has changed and you have requested strict checking. Host key verification failed. (XXX is my hostname, the rest is output exactly like it is!) last but not least, switching to a command is always possible as follow-up = since it's entirely on the client side anyway and requires no coordination = across the cluster - the command would just output the contents of the file= anyhow.