all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH manager] certificate: make sure that any new certificate and key match
@ 2026-05-06 11:04 Shannon Sterz
  2026-05-06 11:28 ` Daniel Herzig
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Shannon Sterz @ 2026-05-06 11:04 UTC (permalink / raw)
  To: pve-devel

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.

 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";
+            };
+
+            my $err = $@;
+
+            if ($err) {
+                # clean up invalid certificate and key
+                unlink $cert_path;
+                unlink $key_path;
+                die $err;
+            }
+        } else {
+            warn "no TLS method to verify certificate and key match, continuing anyway\n";
+        }
     };
     my $err = $@;
 
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH manager] certificate: make sure that any new certificate and key match
  2026-05-06 11:04 [PATCH manager] certificate: make sure that any new certificate and key match Shannon Sterz
@ 2026-05-06 11:28 ` Daniel Herzig
  2026-05-06 11:54 ` Thomas Lamprecht
  2026-05-06 12:50 ` Superseded: " Shannon Sterz
  2 siblings, 0 replies; 5+ messages in thread
From: Daniel Herzig @ 2026-05-06 11:28 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pve-devel

Thanks for this!

I just built, installed and tested the package.

Works perfectly fine, as described in the commit message.

Tested-by: Daniel Herzig <d.herzig@proxmox.com>

Shannon Sterz <s.sterz@proxmox.com> writes:

> 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.
>
>  PVE/CertHelpers.pm | 27 +++++++++++++++++++++++++++




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH manager] certificate: make sure that any new certificate and key match
  2026-05-06 11:04 [PATCH manager] certificate: make sure that any new certificate and key match Shannon Sterz
  2026-05-06 11:28 ` Daniel Herzig
@ 2026-05-06 11:54 ` Thomas Lamprecht
  2026-05-06 12:39   ` Shannon Sterz
  2026-05-06 12:50 ` Superseded: " Shannon Sterz
  2 siblings, 1 reply; 5+ messages in thread
From: Thomas Lamprecht @ 2026-05-06 11:54 UTC (permalink / raw)
  To: Shannon Sterz, pve-devel

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

> +            };
> +
> +            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;
> +            }
> +        } else {
> +            warn "no TLS method to verify certificate and key match, continuing anyway\n";
> +        }
>      };
>      my $err = $@;
>  





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH manager] certificate: make sure that any new certificate and key match
  2026-05-06 11:54 ` Thomas Lamprecht
@ 2026-05-06 12:39   ` Shannon Sterz
  0 siblings, 0 replies; 5+ messages in thread
From: Shannon Sterz @ 2026-05-06 12:39 UTC (permalink / raw)
  To: Thomas Lamprecht, pve-devel

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 = $@;
>>





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Superseded: Re: [PATCH manager] certificate: make sure that any new certificate and key match
  2026-05-06 11:04 [PATCH manager] certificate: make sure that any new certificate and key match Shannon Sterz
  2026-05-06 11:28 ` Daniel Herzig
  2026-05-06 11:54 ` Thomas Lamprecht
@ 2026-05-06 12:50 ` Shannon Sterz
  2 siblings, 0 replies; 5+ messages in thread
From: Shannon Sterz @ 2026-05-06 12:50 UTC (permalink / raw)
  To: Shannon Sterz, pve-devel

Superseded: https://lore.proxmox.com/pve-devel/20260506124832.246682-1-s.sterz@proxmox.com/T/#u

On Wed May 6, 2026 at 1:04 PM CEST, Shannon Sterz wrote:
> 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.
>
>  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";
> +            };
> +
> +            my $err = $@;
> +
> +            if ($err) {
> +                # clean up invalid certificate and key
> +                unlink $cert_path;
> +                unlink $key_path;
> +                die $err;
> +            }
> +        } else {
> +            warn "no TLS method to verify certificate and key match, continuing anyway\n";
> +        }
>      };
>      my $err = $@;
>





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-05-06 12:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 11:04 [PATCH manager] certificate: make sure that any new certificate and key match Shannon Sterz
2026-05-06 11:28 ` Daniel Herzig
2026-05-06 11:54 ` Thomas Lamprecht
2026-05-06 12:39   ` Shannon Sterz
2026-05-06 12:50 ` Superseded: " Shannon Sterz

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