[pmg-devel] [PATCH pmg-api 0/6] add mechanism to update certificate fingerprints in cluster

[pmg-devel] [PATCH pmg-api 0/6] add mechanism to update certificate fingerprints in cluster

[Stoiko Ivanov]

Currently PMG's cluster synchornization relies mostly on rsync+ssh, but
does fetch some information via API call.
Whenever one of the nodes in a cluster changes its api-certificate the
cluster-synchronization breaks (see [0]).

This series addresses the issue by adding an api-call (proxied to master),
which connects to all nodes defined in the cluster via `ssh` and fetches
the current api-certificate fingerprint (by running `openssl x509`) and
updating the cluster.conf.
All nodes in the cluster sync the config (via rsync) at the beginning of
each synchronization and thus will eventually get the updated fingerprint,
before trying to connect to another node via API (with pinned certificate
fingerprint)

the last patch is the addition of that mechanism to the new PMG certificate
managment series by Wolfgang.

[0]
https://forum.proxmox.com/threads/how-to-lets-encrypt-and-pmg.41493/post-207669

Stoiko Ivanov (6):
  cluster: refactor rsync_command
  cluster: add helper to get remote cert fingerprint
  api: cluster: add update-fingerprints call
  cluster: add trigger_update_fingerprints
  pmgcm: add trigger-update-fingerprint
  api: certificates: trigger fingerprint update

 src/PMG/API2/Certificates.pm |  6 ++++
 src/PMG/API2/Cluster.pm      | 40 +++++++++++++++++++++++
 src/PMG/CLI/pmgcm.pm         | 21 +++++++++++++
 src/PMG/Cluster.pm           | 61 ++++++++++++++++++++++++++++++++++--
 4 files changed, 125 insertions(+), 3 deletions(-)

-- 
2.20.1

[pmg-devel] [PATCH pmg-api 1/6] cluster: refactor rsync_command

[Stoiko Ivanov]

pull out the ssh part for later reusal

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Cluster.pm | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index 6bb940a..daaa439 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -280,13 +280,21 @@ my $cond_commit_synced_file = sub {
     return 1;
 };
 
+my $ssh_command = sub {
+    my ($host_key_alias, @args) = @_;
+
+    my $cmd = ['ssh', '-l', 'root', '-o', 'BatchMode=yes'];
+    push @$cmd, '-o', "HostKeyAlias=${host_key_alias}" if $host_key_alias;
+    push @$cmd, @args if @args;
+    return $cmd;
+};
+
 my $rsync_command = sub {
     my ($host_key_alias, @args) = @_;
 
-    my $ssh_cmd = '--rsh=ssh -l root -o BatchMode=yes';
-    $ssh_cmd .=  " -o HostKeyAlias=${host_key_alias}" if $host_key_alias;
+    my $ssh_cmd = join(' ', @{$ssh_command->($host_key_alias)});
 
-    my $cmd = ['rsync', $ssh_cmd,  '-q', @args];
+    my $cmd = ['rsync', "--rsh=$ssh_cmd",  '-q', @args];
 
     return $cmd;
 };
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 2/6] cluster: add helper to get remote cert fingerprint

[Stoiko Ivanov]

via ssh executing 'openssl x509'

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Cluster.pm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index daaa439..49ba7d9 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -289,6 +289,27 @@ my $ssh_command = sub {
     return $cmd;
 };
 
+sub get_remote_cert_fingerprint {
+    my ($ni) = @_;
+
+    my $ssh_cmd = $ssh_command->(
+	$ni->{name}, $ni->{ip},
+	'openssl x509 -noout -fingerprint -sha256 -in /etc/pmg/pmg-api.pem');
+    my $fp;
+    eval {
+	PVE::Tools::run_command($ssh_cmd, outfunc => sub {
+	    my ($line) = @_;
+	    if ($line =~ m/SHA256 Fingerprint=((?:[A-Fa-f0-9]{2}:){31}[A-Fa-f0-9]{2})/) {
+		$fp = $1;
+	    }
+	});
+	die "parsing failed\n" if !$fp;
+    };
+    die "unable to get remote node fingerprint from '$ni->{name}': $@\n" if $@;
+
+    return $fp;
+}
+
 my $rsync_command = sub {
     my ($host_key_alias, @args) = @_;
 
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 3/6] api: cluster: add update-fingerprints call

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/Cluster.pm | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/src/PMG/API2/Cluster.pm b/src/PMG/API2/Cluster.pm
index 7eab761..07aa8fa 100644
--- a/src/PMG/API2/Cluster.pm
+++ b/src/PMG/API2/Cluster.pm
@@ -111,6 +111,7 @@ __PACKAGE__->register_method({
 	    { name => 'status' },
 	    { name => 'create' },
 	    { name => 'join' },
+	    { name => 'update-fingerprints' },
         ];
 
 	return $result;
@@ -451,5 +452,44 @@ __PACKAGE__->register_method({
 	return PMG::ClusterConfig::lock_config($code, "cluster join failed");
     }});
 
+__PACKAGE__->register_method({
+    name => 'update_fingerprints',
+    path => 'update-fingerprints',
+    method => 'POST',
+    description => "Update API certificate fingerprints (by fetching it via ssh).",
+    proxyto => 'master',
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $code = sub {
+	    my $cinfo = PMG::ClusterConfig->new();
+
+	    die "no cluster defined\n" if !scalar(keys %{$cinfo->{ids}});
+
+	    my $localcid = $cinfo->{local}->{cid};
+
+	    foreach my $cid (keys %{$cinfo->{ids}}) {
+	        my $d = $cinfo->{ids}->{$cid};
+		my $fp;
+		if ($d->{cid} == $localcid) {
+		    $fp = PMG::Cluster::read_local_ssl_cert_fingerprint();
+		} else {
+		    $fp = PMG::Cluster::get_remote_cert_fingerprint($d);
+		}
+		$cinfo->{ids}->{$d->{cid}}->{fingerprint} = $fp;
+	    }
+
+	    $cinfo->write();
+
+	    return;
+	};
+
+	PMG::ClusterConfig::lock_config($code, "update fingerprints failed");
+    }});
 
 1;
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 4/6] cluster: add trigger_update_fingerprints

[Stoiko Ivanov]

this commit adds a method that sends a POST request to
'/config/cluster/update-fingerprints' on the master node in a cluster.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/Cluster.pm | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/PMG/Cluster.pm b/src/PMG/Cluster.pm
index 49ba7d9..e9217fe 100644
--- a/src/PMG/Cluster.pm
+++ b/src/PMG/Cluster.pm
@@ -310,6 +310,32 @@ sub get_remote_cert_fingerprint {
     return $fp;
 }
 
+sub trigger_update_fingerprints {
+    my ($cinfo) = @_;
+
+    my $master = $cinfo->{master} || die "unable to lookup master node\n";
+    my $master_fp = $master->{fingerprint};
+
+    # if running on master the current fingerprint for the API-connection is needed
+    if ($cinfo->{local}->{type} eq 'master') {
+	$master_fp = PMG::Cluster::read_local_ssl_cert_fingerprint();
+    }
+
+    my $ticket = PMG::Ticket::assemble_ticket('root@pam');
+    my $csrftoken = PMG::Ticket::assemble_csrf_prevention_token('root@pam');
+    my $conn = PVE::APIClient::LWP->new(
+	ticket => $ticket,
+	csrftoken => $csrftoken,
+	cookie_name => 'PMGAuthCookie',
+	host => $master->{ip},
+	cached_fingerprints => {
+	    $master_fp => 1,
+	});
+
+    $conn->post("/config/cluster/update-fingerprints", {});
+    return undef;
+}
+
 my $rsync_command = sub {
     my ($host_key_alias, @args) = @_;
 
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 5/6] pmgcm: add trigger-update-fingerprint

[Stoiko Ivanov]

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/CLI/pmgcm.pm | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/PMG/CLI/pmgcm.pm b/src/PMG/CLI/pmgcm.pm
index 59bdb06..e37398c 100644
--- a/src/PMG/CLI/pmgcm.pm
+++ b/src/PMG/CLI/pmgcm.pm
@@ -306,6 +306,26 @@ __PACKAGE__->register_method({
 	return undef;
     }});
 
+__PACKAGE__->register_method({
+    name => 'trigger_update_fp',
+    path => 'trigger-update-fingerprint',
+    method => 'POST',
+    description => "Notify master to refresh all certificate fingerprints",
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $cinfo = PMG::ClusterConfig->new();
+
+	die "no cluster defined\n" if !scalar(keys %{$cinfo->{ids}});
+
+	PMG::Cluster::trigger_update_fingerprints($cinfo);
+    }});
+
 our $cmddef = {
     status => [ 'PMG::API2::Cluster', 'status', [], {}, $format_nodelist],
     create => [ 'PMG::API2::Cluster', 'create', [], {}, $upid_exit],
@@ -314,6 +334,7 @@ our $cmddef = {
     join_cmd => [ __PACKAGE__, 'join_cmd', []],
     sync => [ __PACKAGE__, 'sync', []],
     promote => [ __PACKAGE__, 'promote', []],
+    'trigger-update-fingerprint' => [ __PACKAGE__, 'trigger_update_fp'],
 };
 
 1;
-- 
2.20.1

[pmg-devel] [PATCH pmg-api 6/6] api: certificates: trigger fingerprint update

[Stoiko Ivanov]

in clustered systems, so that the cluster-sync remains possible with
the new certificate.

Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
---
 src/PMG/API2/Certificates.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PMG/API2/Certificates.pm b/src/PMG/API2/Certificates.pm
index ca8b75b..5a0f34d 100644
--- a/src/PMG/API2/Certificates.pm
+++ b/src/PMG/API2/Certificates.pm
@@ -43,6 +43,12 @@ my sub restart_after_cert_update : prototype($) {
     if ($type eq 'api') {
 	print "Restarting pmgproxy\n";
 	PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pmgproxy']);
+
+	my $cinfo = PMG::ClusterConfig->new();
+	if (scalar(keys %{$cinfo->{ids}})) {
+	    print "Notify cluster about new fingerprint\n";
+	    PMG::Cluster::trigger_update_fingerprints($cinfo);
+	}
     }
 };
 
-- 
2.20.1

[pmg-devel] applied-series: [PATCH pmg-api 0/6] add mechanism to update certificate fingerprints in cluster

[Thomas Lamprecht]

On 15.03.21 23:01, Stoiko Ivanov wrote:
> Currently PMG's cluster synchornization relies mostly on rsync+ssh, but
> does fetch some information via API call.
> Whenever one of the nodes in a cluster changes its api-certificate the
> cluster-synchronization breaks (see [0]).
> 
> This series addresses the issue by adding an api-call (proxied to master),
> which connects to all nodes defined in the cluster via `ssh` and fetches
> the current api-certificate fingerprint (by running `openssl x509`) and
> updating the cluster.conf.
> All nodes in the cluster sync the config (via rsync) at the beginning of
> each synchronization and thus will eventually get the updated fingerprint,
> before trying to connect to another node via API (with pinned certificate
> fingerprint)
> 
> the last patch is the addition of that mechanism to the new PMG certificate
> managment series by Wolfgang.
> 
> [0]
> https://forum.proxmox.com/threads/how-to-lets-encrypt-and-pmg.41493/post-207669
> 
> Stoiko Ivanov (6):
>   cluster: refactor rsync_command
>   cluster: add helper to get remote cert fingerprint
>   api: cluster: add update-fingerprints call
>   cluster: add trigger_update_fingerprints
>   pmgcm: add trigger-update-fingerprint
>   api: certificates: trigger fingerprint update
> 
>  src/PMG/API2/Certificates.pm |  6 ++++
>  src/PMG/API2/Cluster.pm      | 40 +++++++++++++++++++++++
>  src/PMG/CLI/pmgcm.pm         | 21 +++++++++++++
>  src/PMG/Cluster.pm           | 61 ++++++++++++++++++++++++++++++++++--
>  4 files changed, 125 insertions(+), 3 deletions(-)
> 



applied series, much thanks!

FYI: I did some small (whitespace/indenation) and some medium followups:
* dropped the "trigger-" from the pmgcm "update-fingerprints" comand, two
  verbs are just sounding a little weird
* do not make it an error to call that update method if there's no cluster,
  just note that nothing will be done
* in the api call I used $cid instead of $d->{cid}, which is the same FWICT
  from checking cluster config parser and basic sanity expectations I have
  still left; That avoids nested hash access and allows for shorter code.