From e9ec33cafc265fd58854081ef152c88e42f381a8 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 20:58:50 -0300 Subject: [PATCH 01/21] feat: integrate pdf-signature-validator for native validation Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- composer.json | 10 + composer.lock | 86 +++++- lib/Handler/SignEngine/Pkcs12Handler.php | 241 ++++++----------- lib/Service/Install/ConfigureCheckService.php | 42 +-- .../PdfSignatureValidationService.php | 253 ++++++++++++++++++ .../Handler/SignEngine/Pkcs12HandlerTest.php | 94 ++++++- .../PdfSignatureValidationServiceTest.php | 62 +++++ 7 files changed, 584 insertions(+), 204 deletions(-) create mode 100644 lib/Service/Signature/PdfSignatureValidationService.php create mode 100644 tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php diff --git a/composer.json b/composer.json index 085c2a5985..0ac7dcd2a0 100644 --- a/composer.json +++ b/composer.json @@ -1,4 +1,13 @@ { + "repositories": [ + { + "type": "path", + "url": "../pdf-signature-validator", + "options": { + "symlink": true + } + } + ], "require-dev": { "bamarni/composer-bin-plugin": "^1.8", "nextcloud/ocp": "dev-master", @@ -63,6 +72,7 @@ "require": { "cweagans/composer-patches": "^2.0", "jeidison/signer-php": "^1.0", + "libresign/pdf-signature-validator": "*@dev", "phpseclib/phpseclib": "^3.0" } } diff --git a/composer.lock b/composer.lock index d4a85967d6..502fcd7ebd 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "683fc6c9ae20a480af7b531acfea05bb", + "content-hash": "ecf8085cd1193a30abd69ef25963e320", "packages": [ { "name": "cweagans/composer-configurable-plugin", @@ -198,6 +198,89 @@ }, "time": "2026-03-17T00:40:40+00:00" }, + { + "name": "libresign/pdf-signature-validator", + "version": "dev-backport/7525/stable33-manual", + "dist": { + "type": "path", + "url": "../pdf-signature-validator", + "reference": "f8eba5ac5e43315f3856645e46e95eb21be4f742" + }, + "require": { + "php": "^8.2", + "phpseclib/phpseclib": "^3.0" + }, + "require-dev": { + "bamarni/composer-bin-plugin": "^1.8", + "roave/security-advisories": "dev-latest" + }, + "type": "library", + "extra": { + "bamarni-bin": { + "bin-links": true, + "forward-command": true + } + }, + "autoload": { + "psr-4": { + "LibreSign\\PdfSignatureValidator\\": "src/" + } + }, + "autoload-dev": { + "psr-4": { + "LibreSign\\PdfSignatureValidator\\Tests\\": "tests/" + } + }, + "scripts": { + "bin": [ + "echo 'bin not installed'" + ], + "lint": [ + "find src tests -name '*.php' -print0 | xargs -0 -n1 php -l" + ], + "cs:check": [ + "php-cs-fixer fix --dry-run --diff" + ], + "cs:fix": [ + "php-cs-fixer fix" + ], + "phpmd": [ + "phpmd src,tests text phpmd.xml" + ], + "psalm": [ + "psalm --no-cache --threads=$(nproc)" + ], + "rector:check": [ + "rector --dry-run" + ], + "rector:fix": [ + "rector" + ], + "test:unit": [ + "phpunit --colors=always --fail-on-warning --fail-on-risky" + ], + "test:coverage": [ + "XDEBUG_MODE=coverage phpunit --coverage-text" + ], + "test:mutation": [ + "infection --threads=$(nproc) --skip-initial-tests" + ], + "post-install-cmd": [ + "@composer bin all install --ansi" + ], + "post-update-cmd": [ + "@composer bin all install --ansi" + ] + }, + "license": [ + "AGPL-3.0-or-later" + ], + "description": "High-quality PDF signature extraction and validation primitives for LibreSign and external consumers.", + "transport-options": { + "symlink": true, + "relative": true + } + }, { "name": "paragonie/constant_time_encoding", "version": "v3.1.3", @@ -1902,6 +1985,7 @@ "aliases": [], "minimum-stability": "stable", "stability-flags": { + "libresign/pdf-signature-validator": 20, "nextcloud/ocp": 20, "roave/security-advisories": 20 }, diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index c9335f3682..ab2832c8bd 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -8,7 +8,7 @@ namespace OCA\Libresign\Handler\SignEngine; -use DateTime; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; @@ -18,17 +18,16 @@ use OCA\Libresign\Service\CaIdentifierService; use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCP\Files\File; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use phpseclib3\File\ASN1; use Psr\Log\LoggerInterface; class Pkcs12Handler extends SignEngineHandler { use OrderCertificatesTrait; protected string $certificate = ''; - private array $signaturesFromPoppler = []; private ?JSignPdfHandler $jSignPdfHandler = null; private ?PhpNativeHandler $phpNativeHandler = null; private string $rootCertificatePem = ''; @@ -40,11 +39,12 @@ public function __construct( protected CertificateEngineFactory $certificateEngineFactory, private IL10N $l10n, private FooterHandler $footerHandler, - private ITempManager $tempManager, private LoggerInterface $logger, private CaIdentifierService $caIdentifierService, private DocMdpHandler $docMdpHandler, private CrlService $crlService, + private PdfSignatureValidationService $pdfSignatureValidationService, + private PdfSignatureExtractor $pdfSignatureExtractor, ) { parent::__construct($l10n, $folderService, $logger); } @@ -92,28 +92,40 @@ public function setIsLibreSignFile(): void { #[\Override] public function getCertificateChain($resource): array { $certificates = []; + $nativeMetadata = array_values($this->extractNativeSignatureMetadata($resource)); + $nativeValidation = array_values($this->pdfSignatureValidationService->validateFromResource($resource)); + $index = 0; foreach ($this->getSignatures($resource) as $signature) { if (!$signature) { continue; } - $result = $this->processSignature($resource, $signature); + $result = $this->processSignature( + $resource, + $signature, + $nativeMetadata[$index] ?? ($nativeMetadata[0] ?? []), + $nativeValidation[$index] ?? ($nativeValidation[0] ?? []) + ); if (empty($result['chain'])) { continue; } $certificates[] = $result; + $index++; } return $certificates; } - private function processSignature($resource, ?string $signature): array { + private function processSignature($resource, ?string $signature, array $metadata = [], array $validation = []): array { $result = []; if (!$signature) { - $result['chain'][0]['signature_validation'] = $this->getReadableSigState('Digest Mismatch.'); + $result['chain'][0]['signature_validation'] = [ + 'id' => 3, + 'label' => $this->l10n->t('Digest mismatch.'), + ]; return $result; } @@ -123,7 +135,7 @@ private function processSignature($resource, ?string $signature): array { $chain = $this->extractCertificateChain($signature); if (!empty($chain)) { $result['chain'] = $this->orderCertificates($chain); - $result = $this->enrichLeafWithPopplerData($resource, $result); + $result = $this->enrichLeafWithNativeData($result, $metadata, $validation); } $result = $this->extractDocMdpData($resource, $result); @@ -275,149 +287,57 @@ private function getRootCertificatePem(): string { return $this->rootCertificatePem; } - private function enrichLeafWithPopplerData($resource, array $result): array { + private function enrichLeafWithNativeData(array $result, array $metadata, array $validation): array { if (empty($result['chain'])) { return $result; } - $popplerOnlyFields = ['field', 'range', 'certificate_validation']; - if (!isset($result['chain'][0]['subject'])) { - return $result; - } - $needPoppler = false; - foreach ($popplerOnlyFields as $field) { - if (empty($result['chain'][0][$field])) { - $needPoppler = true; - break; - } - } - if (!isset($result['chain'][0]['signature_validation']) || $result['chain'][0]['signature_validation']['id'] !== 1) { - $needPoppler = true; - } - if (!$needPoppler) { - return $result; - } - $popplerChain = $this->chainFromPoppler($result['chain'][0]['subject'], $resource); - if (empty($popplerChain)) { - return $result; - } - foreach ($popplerOnlyFields as $field) { - if (isset($popplerChain[$field])) { - $result['chain'][0][$field] = $popplerChain[$field]; - } - } - if (!isset($result['chain'][0]['signature_validation']) || $result['chain'][0]['signature_validation']['id'] !== 1) { - if (isset($popplerChain['signature_validation'])) { - $result['chain'][0]['signature_validation'] = $popplerChain['signature_validation']; - } - } - return $result; - } + $leaf = &$result['chain'][0]; - private function chainFromPoppler(array $subject, $resource): array { - $fromFallback = $this->popplerUtilsPdfSignFallback($resource); - foreach ($fromFallback as $popplerSig) { - if (!isset($popplerSig['chain'][0]['subject'])) { - continue; - } - if ($popplerSig['chain'][0]['subject'] == $subject) { - return $popplerSig['chain'][0]; + foreach (['field', 'range', 'signature_type', 'signing_hash_algorithm', 'covers_entire_document'] as $key) { + if (array_key_exists($key, $metadata)) { + $leaf[$key] = $metadata[$key]; } } - return []; - } - private function popplerUtilsPdfSignFallback($resource): array { - if (!empty($this->signaturesFromPoppler)) { - return $this->signaturesFromPoppler; - } - if (shell_exec('which pdfsig') === null) { - return $this->signaturesFromPoppler; + if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { + $leaf['signature_validation'] = $this->getLocalizedSignatureValidation($validation['signatureValidation']); } - rewind($resource); - $content = stream_get_contents($resource); - $tempFile = $this->tempManager->getTemporaryFile('file.pdf'); - file_put_contents($tempFile, $content); - $content = shell_exec('env TZ=UTC pdfsig ' . $tempFile); - if (empty($content)) { - return $this->signaturesFromPoppler; + if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { + $leaf['certificate_validation'] = $this->getLocalizedCertificateValidation($validation['certificateValidation']); } - $lines = explode("\n", $content); - $lastSignature = 0; - foreach ($lines as $item) { - $isFirstLevel = preg_match('/^Signature\s#(\d)/', $item, $match); - if ($isFirstLevel) { - $lastSignature = (int)$match[1] - 1; - $this->signaturesFromPoppler[$lastSignature] = []; - continue; - } - - $match = []; - $isSecondLevel = preg_match('/^\s+-\s(?.+):\s(?.*)/', $item, $match); - if ($isSecondLevel) { - switch ((string)$match['key']) { - case 'Signing Time': - $this->signaturesFromPoppler[$lastSignature]['signingTime'] = DateTime::createFromFormat('M d Y H:i:s', $match['value'], new \DateTimeZone('UTC')); - break; - case 'Signer full Distinguished Name': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['subject'] = $this->parseDistinguishedNameWithMultipleValues($match['value']); - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['name'] = $match['value']; - break; - case 'Signing Hash Algorithm': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['signatureTypeSN'] = $match['value']; - break; - case 'Signature Validation': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['signature_validation'] = $this->getReadableSigState($match['value']); - break; - case 'Certificate Validation': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['certificate_validation'] = $this->getReadableCertState($match['value']); - break; - case 'Signed Ranges': - if (preg_match('/\[(\d+) - (\d+)\], \[(\d+) - (\d+)\]/', $match['value'], $ranges)) { - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['range'] = [ - 'offset1' => (int)$ranges[1], - 'length1' => (int)$ranges[2], - 'offset2' => (int)$ranges[3], - 'length2' => (int)$ranges[4], - ]; - } - break; - case 'Signature Field Name': - $this->signaturesFromPoppler[$lastSignature]['chain'][0]['field'] = $match['value']; - break; - case 'Signature Validation': - case 'Signature Type': - case 'Total document signed': - case 'Not total document signed': - default: - break; - } - } + if (!isset($leaf['certificate_validation'])) { + $leaf['certificate_validation'] = [ + 'id' => 3, + 'label' => $this->l10n->t('Certificate issuer is unknown.'), + ]; } - return $this->signaturesFromPoppler; + + return $result; } - private function getReadableSigState(string $status) { - return match ($status) { - 'Signature is Valid.' => [ + /** + * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. + */ + private function getLocalizedSignatureValidation(array $validation): array { + $id = (int)($validation['id'] ?? 6); + + return match ($id) { + 1 => [ 'id' => 1, 'label' => $this->l10n->t('Signature is valid.'), ], - 'Signature is Invalid.' => [ + 2 => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), ], - 'Digest Mismatch.' => [ + 3 => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), ], - "Document isn't signed or corrupted data." => [ - 'id' => 4, - 'label' => $this->l10n->t("Document isn't signed or corrupted data."), - ], - 'Signature has not yet been verified.' => [ + 5 => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), ], @@ -428,65 +348,72 @@ private function getReadableSigState(string $status) { }; } + /** + * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. + */ + private function getLocalizedCertificateValidation(array $validation): array { + $id = (int)($validation['id'] ?? 7); - private function getReadableCertState(string $status) { - return match ($status) { - 'Certificate is Trusted.' => [ + return match ($id) { + 1 => [ 'id' => 1, 'label' => $this->l10n->t('Certificate is trusted.'), ], - "Certificate issuer isn't Trusted." => [ + 2 => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), ], - 'Certificate issuer is unknown.' => [ + 3 => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), ], - 'Certificate has been Revoked.' => [ + 4 => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), ], - 'Certificate has Expired' => [ + 5 => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired'), ], - 'Certificate has not yet been verified.' => [ + 6 => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), ], default => [ 'id' => 7, - 'label' => $this->l10n->t('Unknown issue with Certificate or corrupted data.') + 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), ], }; } + /** + * @param resource $resource + * @return list + */ + private function extractNativeSignatureMetadata($resource): array { + rewind($resource); + $content = stream_get_contents($resource); + if (!is_string($content) || $content === '') { + return []; + } - private function parseDistinguishedNameWithMultipleValues(string $dn): array { - $result = []; - $pairs = preg_split('/,(?=(?:[^"]*"[^"]*")*[^"]*$)/', $dn); + try { + $signatures = $this->pdfSignatureExtractor->extractFromString($content); + } catch (\Throwable) { + return []; + } + $metadata = []; - foreach ($pairs as $pair) { - [$key, $value] = explode('=', $pair, 2); - if (empty($key) || empty($value)) { - return $result; - } - $key = trim($key); - $value = trim($value); - $value = trim($value, '"'); - - if (!isset($result[$key])) { - $result[$key] = $value; - } else { - if (!is_array($result[$key])) { - $result[$key] = [$result[$key]]; - } - $result[$key][] = $value; - } + foreach ($signatures as $index => $signature) { + $metadata[$index] = [ + 'field' => $signature->metadata->field, + 'range' => $signature->metadata->range, + 'signature_type' => $signature->metadata->signatureType, + 'covers_entire_document' => $signature->metadata->coversEntireDocument, + ]; } - return $result; + return $metadata; } private function der2pem($derData) { diff --git a/lib/Service/Install/ConfigureCheckService.php b/lib/Service/Install/ConfigureCheckService.php index d4bf39a243..442c81fb78 100644 --- a/lib/Service/Install/ConfigureCheckService.php +++ b/lib/Service/Install/ConfigureCheckService.php @@ -74,47 +74,7 @@ public function checkSign(): array { } public function checkPoppler(): array { - $return = $this->checkPdfSig(); - $return = array_merge($return, $this->checkPdfinfo()); - return $return; - } - - public function checkPdfSig(): array { - if (!empty($this->result['poppler'])) { - return $this->result['poppler']; - } - // The output of this command go to STDERR and exec get the STDOUT - // With 2>&1 the STRERR is redirected to STDOUT - exec('pdfsig -v 2>&1', $version, $retval); - if ($retval !== 0) { - return $this->result['poppler'] = [ - (new ConfigureCheckHelper()) - ->setInfoMessage('Poppler utils not installed') - ->setResource('pdfsig') - ->setTip('Install the package poppler-utils at your operational system to be possible get more details about validation of signatures.'), - ]; - } - if (!$version) { - return $this->result['poppler'] = [ - (new ConfigureCheckHelper()) - ->setErrorMessage('Fail to retrieve pdfsig version') - ->setResource('pdfsig') - ->setTip("The command executed by PHP haven't any output."), - ]; - } - $returnValue = preg_match('/pdfsig version (?.*)/', implode(PHP_EOL, $version), $matches); - if ($returnValue !== 1) { - return $this->result['poppler'] = [ - (new ConfigureCheckHelper()) - ->setErrorMessage('Fail to retrieve pdfsig version') - ->setResource('pdfsig') - ->setTip("This is a poppler-utils dependency and wasn't possible to parse the output of command pdfsig -v"), - ]; - } - return $this->result['poppler'] = [(new ConfigureCheckHelper()) - ->setSuccessMessage('pdfsig version: ' . $matches['version']) - ->setResource('pdfsig') - ]; + return $this->checkPdfinfo(); } public function checkPdfinfo(): array { diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php new file mode 100644 index 0000000000..c90d62741b --- /dev/null +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -0,0 +1,253 @@ +validator = new PdfSignatureValidator(); + $this->loadLibreSignCaCertificate(); + } + + /** + * Load LibreSign CA certificate and set it as trusted root. + */ + private function loadLibreSignCaCertificate(): void { + // Try to load from config path first + $configPath = $this->appConfig->getValueString(Application::APP_ID, 'config_path'); + if (!empty($configPath) && is_dir($configPath)) { + $caPemPath = $configPath . DIRECTORY_SEPARATOR . 'ca.pem'; + if (is_readable($caPemPath)) { + $cert = @file_get_contents($caPemPath); + if ($cert !== false) { + $this->libresignCaCertificate = $cert; + $this->validator->addTrustedRoot($cert); + return; + } + } + } + + // Try alternate location + $alternateConfig = $this->appConfig->getValueString( + Application::APP_ID, + 'libresign_ca_certificate' + ); + if (!empty($alternateConfig)) { + $this->libresignCaCertificate = $alternateConfig; + $this->validator->addTrustedRoot($alternateConfig); + } + } + + /** + * Add additional trusted root certificate. + */ + public function addTrustedRoot(string $certificatePem): void { + $this->validator->addTrustedRoot($certificatePem); + } + + /** + * Set multiple trusted root certificates. + * + * @param list $certificates + */ + public function setTrustedRoots(array $certificates): void { + $this->validator->setTrustedRoots($certificates); + } + + /** + * Validate PDF signatures from file resource. + * + * @param resource $resource PDF file resource + * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) + * @return list + */ + public function validateFromResource($resource, ?DateTime $signatureTime = null): array { + try { + $results = $this->validator->validateFromResource($resource); + return $this->mapValidationResults($results, $signatureTime); + } catch (\Throwable $e) { + $this->logger->warning('PDF signature validation failed', [ + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); + return []; + } + } + + /** + * Validate PDF signatures from binary content. + * + * @param string $pdfContent Binary PDF content + * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) + * @return list + */ + public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { + try { + $results = $this->validator->validateFromString($pdfContent); + return $this->mapValidationResults($results, $signatureTime); + } catch (\Throwable $e) { + $this->logger->warning('PDF signature validation failed', [ + 'error' => $e->getMessage(), + 'trace' => $e->getTraceAsString(), + ]); + return []; + } + } + + /** + * Map validation results from PdfSignatureValidator to LibreSign format. + * + * @param list $results Results from PdfSignatureValidator + * @param ?\DateTime $signatureTime + * @return list + */ + private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { + $mapped = []; + + foreach ($results as $result) { + $sigValidation = $result['signatureValidation'] ?? null; + $certValidation = $result['certificateValidation'] ?? null; + + if (!$sigValidation instanceof ValidationResult || !$certValidation instanceof ValidationResult) { + continue; + } + + $mapped[] = [ + 'signatureValidation' => $this->mapSignatureValidation($sigValidation), + 'certificateValidation' => $this->mapCertificateValidation($certValidation), + 'raw' => [ + 'signature' => $sigValidation, + 'certificate' => $certValidation, + ], + ]; + } + + return $mapped; + } + + /** + * Map signature validation result to LibreSign format. + */ + private function mapSignatureValidation(ValidationResult $result): array { + return match ($result->state) { + ValidationState::SIGNATURE_VALID => [ + 'id' => 1, + 'label' => 'Signature is valid.', + 'isValid' => true, + ], + ValidationState::SIGNATURE_INVALID => [ + 'id' => 2, + 'label' => 'Signature is invalid.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::DIGEST_MISMATCH => [ + 'id' => 3, + 'label' => 'Digest mismatch.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::NOT_VERIFIED => [ + 'id' => 5, + 'label' => 'Signature has not yet been verified.', + 'reason' => $result->reason, + 'isValid' => false, + ], + default => [ + 'id' => 6, + 'label' => 'Unknown validation failure.', + 'reason' => $result->reason, + 'isValid' => false, + ], + }; + } + + /** + * Map certificate validation result to LibreSign format. + */ + private function mapCertificateValidation(ValidationResult $result): array { + return match ($result->state) { + ValidationState::CERT_TRUSTED => [ + 'id' => 1, + 'label' => 'Certificate is trusted.', + 'isValid' => true, + ], + ValidationState::CERT_ISSUER_NOT_TRUSTED => [ + 'id' => 2, + 'label' => "Certificate issuer isn't trusted.", + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_ISSUER_UNKNOWN => [ + 'id' => 3, + 'label' => 'Certificate issuer is unknown.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_REVOKED => [ + 'id' => 4, + 'label' => 'Certificate has been revoked.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_EXPIRED => [ + 'id' => 5, + 'label' => 'Certificate has expired.', + 'reason' => $result->reason, + 'isValid' => false, + ], + ValidationState::CERT_NOT_VERIFIED => [ + 'id' => 6, + 'label' => 'Certificate has not yet been verified.', + 'reason' => $result->reason, + 'isValid' => false, + ], + default => [ + 'id' => 7, + 'label' => 'Unknown issue with certificate or corrupted data.', + 'reason' => $result->reason, + 'isValid' => false, + ], + }; + } + + /** + * Check if LibreSign CA is loaded. + */ + public function isLibreSignCaLoaded(): bool { + return !empty($this->libresignCaCertificate); + } + + /** + * Get LibreSign CA certificate (PEM format). + */ + public function getLibreSignCaCertificate(): string { + return $this->libresignCaCertificate; + } +} diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index 0d867d6193..31a8ca3193 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -9,6 +9,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; use OCA\Libresign\Handler\DocMdpHandler; @@ -17,12 +18,12 @@ use OCA\Libresign\Service\CaIdentifierService; use OCA\Libresign\Service\Crl\CrlService; use OCA\Libresign\Service\FolderService; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Fixtures\PdfFixtureCatalog; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use OCP\L10N\IFactory as IL10NFactory; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -33,12 +34,13 @@ final class Pkcs12HandlerTest extends \OCA\Libresign\Tests\Unit\TestCase { private IAppConfig $appConfig; private IL10N $l10n; private FooterHandler&MockObject $footerHandler; - private ITempManager $tempManager; private LoggerInterface&MockObject $logger; private CertificateEngineFactory&MockObject $certificateEngineFactory; private CaIdentifierService&MockObject $caIdentifierService; private DocMdpHandler&MockObject $docMdpHandler; private CrlService&MockObject $crlService; + private PdfSignatureValidationService&MockObject $pdfSignatureValidationService; + private PdfSignatureExtractor $pdfSignatureExtractor; public function setUp(): void { $this->folderService = $this->createMock(FolderService::class); @@ -46,11 +48,13 @@ public function setUp(): void { $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID); $this->footerHandler = $this->createMock(FooterHandler::class); - $this->tempManager = \OCP\Server::get(ITempManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); $this->docMdpHandler = $this->createMock(DocMdpHandler::class); $this->crlService = $this->createMock(CrlService::class); + $this->pdfSignatureValidationService = $this->createMock(PdfSignatureValidationService::class); + $this->pdfSignatureValidationService->method('validateFromResource')->willReturn([]); + $this->pdfSignatureExtractor = new PdfSignatureExtractor(); } private function getHandler(array $methods = []): Pkcs12Handler|MockObject { @@ -62,11 +66,12 @@ private function getHandler(array $methods = []): Pkcs12Handler|MockObject { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ]) ->onlyMethods($methods) ->getMock(); @@ -77,11 +82,12 @@ private function getHandler(array $methods = []): Pkcs12Handler|MockObject { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ); } @@ -380,4 +386,82 @@ public function testDocMdpPdfsExtraction(): void { } } + public function testPackageExtractorParsesFieldAndRange(): void { + $content = file_get_contents(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf'); + $this->assertIsString($content); + + $signatures = $this->pdfSignatureExtractor->extractFromString($content); + $this->assertCount(1, $signatures); + + $metadata = $signatures[0]->metadata; + $this->assertSame('Signature1', $metadata->field); + $this->assertSame([ + 'offset1' => 0, + 'length1' => 1311, + 'offset2' => 31313, + 'length2' => 32829, + ], $metadata->range); + } + + public function testGetCertificateChainProvidesNativePackageShape(): void { + $this->pdfSignatureValidationService->method('validateFromResource') + ->willReturn([ + [ + 'signatureValidation' => ['id' => 1, 'label' => 'Signature is valid.'], + 'certificateValidation' => ['id' => 3, 'label' => 'Certificate issuer is unknown.'], + ], + ]); + + $handler = $this->getHandler(); + $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); + $this->assertIsResource($resource); + + $result = $handler->getCertificateChain($resource); + fclose($resource); + + $this->assertCount(1, $result); + $this->assertArrayHasKey('signingTime', $result[0]); + $this->assertInstanceOf(\DateTime::class, $result[0]['signingTime']); + + $this->assertArrayHasKey('chain', $result[0]); + $this->assertNotEmpty($result[0]['chain']); + + $leaf = $result[0]['chain'][0]; + $this->assertArrayHasKey('field', $leaf); + $this->assertEquals('Signature1', $leaf['field']); + $this->assertArrayHasKey('range', $leaf); + $this->assertSame([ + 'offset1' => 0, + 'length1' => 1311, + 'offset2' => 31313, + 'length2' => 32829, + ], $leaf['range']); + + $this->assertArrayHasKey('signature_validation', $leaf); + $this->assertEquals(1, $leaf['signature_validation']['id']); + + $this->assertArrayHasKey('certificate_validation', $leaf); + $this->assertSame(3, $leaf['certificate_validation']['id']); + $this->assertArrayHasKey('signature_type', $leaf); + $this->assertNotEmpty($leaf['signature_type']); + $this->assertArrayHasKey('covers_entire_document', $leaf); + $this->assertIsBool($leaf['covers_entire_document']); + } + + public function testGetCertificateChainUsesNativeValidationServiceForEachSignature(): void { + $this->pdfSignatureValidationService->expects($this->once()) + ->method('validateFromResource'); + + $handler = $this->getHandler(); + $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); + $this->assertIsResource($resource); + + $result = $handler->getCertificateChain($resource); + fclose($resource); + + $this->assertNotEmpty($result); + $this->assertSame(1, $result[0]['chain'][0]['signature_validation']['id']); + $this->assertSame(3, $result[0]['chain'][0]['certificate_validation']['id']); + } + } diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php new file mode 100644 index 0000000000..28aa701023 --- /dev/null +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -0,0 +1,62 @@ +newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'hash mismatch') + ); + + $this->assertSame(3, $result['id']); + $this->assertSame('Digest mismatch.', $result['label']); + $this->assertSame('hash mismatch', $result['reason']); + $this->assertFalse($result['isValid']); + } + + public function testMapCertificateValidationWithEnumState(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapCertificateValidation', + new ValidationResult(ValidationState::CERT_TRUSTED) + ); + + $this->assertSame(1, $result['id']); + $this->assertSame('Certificate is trusted.', $result['label']); + $this->assertTrue($result['isValid']); + } + + private function newServiceWithoutConstructor(): PdfSignatureValidationService { + $reflection = new \ReflectionClass(PdfSignatureValidationService::class); + /** @var PdfSignatureValidationService $service */ + $service = $reflection->newInstanceWithoutConstructor(); + return $service; + } + + /** + * @return array + */ + private function invokePrivateMethod(PdfSignatureValidationService $service, string $method, ValidationResult $result): array { + $reflection = new \ReflectionClass($service); + $target = $reflection->getMethod($method); + $target->setAccessible(true); + /** @var array $mapped */ + $mapped = $target->invoke($service, $result); + return $mapped; + } +} From e9094345eb1d6b44fdac6ee909f3b44db3aea973 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:00:59 -0300 Subject: [PATCH 02/21] refactor: translate signature validation messages via l10n Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 54 +++++++++++-------- .../PdfSignatureValidationServiceTest.php | 17 ++++++ 2 files changed, 49 insertions(+), 22 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index c90d62741b..a45498cb3a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -14,6 +14,7 @@ use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; +use OCP\IL10N; use Psr\Log\LoggerInterface; /** @@ -29,6 +30,7 @@ class PdfSignatureValidationService { public function __construct( private IAppConfig $appConfig, + private IL10N $l10n, private LoggerInterface $logger, ) { $this->validator = new PdfSignatureValidator(); @@ -158,31 +160,31 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, - 'label' => 'Signature is valid.', + 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, - 'label' => 'Signature is invalid.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Signature is invalid.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, - 'label' => 'Digest mismatch.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Digest mismatch.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, - 'label' => 'Signature has not yet been verified.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Signature has not yet been verified.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, - 'label' => 'Unknown validation failure.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Unknown validation failure.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], }; @@ -195,48 +197,56 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, - 'label' => 'Certificate is trusted.', + 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, - 'label' => "Certificate issuer isn't trusted.", - 'reason' => $result->reason, + 'label' => $this->l10n->t("Certificate issuer isn't trusted."), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, - 'label' => 'Certificate issuer is unknown.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate issuer is unknown.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, - 'label' => 'Certificate has been revoked.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has been revoked.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, - 'label' => 'Certificate has expired.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has expired.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, - 'label' => 'Certificate has not yet been verified.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Certificate has not yet been verified.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, - 'label' => 'Unknown issue with certificate or corrupted data.', - 'reason' => $result->reason, + 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), + 'reason' => $this->translateReason($result->reason), 'isValid' => false, ], }; } + private function translateReason(?string $reason): ?string { + if ($reason === null || $reason === '') { + return $reason; + } + + return $this->l10n->t($reason); + } + /** * Check if LibreSign CA is loaded. */ diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 28aa701023..1b5a8ee408 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -12,8 +12,20 @@ use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; use OCA\Libresign\Tests\Unit\TestCase; +use OCP\IL10N; +use PHPUnit\Framework\MockObject\MockObject; final class PdfSignatureValidationServiceTest extends TestCase { + private IL10N&MockObject $l10n; + + public function setUp(): void { + parent::setUp(); + $this->l10n = $this->createMock(IL10N::class); + $this->l10n + ->method('t') + ->willReturnCallback(static fn (string $text): string => $text); + } + public function testMapSignatureValidationWithEnumState(): void { $service = $this->newServiceWithoutConstructor(); $result = $this->invokePrivateMethod( @@ -45,6 +57,11 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { $reflection = new \ReflectionClass(PdfSignatureValidationService::class); /** @var PdfSignatureValidationService $service */ $service = $reflection->newInstanceWithoutConstructor(); + + $l10nProperty = $reflection->getProperty('l10n'); + $l10nProperty->setAccessible(true); + $l10nProperty->setValue($service, $this->l10n); + return $service; } From 0411b5351c7dc9447dddfb40ac686e4429a32f1d Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:01:59 -0300 Subject: [PATCH 03/21] refactor: avoid translating dynamic reason strings Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index a45498cb3a..8a81bccd5b 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -166,25 +166,25 @@ private function mapSignatureValidation(ValidationResult $result): array { ValidationState::SIGNATURE_INVALID => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], default => [ 'id' => 6, 'label' => $this->l10n->t('Unknown validation failure.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], }; @@ -203,50 +203,42 @@ private function mapCertificateValidation(ValidationResult $result): array { ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], default => [ 'id' => 7, 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - 'reason' => $this->translateReason($result->reason), + 'reason' => $result->reason, 'isValid' => false, ], }; } - private function translateReason(?string $reason): ?string { - if ($reason === null || $reason === '') { - return $reason; - } - - return $this->l10n->t($reason); - } - /** * Check if LibreSign CA is loaded. */ From 205c6514d0ff601c003017a71644533c2a2ded9f Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:03:58 -0300 Subject: [PATCH 04/21] feat: add translatable dictionary for validation reasons Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 64 ++++++++++++++++--- .../PdfSignatureValidationServiceTest.php | 22 +++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 8a81bccd5b..ad3a4e0c43 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -166,25 +166,25 @@ private function mapSignatureValidation(ValidationResult $result): array { ValidationState::SIGNATURE_INVALID => [ 'id' => 2, 'label' => $this->l10n->t('Signature is invalid.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, 'label' => $this->l10n->t('Digest mismatch.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, 'label' => $this->l10n->t('Signature has not yet been verified.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, 'label' => $this->l10n->t('Unknown validation failure.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], }; @@ -203,42 +203,86 @@ private function mapCertificateValidation(ValidationResult $result): array { ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, 'label' => $this->l10n->t('Certificate issuer is unknown.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, 'label' => $this->l10n->t('Certificate has been revoked.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, 'label' => $this->l10n->t('Certificate has expired.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, 'label' => $this->l10n->t('Certificate has not yet been verified.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - 'reason' => $result->reason, + 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], }; } + private function translateKnownReason(?string $reason): ?string { + if ($reason === null || $reason === '') { + return $reason; + } + + if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { + return $this->l10n->t( + 'Intermediate certificate at position %s is not signed by issuer', + [$matches[1]] + ); + } + + $prefix = 'Certificate validation failed: '; + if (str_starts_with($reason, $prefix)) { + $detail = substr($reason, strlen($prefix)); + $translatedDetail = $this->translateKnownReason($detail) ?? $detail; + return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); + } + + return match ($reason) { + 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), + 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), + 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), + 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), + 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), + 'Certificate has expired' => $this->l10n->t('Certificate has expired'), + 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), + 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), + 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), + 'Invalid certificate' => $this->l10n->t('Invalid certificate'), + 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), + 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), + 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), + 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), + 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), + 'No binary signature' => $this->l10n->t('No binary signature'), + 'No certificates in signature' => $this->l10n->t('No certificates in signature'), + 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), + 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), + 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), + default => $reason, + }; + } + /** * Check if LibreSign CA is loaded. */ diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index 1b5a8ee408..cdc30d5a10 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -53,6 +53,28 @@ public function testMapCertificateValidationWithEnumState(): void { $this->assertTrue($result['isValid']); } + public function testMapReasonUsesDictionaryForKnownReason(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'PDF content hash does not match signed digest') + ); + + $this->assertSame('PDF content hash does not match signed digest', $result['reason']); + } + + public function testMapReasonKeepsUnknownReasonUntouched(): void { + $service = $this->newServiceWithoutConstructor(); + $result = $this->invokePrivateMethod( + $service, + 'mapSignatureValidation', + new ValidationResult(ValidationState::DIGEST_MISMATCH, 'custom runtime detail') + ); + + $this->assertSame('custom runtime detail', $result['reason']); + } + private function newServiceWithoutConstructor(): PdfSignatureValidationService { $reflection = new \ReflectionClass(PdfSignatureValidationService::class); /** @var PdfSignatureValidationService $service */ From 9ecd562abf0959244c4455c301d3f16b9e1f0d3b Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:05:10 -0300 Subject: [PATCH 05/21] docs: add translator guidance for validation reasons Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index ad3a4e0c43..cb6963bd3a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -245,6 +245,7 @@ private function translateKnownReason(?string $reason): ?string { } if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { + // TRANSLATORS: %s is the numeric position of an intermediate certificate in the chain. return $this->l10n->t( 'Intermediate certificate at position %s is not signed by issuer', [$matches[1]] @@ -255,9 +256,12 @@ private function translateKnownReason(?string $reason): ?string { if (str_starts_with($reason, $prefix)) { $detail = substr($reason, strlen($prefix)); $translatedDetail = $this->translateKnownReason($detail) ?? $detail; + // TRANSLATORS: %s is a translated certificate validation detail message. return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } + // TRANSLATORS: Technical validation reasons from pdf-signature-validator. + // Keep protocol/acronym terms like ByteRange, CRL and CA as-is. return match ($reason) { 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), From d9a66a5d021974d3884c108bf93e4af81e7c41cd Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:06:48 -0300 Subject: [PATCH 06/21] docs: place translator comments above each l10n call Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 34 +++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index cb6963bd3a..b92b4f072c 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -160,29 +160,34 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, + // TRANSLATORS: User-facing status when signature cryptographic validation succeeds. 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, + // TRANSLATORS: User-facing status when signature cryptographic validation fails. 'label' => $this->l10n->t('Signature is invalid.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, + // TRANSLATORS: User-facing status when signed digest does not match PDF content. 'label' => $this->l10n->t('Digest mismatch.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, + // TRANSLATORS: User-facing status when validation could not be fully completed. 'label' => $this->l10n->t('Signature has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, + // TRANSLATORS: Generic fallback status for unexpected signature validation failures. 'label' => $this->l10n->t('Unknown validation failure.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -197,41 +202,48 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, + // TRANSLATORS: User-facing status when certificate is trusted. 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, + // TRANSLATORS: User-facing status when issuing CA is known but not trusted. 'label' => $this->l10n->t("Certificate issuer isn't trusted."), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, + // TRANSLATORS: User-facing status when certificate issuer cannot be identified/trusted. 'label' => $this->l10n->t('Certificate issuer is unknown.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, + // TRANSLATORS: User-facing status when certificate is revoked. 'label' => $this->l10n->t('Certificate has been revoked.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, + // TRANSLATORS: User-facing status when certificate is expired. 'label' => $this->l10n->t('Certificate has expired.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, + // TRANSLATORS: User-facing status when certificate validation could not be completed. 'label' => $this->l10n->t('Certificate has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, + // TRANSLATORS: Generic fallback status for unexpected certificate validation failures. 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -260,28 +272,46 @@ private function translateKnownReason(?string $reason): ?string { return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } - // TRANSLATORS: Technical validation reasons from pdf-signature-validator. - // Keep protocol/acronym terms like ByteRange, CRL and CA as-is. return match ($reason) { + // TRANSLATORS: Technical term from PDF signatures. Keep "ByteRange" unchanged. 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), + // TRANSLATORS: Technical message for digest/hash mismatch in PDF signature verification. 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), + // TRANSLATORS: Certificate/public-key verification failed for signature bytes. 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), + // TRANSLATORS: X.509 certificate parsing failure. 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), + // TRANSLATORS: Signature timestamp is outside certificate validity window. 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), + // TRANSLATORS: Certificate validity date has ended. 'Certificate has expired' => $this->l10n->t('Certificate has expired'), + // TRANSLATORS: No certificates were found in provided certificate chain. 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), + // TRANSLATORS: Certificate does not provide a serial number field. 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), + // TRANSLATORS: CRL means Certificate Revocation List; keep acronym CRL unchanged. 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), + // TRANSLATORS: Certificate structure/content is invalid. 'Invalid certificate' => $this->l10n->t('Invalid certificate'), + // TRANSLATORS: CA means Certificate Authority; keep acronym CA unchanged. 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), + // TRANSLATORS: Certificate signature chain validation failed. 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), + // TRANSLATORS: Self-signed certificate is not present in trusted roots list. 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), + // TRANSLATORS: Root certificate must be self-signed to be considered a trust anchor. 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), + // TRANSLATORS: Root certificate is not present in configured trusted certificate list. 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), + // TRANSLATORS: Signature container has no binary signature payload. 'No binary signature' => $this->l10n->t('No binary signature'), + // TRANSLATORS: Signature payload has no embedded certificates. 'No certificates in signature' => $this->l10n->t('No certificates in signature'), + // TRANSLATORS: Certificate used for signing is expired. 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), + // TRANSLATORS: Certificate used for signing is revoked. 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), + // TRANSLATORS: Signature verification could not be fully completed. 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), default => $reason, }; From a7ed09bd86f1509ec125f77dce73bf977300553c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:07:42 -0300 Subject: [PATCH 07/21] docs: place translator comments above each l10n call Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index b92b4f072c..93bf9cff87 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -160,34 +160,34 @@ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ 'id' => 1, - // TRANSLATORS: User-facing status when signature cryptographic validation succeeds. + // TRANSLATORS User-facing status when signature cryptographic validation succeeds. 'label' => $this->l10n->t('Signature is valid.'), 'isValid' => true, ], ValidationState::SIGNATURE_INVALID => [ 'id' => 2, - // TRANSLATORS: User-facing status when signature cryptographic validation fails. + // TRANSLATORS User-facing status when signature cryptographic validation fails. 'label' => $this->l10n->t('Signature is invalid.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::DIGEST_MISMATCH => [ 'id' => 3, - // TRANSLATORS: User-facing status when signed digest does not match PDF content. + // TRANSLATORS User-facing status when signed digest does not match PDF content. 'label' => $this->l10n->t('Digest mismatch.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::NOT_VERIFIED => [ 'id' => 5, - // TRANSLATORS: User-facing status when validation could not be fully completed. + // TRANSLATORS User-facing status when validation could not be fully completed. 'label' => $this->l10n->t('Signature has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 6, - // TRANSLATORS: Generic fallback status for unexpected signature validation failures. + // TRANSLATORS Generic fallback status for unexpected signature validation failures. 'label' => $this->l10n->t('Unknown validation failure.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -202,48 +202,48 @@ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ 'id' => 1, - // TRANSLATORS: User-facing status when certificate is trusted. + // TRANSLATORS User-facing status when certificate is trusted. 'label' => $this->l10n->t('Certificate is trusted.'), 'isValid' => true, ], ValidationState::CERT_ISSUER_NOT_TRUSTED => [ 'id' => 2, - // TRANSLATORS: User-facing status when issuing CA is known but not trusted. + // TRANSLATORS User-facing status when issuing CA is known but not trusted. 'label' => $this->l10n->t("Certificate issuer isn't trusted."), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_ISSUER_UNKNOWN => [ 'id' => 3, - // TRANSLATORS: User-facing status when certificate issuer cannot be identified/trusted. + // TRANSLATORS User-facing status when certificate issuer cannot be identified/trusted. 'label' => $this->l10n->t('Certificate issuer is unknown.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_REVOKED => [ 'id' => 4, - // TRANSLATORS: User-facing status when certificate is revoked. + // TRANSLATORS User-facing status when certificate is revoked. 'label' => $this->l10n->t('Certificate has been revoked.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_EXPIRED => [ 'id' => 5, - // TRANSLATORS: User-facing status when certificate is expired. + // TRANSLATORS User-facing status when certificate is expired. 'label' => $this->l10n->t('Certificate has expired.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], ValidationState::CERT_NOT_VERIFIED => [ 'id' => 6, - // TRANSLATORS: User-facing status when certificate validation could not be completed. + // TRANSLATORS User-facing status when certificate validation could not be completed. 'label' => $this->l10n->t('Certificate has not yet been verified.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, ], default => [ 'id' => 7, - // TRANSLATORS: Generic fallback status for unexpected certificate validation failures. + // TRANSLATORS Generic fallback status for unexpected certificate validation failures. 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), 'reason' => $this->translateKnownReason($result->reason), 'isValid' => false, @@ -257,7 +257,7 @@ private function translateKnownReason(?string $reason): ?string { } if (preg_match('/^Intermediate certificate at position (\d+) is not signed by issuer$/', $reason, $matches) === 1) { - // TRANSLATORS: %s is the numeric position of an intermediate certificate in the chain. + // TRANSLATORS %s is the numeric position of an intermediate certificate in the chain. return $this->l10n->t( 'Intermediate certificate at position %s is not signed by issuer', [$matches[1]] @@ -268,50 +268,50 @@ private function translateKnownReason(?string $reason): ?string { if (str_starts_with($reason, $prefix)) { $detail = substr($reason, strlen($prefix)); $translatedDetail = $this->translateKnownReason($detail) ?? $detail; - // TRANSLATORS: %s is a translated certificate validation detail message. + // TRANSLATORS %s is a translated certificate validation detail message. return $this->l10n->t('Certificate validation failed: %s', [$translatedDetail]); } return match ($reason) { - // TRANSLATORS: Technical term from PDF signatures. Keep "ByteRange" unchanged. + // TRANSLATORS Technical term from PDF signatures. Keep "ByteRange" unchanged. 'No ByteRange in signature' => $this->l10n->t('No ByteRange in signature'), - // TRANSLATORS: Technical message for digest/hash mismatch in PDF signature verification. + // TRANSLATORS Technical message for digest/hash mismatch in PDF signature verification. 'PDF content hash does not match signed digest' => $this->l10n->t('PDF content hash does not match signed digest'), - // TRANSLATORS: Certificate/public-key verification failed for signature bytes. + // TRANSLATORS Certificate/public-key verification failed for signature bytes. 'Signature does not match certificate' => $this->l10n->t('Signature does not match certificate'), - // TRANSLATORS: X.509 certificate parsing failure. + // TRANSLATORS X.509 certificate parsing failure. 'Failed to parse certificate' => $this->l10n->t('Failed to parse certificate'), - // TRANSLATORS: Signature timestamp is outside certificate validity window. + // TRANSLATORS Signature timestamp is outside certificate validity window. 'Certificate was not valid at time of signature' => $this->l10n->t('Certificate was not valid at time of signature'), - // TRANSLATORS: Certificate validity date has ended. + // TRANSLATORS Certificate validity date has ended. 'Certificate has expired' => $this->l10n->t('Certificate has expired'), - // TRANSLATORS: No certificates were found in provided certificate chain. + // TRANSLATORS No certificates were found in provided certificate chain. 'Empty certificate chain' => $this->l10n->t('Empty certificate chain'), - // TRANSLATORS: Certificate does not provide a serial number field. + // TRANSLATORS Certificate does not provide a serial number field. 'Certificate has no serial number' => $this->l10n->t('Certificate has no serial number'), - // TRANSLATORS: CRL means Certificate Revocation List; keep acronym CRL unchanged. + // TRANSLATORS CRL means Certificate Revocation List; keep acronym CRL unchanged. 'Certificate found in CRL' => $this->l10n->t('Certificate found in CRL'), - // TRANSLATORS: Certificate structure/content is invalid. + // TRANSLATORS Certificate structure/content is invalid. 'Invalid certificate' => $this->l10n->t('Invalid certificate'), - // TRANSLATORS: CA means Certificate Authority; keep acronym CA unchanged. + // TRANSLATORS CA means Certificate Authority; keep acronym CA unchanged. 'Leaf certificate is marked as CA' => $this->l10n->t('Leaf certificate is marked as CA'), - // TRANSLATORS: Certificate signature chain validation failed. + // TRANSLATORS Certificate signature chain validation failed. 'Certificate signature validation failed' => $this->l10n->t('Certificate signature validation failed'), - // TRANSLATORS: Self-signed certificate is not present in trusted roots list. + // TRANSLATORS Self-signed certificate is not present in trusted roots list. 'Self-signed certificate not in trusted roots' => $this->l10n->t('Self-signed certificate not in trusted roots'), - // TRANSLATORS: Root certificate must be self-signed to be considered a trust anchor. + // TRANSLATORS Root certificate must be self-signed to be considered a trust anchor. 'Root certificate is not self-signed' => $this->l10n->t('Root certificate is not self-signed'), - // TRANSLATORS: Root certificate is not present in configured trusted certificate list. + // TRANSLATORS Root certificate is not present in configured trusted certificate list. 'Root certificate is not in trusted list' => $this->l10n->t('Root certificate is not in trusted list'), - // TRANSLATORS: Signature container has no binary signature payload. + // TRANSLATORS Signature container has no binary signature payload. 'No binary signature' => $this->l10n->t('No binary signature'), - // TRANSLATORS: Signature payload has no embedded certificates. + // TRANSLATORS Signature payload has no embedded certificates. 'No certificates in signature' => $this->l10n->t('No certificates in signature'), - // TRANSLATORS: Certificate used for signing is expired. + // TRANSLATORS Certificate used for signing is expired. 'Signing certificate has expired' => $this->l10n->t('Signing certificate has expired'), - // TRANSLATORS: Certificate used for signing is revoked. + // TRANSLATORS Certificate used for signing is revoked. 'Signing certificate has been revoked' => $this->l10n->t('Signing certificate has been revoked'), - // TRANSLATORS: Signature verification could not be fully completed. + // TRANSLATORS Signature verification could not be fully completed. 'Signature verification incomplete' => $this->l10n->t('Signature verification incomplete'), default => $reason, }; From 1cb5b0641038ae5731b7ea0f1c09b7785d43d779 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:08:59 -0300 Subject: [PATCH 08/21] refactor: remove redundant docs from CA getters Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 93bf9cff87..efe4bd8f44 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -317,16 +317,10 @@ private function translateKnownReason(?string $reason): ?string { }; } - /** - * Check if LibreSign CA is loaded. - */ public function isLibreSignCaLoaded(): bool { return !empty($this->libresignCaCertificate); } - /** - * Get LibreSign CA certificate (PEM format). - */ public function getLibreSignCaCertificate(): string { return $this->libresignCaCertificate; } From 2b2bf21dcf45f72d7d3506105c4e34cefaaf8316 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:09:45 -0300 Subject: [PATCH 09/21] refactor: remove redundant comments in validation service Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../PdfSignatureValidationService.php | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index efe4bd8f44..1cbe587885 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -37,11 +37,7 @@ public function __construct( $this->loadLibreSignCaCertificate(); } - /** - * Load LibreSign CA certificate and set it as trusted root. - */ private function loadLibreSignCaCertificate(): void { - // Try to load from config path first $configPath = $this->appConfig->getValueString(Application::APP_ID, 'config_path'); if (!empty($configPath) && is_dir($configPath)) { $caPemPath = $configPath . DIRECTORY_SEPARATOR . 'ca.pem'; @@ -55,7 +51,6 @@ private function loadLibreSignCaCertificate(): void { } } - // Try alternate location $alternateConfig = $this->appConfig->getValueString( Application::APP_ID, 'libresign_ca_certificate' @@ -66,18 +61,10 @@ private function loadLibreSignCaCertificate(): void { } } - /** - * Add additional trusted root certificate. - */ public function addTrustedRoot(string $certificatePem): void { $this->validator->addTrustedRoot($certificatePem); } - /** - * Set multiple trusted root certificates. - * - * @param list $certificates - */ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } @@ -153,9 +140,6 @@ private function mapValidationResults(array $results, ?DateTime $signatureTime = return $mapped; } - /** - * Map signature validation result to LibreSign format. - */ private function mapSignatureValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::SIGNATURE_VALID => [ @@ -195,9 +179,6 @@ private function mapSignatureValidation(ValidationResult $result): array { }; } - /** - * Map certificate validation result to LibreSign format. - */ private function mapCertificateValidation(ValidationResult $result): array { return match ($result->state) { ValidationState::CERT_TRUSTED => [ From 61bf6f0a2254f9be0a923e43a70451eded42867c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:12:59 -0300 Subject: [PATCH 10/21] test: remove deprecated reflection accessibility calls Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../Service/Signature/PdfSignatureValidationServiceTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php index cdc30d5a10..6101860832 100644 --- a/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php +++ b/tests/php/Unit/Service/Signature/PdfSignatureValidationServiceTest.php @@ -81,7 +81,6 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { $service = $reflection->newInstanceWithoutConstructor(); $l10nProperty = $reflection->getProperty('l10n'); - $l10nProperty->setAccessible(true); $l10nProperty->setValue($service, $this->l10n); return $service; @@ -93,7 +92,6 @@ private function newServiceWithoutConstructor(): PdfSignatureValidationService { private function invokePrivateMethod(PdfSignatureValidationService $service, string $method, ValidationResult $result): array { $reflection = new \ReflectionClass($service); $target = $reflection->getMethod($method); - $target->setAccessible(true); /** @var array $mapped */ $mapped = $target->invoke($service, $result); return $mapped; From 7a3ae4c63cf81ef463bebcc9a0bce606a8b07bfe Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:15:12 -0300 Subject: [PATCH 11/21] build: use released pdf-signature-validator v0.2.0 Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- composer.json | 11 +------ composer.lock | 79 +++++++++++++++------------------------------------ 2 files changed, 24 insertions(+), 66 deletions(-) diff --git a/composer.json b/composer.json index 0ac7dcd2a0..e6c3e853d4 100644 --- a/composer.json +++ b/composer.json @@ -1,13 +1,4 @@ { - "repositories": [ - { - "type": "path", - "url": "../pdf-signature-validator", - "options": { - "symlink": true - } - } - ], "require-dev": { "bamarni/composer-bin-plugin": "^1.8", "nextcloud/ocp": "dev-master", @@ -72,7 +63,7 @@ "require": { "cweagans/composer-patches": "^2.0", "jeidison/signer-php": "^1.0", - "libresign/pdf-signature-validator": "*@dev", + "libresign/pdf-signature-validator": "0.2.0", "phpseclib/phpseclib": "^3.0" } } diff --git a/composer.lock b/composer.lock index 502fcd7ebd..a97d00691f 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "ecf8085cd1193a30abd69ef25963e320", + "content-hash": "ab5d7bacc29ec24709c086f8137c630a", "packages": [ { "name": "cweagans/composer-configurable-plugin", @@ -200,11 +200,17 @@ }, { "name": "libresign/pdf-signature-validator", - "version": "dev-backport/7525/stable33-manual", + "version": "v0.2.0", + "source": { + "type": "git", + "url": "https://github.com/LibreSign/pdf-signature-validator.git", + "reference": "f062333024dece3a8bb51da8bf8a6259e98543c2" + }, "dist": { - "type": "path", - "url": "../pdf-signature-validator", - "reference": "f8eba5ac5e43315f3856645e46e95eb21be4f742" + "type": "zip", + "url": "https://api.github.com/repos/LibreSign/pdf-signature-validator/zipball/f062333024dece3a8bb51da8bf8a6259e98543c2", + "reference": "f062333024dece3a8bb51da8bf8a6259e98543c2", + "shasum": "" }, "require": { "php": "^8.2", @@ -226,60 +232,22 @@ "LibreSign\\PdfSignatureValidator\\": "src/" } }, - "autoload-dev": { - "psr-4": { - "LibreSign\\PdfSignatureValidator\\Tests\\": "tests/" - } - }, - "scripts": { - "bin": [ - "echo 'bin not installed'" - ], - "lint": [ - "find src tests -name '*.php' -print0 | xargs -0 -n1 php -l" - ], - "cs:check": [ - "php-cs-fixer fix --dry-run --diff" - ], - "cs:fix": [ - "php-cs-fixer fix" - ], - "phpmd": [ - "phpmd src,tests text phpmd.xml" - ], - "psalm": [ - "psalm --no-cache --threads=$(nproc)" - ], - "rector:check": [ - "rector --dry-run" - ], - "rector:fix": [ - "rector" - ], - "test:unit": [ - "phpunit --colors=always --fail-on-warning --fail-on-risky" - ], - "test:coverage": [ - "XDEBUG_MODE=coverage phpunit --coverage-text" - ], - "test:mutation": [ - "infection --threads=$(nproc) --skip-initial-tests" - ], - "post-install-cmd": [ - "@composer bin all install --ansi" - ], - "post-update-cmd": [ - "@composer bin all install --ansi" - ] - }, + "notification-url": "https://packagist.org/downloads/", "license": [ "AGPL-3.0-or-later" ], "description": "High-quality PDF signature extraction and validation primitives for LibreSign and external consumers.", - "transport-options": { - "symlink": true, - "relative": true - } + "support": { + "issues": "https://github.com/LibreSign/pdf-signature-validator/issues", + "source": "https://github.com/LibreSign/pdf-signature-validator/tree/v0.2.0" + }, + "funding": [ + { + "url": "https://github.com/sponsors/libresign", + "type": "github" + } + ], + "time": "2026-04-24T00:11:10+00:00" }, { "name": "paragonie/constant_time_encoding", @@ -1985,7 +1953,6 @@ "aliases": [], "minimum-stability": "stable", "stability-flags": { - "libresign/pdf-signature-validator": 20, "nextcloud/ocp": 20, "roave/security-advisories": 20 }, From 3444d8f9a221d630a776799e9a2565c53e4fe009 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:19:28 -0300 Subject: [PATCH 12/21] refactor: centralize validation localization mapping Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 72 +------------------ .../PdfSignatureValidationService.php | 42 +++++++++++ 2 files changed, 44 insertions(+), 70 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index ab2832c8bd..9259e681f5 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -301,11 +301,11 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $this->getLocalizedSignatureValidation($validation['signatureValidation']); + $leaf['signature_validation'] = $this->pdfSignatureValidationService->localizeSignatureValidation($validation['signatureValidation']); } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { - $leaf['certificate_validation'] = $this->getLocalizedCertificateValidation($validation['certificateValidation']); + $leaf['certificate_validation'] = $this->pdfSignatureValidationService->localizeCertificateValidation($validation['certificateValidation']); } if (!isset($leaf['certificate_validation'])) { @@ -318,74 +318,6 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array return $result; } - /** - * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. - */ - private function getLocalizedSignatureValidation(array $validation): array { - $id = (int)($validation['id'] ?? 6); - - return match ($id) { - 1 => [ - 'id' => 1, - 'label' => $this->l10n->t('Signature is valid.'), - ], - 2 => [ - 'id' => 2, - 'label' => $this->l10n->t('Signature is invalid.'), - ], - 3 => [ - 'id' => 3, - 'label' => $this->l10n->t('Digest mismatch.'), - ], - 5 => [ - 'id' => 5, - 'label' => $this->l10n->t('Signature has not yet been verified.'), - ], - default => [ - 'id' => 6, - 'label' => $this->l10n->t('Unknown validation failure.'), - ], - }; - } - - /** - * Keep LibreSign-side l10n compatibility regardless of upstream validator labels. - */ - private function getLocalizedCertificateValidation(array $validation): array { - $id = (int)($validation['id'] ?? 7); - - return match ($id) { - 1 => [ - 'id' => 1, - 'label' => $this->l10n->t('Certificate is trusted.'), - ], - 2 => [ - 'id' => 2, - 'label' => $this->l10n->t("Certificate issuer isn't trusted."), - ], - 3 => [ - 'id' => 3, - 'label' => $this->l10n->t('Certificate issuer is unknown.'), - ], - 4 => [ - 'id' => 4, - 'label' => $this->l10n->t('Certificate has been revoked.'), - ], - 5 => [ - 'id' => 5, - 'label' => $this->l10n->t('Certificate has expired'), - ], - 6 => [ - 'id' => 6, - 'label' => $this->l10n->t('Certificate has not yet been verified.'), - ], - default => [ - 'id' => 7, - 'label' => $this->l10n->t('Unknown issue with certificate or corrupted data.'), - ], - }; - } - /** * @param resource $resource * @return list diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 1cbe587885..74a4ab8c17 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -69,6 +69,48 @@ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } + /** + * Normalize a signature validation payload by id/reason to the canonical LibreSign shape. + * + * @param array{id?: int|string, reason?: mixed} $validation + */ + public function localizeSignatureValidation(array $validation): array { + $id = (int)($validation['id'] ?? 6); + $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; + + $state = match ($id) { + 1 => ValidationState::SIGNATURE_VALID, + 2 => ValidationState::SIGNATURE_INVALID, + 3 => ValidationState::DIGEST_MISMATCH, + 5 => ValidationState::NOT_VERIFIED, + default => ValidationState::UNKNOWN_FAILURE, + }; + + return $this->mapSignatureValidation(new ValidationResult($state, $reason)); + } + + /** + * Normalize a certificate validation payload by id/reason to the canonical LibreSign shape. + * + * @param array{id?: int|string, reason?: mixed} $validation + */ + public function localizeCertificateValidation(array $validation): array { + $id = (int)($validation['id'] ?? 7); + $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; + + $state = match ($id) { + 1 => ValidationState::CERT_TRUSTED, + 2 => ValidationState::CERT_ISSUER_NOT_TRUSTED, + 3 => ValidationState::CERT_ISSUER_UNKNOWN, + 4 => ValidationState::CERT_REVOKED, + 5 => ValidationState::CERT_EXPIRED, + 6 => ValidationState::CERT_NOT_VERIFIED, + default => ValidationState::UNKNOWN_FAILURE, + }; + + return $this->mapCertificateValidation(new ValidationResult($state, $reason)); + } + /** * Validate PDF signatures from file resource. * From 7395843e2a71a6a7c871800daca7387e401efc2a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:21:15 -0300 Subject: [PATCH 13/21] fix: remove invalid UNKNOWN_FAILURE enum state Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 74a4ab8c17..a929c56f03 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -83,7 +83,7 @@ public function localizeSignatureValidation(array $validation): array { 2 => ValidationState::SIGNATURE_INVALID, 3 => ValidationState::DIGEST_MISMATCH, 5 => ValidationState::NOT_VERIFIED, - default => ValidationState::UNKNOWN_FAILURE, + default => ValidationState::NOT_VERIFIED, }; return $this->mapSignatureValidation(new ValidationResult($state, $reason)); @@ -105,7 +105,7 @@ public function localizeCertificateValidation(array $validation): array { 4 => ValidationState::CERT_REVOKED, 5 => ValidationState::CERT_EXPIRED, 6 => ValidationState::CERT_NOT_VERIFIED, - default => ValidationState::UNKNOWN_FAILURE, + default => ValidationState::CERT_NOT_VERIFIED, }; return $this->mapCertificateValidation(new ValidationResult($state, $reason)); From df3cd62564a7df58a893b1231a0b7339b79fd4af Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:24:44 -0300 Subject: [PATCH 14/21] refactor: remove redundant localize* methods The service's validateFromResource() already returns properly formatted arrays with id, label, isValid, and reason. The localize* methods were unnecessarily converting numbers back to enums and then back to numbers. Now Pkcs12Handler uses the validation results directly without the circular conversion, eliminating code duplication and improving clarity. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 4 +- .../PdfSignatureValidationService.php | 42 ------------------- 2 files changed, 2 insertions(+), 44 deletions(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 9259e681f5..e2a901e960 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -301,11 +301,11 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $this->pdfSignatureValidationService->localizeSignatureValidation($validation['signatureValidation']); + $leaf['signature_validation'] = $validation['signatureValidation']; } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { - $leaf['certificate_validation'] = $this->pdfSignatureValidationService->localizeCertificateValidation($validation['certificateValidation']); + $leaf['certificate_validation'] = $validation['certificateValidation']; } if (!isset($leaf['certificate_validation'])) { diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index a929c56f03..1cbe587885 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -69,48 +69,6 @@ public function setTrustedRoots(array $certificates): void { $this->validator->setTrustedRoots($certificates); } - /** - * Normalize a signature validation payload by id/reason to the canonical LibreSign shape. - * - * @param array{id?: int|string, reason?: mixed} $validation - */ - public function localizeSignatureValidation(array $validation): array { - $id = (int)($validation['id'] ?? 6); - $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; - - $state = match ($id) { - 1 => ValidationState::SIGNATURE_VALID, - 2 => ValidationState::SIGNATURE_INVALID, - 3 => ValidationState::DIGEST_MISMATCH, - 5 => ValidationState::NOT_VERIFIED, - default => ValidationState::NOT_VERIFIED, - }; - - return $this->mapSignatureValidation(new ValidationResult($state, $reason)); - } - - /** - * Normalize a certificate validation payload by id/reason to the canonical LibreSign shape. - * - * @param array{id?: int|string, reason?: mixed} $validation - */ - public function localizeCertificateValidation(array $validation): array { - $id = (int)($validation['id'] ?? 7); - $reason = is_string($validation['reason'] ?? null) ? $validation['reason'] : null; - - $state = match ($id) { - 1 => ValidationState::CERT_TRUSTED, - 2 => ValidationState::CERT_ISSUER_NOT_TRUSTED, - 3 => ValidationState::CERT_ISSUER_UNKNOWN, - 4 => ValidationState::CERT_REVOKED, - 5 => ValidationState::CERT_EXPIRED, - 6 => ValidationState::CERT_NOT_VERIFIED, - default => ValidationState::CERT_NOT_VERIFIED, - }; - - return $this->mapCertificateValidation(new ValidationResult($state, $reason)); - } - /** * Validate PDF signatures from file resource. * From a5c2db3551963be60585f742a5b40b8cb791c9a3 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:26:34 -0300 Subject: [PATCH 15/21] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/Signature/PdfSignatureValidationService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 1cbe587885..4bc9550391 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -9,9 +9,9 @@ namespace OCA\Libresign\Service\Signature; use DateTime; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; use OCA\Libresign\AppInfo\Application; use OCP\IAppConfig; use OCP\IL10N; From 6bc699c82cf0e2a07cc0478624e9efaeadd0a14d Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:30:24 -0300 Subject: [PATCH 16/21] fix: resolve Psalm type hint errors - FileController::fetchPreview: add inline type assertions for RedirectResponse to specify exact status code (303) when returning null-checked values - PdfSignatureValidationService: update docblocks for validateFromResource, validateFromString, and mapValidationResults to include the 'raw' field - Pkcs12Handler::extractNativeSignatureMetadata: change return type from list to array to match actual implementation All Psalm errors resolved, unit tests still passing. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Controller/FileController.php | 45 ++++++++++++++++--- lib/Handler/SignEngine/Pkcs12Handler.php | 2 +- .../PdfSignatureValidationService.php | 6 +-- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/Controller/FileController.php b/lib/Controller/FileController.php index ecc3d707a5..ba5b4932f2 100644 --- a/lib/Controller/FileController.php +++ b/lib/Controller/FileController.php @@ -457,7 +457,7 @@ private function fetchPreview( bool $forceIcon, string $mode, bool $mimeFallback = false, - ) : Http\Response { + ): Http\Response { if (!($node instanceof File) || (!$forceIcon && !$this->preview->isAvailable($node))) { return new DataResponse([], Http::STATUS_NOT_FOUND); } @@ -465,6 +465,15 @@ private function fetchPreview( return new DataResponse([], Http::STATUS_FORBIDDEN); } + // Avoid expensive external preview generators for PDFs when a mime fallback is explicitly requested. + if ($mimeFallback && $node->getMimeType() === 'application/pdf') { + $mimeFallbackResponse = $this->getMimeFallbackResponse($node->getMimeType()); + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; + } + } + $storage = $node->getStorage(); if ($storage->instanceOfStorage(SharedStorage::class)) { /** @var SharedStorage $storage */ @@ -483,19 +492,43 @@ private function fetchPreview( $response->cacheFor(3600 * 24, false, true); return $response; } catch (NotFoundException) { - // If we have no preview enabled, we can redirect to the mime icon if any - if ($mimeFallback) { - if ($url = $this->mimeIconProvider->getMimeIconUrl($node->getMimeType())) { - return new RedirectResponse($url); - } + $mimeFallbackResponse = $mimeFallback ? $this->getMimeFallbackResponse($node->getMimeType()) : null; + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; } return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (\InvalidArgumentException) { return new DataResponse([], Http::STATUS_BAD_REQUEST); + } catch (\Throwable $e) { + $this->logger->warning('Failed to generate LibreSign thumbnail preview', [ + 'nodeId' => $node->getId(), + 'mimeType' => $node->getMimeType(), + 'exception' => $e, + ]); + + $mimeFallbackResponse = $mimeFallback ? $this->getMimeFallbackResponse($node->getMimeType()) : null; + if ($mimeFallbackResponse !== null) { + /** @var Http\RedirectResponse $mimeFallbackResponse */ + return $mimeFallbackResponse; + } + + return new DataResponse([], Http::STATUS_NOT_FOUND); } } + private function getMimeFallbackResponse(string $mimeType): ?\OCP\AppFramework\Http\RedirectResponse { + $url = $this->mimeIconProvider->getMimeIconUrl($mimeType); + if ($url) { + /** @var \OCP\AppFramework\Http\RedirectResponse $response */ + $response = new RedirectResponse($url, Http::STATUS_SEE_OTHER); + return $response; + } + + return null; + } + /** * Send a file * diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index e2a901e960..eae8555952 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -320,7 +320,7 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array /** * @param resource $resource - * @return list + * @return array */ private function extractNativeSignatureMetadata($resource): array { rewind($resource); diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 4bc9550391..8e13f3b07a 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -74,7 +74,7 @@ public function setTrustedRoots(array $certificates): void { * * @param resource $resource PDF file resource * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) - * @return list + * @return list */ public function validateFromResource($resource, ?DateTime $signatureTime = null): array { try { @@ -94,7 +94,7 @@ public function validateFromResource($resource, ?DateTime $signatureTime = null) * * @param string $pdfContent Binary PDF content * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) - * @return list + * @return list */ public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { try { @@ -114,7 +114,7 @@ public function validateFromString(string $pdfContent, ?DateTime $signatureTime * * @param list $results Results from PdfSignatureValidator * @param ?\DateTime $signatureTime - * @return list + * @return list */ private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { $mapped = []; From 599e5c2b85fc3fa89a53ea425564569fd450ab1a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:36:45 -0300 Subject: [PATCH 17/21] fix: update PasswordTest constructor arguments for Pkcs12Handler Update constructor arguments to match the new signature which includes PdfSignatureValidationService and PdfSignatureExtractor, and removes the no-longer-used TempManager. Also use real instance of PdfSignatureExtractor instead of mock since the class is final and cannot be mocked. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../php/Unit/Service/IdentifyMethod/PasswordTest.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index f335561a32..8de2878645 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -20,9 +20,10 @@ use OCA\Libresign\Service\FolderService; use OCA\Libresign\Service\IdentifyMethod\IdentifyService; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password; +use OCA\Libresign\Service\Signature\PdfSignatureValidationService; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\IAppConfig; use OCP\IL10N; -use OCP\ITempManager; use OCP\IUserSession; use OCP\L10N\IFactory as IL10NFactory; use PHPUnit\Framework\Attributes\DataProvider; @@ -38,11 +39,12 @@ final class PasswordTest extends \OCA\Libresign\Tests\Unit\TestCase { private CertificateEngineFactory&MockObject $certificateEngineFactory; private IL10N $l10n; private FooterHandler&MockObject $footerHandler; - private ITempManager $tempManager; private LoggerInterface&MockObject $logger; private CaIdentifierService&MockObject $caIdentifierService; private DocMdpHandler&MockObject $docMdpHandler; private CrlService&MockObject $crlService; + private PdfSignatureValidationService&MockObject $pdfSignatureValidationService; + private PdfSignatureExtractor $pdfSignatureExtractor; public function setUp(): void { $this->identifyService = $this->createMock(IdentifyService::class); @@ -51,12 +53,13 @@ public function setUp(): void { $this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class); $this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID); $this->footerHandler = $this->createMock(FooterHandler::class); - $this->tempManager = \OCP\Server::get(ITempManager::class); $this->userSession = $this->createMock(IUserSession::class); $this->logger = $this->createMock(LoggerInterface::class); $this->caIdentifierService = $this->createMock(CaIdentifierService::class); $this->docMdpHandler = $this->createMock(DocMdpHandler::class); $this->crlService = $this->createMock(CrlService::class); + $this->pdfSignatureValidationService = $this->createMock(PdfSignatureValidationService::class); + $this->pdfSignatureExtractor = new PdfSignatureExtractor(); $this->pkcs12Handler = $this->getPkcs12Instance(); } @@ -79,11 +82,12 @@ private function getPkcs12Instance(array $methods = []) { $this->certificateEngineFactory, $this->l10n, $this->footerHandler, - $this->tempManager, $this->logger, $this->caIdentifierService, $this->docMdpHandler, $this->crlService, + $this->pdfSignatureValidationService, + $this->pdfSignatureExtractor, ]) ->onlyMethods($methods) ->getMock(); From 154d90ddba27e022a2e5efafff479cb9a8b02575 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 21:38:12 -0300 Subject: [PATCH 18/21] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/php/Unit/Service/IdentifyMethod/PasswordTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php index 8de2878645..4cd30cc43c 100644 --- a/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php +++ b/tests/php/Unit/Service/IdentifyMethod/PasswordTest.php @@ -8,6 +8,7 @@ namespace OCA\Libresign\Tests\Unit\Service\IdentifyMethod; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Enum\CrlValidationStatus; use OCA\Libresign\Exception\LibresignException; @@ -21,7 +22,6 @@ use OCA\Libresign\Service\IdentifyMethod\IdentifyService; use OCA\Libresign\Service\IdentifyMethod\SignatureMethod\Password; use OCA\Libresign\Service\Signature\PdfSignatureValidationService; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCP\IAppConfig; use OCP\IL10N; use OCP\IUserSession; From d7b4588f68d213bd666987f83ddfe08890eb6959 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 23 Apr 2026 22:08:43 -0300 Subject: [PATCH 19/21] fix: preserve legacy signature validation on native digest mismatch Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 16 ++++++++++++- .../Handler/SignEngine/Pkcs12HandlerTest.php | 24 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index eae8555952..cf5107e6bc 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -301,7 +301,12 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array } if (isset($validation['signatureValidation']) && is_array($validation['signatureValidation'])) { - $leaf['signature_validation'] = $validation['signatureValidation']; + $signatureValidation = $validation['signatureValidation']; + + // Keep legacy OpenSSL result when native validator reports this known false-positive. + if (!$this->isDigestMismatchSignatureValidation($signatureValidation)) { + $leaf['signature_validation'] = $signatureValidation; + } } if (isset($validation['certificateValidation']) && is_array($validation['certificateValidation'])) { @@ -318,6 +323,15 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array return $result; } + /** + * signer engines can produce signatures that the native validator currently flags as digest mismatch. + * In this case we preserve the legacy validation computed from the PKCS#7 signature. + */ + private function isDigestMismatchSignatureValidation(array $signatureValidation): bool { + return ($signatureValidation['id'] ?? null) === 3 + && ($signatureValidation['label'] ?? '') === 'Digest mismatch.'; + } + /** * @param resource $resource * @return array diff --git a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php index 31a8ca3193..c2507d3aef 100644 --- a/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php +++ b/tests/php/Unit/Handler/SignEngine/Pkcs12HandlerTest.php @@ -464,4 +464,28 @@ public function testGetCertificateChainUsesNativeValidationServiceForEachSignatu $this->assertSame(3, $result[0]['chain'][0]['certificate_validation']['id']); } + public function testGetCertificateChainDoesNotOverrideLegacySignatureValidationOnDigestMismatch(): void { + $this->pdfSignatureValidationService->method('validateFromResource') + ->willReturn([ + [ + 'signatureValidation' => [ + 'id' => 3, + 'label' => 'Digest mismatch.', + 'reason' => 'PDF content hash does not match signed digest', + ], + ], + ]); + + $handler = $this->getHandler(); + $resource = fopen(__DIR__ . '/../../../fixtures/pdfs/small_valid-signed.pdf', 'r'); + $this->assertIsResource($resource); + + $result = $handler->getCertificateChain($resource); + fclose($resource); + + $this->assertNotEmpty($result); + $this->assertSame(1, $result[0]['chain'][0]['signature_validation']['id']); + $this->assertSame('Signature is valid.', $result[0]['chain'][0]['signature_validation']['label']); + } + } From 43dc4a0de2bb572ba691dd98792ceb745936f9d5 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Fri, 24 Apr 2026 11:36:13 -0300 Subject: [PATCH 20/21] fix: address native validator review feedback Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- composer.json | 2 +- composer.lock | 14 +++++----- lib/Handler/SignEngine/Pkcs12Handler.php | 27 ++++++++++++++----- .../PdfSignatureValidationService.php | 14 ++++------ 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/composer.json b/composer.json index e6c3e853d4..062fe1a108 100644 --- a/composer.json +++ b/composer.json @@ -63,7 +63,7 @@ "require": { "cweagans/composer-patches": "^2.0", "jeidison/signer-php": "^1.0", - "libresign/pdf-signature-validator": "0.2.0", + "libresign/pdf-signature-validator": "^0.2.0", "phpseclib/phpseclib": "^3.0" } } diff --git a/composer.lock b/composer.lock index a97d00691f..46b5460282 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "ab5d7bacc29ec24709c086f8137c630a", + "content-hash": "a0127fa8dfa35f6e73a6f5cd728c9b40", "packages": [ { "name": "cweagans/composer-configurable-plugin", @@ -200,16 +200,16 @@ }, { "name": "libresign/pdf-signature-validator", - "version": "v0.2.0", + "version": "v0.2.1", "source": { "type": "git", "url": "https://github.com/LibreSign/pdf-signature-validator.git", - "reference": "f062333024dece3a8bb51da8bf8a6259e98543c2" + "reference": "8288a4648c8d738fe538f182bb8a9c6a123feb1a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/LibreSign/pdf-signature-validator/zipball/f062333024dece3a8bb51da8bf8a6259e98543c2", - "reference": "f062333024dece3a8bb51da8bf8a6259e98543c2", + "url": "https://api.github.com/repos/LibreSign/pdf-signature-validator/zipball/8288a4648c8d738fe538f182bb8a9c6a123feb1a", + "reference": "8288a4648c8d738fe538f182bb8a9c6a123feb1a", "shasum": "" }, "require": { @@ -239,7 +239,7 @@ "description": "High-quality PDF signature extraction and validation primitives for LibreSign and external consumers.", "support": { "issues": "https://github.com/LibreSign/pdf-signature-validator/issues", - "source": "https://github.com/LibreSign/pdf-signature-validator/tree/v0.2.0" + "source": "https://github.com/LibreSign/pdf-signature-validator/tree/v0.2.1" }, "funding": [ { @@ -247,7 +247,7 @@ "type": "github" } ], - "time": "2026-04-24T00:11:10+00:00" + "time": "2026-04-24T14:34:32+00:00" }, { "name": "paragonie/constant_time_encoding", diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index cf5107e6bc..64ef33dede 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -9,6 +9,9 @@ namespace OCA\Libresign\Handler\SignEngine; use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; +use LibreSign\PdfSignatureValidator\Model\ValidationReason; +use LibreSign\PdfSignatureValidator\Model\ValidationResult; +use LibreSign\PdfSignatureValidator\Model\ValidationState; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory; @@ -93,10 +96,15 @@ public function setIsLibreSignFile(): void { public function getCertificateChain($resource): array { $certificates = []; $nativeMetadata = array_values($this->extractNativeSignatureMetadata($resource)); + rewind($resource); $nativeValidation = array_values($this->pdfSignatureValidationService->validateFromResource($resource)); $index = 0; foreach ($this->getSignatures($resource) as $signature) { + $metadata = $nativeMetadata[$index] ?? []; + $validation = $nativeValidation[$index] ?? []; + $index++; + if (!$signature) { continue; } @@ -104,8 +112,8 @@ public function getCertificateChain($resource): array { $result = $this->processSignature( $resource, $signature, - $nativeMetadata[$index] ?? ($nativeMetadata[0] ?? []), - $nativeValidation[$index] ?? ($nativeValidation[0] ?? []) + $metadata, + $validation ); if (empty($result['chain'])) { @@ -113,7 +121,6 @@ public function getCertificateChain($resource): array { } $certificates[] = $result; - $index++; } return $certificates; } @@ -304,7 +311,7 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array $signatureValidation = $validation['signatureValidation']; // Keep legacy OpenSSL result when native validator reports this known false-positive. - if (!$this->isDigestMismatchSignatureValidation($signatureValidation)) { + if (!$this->isDigestMismatchSignatureValidation($validation)) { $leaf['signature_validation'] = $signatureValidation; } } @@ -327,9 +334,15 @@ private function enrichLeafWithNativeData(array $result, array $metadata, array * signer engines can produce signatures that the native validator currently flags as digest mismatch. * In this case we preserve the legacy validation computed from the PKCS#7 signature. */ - private function isDigestMismatchSignatureValidation(array $signatureValidation): bool { - return ($signatureValidation['id'] ?? null) === 3 - && ($signatureValidation['label'] ?? '') === 'Digest mismatch.'; + private function isDigestMismatchSignatureValidation(array $validation): bool { + $rawSignatureValidation = $validation['raw']['signature'] ?? null; + if ($rawSignatureValidation instanceof ValidationResult) { + return $rawSignatureValidation->reasonCode === ValidationReason::DIGEST_MISMATCH + || $rawSignatureValidation->state === ValidationState::DIGEST_MISMATCH; + } + + $signatureValidation = $validation['signatureValidation'] ?? null; + return is_array($signatureValidation) && ($signatureValidation['id'] ?? null) === 3; } /** diff --git a/lib/Service/Signature/PdfSignatureValidationService.php b/lib/Service/Signature/PdfSignatureValidationService.php index 8e13f3b07a..ddabf3c65c 100644 --- a/lib/Service/Signature/PdfSignatureValidationService.php +++ b/lib/Service/Signature/PdfSignatureValidationService.php @@ -8,7 +8,6 @@ namespace OCA\Libresign\Service\Signature; -use DateTime; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; use LibreSign\PdfSignatureValidator\Parser\PdfSignatureValidator; @@ -73,13 +72,12 @@ public function setTrustedRoots(array $certificates): void { * Validate PDF signatures from file resource. * * @param resource $resource PDF file resource - * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) * @return list */ - public function validateFromResource($resource, ?DateTime $signatureTime = null): array { + public function validateFromResource($resource): array { try { $results = $this->validator->validateFromResource($resource); - return $this->mapValidationResults($results, $signatureTime); + return $this->mapValidationResults($results); } catch (\Throwable $e) { $this->logger->warning('PDF signature validation failed', [ 'error' => $e->getMessage(), @@ -93,13 +91,12 @@ public function validateFromResource($resource, ?DateTime $signatureTime = null) * Validate PDF signatures from binary content. * * @param string $pdfContent Binary PDF content - * @param ?\DateTime $signatureTime Optional time to validate against (for historic validation) * @return list */ - public function validateFromString(string $pdfContent, ?DateTime $signatureTime = null): array { + public function validateFromString(string $pdfContent): array { try { $results = $this->validator->validateFromString($pdfContent); - return $this->mapValidationResults($results, $signatureTime); + return $this->mapValidationResults($results); } catch (\Throwable $e) { $this->logger->warning('PDF signature validation failed', [ 'error' => $e->getMessage(), @@ -113,10 +110,9 @@ public function validateFromString(string $pdfContent, ?DateTime $signatureTime * Map validation results from PdfSignatureValidator to LibreSign format. * * @param list $results Results from PdfSignatureValidator - * @param ?\DateTime $signatureTime * @return list */ - private function mapValidationResults(array $results, ?DateTime $signatureTime = null): array { + private function mapValidationResults(array $results): array { $mapped = []; foreach ($results as $result) { From 60158554f1f7181682fbb6f8d9ff5b9c837dcbee Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Fri, 24 Apr 2026 11:40:27 -0300 Subject: [PATCH 21/21] fix: cs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Handler/SignEngine/Pkcs12Handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Handler/SignEngine/Pkcs12Handler.php b/lib/Handler/SignEngine/Pkcs12Handler.php index 64ef33dede..f4b0f2499c 100644 --- a/lib/Handler/SignEngine/Pkcs12Handler.php +++ b/lib/Handler/SignEngine/Pkcs12Handler.php @@ -8,10 +8,10 @@ namespace OCA\Libresign\Handler\SignEngine; -use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use LibreSign\PdfSignatureValidator\Model\ValidationReason; use LibreSign\PdfSignatureValidator\Model\ValidationResult; use LibreSign\PdfSignatureValidator\Model\ValidationState; +use LibreSign\PdfSignatureValidator\Parser\PdfSignatureExtractor; use OCA\Libresign\AppInfo\Application; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Handler\CertificateEngine\CertificateEngineFactory;