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 D58DE95239 for ; Tue, 28 Feb 2023 17:37:10 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B3CF195FD for ; Tue, 28 Feb 2023 17:36:40 +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, 28 Feb 2023 17:36:39 +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 908B848BB8 for ; Tue, 28 Feb 2023 17:36:39 +0100 (CET) From: Max Carrara To: pve-devel@lists.proxmox.com Date: Tue, 28 Feb 2023 17:36:33 +0100 Message-Id: <20230228163634.299137-4-m.carrara@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230228163634.299137-1-m.carrara@proxmox.com> References: <20230228163634.299137-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.012 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [certhelpers.pm] Subject: [pve-devel] [PATCH manager 1/2] fix #4552: certhelpers: check if custom cert and key match on change 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, 28 Feb 2023 16:37:10 -0000 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 --- 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