[PATCH common/proxmox-acme v3 0/2] fix #5978: pem parser: relax parsing of chain entries

[PATCH common/proxmox-acme v3 0/2] fix #5978: pem parser: relax parsing of chain entries

[Thomas Ellmenreich]

According to RFC 8555, expected certchains should come
without whitespace or explanatory texts inbetween chain
entries. These two patches relax our parser to also
accept text or whitespaces inbetween chain entries.

To make sure that the acme changes work as expected I
setup the pebble acme server [1] locally, and worked
through the acme flow to get a new certificate. I then
manually modified the final certificate to contain
descriptive text which worked without issues.

changes since v2:
- cleaner implementation and correction of mistakes in
  check_pem in pve-common

- get_certificate in proxmox-acme now correctly calls
  check_pem with the 'multiple' option enabled

- removed ambiguity in the error messages of
  get_certificate

- correction of tests, to better compare returned value
  to expected value

- performed proper end-to-end test with pebble [1]

- proper formatting (hopefully)

changes since v1:
- Where in v1 check_pem was just a wrapper of split_pem,
  they now perform different functions

- split_pem now purely splits the PEM chain into separate
  entries and does no further validation. Returning each
  entry with its leading text.

- check_pem retains the original functionality, except
  when the multiple option is active, in which case it
  uses split_pem to get single entries and then calls
  itself recursively

- On the ACME side, errors are now captured, wrapped,
  and then rethrown.

[1] https://github.com/letsencrypt/pebble


pve-common:

Thomas Ellmenreich (1):
  fix #5978: pem parser: relax parsing of chain entries

 src/PVE/Certificate.pm |  37 ++++-
 test/Makefile          |   2 +
 test/check_pem_test.pl | 357 +++++++++++++++++++++++++++++++++++++++++
 test/split_pem_test.pl | 279 ++++++++++++++++++++++++++++++++
 4 files changed, 667 insertions(+), 8 deletions(-)
 create mode 100755 test/check_pem_test.pl
 create mode 100755 test/split_pem_test.pl


proxmox-acme:

Thomas Ellmenreich (1):
  fix #5978: pem parser: relax parsing of chain entries:

 src/PVE/ACME.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)


Summary over all repositories:
  5 files changed, 673 insertions(+), 13 deletions(-)

-- 
Generated by murpp 0.12.0

[PATCH common v3 1/2] fix #5978: pem parser: relax parsing of chain entries

[Thomas Ellmenreich]

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.

Added testsuite to cover a number of parsing edge cases.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/Certificate.pm |  37 ++++-
 test/Makefile          |   2 +
 test/check_pem_test.pl | 357 +++++++++++++++++++++++++++++++++++++++++
 test/split_pem_test.pl | 279 ++++++++++++++++++++++++++++++++
 4 files changed, 667 insertions(+), 8 deletions(-)
 create mode 100755 test/check_pem_test.pl
 create mode 100755 test/split_pem_test.pl

diff --git a/src/PVE/Certificate.pm b/src/PVE/Certificate.pm
index b8415e2..e74887c 100644
--- a/src/PVE/Certificate.pm
+++ b/src/PVE/Certificate.pm
@@ -1,5 +1,4 @@
 package PVE::Certificate;
-
 use strict;
 use warnings;
 
@@ -134,26 +133,48 @@ 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) = @_;
 
+    my $label = $opts{label} // 'CERTIFICATE';
     $content = strip_leading_text($content);
 
-    my $re = $pem_re->($opts{label} // 'CERTIFICATE');
-    $re = qr/($re\n+)*$re/ if $opts{multiple};
+    my $result_pem = "";
+    if (delete $opts{multiple}) {
+        my @split = split_pem($content, label => $label);
+
+        if (!@split) {
+            return undef if $opts{noerr};
+            die "pem chain could not be split into separate entries\n";
+        }
+
+        for my $entry (@split) {
+            my $entry_pem = check_pem($entry, %opts);
+
+            return undef if !$entry_pem;
+
+            $result_pem .= $entry_pem;
+        }
+    } else {
+        my $re = $pem_re->($label);
+        $result_pem = $content if $content =~ /^$re$/;
+    }
 
-    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 {
diff --git a/test/Makefile b/test/Makefile
index 9b9f81b..8b725c5 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -14,6 +14,8 @@ TESTS = lock_file.test			\
 	is_deeply_test.test		\
 	section_config_property_isolation_test.pl \
 	file-test.pl \
+	check_pem_test.pl \
+	split_pem_test.pl \
 
 all:
 
diff --git a/test/check_pem_test.pl b/test/check_pem_test.pl
new file mode 100755
index 0000000..f26a38b
--- /dev/null
+++ b/test/check_pem_test.pl
@@ -0,0 +1,357 @@
+#!/usr/bin/perl
+# Tests the PVE::Certificate::check_pem function for
+# correctness and coverage of edgecases.
+use strict;
+use warnings;
+
+use lib '../src';
+
+use Test::More;
+
+use PVE::Certificate;
+
+# Arrange
+my $setup = [
+    {
+        expected_success => 1,
+        name => "full pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+MIIBsjCCAVugAwIBAgIJAO2g8Z0dXk9tMAoGCCqGSM49BAMCMEUxCzAJBgNVBAYT
+AlVTMQswCQYDVQQIDAJDQTEQMA4GA1UEBwwHQmVya2VsZXkxEDAOBgNVBAoMB1Rl
+c3QgQ0EwHhcNMjAwMTAxMDAwMDAwWhcNMzAwMTAxMDAwMDAwWjBFMQswCQYDVQQG
+EwJVUzELMAkGA1UECAwCQ0ExEDAOBgNVBAcMB0JlcmtlbGV5MRAwDgYDVQQKDAdU
+ZXN0IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEv5Q8q1p7qZ2gqkQ0Qn5x
+0n9yqv8n8n7n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8aNT
+MFEwHQYDVR0OBBYEFOu2Y0bq8v3z7qkq1m1Qwqkq1m1QMB8GA1UdIwQYMBaAFOu2
+Y0bq8v3z7qkq1m1Qwqkq1m1QMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID
+SAAwRQIhANfakefakefakefakefakefakefakefakefake
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "other label pem",
+        label => "SOME LABEL",
+        pem => <<'EOF',
+-----BEGIN SOME LABEL-----
+MIIBsjCCAVugAwIBAgIJAO2g8Z0dXk9tMAoGCCqGSM49BAMCMEUxCzAJBgNVBAYT
+AlVTMQswCQYDVQQIDAJDQTEQMA4GA1UEBwwHQmVya2VsZXkxEDAOBgNVBAoMB1Rl
+c3QgQ0EwHhcNMjAwMTAxMDAwMDAwWhcNMzAwMTAxMDAwMDAwWjBFMQswCQYDVQQG
+EwJVUzELMAkGA1UECAwCQ0ExEDAOBgNVBAcMB0JlcmtlbGV5MRAwDgYDVQQKDAdU
+ZXN0IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEv5Q8q1p7qZ2gqkQ0Qn5x
+0n9yqv8n8n7n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8aNT
+MFEwHQYDVR0OBBYEFOu2Y0bq8v3z7qkq1m1Qwqkq1m1QMB8GA1UdIwQYMBaAFOu2
+Y0bq8v3z7qkq1m1Qwqkq1m1QMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID
+SAAwRQIhANfakefakefakefakefakefakefakefakefake
+-----END SOME LABEL-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "simple pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+test
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "spaced out pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+
+test
+
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "only newline pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "many newlines pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+
+
+
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "no content pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE----------END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "preceding content pem",
+        pem => <<'EOF',
+some extended content
+-----BEGIN CERTIFICATE-----
+test
+-----END CERTIFICATE-----
+EOF
+
+        expected_result => <<'EOF',
+-----BEGIN CERTIFICATE-----
+test
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "following content pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+test
+-----END CERTIFICATE-----
+some extended content
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "empty pem",
+        pem => <<'EOF',
+
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "upsidedown pem",
+        pem => <<'EOF',
+-----END CERTIFICATE-----
+test
+-----BEGIN CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "multi pem",
+        multiple => 1,
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+12JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+22JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+32JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+42JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "multiple as single pem",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "big pem different label",
+        multiple => 1,
+        label => "LABEL",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+-----BEGIN LABEL-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+-----BEGIN LABEL-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+-----BEGIN LABEL-----
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+EOF
+
+        expected_result => <<'EOF',
+-----BEGIN LABEL-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+-----BEGIN LABEL-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+-----BEGIN LABEL-----
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END LABEL-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "other real pem",
+        pem => <<'EOF',
+Subject: CN=Some subject with 2 Points,O=A Organization,L=Some Lation,C=TT
+Issuer: CN=The Orgs Name RootCA 2015,O=The Orgs name Institutions Cert. Authority,L=Location,C=TT
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        expected_result => <<'EOF',
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "invalid single pem",
+        pem => <<'EOF',
+Subject: CN=Some subject with 2 Points,O=A Organization,L=Some Lation,C=TT
+Issuer: CN=The Orgs Name RootCA 2015,O=The Orgs name Institutions Cert. Authority,L=Location,C=TT
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdthese !?''*:;$%&/  are not allowedWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 0,
+        name => "invalid multi pem",
+        multiple => 1,
+        pem => <<'EOF',
+Subject: CN=Some subject with 2 Points,O=A Organization,L=Some Lation,C=TT
+Issuer: CN=The Orgs Name RootCA 2015,O=The Orgs name Institutions Cert. Authority,L=Location,C=TT
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdthese !?''*:;$%&/  are not allowedWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+Subject: CN=Some subject with 2 Points,O=A Organization,L=Some Lation,C=TT
+Issuer: CN=The Orgs Name RootCA 2015,O=The Orgs name Institutions Cert. Authority,L=Location,C=TT
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "single as multiple pem",
+        multiple => 1,
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdtheseZ2RmaXVwYWJ2aXV1YXdlYmxmamlraWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "single as multiple pem",
+        multiple => 1,
+        pem => <<'EOF',
+Subject: CN=Some subject with 2 Points,O=A Organization,L=Some Lation,C=TT
+Issuer: CN=The Orgs Name RootCA 2015,O=The Orgs name Institutions Cert. Authority,L=Location,C=TT
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdtheseZ2RmaXVwYWJ2aXV1YXdlYmxmamlraWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        expected_result => <<'EOF',
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdtheseZ2RmaXVwYWJ2aXV1YXdlYmxmamlraWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+];
+
+for my $data ($setup->@*) {
+    subtest $data->{name} => sub {
+        # Act
+        my $check_result = PVE::Certificate::check_pem(
+            $data->{pem},
+            noerr => 1,
+            multiple => $data->{multiple},
+            label => $data->{label},
+        );
+
+        # Assert
+        if ($data->{expected_success}) {
+            ok($check_result, "is successful");
+
+            my $expected_result =
+                $data->{expected_result}
+                ? $data->{expected_result}
+                : $data->{pem};
+            is(
+                $check_result, $expected_result, "has correct result content",
+            );
+        } else {
+            is($check_result, undef, "has correctly returned undef");
+        }
+    }
+}
+
+done_testing();
diff --git a/test/split_pem_test.pl b/test/split_pem_test.pl
new file mode 100755
index 0000000..4239e5c
--- /dev/null
+++ b/test/split_pem_test.pl
@@ -0,0 +1,279 @@
+#!/usr/bin/perl
+# Tests the PVE::Certificate::split_pem function for
+# correctness and coverage of edgecases.
+use strict;
+use warnings;
+
+use lib '../src';
+
+use Test::More;
+
+use PVE::Certificate;
+
+# Arrange
+my $setup = [
+    {
+        expected_success => 1,
+        name => "single pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+MIIBsjCCAVugAwIBAgIJAO2g8Z0dXk9tMAoGCCqGSM49BAMCMEUxCzAJBgNVBAYT
+AlVTMQswCQYDVQQIDAJDQTEQMA4GA1UEBwwHQmVya2VsZXkxEDAOBgNVBAoMB1Rl
+c3QgQ0EwHhcNMjAwMTAxMDAwMDAwWhcNMzAwMTAxMDAwMDAwWjBFMQswCQYDVQQG
+EwJVUzELMAkGA1UECAwCQ0ExEDAOBgNVBAcMB0JlcmtlbGV5MRAwDgYDVQQKDAdU
+ZXN0IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEv5Q8q1p7qZ2gqkQ0Qn5x
+0n9yqv8n8n7n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8aNT
+MFEwHQYDVR0OBBYEFOu2Y0bq8v3z7qkq1m1Qwqkq1m1QMB8GA1UdIwQYMBaAFOu2
+Y0bq8v3z7qkq1m1Qwqkq1m1QMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID
+SAAwRQIhANfakefakefakefakefakefakefakefakefake
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "no extra text pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+12JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+22JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+32JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+42JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+        expected_result => [
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+12JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+22JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+32JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+42JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        ],
+    },
+    {
+        expected_success => 1,
+        name => "leading and interleaved text pem",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+        expected_result => [
+            <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        ],
+    },
+    {
+        expected_success => 1,
+        name => "massive glob pem",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+-----BEGIN CERTIFICATE-----
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+-----BEGIN CERTIFICATE-----
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "no beginning pem",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+        expected_result => [
+            <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+2WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+Subject: CN=Org Root CA
+Issuer: CN=Org Root CA
+3WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        ],
+
+    },
+    {
+        expected_success => 0,
+        name => "not a pem pem",
+        pem => <<'EOF',
+Subject: CN=some.test.content
+Issuer: CN=Org Issuing CA 3
+1WtzZGhmbGtqYXNoZ2RmaXVwYWJ2aXV1YXdlYmxmamlraGFzZGtmYWlzdXBkdW5i
+dmFpdXdlYmdmw7ZrYWpkc2JoZmdpYXNkbmJpZnVhYmdzaWprZmdyYXNkaHJmaW9h
+dXNkbmF1c2JmZ3ZramFzZGJma2phc2RibmZpanVhIHNkb3ZuYmF3b3VlYmZnb2F3
+c2JlZ2tseTxzZWl2IGF3ZWlidm4=
+Subject: CN=Org Issuing CA 3
+Issuer: CN=Org Root CA
+EOF
+    },
+    {
+        expected_success => 1,
+        name => "following text pem",
+        pem => <<'EOF',
+-----BEGIN CERTIFICATE-----
+32JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+42JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+some more text that we would like to remove
+EOF
+
+        expected_result => [
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+32JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+
+            <<'EOF',
+-----BEGIN CERTIFICATE-----
+42JlZ2tseTxzZWl2IGF3ZWlidm4=
+-----END CERTIFICATE-----
+EOF
+        ],
+    },
+];
+
+for my $data ($setup->@*) {
+    subtest $data->{name} => sub {
+        # Act
+        my @split_result = PVE::Certificate::split_pem(
+            $data->{pem}, label => $data->{label},
+        );
+
+        # Assert
+        if ($data->{expected_success}) {
+            ok(@split_result, "is successful");
+            my $expected_result =
+                $data->{expected_result}
+                ? $data->{expected_result}
+                : [$data->{pem}];
+
+            is_deeply(
+                \@split_result, \@$expected_result, "has correct split output",
+            );
+        } else {
+            is_deeply(\@split_result, [], "has correctly returned empty");
+        }
+    }
+}
+
+done_testing();
-- 
2.47.3

[PATCH proxmox-acme v3 2/2] fix #5978: pem parser: relax parsing of chain entries:

[Thomas Ellmenreich]

Instead of using a custom regex to parse pem chains, now uses
the pve-common Certificate::check_pem function to do so. This
now allows for additional text and whitespace inbetween the
chain entries.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/ACME.pm | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
index e6fb9c2..7adda25 100644
--- a/src/PVE/ACME.pm
+++ b/src/PVE/ACME.pm
@@ -526,14 +526,15 @@ sub get_certificate {
                     }
                 }
             }
-            die "no matching alternate chain for '$root' returned by server\n"
+            die "no matching alternate chain for '$root' provided by the acme server\n"
                 if !defined($res);
         }
 
-        if ($res =~ /^(-----BEGIN CERTIFICATE-----)(.+)(-----END CERTIFICATE-----)$/s) { # untaint
-            return $1 . $2 . $3;
-        }
-        die "Server reply does not look like a PEM encoded certificate\n";
+        my $parsed = eval { PVE::Certificate::check_pem($res, multiple => 1) };
+        die
+            "value returned by acme certificate url does not look like a PEM encoded certificate: $@\n"
+            if $@;
+        return $parsed;
     };
     $self->fatal("POST of '$order->{certificate}' failed - $@", $r) if $@;
     return $return;
-- 
2.47.3

Re: [PATCH common v3 1/2] fix #5978: pem parser: relax parsing of chain entries

[Elias Huhsovitz]

On Fri Jul 3, 2026 at 12:51 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.
>
> Added testsuite to cover a number of parsing edge cases.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>

Summary
-------
* Patch needs a rebase onto master, in order to be applied using b4 pain
free.

* Consider splitting tests into separate patch. This allows reviewing
  the bug-fix and tests separately

* die-path of check_pem is not tested

* use iteration instead of recursion in check_pem

* improve test names

See more comments inline

> ---
>  src/PVE/Certificate.pm |  37 ++++-
>  test/Makefile          |   2 +
>  test/check_pem_test.pl | 357 +++++++++++++++++++++++++++++++++++++++++
>  test/split_pem_test.pl | 279 ++++++++++++++++++++++++++++++++
>  4 files changed, 667 insertions(+), 8 deletions(-)
>  create mode 100755 test/check_pem_test.pl
>  create mode 100755 test/split_pem_test.pl
>
> diff --git a/src/PVE/Certificate.pm b/src/PVE/Certificate.pm
> index b8415e2..e74887c 100644
> --- a/src/PVE/Certificate.pm
> +++ b/src/PVE/Certificate.pm
> @@ -1,5 +1,4 @@
>  package PVE::Certificate;
> -

nit: Why remove this newline?

>  use strict;
>  use warnings;
>  
> @@ -134,26 +133,48 @@ 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;
>  }

Splitting at the END instead of the BEGIN makes sense. Very nice fix.

>  
> +# 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) = @_;
>  
> +    my $label = $opts{label} // 'CERTIFICATE';
>      $content = strip_leading_text($content);
>  
> -    my $re = $pem_re->($opts{label} // 'CERTIFICATE');
> -    $re = qr/($re\n+)*$re/ if $opts{multiple};
> +    my $result_pem = "";
> +    if (delete $opts{multiple}) {
> +        my @split = split_pem($content, label => $label);
> +
> +        if (!@split) {
> +            return undef if $opts{noerr};
> +            die "pem chain could not be split into separate entries\n";
> +        }

nit: Capitalize Acronyms. e.g.,
die "PEM chain could not be split into separate entries\n";

> +
> +        for my $entry (@split) {
> +            my $entry_pem = check_pem($entry, %opts);
> +
> +            return undef if !$entry_pem;
> +
> +            $result_pem .= $entry_pem;
> +        }
> +    } else {
> +        my $re = $pem_re->($label);
> +        $result_pem = $content if $content =~ /^$re$/;
> +    }
>  
> -    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";

This die-path is not included in the test suite.

>  }

Stylistic nit: I like the ingenuity of the recurisve approach, but IMO
this complicates the code unnecessarily. I would go for an iterative
approach, since it is usually less error prone and easier to maintain.
For example it could look like this (not-tested, just for
visualization):


my sub validate_single_pem {
    my ($chunk, $label, $noerr) = @_;

    my $clean = strip_leading_text($chunk);
    my $re = $pem_re->($label);

    # Guard clause: return immediately if valid
    return $clean if $clean =~ /^$re$/;

    # Guard clause: handle error state without deep nesting
    return undef if $noerr;
    die "Not a valid PEM-formatted string\n";
}

sub check_pem {
    my ($content, %opts) = @_;

    my $label = $opts{label} // 'CERTIFICATE';
    my $noerr = $opts{noerr};

    # Treat a single PEM as an array of one chunk, or split if multiple
    my @chunks = $opts{multiple} ? split_pem($content, label => $label) : ($content);

    if ($opts{multiple} && !@chunks) {
        return undef if $noerr;
        die "The PEM chain could not be split into separate entries\n";
    }

    my $result_pem = "";
    for my $chunk (@chunks) {
        my $valid = validate_single_pem($chunk, $label, $noerr);
        
        return $valid if !defined($valid);

        $result_pem .= $valid;
    }

    return $result_pem if $result_pem;

    return undef if $noerr;
    die "Not a valid PEM-formatted string\n";
}

If you think a recursive approach is better suited here, then i would
recommend reducing the level of indenation to indentation. Long complex
recursive functions are usually hard to maintain.

>  
>  sub pem_to_der {
> diff --git a/test/Makefile b/test/Makefile
> index 9b9f81b..8b725c5 100644
> --- a/test/Makefile
> +++ b/test/Makefile
> @@ -14,6 +14,8 @@ TESTS = lock_file.test			\
>  	is_deeply_test.test		\
>  	section_config_property_isolation_test.pl \
>  	file-test.pl \
> +	check_pem_test.pl \
> +	split_pem_test.pl \

This currently causes issues when apllying the patch using b4. Please
rebase onto master.

>  
>  all:
>  
> diff --git a/test/check_pem_test.pl b/test/check_pem_test.pl
> new file mode 100755
> index 0000000..f26a38b
> --- /dev/null
> +++ b/test/check_pem_test.pl
> @@ -0,0 +1,357 @@
> +#!/usr/bin/perl
> +# Tests the PVE::Certificate::check_pem function for
> +# correctness and coverage of edgecases.
> +use strict;
> +use warnings;
> +
> +use lib '../src';
> +
> +use Test::More;
> +
> +use PVE::Certificate;
> +
> +# Arrange
> +my $setup = [
> +    {
> +        expected_success => 1,
> +        name => "full pem",

nit: include success/failure in the name. e.g.,
"full pem succeeds" or "invalid full pem fails with error XYZ"

I know you already set expected_success, but this is easy to miss when
reading the test output.

> +        pem => <<'EOF',
> +-----BEGIN CERTIFICATE-----
> +MIIBsjCCAVugAwIBAgIJAO2g8Z0dXk9tMAoGCCqGSM49BAMCMEUxCzAJBgNVBAYT
> +AlVTMQswCQYDVQQIDAJDQTEQMA4GA1UEBwwHQmVya2VsZXkxEDAOBgNVBAoMB1Rl
> +c3QgQ0EwHhcNMjAwMTAxMDAwMDAwWhcNMzAwMTAxMDAwMDAwWjBFMQswCQYDVQQG
> +EwJVUzELMAkGA1UECAwCQ0ExEDAOBgNVBAcMB0JlcmtlbGV5MRAwDgYDVQQKDAdU
> +ZXN0IENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEv5Q8q1p7qZ2gqkQ0Qn5x
> +0n9yqv8n8n7n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8n8aNT
> +MFEwHQYDVR0OBBYEFOu2Y0bq8v3z7qkq1m1Qwqkq1m1QMB8GA1UdIwQYMBaAFOu2
> +Y0bq8v3z7qkq1m1Qwqkq1m1QMA8GA1UdEwEB/wQFMAMBAf8wCgYIKoZIzj0EAwID
> +SAAwRQIhANfakefakefakefakefakefakefakefakefake
> +-----END CERTIFICATE-----
> +EOF
> +    },

[snip]

> +    {
> +        expected_success => 1,
> +        name => "many newlines pem",
> +        pem => <<'EOF',
> +-----BEGIN CERTIFICATE-----
> +
> +
> +
> +-----END CERTIFICATE-----
> +EOF
> +    },
> +    {
> +        expected_success => 0,
> +        name => "no content pem",

Same as above, i would prefer a more descriptive name. Something like
"no content pem returns undef"

[snip]

Re: [PATCH proxmox-acme v3 2/2] fix #5978: pem parser: relax parsing of chain entries:

[Elias Huhsovitz]

On Fri Jul 3, 2026 at 12:51 PM CEST, Thomas Ellmenreich wrote:
> Instead of using a custom regex to parse pem chains, now uses
> the pve-common Certificate::check_pem function to do so. This
> now allows for additional text and whitespace inbetween the
> chain entries.
>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---

Good change to use standard pve-common implementation instead of custom
regex!

See comments below.

>  src/PVE/ACME.pm | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/PVE/ACME.pm b/src/PVE/ACME.pm
> index e6fb9c2..7adda25 100644
> --- a/src/PVE/ACME.pm
> +++ b/src/PVE/ACME.pm
> @@ -526,14 +526,15 @@ sub get_certificate {
>                      }
>                  }
>              }
> -            die "no matching alternate chain for '$root' returned by server\n"
> +            die "no matching alternate chain for '$root' provided by the acme server\n"
>                  if !defined($res);
>          }
>  
> -        if ($res =~ /^(-----BEGIN CERTIFICATE-----)(.+)(-----END CERTIFICATE-----)$/s) { # untaint
> -            return $1 . $2 . $3;
> -        }
> -        die "Server reply does not look like a PEM encoded certificate\n";
> +        my $parsed = eval { PVE::Certificate::check_pem($res, multiple => 1) };
> +        die
> +            "value returned by acme certificate url does not look like a PEM encoded certificate: $@\n"
> +            if $@;

nit: Error message inside of $@ might contain trailing newline. Consider
remvoing the newline before die statement. e.g.,

die "value returned by ACME certificate url does not look like a PEM encoded certificate: " . chomp($@) . "\n"
    if $@;

nit: in the error message "PEM" is capitalized by "acme" is not. Capitalize both for
consistency. e.g., code sample above.


> +        return $parsed;
>      };
>      $self->fatal("POST of '$order->{certificate}' failed - $@", $r) if $@;
>      return $return;