[pve-devel] [PATCH cluster 4/4] pvecm: stop merging SSH known hosts by default

[Esi Y]

On Thu, Jan 11, 2024 at 11:51:18AM +0100, Fabian Grünbichler wrote:
> and allow explicitly unmerging to remove the symlink altogether.

Apologies if I am second guessing here, but this is meant to be explicitly later "unmerging" on pveproxy start of new version of PVE? If so, this is risky if people used the shared known_hosts for own keys to reach e.g. backup servers, etc. It would be much safer to leave it as-is (or provide the facility otherwise as opt-in).

> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
>  src/PVE/CLI/pvecm.pm     | 10 ++++++++--
>  src/PVE/Cluster/Setup.pm |  9 ++++++---
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/PVE/CLI/pvecm.pm b/src/PVE/CLI/pvecm.pm
> index 0005e4b..0e8ca8f 100755
> --- a/src/PVE/CLI/pvecm.pm
> +++ b/src/PVE/CLI/pvecm.pm
> @@ -567,12 +567,18 @@ __PACKAGE__->register_method ({
>  		type => 'boolean',
>  		optional => 1,
>  	    },
> +	    'unmerge-known-hosts' => {
> +		description => "Unmerge legacy SSH known hosts.",
> +		type => 'boolean',
> +		optional => 1,
> +		default => 0,
> +	    },
>  	},
>      },
>      returns => { type => 'null' },
>      code => sub {
>  	my ($param) = @_;
> -	my ($force_new_cert, $silent) = $param->@{qw(force silent)};
> +	my ($force_new_cert, $silent, $unmerge) = $param->@{qw(force silent unmerge-known-hosts)};
>  
>  	# pveproxy's ExecStartPre calls this, and as we do IO (on /etc/pve) that can hang
>  	# (uninterruptible D state) we could fail the whole service, rendering the API guaranteed
> @@ -585,7 +591,7 @@ __PACKAGE__->register_method ({
>  		usleep(100 * 1000);
>  	    }
>  
> -	    PVE::Cluster::Setup::updatecerts_and_ssh($force_new_cert, $silent);
> +	    PVE::Cluster::Setup::updatecerts_and_ssh($force_new_cert, $silent, $unmerge);
>  	    PVE::Cluster::prepare_observed_file_basedirs();
>  	});
>  	if ($got_timeout) {
> diff --git a/src/PVE/Cluster/Setup.pm b/src/PVE/Cluster/Setup.pm
> index 4b6f013..42dff85 100644
> --- a/src/PVE/Cluster/Setup.pm
> +++ b/src/PVE/Cluster/Setup.pm
> @@ -816,7 +816,7 @@ sub generate_local_files {
>  }
>  
>  sub updatecerts_and_ssh {
> -    my ($force_new_cert, $silent) = @_;
> +    my ($force_new_cert, $silent, $unmerge_ssh) = @_;
>  
>      my $p = sub { print "$_[0]\n" if !$silent };
>  
> @@ -834,9 +834,12 @@ sub updatecerts_and_ssh {
>      $p->("generate new node certificate") if $force_new_cert;
>      gen_pve_node_files($nodename, $local_ip_address, $force_new_cert);
>  
> -    $p->("merge authorized SSH keys and known hosts");
> +    $p->("merge authorized SSH keys");
>      ssh_merge_keys();
> -    ssh_merge_known_hosts($nodename, $local_ip_address, 1);
> +    if ($unmerge_ssh) {
> +	$p->("unmerge SSH known hosts");
> +	ssh_unmerge_known_hosts();
> +    }
>      ssh_create_node_known_hosts($nodename);
>      gen_pve_vzdump_files();
>  }
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel