[pve-devel] [PATCH manager 1/2] fix #4552: certhelpers: check if custom cert and key match on change

Max Carrara m.carrara at proxmox.com
Tue Feb 28 17:36:33 CET 2023


It is now checked whether the new custom SSL certificate actually
matches the provided or existing custom key.

Also, the new custom certificate and key pair is now validated
*before* it is used or replaced with an existing pair. Safety copies
are still made before existing files are replaced.

Signed-off-by: Max Carrara <m.carrara at proxmox.com>
---
 PVE/CertHelpers.pm | 65 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 8 deletions(-)

diff --git a/PVE/CertHelpers.pm b/PVE/CertHelpers.pm
index 7e088cb9..83be9909 100644
--- a/PVE/CertHelpers.pm
+++ b/PVE/CertHelpers.pm
@@ -53,12 +53,15 @@ sub cert_lock {
 sub set_cert_files {
     my ($cert, $key, $path_prefix, $force) = @_;
 
-    my ($old_cert, $old_key, $info);
+    my $info;
 
     my $cert_path = "${path_prefix}.pem";
     my $cert_path_tmp = "${path_prefix}.pem.old";
+    my $cert_path_new = "${path_prefix}.pem.new";
+
     my $key_path = "${path_prefix}.key";
     my $key_path_tmp = "${path_prefix}.key.old";
+    my $key_path_new = "${path_prefix}.key.new";
 
     die "Custom certificate file exists but force flag is not set.\n"
 	if !$force && -e $cert_path;
@@ -69,26 +72,72 @@ sub set_cert_files {
     PVE::Tools::file_copy($key_path, $key_path_tmp) if -e $key_path;
 
     eval {
-	PVE::Tools::file_set_contents($cert_path, $cert);
-	PVE::Tools::file_set_contents($key_path, $key) if $key;
+	PVE::Tools::file_set_contents($cert_path_new, $cert);
+
+	if ($key) {
+	    PVE::Tools::file_set_contents($key_path_new, $key);
+	} elsif (-e $key_path) {
+	    PVE::Tools::file_copy($key_path, $key_path_new);
+	} else {
+	    die "Cannot set custom certificate without key.\n";
+	}
+
+	die "Custom certificate does not match provided key.\n"
+	    if !PVE::Certificate::certificate_matches_key($cert_path_new, $key_path_new);
+
+	PVE::Tools::file_copy($cert_path_new, $cert_path);
+	PVE::Tools::file_copy($key_path_new, $key_path);
+
 	$info = PVE::Certificate::get_certificate_info($cert_path);
     };
     my $err = $@;
 
     if ($err) {
-	if (-e $cert_path_tmp && -e $key_path_tmp) {
+	if (-e $cert_path_new) {
+	    eval {
+		warn "Removing uploaded certificate...\n";
+		unlink $cert_path_new;
+	    };
+	    warn "$@\n" if $@;
+	}
+	if (-e $cert_path_tmp) {
 	    eval {
-		warn "Attempting to restore old certificate files..\n";
+		warn "Restoring previous certificate...\n";
 		PVE::Tools::file_copy($cert_path_tmp, $cert_path);
+		unlink $cert_path_tmp;
+	    };
+	    warn "$@\n" if $@;
+	}
+	if (-e $key_path_new) {
+	    eval {
+		if ($key) {
+		    warn "Removing uploaded key...\n";
+		} else {
+		    warn "Removing temporary copy of key...\n";
+		}
+		unlink $key_path_new;
+	    };
+	    warn "$@\n" if $@;
+	}
+	if (-e $key_path_tmp) {
+	    eval {
+		warn "Restoring previous key...\n";
 		PVE::Tools::file_copy($key_path_tmp, $key_path);
+		unlink $key_path_tmp;
 	    };
 	    warn "$@\n" if $@;
 	}
-	die "Setting certificate files failed - $err\n"
+	die "Setting certificate files failed - $err\n";
     }
 
-    unlink $cert_path_tmp;
-    unlink $key_path_tmp;
+    for my $path (
+	$cert_path_tmp,
+	$cert_path_new,
+	$key_path_tmp,
+	$key_path_new
+    ) {
+	unlink $path if -e $path;
+    }
 
     return $info;
 }
-- 
2.30.2






More information about the pve-devel mailing list