Skip to content

Commit b5b89f4

Browse files
authored
Merge pull request #5584 from LibreSign/chore/certificate-chain-validation-improvements
chore: Refactor certificate chain processing with ordering
2 parents f279882 + 456623c commit b5b89f4

File tree

4 files changed

+798
-166
lines changed

4 files changed

+798
-166
lines changed

lib/Handler/CertificateEngine/OrderCertificatesTrait.php

Lines changed: 123 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,66 +12,154 @@
1212

1313
trait OrderCertificatesTrait {
1414
public function orderCertificates(array $certificates): array {
15-
$this->validateCertificateStructure($certificates);
16-
$remainingCerts = [];
15+
if (empty($certificates)) {
16+
throw new InvalidArgumentException('Certificate list cannot be empty');
17+
}
18+
19+
$this->ensureValidStructure($certificates);
20+
21+
return match (true) {
22+
count($certificates) === 1 => $certificates,
23+
default => $this->buildChain($certificates)
24+
};
25+
}
26+
27+
private function buildChain(array $certificates): array {
28+
$leaf = $this->findLeafCertificate($certificates);
29+
if (!$leaf) {
30+
return $certificates;
31+
}
1732

18-
// Add the root cert at ordered list and collect the remaining certs
33+
$ordered = [$leaf];
34+
$remaining = $this->excludeCertificate($certificates, $leaf);
35+
36+
while ($remaining && !$this->isRootCertificate(end($ordered))) {
37+
[$next, $remaining] = $this->findIssuer(end($ordered), $remaining);
38+
if (!$next) {
39+
break;
40+
}
41+
$ordered[] = $next;
42+
}
43+
44+
return [...$ordered, ...$remaining];
45+
}
46+
47+
private function ensureValidStructure(array $certificates): void {
1948
foreach ($certificates as $cert) {
20-
if (!$this->arrayDiffCanonicalized($cert['subject'], $cert['issuer'])) {
21-
$ordered = [$cert];
22-
continue;
49+
if (!is_array($cert) || !isset($cert['subject'], $cert['issuer'], $cert['name'])) {
50+
throw new InvalidArgumentException('Invalid certificate structure. Certificate must have "subject", "issuer", and "name".');
51+
}
52+
if (!is_array($cert['subject']) || !is_array($cert['issuer'])) {
53+
throw new InvalidArgumentException('Invalid certificate structure. Certificate must have "subject", "issuer", and "name".');
2354
}
24-
$remainingCerts[$cert['name']] = $cert;
2555
}
2656

27-
if (!isset($ordered)) {
28-
return $certificates;
57+
$names = array_column($certificates, 'name');
58+
if (count($names) !== count(array_unique($names))) {
59+
throw new InvalidArgumentException('Duplicate certificate names detected');
2960
}
61+
}
3062

63+
private function findLeafCertificate(array $certificates): ?array {
64+
$issuers = [];
65+
foreach ($certificates as $cert) {
66+
if (isset($cert['issuer'])) {
67+
$issuers[] = $this->normalizeDistinguishedName($cert['issuer']);
68+
}
69+
}
3170

32-
while (!empty($remainingCerts)) {
33-
$found = false;
34-
foreach ($remainingCerts as $name => $cert) {
35-
$first = reset($ordered);
36-
if (!$this->arrayDiffCanonicalized($first['subject'], $cert['issuer'])) {
37-
array_unshift($ordered, $cert);
38-
unset($remainingCerts[$name]);
39-
$found = true;
40-
break;
41-
}
71+
foreach ($certificates as $cert) {
72+
if (!isset($cert['subject'])) {
73+
continue;
4274
}
75+
$subject = $this->normalizeDistinguishedName($cert['subject']);
76+
if (!in_array($subject, $issuers, true)) {
77+
return $cert;
78+
}
79+
}
80+
81+
return $certificates[0] ?? null;
82+
}
4383

44-
if (!$found) {
45-
throw new InvalidArgumentException('Certificate chain is incomplete or invalid.');
84+
private function findIssuer(array $cert, array $certificates): array {
85+
foreach ($certificates as $index => $candidate) {
86+
if ($this->isIssuedBy($cert, $candidate)) {
87+
unset($certificates[$index]);
88+
return [$candidate, array_values($certificates)];
4689
}
4790
}
91+
return [null, $certificates];
92+
}
93+
94+
private function excludeCertificate(array $certificates, array $toExclude): array {
95+
return array_values(array_filter($certificates, fn ($cert) => $cert['name'] !== $toExclude['name']));
96+
}
97+
98+
private function isRootCertificate(array $cert): bool {
99+
if (!isset($cert['subject'], $cert['issuer']) || !is_array($cert['subject']) || !is_array($cert['issuer'])) {
100+
return false;
101+
}
102+
return $this->normalizeDistinguishedName($cert['subject']) === $this->normalizeDistinguishedName($cert['issuer']);
103+
}
48104

49-
return $ordered;
105+
private function isIssuedBy(array $child, array $parent): bool {
106+
if (!isset($child['issuer'], $parent['subject']) || !is_array($child['issuer']) || !is_array($parent['subject'])) {
107+
return false;
108+
}
109+
return $this->normalizeDistinguishedName($child['issuer']) === $this->normalizeDistinguishedName($parent['subject']);
50110
}
51111

52-
private function validateCertificateStructure(array $certificates): void {
112+
private function normalizeDistinguishedName(array $dn): string {
113+
ksort($dn);
114+
return json_encode($dn, JSON_THROW_ON_ERROR);
115+
}
116+
117+
public function validateCertificateChain(array $certificates): array {
118+
return [
119+
'valid' => $this->isValidChain($certificates),
120+
'hasRoot' => $this->hasRootCertificate($certificates),
121+
'isComplete' => $this->isCompleteChain($certificates),
122+
'length' => count($certificates),
123+
];
124+
}
125+
126+
private function isValidChain(array $certificates): bool {
53127
if (empty($certificates)) {
54-
throw new InvalidArgumentException('Certificate list cannot be empty');
128+
return false;
55129
}
56130

57131
foreach ($certificates as $cert) {
58-
if (!isset($cert['subject'], $cert['issuer'], $cert['name'])) {
59-
throw new InvalidArgumentException(
60-
'Invalid certificate structure. Certificate must have "subject", "issuer", and "name".'
61-
);
132+
if (!is_array($cert) || !isset($cert['subject'], $cert['issuer'], $cert['name'])
133+
|| !is_array($cert['subject']) || !is_array($cert['issuer'])
134+
|| empty($cert['subject']['CN']) || empty($cert['issuer']['CN'])) {
135+
return false;
62136
}
63137
}
64138

65-
$names = array_column($certificates, 'name');
66-
if (count($names) !== count(array_unique($names))) {
67-
throw new InvalidArgumentException('Duplicate certificate names detected');
139+
return true;
140+
}
141+
142+
private function hasRootCertificate(array $certificates): bool {
143+
foreach ($certificates as $cert) {
144+
if ($this->isRootCertificate($cert)) {
145+
return true;
146+
}
68147
}
148+
return false;
69149
}
70150

71-
private function arrayDiffCanonicalized(array $array1, array $array2): array {
72-
sort($array1);
73-
sort($array2);
151+
private function isCompleteChain(array $certificates): bool {
152+
if (!$this->hasRootCertificate($certificates)) {
153+
return false;
154+
}
155+
156+
$ordered = $this->orderCertificates($certificates);
157+
for ($i = 0; $i < count($ordered) - 1; $i++) {
158+
if (!$this->isIssuedBy($ordered[$i], $ordered[$i + 1])) {
159+
return false;
160+
}
161+
}
74162

75-
return array_diff($array1, $array2);
163+
return true;
76164
}
77165
}

lib/Handler/SignEngine/Pkcs12Handler.php

Lines changed: 80 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,6 @@ class Pkcs12Handler extends SignEngineHandler {
2626
use OrderCertificatesTrait;
2727
protected string $certificate = '';
2828
private array $signaturesFromPoppler = [];
29-
/**
30-
* Used by method self::getHandler()
31-
*/
3229
private ?JSignPdfHandler $jSignPdfHandler = null;
3330

3431
public function __construct(
@@ -58,11 +55,8 @@ private function getSignatures($resource): iterable {
5855
}
5956

6057
for ($i = 0; $i < count($bytes['offset1']); $i++) {
61-
// Starting position (in bytes) of the first part of the PDF that will be included in the validation.
6258
$offset1 = (int)$bytes['offset1'][$i];
63-
// Length (in bytes) of the first part.
6459
$length1 = (int)$bytes['length1'][$i];
65-
// Starting position (in bytes) of the second part, immediately after the signature.
6660
$offset2 = (int)$bytes['offset2'][$i];
6761

6862
$signatureStart = $offset1 + $length1 + 1;
@@ -85,59 +79,95 @@ private function getSignatures($resource): iterable {
8579
*/
8680
#[\Override]
8781
public function getCertificateChain($resource): array {
88-
$signerCounter = 0;
8982
$certificates = [];
83+
$signerCounter = 0;
84+
9085
foreach ($this->getSignatures($resource) as $signature) {
91-
// The signature could be invalid
92-
$fromFallback = $this->popplerUtilsPdfSignFallback($resource, $signerCounter);
93-
if ($fromFallback) {
94-
$certificates[$signerCounter] = $fromFallback;
95-
}
96-
if (!$signature) {
97-
$certificates[$signerCounter]['chain'][0]['signature_validation'] = $this->getReadableSigState('Digest Mismatch.');
98-
$signerCounter++;
99-
continue;
86+
$certificates[$signerCounter] = $this->processSignature($resource, $signature, $signerCounter);
87+
$signerCounter++;
88+
}
89+
return $certificates;
90+
}
91+
92+
private function processSignature($resource, ?string $signature, int $signerCounter): array {
93+
$fromFallback = $this->popplerUtilsPdfSignFallback($resource, $signerCounter);
94+
$result = $fromFallback ?: [];
95+
96+
if (!$signature) {
97+
$result['chain'][0]['signature_validation'] = $this->getReadableSigState('Digest Mismatch.');
98+
return $result;
99+
}
100+
101+
$decoded = ASN1::decodeBER($signature);
102+
$result = $this->extractTimestampData($decoded, $result);
103+
104+
$chain = $this->extractCertificateChain($signature);
105+
if (!empty($chain)) {
106+
$result['chain'] = $this->orderCertificates($chain);
107+
$this->enrichLeafWithPopplerData($result, $fromFallback);
108+
}
109+
return $result;
110+
}
111+
112+
private function extractTimestampData(array $decoded, array $result): array {
113+
$tsa = new TSA();
114+
115+
try {
116+
$timestampData = $tsa->extract($decoded);
117+
if (!empty($timestampData['genTime']) || !empty($timestampData['policy']) || !empty($timestampData['serialNumber'])) {
118+
$result['timestamp'] = $timestampData;
100119
}
120+
} catch (\Throwable $e) {
121+
}
101122

102-
$tsa = new TSA();
103-
$decoded = ASN1::decodeBER($signature);
104-
try {
105-
$timestampData = $tsa->extract($decoded);
106-
if (!empty($timestampData['genTime']) || !empty($timestampData['policy']) || !empty($timestampData['serialNumber'])) {
107-
$certificates[$signerCounter]['timestamp'] = $timestampData;
108-
}
109-
} catch (\Throwable $e) {
123+
if (!isset($result['signingTime']) || !$result['signingTime'] instanceof \DateTime) {
124+
$result['signingTime'] = $tsa->getSigninTime($decoded);
125+
}
126+
return $result;
127+
}
128+
129+
private function extractCertificateChain(string $signature): array {
130+
$pkcs7PemSignature = $this->der2pem($signature);
131+
$pemCertificates = [];
132+
133+
if (!openssl_pkcs7_read($pkcs7PemSignature, $pemCertificates)) {
134+
return [];
135+
}
136+
137+
$chain = [];
138+
foreach ($pemCertificates as $index => $pemCertificate) {
139+
$parsed = openssl_x509_parse($pemCertificate);
140+
if ($parsed) {
141+
$parsed['signature_validation'] = [
142+
'id' => 1,
143+
'label' => $this->l10n->t('Signature is valid.'),
144+
];
145+
$chain[$index] = $parsed;
110146
}
147+
}
111148

112-
if (!isset($fromFallback['signingTime']) || !$fromFallback['signingTime'] instanceof \DateTime) {
113-
$certificates[$signerCounter]['signingTime'] = $tsa->getSigninTime($decoded);
149+
return $chain;
150+
}
151+
152+
private function enrichLeafWithPopplerData(array &$result, array $fromFallback): void {
153+
if (empty($fromFallback['chain'][0]) || empty($result['chain'][0])) {
154+
return;
155+
}
156+
157+
$popplerData = $fromFallback['chain'][0];
158+
$leafCert = &$result['chain'][0];
159+
160+
$popplerOnlyFields = ['field', 'range', 'certificate_validation'];
161+
foreach ($popplerOnlyFields as $field) {
162+
if (isset($popplerData[$field])) {
163+
$leafCert[$field] = $popplerData[$field];
114164
}
165+
}
115166

116-
$pkcs7PemSignature = $this->der2pem($signature);
117-
if (openssl_pkcs7_read($pkcs7PemSignature, $pemCertificates)) {
118-
foreach ($pemCertificates as $certificateIndex => $pemCertificate) {
119-
$parsed = openssl_x509_parse($pemCertificate);
120-
foreach ($parsed as $key => $value) {
121-
if (!isset($certificates[$signerCounter]['chain'][$certificateIndex][$key])) {
122-
$certificates[$signerCounter]['chain'][$certificateIndex][$key] = $value;
123-
} elseif ($key === 'name') {
124-
$certificates[$signerCounter]['chain'][$certificateIndex][$key] = $value;
125-
} elseif ($key === 'signatureTypeSN' && $certificates[$signerCounter]['chain'][$certificateIndex][$key] !== $value) {
126-
$certificates[$signerCounter]['chain'][$certificateIndex][$key] = $value;
127-
}
128-
}
129-
if (empty($certificates[$signerCounter]['chain'][$certificateIndex]['signature_validation'])) {
130-
$certificates[$signerCounter]['chain'][$certificateIndex]['signature_validation'] = [
131-
'id' => 1,
132-
'label' => $this->l10n->t('Signature is valid.'),
133-
];
134-
}
135-
}
136-
};
137-
$certificates[$signerCounter]['chain'] = $this->orderCertificates($certificates[$signerCounter]['chain']);
138-
$signerCounter++;
167+
if (isset($popplerData['signature_validation'])
168+
&& (!isset($leafCert['signature_validation']) || $leafCert['signature_validation']['id'] !== 1)) {
169+
$leafCert['signature_validation'] = $popplerData['signature_validation'];
139170
}
140-
return $certificates;
141171
}
142172

143173
private function popplerUtilsPdfSignFallback($resource, int $signerCounter): array {
@@ -167,7 +197,6 @@ private function popplerUtilsPdfSignFallback($resource, int $signerCounter): arr
167197
continue;
168198
}
169199

170-
171200
$match = [];
172201
$isSecondLevel = preg_match('/^\s+-\s(?<key>.+):\s(?<value>.*)/', $item, $match);
173202
if ($isSecondLevel) {

0 commit comments

Comments
 (0)