[pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains

2021-10-08 Thread Fabian Grünbichler
the current default chains end with an expired root certificate for
maximum compatibility with old Android versions. this breaks some other
older clients (openssl, gnutls) which don't expect chains to contain any
expired certificates, even if they are 'above' the trust anchor.

by setting $root, it is possible to specify which root the ACME provided
certificate chain should end with, downloading alternate chains as
necessary.

Signed-off-by: Fabian Grünbichler 
---

Notes:
v2: 
- only check issuer
- also check default chain
- add 'i' to RE check

only tested with pebble

 src/PVE/ACME.pm | 35 ++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
index 265482d..57578d7 100644
--- a/src/PVE/ACME.pm
+++ b/src/PVE/ACME.pm
@@ -442,17 +442,50 @@ sub deactivate_authorization {
 
 # Get certificate
 # GET-as-POST to order's certificate URL
+# if $root is specified, attempts to find a matching (alternate) chain
 # Expects a '200 OK' reply
 # returns certificate chain in PEM format
 sub get_certificate {
-my ($self, $order) = @_;
+my ($self, $order, $root) = @_;
 
 $self->fatal("no certificate URL available (yet?)", $order)
if !$order->{certificate};
 
+my $check_root = sub {
+   my ($chain) = @_;
+
+   my @certs = PVE::Certificate::split_pem($chain);
+   my $root_pem = $certs[-1];
+
+   my ($file, $fh) = PVE::Tools::tempfile_contents($root_pem);
+   my $info = PVE::Certificate::get_certificate_info($file);
+
+   return defined($info->{issuer}) && $info->{issuer} =~ m/\Q$root\E/i;
+};
+
 my $r = $self->do(POST => $order->{certificate}, '');
 my $return = eval {
+   # default chain
my $res = __get_result($r, 200, 1);
+   if ($root && !$check_root->($res)) {
+   # alternate chains if requested and default didn't match
+   $res = undef;
+   my @links = $r->header('link');
+   for my $link (@links) {
+   if ($link =~ /^<(.*)>;rel="alternate"$/) {
+   my $url = $1;
+   my $chain = eval { __get_result($self->do(POST => $url, 
''), 200, 1); };
+   die "failed to retrieve alternate chain from '$url' - $@\n" 
if $@;
+   if ($check_root->($chain)) {
+   $res = $chain;
+   last;
+   }
+   }
+   }
+   die "no matching alternate chain for '$root' returned by server\n"
+   if !defined($res);
+   }
+
if ($res =~ /^(-BEGIN CERTIFICATE-)(.+)(-END 
CERTIFICATE-)$/s) { # untaint
return $1 . $2 . $3;
}
-- 
2.30.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains

2021-10-08 Thread Stoiko Ivanov
Tested again against LE production endpoint - LGTM :)
Thanks!

Reviewed-By: Stoiko Ivanov 
Tested-By: Stoiko Ivanov 

On Fri,  8 Oct 2021 10:18:21 +0200
Fabian Grünbichler  wrote:

> the current default chains end with an expired root certificate for
> maximum compatibility with old Android versions. this breaks some other
> older clients (openssl, gnutls) which don't expect chains to contain any
> expired certificates, even if they are 'above' the trust anchor.
> 
> by setting $root, it is possible to specify which root the ACME provided
> certificate chain should end with, downloading alternate chains as
> necessary.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
> 
> Notes:
> v2: 
> - only check issuer
> - also check default chain
> - add 'i' to RE check
> 
> only tested with pebble
> 
>  src/PVE/ACME.pm | 35 ++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
> index 265482d..57578d7 100644
> --- a/src/PVE/ACME.pm
> +++ b/src/PVE/ACME.pm
> @@ -442,17 +442,50 @@ sub deactivate_authorization {
>  
>  # Get certificate
>  # GET-as-POST to order's certificate URL
> +# if $root is specified, attempts to find a matching (alternate) chain
>  # Expects a '200 OK' reply
>  # returns certificate chain in PEM format
>  sub get_certificate {
> -my ($self, $order) = @_;
> +my ($self, $order, $root) = @_;
>  
>  $self->fatal("no certificate URL available (yet?)", $order)
> if !$order->{certificate};
>  
> +my $check_root = sub {
> + my ($chain) = @_;
> +
> + my @certs = PVE::Certificate::split_pem($chain);
> + my $root_pem = $certs[-1];
> +
> + my ($file, $fh) = PVE::Tools::tempfile_contents($root_pem);
> + my $info = PVE::Certificate::get_certificate_info($file);
> +
> + return defined($info->{issuer}) && $info->{issuer} =~ m/\Q$root\E/i;
> +};
> +
>  my $r = $self->do(POST => $order->{certificate}, '');
>  my $return = eval {
> + # default chain
>   my $res = __get_result($r, 200, 1);
> + if ($root && !$check_root->($res)) {
> + # alternate chains if requested and default didn't match
> + $res = undef;
> + my @links = $r->header('link');
> + for my $link (@links) {
> + if ($link =~ /^<(.*)>;rel="alternate"$/) {
> + my $url = $1;
> + my $chain = eval { __get_result($self->do(POST => $url, 
> ''), 200, 1); };
> + die "failed to retrieve alternate chain from '$url' - $@\n" 
> if $@;
> + if ($check_root->($chain)) {
> + $res = $chain;
> + last;
> + }
> + }
> + }
> + die "no matching alternate chain for '$root' returned by server\n"
> + if !defined($res);
> + }
> +
>   if ($res =~ /^(-BEGIN CERTIFICATE-)(.+)(-END 
> CERTIFICATE-)$/s) { # untaint
>   return $1 . $2 . $3;
>   }



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] applied: [PATCH v2 proxmox-acme] support downloading alternate chains

2021-10-08 Thread Thomas Lamprecht
On 08.10.21 10:52, Stoiko Ivanov wrote:
> Tested again against LE production endpoint - LGTM :)
> Thanks!
> 
> Reviewed-By: Stoiko Ivanov 
> Tested-By: Stoiko Ivanov 

with s/By/by/ for those trailers: 

applied, thanks!


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH docs] fix PVE 6 repository entries

2021-10-08 Thread Mira Limbeck
With the release of PBS, and switching /etc/apt/auth.conf to use
/debian/pve as path instead of just /debian, the sources.list for
pve-enterprise must contain the /pve at the end. Otherwise there is no
match and updating via the pve-enterprise repository doesn't work.

Signed-off-by: Mira Limbeck 
---
 pve-package-repos.adoc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pve-package-repos.adoc b/pve-package-repos.adoc
index edb9ae0..82c8a36 100644
--- a/pve-package-repos.adoc
+++ b/pve-package-repos.adoc
@@ -226,11 +226,11 @@ these repositories is still provided.
 |===
 |Repository | sources.list entry
 |{pve} 6.x Enterprise   |
-deb https://enterprise.proxmox.com/debian buster pve-enterprise
+deb https://enterprise.proxmox.com/debian/pve buster pve-enterprise
 |{pve} 6.x No-Subscription  |
-deb http://download.proxmox.com/debian buster pve-no-subscription
+deb http://download.proxmox.com/debian/pve buster pve-no-subscription
 |{pve} 6.x Test |
-deb http://download.proxmox.com/debian buster pvetest
+deb http://download.proxmox.com/debian/pve buster pvetest
 |===
 
 
-- 
2.30.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [RFC ha-manager] manage: handle edge case where a node gets stuck in 'fence' state

2021-10-08 Thread Fabian Ebner
If all services in 'fence' state are gone from a node (e.g. by
removing the services) before fence_node() was successful, a node
would get stuck in the 'fence' state. Avoid this by calling
fence_node() if the node is in 'fence' state, regardless of service
state.

Reported in the community forum:
https://forum.proxmox.com/threads/ha-migration-stuck-is-doing-nothing.94469/

Signed-off-by: Fabian Ebner 
---

Not really sure if this is worth it, because it's a hard to reach edge
case, but AFAICT there is no good way to get out of being stuck. What
would work is either of:
* Manually correcting the node state.
* Adding a service to the stuck node and triggering a fence
  situation.

An alternative would be to keep services in 'fence' state in the
manager state, even if they were removed from the config. But the
approach from this patch seemed a bit more robust: for example, it
will fix an already existing stuck state, rather than just avoid
creating one.

 src/PVE/HA/Manager.pm | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/PVE/HA/Manager.pm b/src/PVE/HA/Manager.pm
index 1c66b43..fc445b1 100644
--- a/src/PVE/HA/Manager.pm
+++ b/src/PVE/HA/Manager.pm
@@ -472,6 +472,14 @@ sub manage {
$repeat = 1; # for faster execution
}
 
+   # Avoid that a node without services in 'fence' state gets stuck in 
'fence' state.
+   for my $node (sort keys $ns->{status}->%*) {
+   next if $ns->get_node_state($node) ne 'fence';
+   next if defined($fenced_nodes->{$node});
+
+   $fenced_nodes->{$node} = $ns->fence_node($node) || 0;
+   }
+
last if !$repeat;
 }
 
-- 
2.30.2



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel