From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 5C92B1FF129 for ; Thu, 06 Aug 2026 17:39:13 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A65F621501; Thu, 06 Aug 2026 17:39:12 +0200 (CEST) Date: Thu, 06 Aug 2026 17:39:03 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= Subject: Re: [PATCH manager v2 03/16] bin/api: add a new staged certificate when renewing self-signed cert To: pdm-devel@lists.proxmox.com, Shannon Sterz References: <20260805131838.254723-2-s.sterz@proxmox.com> <20260805131838.254723-5-s.sterz@proxmox.com> In-Reply-To: <20260805131838.254723-5-s.sterz@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1786028636.awgbclb3om.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1786030731859 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.093 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: FK2HZ34BMH3OGW4ZTOXRDUQHY5GJX67T X-Message-ID-Hash: FK2HZ34BMH3OGW4ZTOXRDUQHY5GJX67T X-MailFrom: f.gruenbichler@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On August 5, 2026 3:18 pm, Shannon Sterz wrote: > when a certificate is about to expire (within the next four weeks, up > to two weeks before it expires), create a new staged "next" > certificate. by including this certificate when querying the api, > clients can prepare for when the certificate is about to be rotated. > this allows them to update to the new fingerprint, without needing an > additional out-of-band communication channel. >=20 > Signed-off-by: Shannon Sterz > --- > PVE/API2/Certificates.pm | 5 ++- > PVE/CertHelpers.pm | 6 ++++ > bin/pveupdate | 72 +++++++++++++++++++++++++++++++++------- > 3 files changed, 70 insertions(+), 13 deletions(-) >=20 > diff --git a/PVE/API2/Certificates.pm b/PVE/API2/Certificates.pm > index de8762c53..bcd650c3b 100644 > --- a/PVE/API2/Certificates.pm > +++ b/PVE/API2/Certificates.pm > @@ -70,7 +70,10 @@ __PACKAGE__->register_method({ > =20 > my $res =3D []; > my $cert_paths =3D [ > - '/etc/pve/pve-root-ca.pem', "$node_path/pve-ssl.pem", "$node= _path/pveproxy-ssl.pem", > + '/etc/pve/pve-root-ca.pem', > + "$node_path/pve-ssl.pem", > + "$node_path/pveproxy-ssl.pem", > + "$node_path/pve-ssl-next.pem", should we add some flags to the CertificateInfo? e.g., the product code here knows which certificate has which purpose: - CA - signed-by-ca - custom/acme - currently-used - staged > ]; > for my $path (@$cert_paths) { > eval { > diff --git a/PVE/CertHelpers.pm b/PVE/CertHelpers.pm > index 202dec0ee..4272eaf57 100644 > --- a/PVE/CertHelpers.pm > +++ b/PVE/CertHelpers.pm > @@ -55,6 +55,12 @@ sub default_cert_path_prefix { > return "/etc/pve/nodes/${node}/pve-ssl"; > } > =20 > +sub default_next_cert_path_prefix { > + my ($node) =3D @_; > + > + return "/etc/pve/nodes/${node}/pve-ssl-next"; > +} high-level question - do we want to rotate the key as well? what about CA rotation? (#5215) > + > sub cert_lock { > my ($timeout, $code, @param) =3D @_; > =20 > diff --git a/bin/pveupdate b/bin/pveupdate > index b1960c353..e44c614ee 100755 > --- a/bin/pveupdate > +++ b/bin/pveupdate > @@ -103,10 +103,12 @@ syslog('err', "Renewing ACME certificate failed: $@= ") if $@; > =20 > eval { > my $certpath =3D PVE::CertHelpers::default_cert_path_prefix($nodenam= e) . ".pem"; > + my $next_certpath =3D PVE::CertHelpers::default_next_cert_path_prefi= x($nodename) . ".pem"; > my $capath =3D "/etc/pve/pve-root-ca.pem"; > + my $now =3D time(); > =20 > my $renew =3D sub { > - my ($msg) =3D @_; > + my ($msg, $use_later) =3D @_; > =20 > # get CA info > my $cainfo =3D PVE::Certificate::get_certificate_info($capath); > @@ -127,21 +129,67 @@ eval { > print "PVE certificate $msg\n"; > # create new certificate > my $ip =3D PVE::Cluster::remote_node_ip($nodename); > - PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip); > =20 > - print "Restarting pveproxy after renewing certificate\n"; > - PVE::Tools::run_command(['systemctl', 'reload-or-restart', 'pvep= roxy']); > + if ($use_later) { > + PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip, $ne= xt_certpath); > + } else { > + PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip); > + print "Restarting pveproxy after renewing certificate\n"; > + PVE::Tools::run_command(['systemctl', 'reload-or-restart', '= pveproxy']); > + } > }; > =20 > - if (PVE::Certificate::check_expiry($certpath)) { > - # already expired > - $renew->("expired, renewing..."); > - } elsif (PVE::Certificate::check_expiry($certpath, time() + 14 * 24 = * 60 * 60)) { > - # expires in next 2 weeks > - $renew->("expires soon, renewing..."); > - } elsif (!PVE::Certificate::check_expiry($certpath, time() + 2 * 365= * 24 * 60 * 60)) { > + my $rotate_in_next_certificate_or_renew =3D sub { > + my ($msg) =3D @_; > + > + # if a staged certificate: > + # > + # 1. exists > + # 2. does not expire within two weeks > + # 3. does expire within two years > + # 4. and is signed by the currently active ca > + # > + # rotate it in, otherwise create a new certificate > + if ( > + -f $next_certpath > + && !PVE::Certificate::check_expiry($next_certpath, $now + 14= * 24 * 60 * 60) > + && PVE::Certificate::check_expiry($next_certpath, $now + 2 *= 365 * 24 * 60 * 60) these constants are also used below, they should be in a single place.. the comments from the other (rotation) series apply here as well I guess. > + && PVE::Certificate::check_certificate_signed_by_ca($next_ce= rtpath, $capath) > + ) { > + rename($next_certpath, $certpath); > + print("Restarting pveproxy after rotating certificate\n"); > + PVE::Tools::run_command(['systemctl', 'reload-or-restart', '= pveproxy']); these two lines could maybe go into their own helper? > + } else { > + $renew->($msg); > + # clean up the now useless staged certificate, if there is o= ne > + if (-f $next_certpath) { > + unlink $next_certpath > + or $!{ENOENT} > + or warn "failed to clean-up $next_certpath - $!\n"; this should be a fat warning in any case - it should never happen, and if it does, we possibly just prevented all existing clients pinning the previous and the staged fingerprints from connecting.. > + } > + } > + > + }; > + > + if (PVE::Certificate::check_expiry($certpath, $now + 14 * 24 * 60 * = 60)) { > + $rotate_in_next_certificate_or_renew->("expired or expires soon,= renewing..."); I know this was there before, but it's kind of weird to call this "renewing".. we should probably let the called helper decide the message here, since rotating in a staged certificate vs. skipping that and putting in a newly generated one is pretty different? > + } elsif (PVE::Certificate::check_expiry($certpath, $now + 28 * 24 * = 60 * 60)) { > + # if the certificate expires in 4 weeks, stage a new one if no > + # certificate has been staged yet or the staged one is invalid b= y now > + if ( > + !-f $next_certpath > + || PVE::Certificate::check_expiry($next_certpath, $now + 28 = * 24 * 60 * 60) > + || !PVE::Certificate::check_expiry($next_certpath, $now + 2 = * 365 * 24 * 60 * 60) > + || !PVE::Certificate::check_certificate_signed_by_ca($next_c= ertpath, $capath) > + ) { > + unlink $next_certpath > + or $!{ENOENT} > + or warn "failed to clean-up $next_certpath - $!\n"; > + $renew->("staging new certificate...", 1); this messages is incomplete ("expires soon, staging new ceriticate..." ?) > + } > + } elsif (!PVE::Certificate::check_expiry($certpath, $now + 2 * 365 *= 24 * 60 * 60)) { > # expires in more than 2 years > - $renew->( > + $rotate_in_next_certificate_or_renew->( > "expires in more than 2 years, renewing to reduce certificat= e life-span for client compatibility..." > ); this should also stage a new one and handle rotating it in two weeks later? > } > --=20 > 2.47.3 >=20 >=20 >=20 >=20 >=20 >=20