* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox