fix(crl): detect distribution points across extension key variants

Signed-off-by: Vitor Mattos <[email protected]>

Author: vitormattos

lib/Handler/CertificateEngine/AEngineHandler.php

@@ -182,23 +182,76 @@ private function parseX509(string $x509): array {
 	}
 
 	private function addCrlValidationInfo(array &$certData, string $certPem): void {
-		if (isset($certData['extensions']['crlDistributionPoints'])) {
-			preg_match_all('/URI:([^\s,\n]+)/', $certData['extensions']['crlDistributionPoints'], $matches);
-			$extractedUrls = $matches[1] ?? [];
-
-			$certData['crl_urls'] = $extractedUrls;
-			$crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem);
-			$certData['crl_validation'] = $crlDetails['status'];
-			if (!empty($crlDetails['revoked_at'])) {
-				$certData['crl_revoked_at'] = $crlDetails['revoked_at'];
+		$extensions = $certData['extensions'] ?? [];
+		if (is_array($extensions)) {
+			['hasExtension' => $hasCrlExtension, 'urls' => $extractedUrls] = $this->extractCrlUrlsFromExtensions($extensions);
+			if ($hasCrlExtension) {
+				$certData['crl_urls'] = $extractedUrls;
+				if (empty($extractedUrls)) {
+					$certData['crl_validation'] = CrlValidationStatus::NO_URLS;
+					return;
+				}
+
+				$crlDetails = $this->crlRevocationChecker->validate($extractedUrls, $certPem);
+				$certData['crl_validation'] = $crlDetails['status'];
+				if (!empty($crlDetails['revoked_at'])) {
+					$certData['crl_revoked_at'] = $crlDetails['revoked_at'];
+				}
+				return;
 			}
-		} else {
+		}
+
 			$externalValidationEnabled = $this->appConfig->getValueBool(Application::APP_ID, 'crl_external_validation_enabled', true);
 			$certData['crl_validation'] = $externalValidationEnabled
 				? CrlValidationStatus::MISSING
 				: CrlValidationStatus::DISABLED;
 			$certData['crl_urls'] = [];
+	}
+
+	/**
+	 * @return array{hasExtension: bool, urls: array<int, string>}
+	 */
+	private function extractCrlUrlsFromExtensions(array $extensions): array {
+		$values = [];
+		foreach ($extensions as $extensionName => $extensionValue) {
+			if (!is_string($extensionName)) {
+				continue;
+			}
+
+			$normalizedName = strtolower(trim($extensionName));
+			$isCrlDistributionPoints =
+				$normalizedName === 'crldistributionpoints'
+				|| $normalizedName === 'x509v3 crl distribution points'
+				|| $normalizedName === '2.5.29.31'
+				|| str_contains($normalizedName, 'crl distribution points');
+
+			if (!$isCrlDistributionPoints) {
+				continue;
+			}
+
+			if (is_string($extensionValue)) {
+				$values[] = $extensionValue;
+			} elseif (is_array($extensionValue)) {
+				$values[] = implode("\n", array_filter($extensionValue, 'is_string'));
+			}
+		}
+
+		if (empty($values)) {
+			return ['hasExtension' => false, 'urls' => []];
 		}
+
+		$urls = [];
+		foreach ($values as $value) {
+			preg_match_all('/URI\s*:\s*([^\s,\n]+)/i', $value, $matches);
+			if (!empty($matches[1])) {
+				$urls = [...$urls, ...$matches[1]];
+			}
+		}
+
+		return [
+			'hasExtension' => true,
+			'urls' => array_values(array_unique($urls)),
+		];
 	}
 
 	private static function convertArrayToUtf8($array) {

lib/Service/IdentifyMethod/SignatureMethod/Password.php

@@ -59,14 +59,6 @@ private function validateCertificateRevocation(array $certificateData): void {
 		if ($status === CrlValidationStatus::DISABLED) {
 			return;
 		}
-		// Backward compatibility for legacy certificates issued before CRL metadata existed.
-		if ($status === CrlValidationStatus::MISSING) {
-			$this->identifyService->getLogger()->warning('Signing allowed for certificate without revocation metadata', [
-				'status' => $status->value,
-				'signer_uid' => $this->userSession->getUser()?->getUID(),
-			]);
-			return;
-		}
 		$this->logRevocationBlockedSigning($status);
 		throw new LibresignException($this->getRevocationErrorMessage($status), 422);
 	}

tests/php/Unit/Handler/CertificateEngine/OpenSslHandlerTest.php

@@ -468,6 +468,34 @@ public function testUniqueSerialNumbers(): void {
 		$this->assertCount($numCertificates, array_unique($serialNumbers), 'All serial numbers should be unique');
 	}
 
+	public function testExtractCrlUrlsFromOidExtensionName(): void {
+		$handler = $this->getInstance();
+
+		$method = new \ReflectionMethod('OCA\\Libresign\\Handler\\CertificateEngine\\AEngineHandler', 'extractCrlUrlsFromExtensions');
+		$method->setAccessible(true);
+
+		$result = $method->invoke($handler, [
+			'2.5.29.31' => "Full Name:\nURI:https://example.org/crl/root.crl",
+		]);
+
+		$this->assertTrue($result['hasExtension']);
+		$this->assertSame(['https://example.org/crl/root.crl'], $result['urls']);
+	}
+
+	public function testExtractCrlUrlsFromX509LabelExtensionName(): void {
+		$handler = $this->getInstance();
+
+		$method = new \ReflectionMethod('OCA\\Libresign\\Handler\\CertificateEngine\\AEngineHandler', 'extractCrlUrlsFromExtensions');
+		$method->setAccessible(true);
+
+		$result = $method->invoke($handler, [
+			'X509v3 CRL Distribution Points' => "Full Name:\n URI : https://example.org/crl/issuer.crl",
+		]);
+
+		$this->assertTrue($result['hasExtension']);
+		$this->assertSame(['https://example.org/crl/issuer.crl'], $result['urls']);
+	}
+
 	public function testRealCertificateRevocationInCrl(): void {
 		$this->caIdentifierService->generateCaId('openssl');
 

tests/php/Unit/Service/IdentifyMethod/PasswordTest.php

@@ -290,7 +290,8 @@ public static function providerValidateToSignWithCertificateData(): array {
 					'validTo_time_t' => $futureTimestamp,
 					'crl_validation' => CrlValidationStatus::MISSING,
 				],
-				'shouldThrow' => false,
+				'shouldThrow' => true,
+				'expectedCode' => 422,
 			],
 			'revoked and expired certificate' => [
 				'certificateData' => [
@@ -305,7 +306,8 @@ public static function providerValidateToSignWithCertificateData(): array {
 					'validTo_time_t' => $futureTimestamp,
 					'crl_validation' => CrlValidationStatus::MISSING,
 				],
-				'shouldThrow' => false,
+				'shouldThrow' => true,
+				'expectedCode' => 422,
 			],
 			'valid certificate - old date but valid (1970s timestamp)' => [
 				'certificateData' => [