* [pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains
@ 2021-10-08 8:18 Fabian Grünbichler
2021-10-08 8:52 ` Stoiko Ivanov
0 siblings, 1 reply; 3+ messages in thread
From: Fabian Grünbichler @ 2021-10-08 8:18 UTC (permalink / raw)
To: pve-devel
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 <f.gruenbichler@proxmox.com>
---
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains
2021-10-08 8:18 [pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains Fabian Grünbichler
@ 2021-10-08 8:52 ` Stoiko Ivanov
2021-10-08 9:23 ` [pve-devel] applied: " Thomas Lamprecht
0 siblings, 1 reply; 3+ messages in thread
From: Stoiko Ivanov @ 2021-10-08 8:52 UTC (permalink / raw)
To: Fabian Grünbichler; +Cc: Proxmox VE development discussion
Tested again against LE production endpoint - LGTM :)
Thanks!
Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>
On Fri, 8 Oct 2021 10:18:21 +0200
Fabian Grünbichler <f.gruenbichler@proxmox.com> 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 <f.gruenbichler@proxmox.com>
> ---
>
> 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;
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* [pve-devel] applied: [PATCH v2 proxmox-acme] support downloading alternate chains
2021-10-08 8:52 ` Stoiko Ivanov
@ 2021-10-08 9:23 ` Thomas Lamprecht
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Lamprecht @ 2021-10-08 9:23 UTC (permalink / raw)
To: Proxmox VE development discussion, Stoiko Ivanov,
Fabian Grünbichler
On 08.10.21 10:52, Stoiko Ivanov wrote:
> Tested again against LE production endpoint - LGTM :)
> Thanks!
>
> Reviewed-By: Stoiko Ivanov <s.ivanov@proxmox.com>
> Tested-By: Stoiko Ivanov <s.ivanov@proxmox.com>
with s/By/by/ for those trailers:
applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-08 9:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-08 8:18 [pve-devel] [PATCH v2 proxmox-acme] support downloading alternate chains Fabian Grünbichler
2021-10-08 8:52 ` Stoiko Ivanov
2021-10-08 9:23 ` [pve-devel] applied: " Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox