Skip to content

Commit 882ccab

Browse files
vitormattosvitormattos
authored
Merge pull request #7605 from LibreSign/fix/7597-missing-revocation-legacy-cert
fix: allow signing for legacy certificates missing CRL metadata (fixes #7597)
2 parents 7922186 + b450623 commit 882ccab

3 files changed

Lines changed: 277 additions & 15 deletions

File tree

‎lib/Handler/CertificateEngine/AEngineHandler.php‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCA\Libresign\Helper\MagicGetterSetterTrait;
1818
use OCA\Libresign\Service\CaIdentifierService;
1919
use OCA\Libresign\Service\CertificatePolicyService;
20+
use OCA\Libresign\Service\Crl\CrlDistributionPointsExtractor;
2021
use OCA\Libresign\Service\Crl\CrlRevocationChecker;
2122
use OCP\Files\AppData\IAppDataFactory;
2223
use OCP\Files\IAppData;
@@ -73,6 +74,7 @@ abstract class AEngineHandler implements IEngineHandler {
7374
protected string $certificate = '';
7475
protected string $currentCaId = '';
7576
protected IAppData $appData;
77+
private CrlDistributionPointsExtractor $crlDistributionPointsExtractor;
7678

7779
public function __construct(
7880
protected IConfig $config,
@@ -87,6 +89,7 @@ public function __construct(
8789
private CrlRevocationChecker $crlRevocationChecker,
8890
) {
8991
$this->appData = $appDataFactory->get('libresign');
92+
$this->crlDistributionPointsExtractor = new CrlDistributionPointsExtractor();
9093
}
9194

9295
protected function exportToPkcs12(
@@ -182,23 +185,30 @@ private function parseX509(string $x509): array {
182185
}
183186

184187
private function addCrlValidationInfo(array &$certData, string $certPem): void {
185-
if (isset($certData['extensions']['crlDistributionPoints'])) {
186-
preg_match_all('/URI:([^\s,\n]+)/', $certData['extensions']['crlDistributionPoints'], $matches);
187-
$extractedUrls = $matches[1] ?? [];
188-
189-
$certData['crl_urls'] = $extractedUrls;
190-
$crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem);
191-
$certData['crl_validation'] = $crlDetails['status'];
192-
if (!empty($crlDetails['revoked_at'])) {
193-
$certData['crl_revoked_at'] = $crlDetails['revoked_at'];
188+
$extensions = $certData['extensions'] ?? [];
189+
if (is_array($extensions)) {
190+
['hasExtension' => $hasCrlExtension, 'urls' => $extractedUrls] = $this->crlDistributionPointsExtractor->extractFromExtensions($extensions);
191+
if ($hasCrlExtension) {
192+
$certData['crl_urls'] = $extractedUrls;
193+
if (empty($extractedUrls)) {
194+
$certData['crl_validation'] = CrlValidationStatus::NO_URLS;
195+
return;
196+
}
197+
198+
$crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem);
199+
$certData['crl_validation'] = $crlDetails['status'];
200+
if (!empty($crlDetails['revoked_at'])) {
201+
$certData['crl_revoked_at'] = $crlDetails['revoked_at'];
202+
}
203+
return;
194204
}
195-
} else {
196-
$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
197-
$certData['crl_validation'] = $externalValidationEnabled
198-
? CrlValidationStatus::MISSING
199-
: CrlValidationStatus::DISABLED;
200-
$certData['crl_urls'] = [];
201205
}
206+
207+
$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
208+
$certData['crl_validation'] = $externalValidationEnabled
209+
? CrlValidationStatus::MISSING
210+
: CrlValidationStatus::DISABLED;
211+
$certData['crl_urls'] = [];
202212
}
203213

204214
private static function convertArrayToUtf8($array) {

‎lib/Service/Crl/CrlDistributionPointsExtractor.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Libresign\Service\Crl;
11+
12+
final class CrlDistributionPointsExtractor {
13+
/** @var array<string, true> */
14+
private const ACCEPTED_EXTENSION_NAMES = [
15+
'crldistributionpoints' => true,
16+
'x509v3 crl distribution points' => true,
17+
'2.5.29.31' => true,
18+
];
19+
20+
private const URI_PATTERN = '/URI\s*:\s*([^\s\n]+)/i';
21+
22+
/**
23+
* @param array<array-key, mixed> $extensions
24+
* @return array{hasExtension: bool, urls: list<string>}
25+
*/
26+
public function extractFromExtensions(array $extensions): array {
27+
$hasCrlExtension = false;
28+
$orderedUrls = [];
29+
$seenUrls = [];
30+
foreach ($extensions as $extensionName => $extensionValue) {
31+
if (!is_string($extensionName)) {
32+
continue;
33+
}
34+
35+
$normalizedName = strtolower(trim($extensionName));
36+
if (!isset(self::ACCEPTED_EXTENSION_NAMES[$normalizedName])) {
37+
continue;
38+
}
39+
$hasCrlExtension = true;
40+
41+
if (is_string($extensionValue)) {
42+
$this->appendUrlsFromText($extensionValue, $orderedUrls, $seenUrls);
43+
} elseif (is_array($extensionValue)) {
44+
foreach ($extensionValue as $extensionPart) {
45+
if (is_string($extensionPart)) {
46+
$this->appendUrlsFromText($extensionPart, $orderedUrls, $seenUrls);
47+
}
48+
}
49+
}
50+
}
51+
52+
if (!$hasCrlExtension) {
53+
return ['hasExtension' => false, 'urls' => []];
54+
}
55+
56+
return [
57+
'hasExtension' => true,
58+
'urls' => $orderedUrls,
59+
];
60+
}
61+
62+
/**
63+
* @param list<string> $orderedUrls
64+
* @param array<string, true> $seenUrls
65+
*/
66+
private function appendUrlsFromText(string $value, array &$orderedUrls, array &$seenUrls): void {
67+
if (stripos($value, 'URI') === false) {
68+
return;
69+
}
70+
71+
preg_match_all(self::URI_PATTERN, $value, $matches);
72+
if (empty($matches[1])) {
73+
return;
74+
}
75+
76+
foreach ($matches[1] as $url) {
77+
$normalizedUrl = $this->normalizeUrlToken($url);
78+
if ($normalizedUrl === '' || isset($seenUrls[$normalizedUrl])) {
79+
continue;
80+
}
81+
82+
$seenUrls[$normalizedUrl] = true;
83+
$orderedUrls[] = $normalizedUrl;
84+
}
85+
}
86+
87+
private function normalizeUrlToken(string $url): string {
88+
return rtrim($url, ')]');
89+
}
90+
}

‎tests/php/Unit/Service/Crl/CrlDistributionPointsExtractorTest.php‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 LibreCode coop and contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Libresign\Tests\Unit\Service\Crl;
11+
12+
use OCA\Libresign\Service\Crl\CrlDistributionPointsExtractor;
13+
use PHPUnit\Framework\Attributes\DataProvider;
14+
use PHPUnit\Framework\TestCase;
15+
16+
final class CrlDistributionPointsExtractorTest extends TestCase {
17+
private CrlDistributionPointsExtractor $extractor;
18+
19+
protected function setUp(): void {
20+
$this->extractor = new CrlDistributionPointsExtractor();
21+
}
22+
23+
#[DataProvider('crlDistributionPointExtractionProvider')]
24+
public function testExtractFromExtensions(array $extensions, bool $expectedHasExtension, array $expectedUrls): void {
25+
$result = $this->extractor->extractFromExtensions($extensions);
26+
27+
$this->assertSame($expectedHasExtension, $result['hasExtension']);
28+
$this->assertSame($expectedUrls, $result['urls']);
29+
}
30+
31+
/**
32+
* RFC 5280 4.2.1.13 defines cRLDistributionPoints as DistributionPointName
33+
* with URI represented in GeneralNames. Tests cover common OpenSSL textual
34+
* outputs for HTTP and LDAP URIs and multiple distribution points.
35+
*
36+
* @return array<string, array{0: array<string, mixed>, 1: bool, 2: list<string>}>
37+
*/
38+
public static function crlDistributionPointExtractionProvider(): array {
39+
return [
40+
'oid-extension-with-http-uri' => [
41+
[
42+
'2.5.29.31' => "Full Name:\nURI:https://example.org/crl/root.crl",
43+
],
44+
true,
45+
['https://example.org/crl/root.crl'],
46+
],
47+
'x509v3-label-with-http-uri' => [
48+
[
49+
'X509v3 CRL Distribution Points' => "Full Name:\n URI : https://example.org/crl/issuer.crl",
50+
],
51+
true,
52+
['https://example.org/crl/issuer.crl'],
53+
],
54+
'rfc-ldap-uri-with-dn-and-query' => [
55+
[
56+
'crlDistributionPoints' => "Full Name:\nURI:ldap://ldap.example.com/cn=Example%20CA,ou=PKI,dc=example,dc=com?certificateRevocationList;binary",
57+
],
58+
true,
59+
['ldap://ldap.example.com/cn=Example%20CA,ou=PKI,dc=example,dc=com?certificateRevocationList;binary'],
60+
],
61+
'multiple-distribution-points-in-single-extension' => [
62+
[
63+
'2.5.29.31' => "Full Name:\nURI:https://pki.example.org/root.crl\nFull Name:\nURI:ldap://ldap.example.org/cn=RootCA,dc=example,dc=org?certificateRevocationList;binary",
64+
],
65+
true,
66+
[
67+
'https://pki.example.org/root.crl',
68+
'ldap://ldap.example.org/cn=RootCA,dc=example,dc=org?certificateRevocationList;binary',
69+
],
70+
],
71+
'rfc-structure-with-reasons-and-crl-issuer' => [
72+
[
73+
'2.5.29.31' => "Full Name:\n URI:http://crl.example.org/root.crl\nReasons: keyCompromise, cACompromise\nCRL Issuer:\n DirName:/C=BR/O=Example/CN=Example CRL Issuer",
74+
],
75+
true,
76+
['http://crl.example.org/root.crl'],
77+
],
78+
'extension-name-is-trimmed-and-case-insensitive' => [
79+
[
80+
' X509V3 CRL Distribution Points ' => "Full Name:\n URI:https://example.org/crl/mixed-case.crl",
81+
],
82+
true,
83+
['https://example.org/crl/mixed-case.crl'],
84+
],
85+
'uri-token-is-case-insensitive' => [
86+
[
87+
'2.5.29.31' => "Full Name:\nuri:ldap://ldap.example.net/cn=CA,dc=example,dc=net?certificateRevocationList;binary",
88+
],
89+
true,
90+
['ldap://ldap.example.net/cn=CA,dc=example,dc=net?certificateRevocationList;binary'],
91+
],
92+
'uri-with-tabs-and-extra-whitespace' => [
93+
[
94+
'2.5.29.31' => "Full Name:\n\tURI\t:\t https://example.org/crl/with-tabs.crl",
95+
],
96+
true,
97+
['https://example.org/crl/with-tabs.crl'],
98+
],
99+
'uri-line-with-closing-parenthesis-from-formatted-output' => [
100+
[
101+
'2.5.29.31' => "Distribution Point (1):\nURI:https://example.org/crl/formatted.crl)",
102+
],
103+
true,
104+
['https://example.org/crl/formatted.crl'],
105+
],
106+
'multiple-supported-extension-keys-are-merged-and-deduplicated' => [
107+
[
108+
'2.5.29.31' => "Full Name:\nURI:https://example.org/crl/shared.crl",
109+
'crlDistributionPoints' => "Full Name:\nURI:https://example.org/crl/shared.crl\nURI:https://example.org/crl/extra.crl",
110+
],
111+
true,
112+
[
113+
'https://example.org/crl/shared.crl',
114+
'https://example.org/crl/extra.crl',
115+
],
116+
],
117+
'array-extension-value-and-duplicates' => [
118+
[
119+
'2.5.29.31' => [
120+
'Full Name:',
121+
'URI:https://example.org/crl/root.crl',
122+
'URI:https://example.org/crl/root.crl',
123+
],
124+
],
125+
true,
126+
['https://example.org/crl/root.crl'],
127+
],
128+
'known-extension-without-uri' => [
129+
[
130+
'2.5.29.31' => 'Distribution Point Name: relativeName=CN=DP1',
131+
],
132+
true,
133+
[],
134+
],
135+
'known-extension-with-general-names-but-no-uri' => [
136+
[
137+
'X509v3 CRL Distribution Points' => "Full Name:\nDNS:crl.example.org\nDirName:/C=BR/O=Example/CN=CRL Directory",
138+
],
139+
true,
140+
[],
141+
],
142+
'multiple-supported-keys-preserve-first-seen-order' => [
143+
[
144+
'crlDistributionPoints' => "Full Name:\nURI:https://example.org/crl/first.crl",
145+
'2.5.29.31' => "Full Name:\nURI:https://example.org/crl/second.crl",
146+
],
147+
true,
148+
[
149+
'https://example.org/crl/first.crl',
150+
'https://example.org/crl/second.crl',
151+
],
152+
],
153+
'unknown-extension-name-should-not-match' => [
154+
[
155+
'Issuer CRL Distribution Points' => "Full Name:\nURI:https://example.org/crl/issuer.crl",
156+
],
157+
false,
158+
[],
159+
],
160+
];
161+
}
162+
}

0 commit comments

Comments
 (0)