all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pdm-devel@lists.proxmox.com, Shannon Sterz <s.sterz@proxmox.com>
Subject: Re: [PATCH manager v2 03/16] bin/api: add a new staged certificate when renewing self-signed cert
Date: Thu, 06 Aug 2026 17:39:03 +0200	[thread overview]
Message-ID: <1786028636.awgbclb3om.astroid@yuna.none> (raw)
In-Reply-To: <20260805131838.254723-5-s.sterz@proxmox.com>

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.
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  PVE/API2/Certificates.pm |  5 ++-
>  PVE/CertHelpers.pm       |  6 ++++
>  bin/pveupdate            | 72 +++++++++++++++++++++++++++++++++-------
>  3 files changed, 70 insertions(+), 13 deletions(-)
> 
> 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({
>  
>          my $res = [];
>          my $cert_paths = [
> -            '/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";
>  }
>  
> +sub default_next_cert_path_prefix {
> +    my ($node) = @_;
> +
> +    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) = @_;
>  
> 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 $@;
>  
>  eval {
>      my $certpath = PVE::CertHelpers::default_cert_path_prefix($nodename) . ".pem";
> +    my $next_certpath = PVE::CertHelpers::default_next_cert_path_prefix($nodename) . ".pem";
>      my $capath = "/etc/pve/pve-root-ca.pem";
> +    my $now = time();
>  
>      my $renew = sub {
> -        my ($msg) = @_;
> +        my ($msg, $use_later) = @_;
>  
>          # get CA info
>          my $cainfo = PVE::Certificate::get_certificate_info($capath);
> @@ -127,21 +129,67 @@ eval {
>          print "PVE certificate $msg\n";
>          # create new certificate
>          my $ip = PVE::Cluster::remote_node_ip($nodename);
> -        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']);
> +        if ($use_later) {
> +            PVE::Cluster::Setup::gen_pve_ssl_cert(1, $nodename, $ip, $next_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']);
> +        }
>      };
>  
> -    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 = sub {
> +        my ($msg) = @_;
> +
> +        # 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_certpath, $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 one
> +            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 by 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_certpath, $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 certificate life-span for client compatibility..."
>          );

this should also stage a new one and handle rotating it in two weeks
later?

>      }
> -- 
> 2.47.3
> 
> 
> 
> 
> 
> 




  reply	other threads:[~2026-08-06 15:39 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 13:18 [PATCH cluster/common/datacenter-manager/manager/proxmox v2 00/16] TLS Certificate Staging Shannon Sterz
2026-08-05 13:18 ` [PATCH cluster v2 01/16] setup: allow caller to provide the certificate filename Shannon Sterz
2026-08-05 13:18 ` [PATCH pve-common v2 02/16] certificate: add helper to verify that a certificate was signed by a ca Shannon Sterz
2026-08-06 15:02   ` Fabian Grünbichler
2026-08-05 13:18 ` [PATCH manager v2 03/16] bin/api: add a new staged certificate when renewing self-signed cert Shannon Sterz
2026-08-06 15:39   ` Fabian Grünbichler [this message]
2026-08-05 13:18 ` [PATCH manager v2 04/16] api: certificates: if node parameter is 'localhost' return local certs Shannon Sterz
2026-08-05 13:18 ` [PATCH proxmox v2 05/16] pve-api-types: expose certificates info endpoint Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 06/16] client: allow users to update a changed fingerprint interactively Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 07/16] cli/api-types: move Fingerprint to common api type crate Shannon Sterz
2026-08-06 15:23   ` Fabian Grünbichler
2026-08-05 13:18 ` [PATCH datacenter-manager v2 08/16] server: connection: report mismatching fingerprint as untrusted on probe Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 09/16] ui: wizard: add context if a provided fingerprint did not match remote Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 10/16] ui: wizard: nodes page: always update fingerprints on user confirmation Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 11/16] pdm-api-types: implement ApiType for Fingerprint Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 12/16] pdm-api-types: add staged_fingerprints field to NodeUrl Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 13/16] server: remotes: lock remotes config when updating it Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 14/16] server: connection: rotate in staged fingerprints when encountering them Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 15/16] server: api: tasks: move `spawn_aborted_on_shutdown()` to super module Shannon Sterz
2026-08-05 13:18 ` [PATCH datacenter-manager v2 16/16] server: bin: api: tasks: add task to discover new staged certificates Shannon Sterz
2026-08-06 15:23 ` partially-applied [PATCH cluster/common/datacenter-manager/manager/proxmox v2 00/16] TLS Certificate Staging Fabian Grünbichler
2026-08-06 15:41 ` Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1786028636.awgbclb3om.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.sterz@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal