public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Thomas Ellmenreich" <t.ellmenreich@proxmox.com>,
	<pve-devel@lists.proxmox.com>
Subject: Re: [PATCH common v4 1/3] fix #5978: pem parser: relax parsing of chain entries
Date: Fri, 14 Aug 2026 16:26:49 +0200	[thread overview]
Message-ID: <DKOQJ67KWCA9.13OZHQ54PZ3A@proxmox.com> (raw)
In-Reply-To: <20260805141114.190477-2-t.ellmenreich@proxmox.com>

Good refresh. IMO the iterative approach is much easier to maintain,
than the previous recurive solution.

Testing
-------
I tested this by setting up a local Pebble ACME server and
successfully ordering certificates. To verify the relaxed parsing,
I injected fake interleaved text into the ACME response that the parser 
correctly strips it and accepts the chain.

Additionally I manually malformed the header/footer of the certificate
which correctly throws an error.

Everything seemes to work as intended.

Consider this.

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Wed Aug 5, 2026 at 4:11 PM CEST, Thomas Ellmenreich wrote:
> Relaxes the parser to allow for text and whitespaces inbetween certchain
> entries. The splitting of PEM chains was also reworked to split each entry at
> its end, grouping it with its leading text.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
>  src/PVE/Certificate.pm | 39 +++++++++++++++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/src/PVE/Certificate.pm b/src/PVE/Certificate.pm
> index b8415e2..53619a3 100644
> --- a/src/PVE/Certificate.pm
> +++ b/src/PVE/Certificate.pm
> @@ -134,26 +134,49 @@ sub strip_leading_text {
>      return $content;
>  }
>  
> +# Splits the pem chain into entries with their leading text
>  sub split_pem {
>      my ($content, %opts) = @_;
> -    my $label = $opts{label} // 'CERTIFICATE';
>  
> -    my $header = $header_re->($label);
> -    return split(/(?=$header)/, $content);
> +    my $footer = $footer_re->($opts{label} // 'CERTIFICATE');
> +
> +    return $content =~ /(.*?$footer)/sg;
>  }
>  
> +# Parses the pem or pem chain for complete validity and returns
> +# only the pem/pem chain removing any extra text
>  sub check_pem {
>      my ($content, %opts) = @_;
>  
> -    $content = strip_leading_text($content);
> +    my $label = $opts{label} // 'CERTIFICATE';
> +    my $re = $pem_re->($label);
> +
> +    my @split =
> +        $opts{multiple}
> +        ? split_pem($content, label => $label)
> +        : ($content);
>  
> -    my $re = $pem_re->($opts{label} // 'CERTIFICATE');
> -    $re = qr/($re\n+)*$re/ if $opts{multiple};
> +    if (!@split) {
> +        return undef if $opts{noerr};
> +        die "PEM chain could not be split into separate entries\n";
> +    }
> +
> +    my $result_pem = "";
> +    for my $entry (@split) {
> +        my $stripped = strip_leading_text($entry);
> +
> +        if ($stripped !~ /^$re$/) {
> +            $result_pem = "";
> +            last;
> +        }
> +
> +        $result_pem .= $stripped;
> +    }
>  
> -    return $content if $content =~ /^$re$/; # OK
> +    return $result_pem if $result_pem;
>  
>      return undef if $opts{noerr};
> -    die "not a valid PEM-formatted string.\n";
> +    die "not a valid PEM-formatted string\n";
>  }
>  
>  sub pem_to_der {





  reply	other threads:[~2026-08-14 14:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 14:11 [PATCH common/proxmox-acme v4 0/3] fix #5978: pem parser: relax parsing of chain entries Thomas Ellmenreich
2026-08-05 14:11 ` [PATCH common v4 1/3] " Thomas Ellmenreich
2026-08-14 14:26   ` Elias Huhsovitz [this message]
2026-08-05 14:11 ` [PATCH common v4 2/3] pem parser: add testsuite for check_ and split_pem Thomas Ellmenreich
2026-08-14 14:30   ` Elias Huhsovitz
2026-08-05 14:11 ` [PATCH common v4 2/3] pem parser: add testsuite for check_pem " Thomas Ellmenreich
2026-08-05 14:23   ` Thomas Ellmenreich
2026-08-05 14:11 ` [PATCH proxmox-acme v4 3/3] fix #5978: pem parser: relax parsing of chain entries: Thomas Ellmenreich
2026-08-14 14:32   ` Elias Huhsovitz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKOQJ67KWCA9.13OZHQ54PZ3A@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.ellmenreich@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal