Re: [PATCH manager] certificate: make sure that any new certificate and key match

["Shannon Sterz" <s.sterz@proxmox.com>]

On Wed May 6, 2026 at 1:54 PM CEST, Thomas Lamprecht wrote:
> Am 06.05.26 um 13:03 schrieb Shannon Sterz:
>> previously it was possible to upload and set a key and certificate
>> combination, that did not match each other. this lead to confusing
>> errors as pveproxy would seemingly start, but not actually serve any
>> http connections. in a cluster context this leads to "broken pipe"
>> errors when connecting to such a misconfigured node. since this is
>> rather confusing, verify that a key and certificate can actually be
>> used before setting them as the current certificates by loading them
>> into a TLS context.
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>
>> Notes:
>>     this came up in the enterprise support and caused quite a bit of
>>     confusion.
>
> nice UX polishing.
>
>>
>>  PVE/CertHelpers.pm | 27 +++++++++++++++++++++++++++
>>  1 file changed, 27 insertions(+)
>>
>> diff --git a/PVE/CertHelpers.pm b/PVE/CertHelpers.pm
>> index ee945827f..2fc241c24 100644
>> --- a/PVE/CertHelpers.pm
>> +++ b/PVE/CertHelpers.pm
>> @@ -7,6 +7,8 @@ use PVE::Certificate;
>>  use PVE::JSONSchema;
>>  use PVE::Tools;
>>
>> +use Net::SSLeay;
>> +
>>  my $account_prefix = '/etc/pve/priv/acme';
>>
>>  PVE::JSONSchema::register_standard_option(
>> @@ -81,6 +83,31 @@ sub set_cert_files {
>>          PVE::Tools::file_set_contents($cert_path, $cert);
>>          PVE::Tools::file_set_contents($key_path, $key) if $key;
>
>
>
>>          $info = PVE::Certificate::get_certificate_info($cert_path);
>> +
>> +        if (my $method = Net::SSLeay::TLS_method()) {
>> +            my $ctx = Net::SSLeay::CTX_new_with_method($method);
>> +
>> +            eval {
>> +                Net::SSLeay::CTX_use_certificate_chain_file($ctx, $cert_path)
>> +                    or die "could not load certificate (chain)\n";
>> +                Net::SSLeay::CTX_use_PrivateKey_file(
>> +                    $ctx,
>> +                    $key_path,
>> +                    Net::SSLeay::FILETYPE_PEM(),
>> +                ) or die "key does not match the certificate (chain)\n";
>
> Should we include the error stack [0] here too in these calls? Might
> make debugging even easier, and as the user provided these certs, (or ACME
> generated them), we cannot really leak anything with doing that I think.
>
> [0]: https://metacpan.org/pod/Net::SSLeay#Error-handling-functions

in my testing neither `die_now` nor `die_if_ssl_error` added any context
for the mismatched key scenario. in my general experience, the openssl
error stack rarely returns error messages that are very useful to
end-users. however, it may come in handy for scenarios that i'm
currently not considering and we don't really lose anything by using
`die_now`. so i'll send a v2 with that.

>
>> +            };
>> +
>> +            my $err = $@;
>> +
>> +            if ($err) {
>> +                # clean up invalid certificate and key
>> +                unlink $cert_path;
>
>
> error handling would be nice here:
>
> unlink $cert_path or $!{ENOENT} or warn "failed to clean-up $cert_path - $!\n";
>
>> +                unlink $key_path;
>
> same here> +                die $err;

done.

>> +            }
>> +        } else {
>> +            warn "no TLS method to verify certificate and key match, continuing anyway\n";
>> +        }
>>      };
>>      my $err = $@;
>>