From f55d4ce090bc4aadc3a3332227a8d8d88adb477d Mon Sep 17 00:00:00 2001 From: Vitor Mattos Date: Tue, 4 Jun 2024 16:54:18 -0300 Subject: [PATCH] chore: validate signature by resource Signed-off-by: Vitor Mattos --- lib/Service/Install/ConfigureCheckService.php | 49 ++++++---- lib/Service/Install/InstallService.php | 2 +- lib/Service/Install/SignSetupService.php | 89 +++++++++++++------ .../Service/Install/SignSetupServiceTest.php | 39 +++++--- tests/psalm-baseline.xml | 3 +- 5 files changed, 124 insertions(+), 58 deletions(-) diff --git a/lib/Service/Install/ConfigureCheckService.php b/lib/Service/Install/ConfigureCheckService.php index b1e0805eae..4de4cf3ec4 100644 --- a/lib/Service/Install/ConfigureCheckService.php +++ b/lib/Service/Install/ConfigureCheckService.php @@ -15,13 +15,15 @@ use OCP\AppFramework\Services\IAppConfig; class ConfigureCheckService { + private string $architecture; public function __construct( private IAppConfig $appConfig, private SystemConfig $systemConfig, private JSignPdfHandler $jSignPdfHandler, private CertificateEngine $certificateEngine, - private InstallService $installService, + private SignSetupService $signSetupService, ) { + $this->architecture = php_uname('m'); } /** @@ -43,27 +45,12 @@ public function checkAll(): array { */ public function checkSign(): array { $return = []; - $return = array_merge($return, $this->checkHash()); $return = array_merge($return, $this->checkJava()); $return = array_merge($return, $this->checkPdftk()); $return = array_merge($return, $this->checkJSignPdf()); return $return; } - public function checkHash(): array { - if (!$this->installService->isDownloadedFilesOk()) { - return [ - (new ConfigureCheckHelper()) - ->setErrorMessage( - 'Invalid hash of binaries files.' - ) - ->setResource('java') - ->setTip('Run occ libresign:install --all'), - ]; - } - return []; - } - /** * Check all requirements to use JSignPdf * @@ -72,6 +59,16 @@ public function checkHash(): array { public function checkJSignPdf(): array { $jsignpdJarPath = $this->appConfig->getAppValue('jsignpdf_jar_path'); if ($jsignpdJarPath) { + if (count($this->signSetupService->verify($this->architecture, 'jsignpdf'))) { + return [ + (new ConfigureCheckHelper()) + ->setErrorMessage( + 'Invalid hash of binaries files.' + ) + ->setResource('jsignpdf') + ->setTip('Run occ libresign:install --all'), + ]; + } if (file_exists($jsignpdJarPath)) { if (!$this->isJavaOk()) { return [ @@ -132,6 +129,16 @@ public function checkJSignPdf(): array { public function checkPdftk(): array { $pdftkPath = $this->appConfig->getAppValue('pdftk_path'); if ($pdftkPath) { + if (count($this->signSetupService->verify($this->architecture, 'pdftk'))) { + return [ + (new ConfigureCheckHelper()) + ->setErrorMessage( + 'Invalid hash of binaries files.' + ) + ->setResource('pdftk') + ->setTip('Run occ libresign:install --all'), + ]; + } if (file_exists($pdftkPath)) { if (!$this->isJavaOk()) { return [ @@ -201,6 +208,16 @@ public function checkPdftk(): array { private function checkJava(): array { $javaPath = $this->appConfig->getAppValue('java_path'); if ($javaPath) { + if (count($this->signSetupService->verify($this->architecture, 'java'))) { + return [ + (new ConfigureCheckHelper()) + ->setErrorMessage( + 'Invalid hash of binaries files.' + ) + ->setResource('java') + ->setTip('Run occ libresign:install --all'), + ]; + } if (file_exists($javaPath)) { \exec($javaPath . " -version 2>&1", $javaVersion, $resultCode); if (empty($javaVersion)) { diff --git a/lib/Service/Install/InstallService.php b/lib/Service/Install/InstallService.php index 8e23eb0279..48261d8ecd 100644 --- a/lib/Service/Install/InstallService.php +++ b/lib/Service/Install/InstallService.php @@ -331,7 +331,7 @@ public function setResource(string $resource): self { public function isDownloadedFilesOk(): bool { try { - return count($this->signSetupService->verify($this->architecture)) === 0; + return count($this->signSetupService->verify($this->architecture, $this->resource)) === 0; } catch (InvalidSignatureException $e) { return false; } diff --git a/lib/Service/Install/SignSetupService.php b/lib/Service/Install/SignSetupService.php index b36ffbd2af..4d208859b7 100644 --- a/lib/Service/Install/SignSetupService.php +++ b/lib/Service/Install/SignSetupService.php @@ -29,6 +29,9 @@ class SignSetupService { 'unauthetnicated', ]; private string $architecture; + private string $resource; + private array $signatureData = []; + private ?x509 $x509 = null; public function __construct( private EnvironmentHelper $environmentHelper, private FileAccessHelper $fileAccessHelper, @@ -59,14 +62,11 @@ public function writeAppSignature( X509 $certificate, RSA $privateKey, string $architecture, - string $appInfoDir = '', ) { $this->architecture = $architecture; - $appInfoDir = $this->getAppInfoDirectory($appInfoDir); + $appInfoDir = $this->getAppInfoDirectory(); try { - $this->fileAccessHelper->assertDirectoryExists($appInfoDir); - - $iterator = $this->getFolderIterator(); + $iterator = $this->getFolderIterator($this->getInstallPath()); $hashes = $this->generateHashes($iterator); $signature = $this->createSignatureData($hashes, $certificate, $privateKey); $this->fileAccessHelper->file_put_contents( @@ -83,11 +83,10 @@ public function writeAppSignature( } } - private function getAppInfoDirectory(string $appInfoDir): string { - if (is_dir($appInfoDir)) { - return $appInfoDir; - } - return realpath(__DIR__ . '/../../../appinfo'); + protected function getAppInfoDirectory(): string { + $appInfoDir = realpath(__DIR__ . '/../../../appinfo'); + $this->fileAccessHelper->assertDirectoryExists($appInfoDir); + return $appInfoDir; } /** @@ -102,10 +101,11 @@ private function splitCerts(string $cert): array { return $matches[0]; } - public function verify(string $architecture, string $appInfoDir = '', string $certificateCN = Application::APP_ID): array { - $this->architecture = $architecture; - $appInfoDir = $this->getAppInfoDirectory($appInfoDir); - + private function getSignatureData(): array { + if (!empty($this->signatureData)) { + return $this->signatureData; + } + $appInfoDir = $this->getAppInfoDirectory(); $signaturePath = $appInfoDir . '/install-' . $this->architecture . '.json'; $content = $this->fileAccessHelper->file_get_contents($signaturePath); $signatureData = null; @@ -116,31 +116,58 @@ public function verify(string $architecture, string $appInfoDir = '', string $ce if (!\is_array($signatureData)) { throw new InvalidSignatureException('Signature data not found.'); } + $this->signatureData = $signatureData; + + $this->validateIfIssignedByLibresignAppCertificate($signatureData['hashes']); + return $this->signatureData; + } + + private function getHashesOfResource(): array { + $signatureData = $this->getSignatureData(); $expectedHashes = $signatureData['hashes']; - ksort($expectedHashes); - $signature = base64_decode($signatureData['signature']); + $filtered = array_filter($expectedHashes, function (string $key) { + return str_starts_with($key, $this->resource); + }, ARRAY_FILTER_USE_KEY); + if (!$filtered) { + throw new InvalidSignatureException('No signature files to ' . $this->resource); + } + return $filtered; + } + + private function getLibresignAppCertificate(): X509 { + if ($this->x509 instanceof X509) { + return $this->x509; + } + $signatureData = $this->getSignatureData(); $certificate = $signatureData['certificate']; // Check if certificate is signed by Nextcloud Root Authority - $x509 = new \phpseclib\File\X509(); + $this->x509 = new X509(); $rootCertificatePublicKey = $this->fileAccessHelper->file_get_contents($this->environmentHelper->getServerRoot().'/resources/codesigning/root.crt'); $rootCerts = $this->splitCerts($rootCertificatePublicKey); foreach ($rootCerts as $rootCert) { - $x509->loadCA($rootCert); + $this->x509->loadCA($rootCert); } - $x509->loadX509($certificate); - if (!$x509->validateSignature()) { + $this->x509->loadX509($certificate); + if (!$this->x509->validateSignature()) { throw new InvalidSignatureException('Certificate is not valid.'); } + // Verify if certificate has proper CN. "core" CN is always trusted. - if ($x509->getDN(X509::DN_OPENSSL)['CN'] !== $certificateCN && $x509->getDN(X509::DN_OPENSSL)['CN'] !== 'core') { + if ($this->x509->getDN(X509::DN_OPENSSL)['CN'] !== Application::APP_ID && $this->x509->getDN(X509::DN_OPENSSL)['CN'] !== 'core') { throw new InvalidSignatureException( - sprintf('Certificate is not valid for required scope. (Requested: %s, current: CN=%s)', $certificateCN, $x509->getDN(true)['CN']) + sprintf('Certificate is not valid for required scope. (Requested: %s, current: CN=%s)', Application::APP_ID, $this->x509->getDN(true)['CN']) ); } + return $this->x509; + } + + private function validateIfIssignedByLibresignAppCertificate(array $expectedHashes): void { + $x509 = $this->getLibresignAppCertificate(); + // Check if the signature of the files is valid $rsa = new \phpseclib\Crypt\RSA(); $rsa->loadKey($x509->currentCert['tbsCertificate']['subjectPublicKeyInfo']['subjectPublicKey']); @@ -148,13 +175,23 @@ public function verify(string $architecture, string $appInfoDir = '', string $ce $rsa->setMGFHash('sha512'); // See https://tools.ietf.org/html/rfc3447#page-38 $rsa->setSaltLength(0); + + $signatureData = $this->getSignatureData(); + $signature = base64_decode($signatureData['signature']); if (!$rsa->verify(json_encode($expectedHashes), $signature)) { throw new InvalidSignatureException('Signature could not get verified.'); } + } + + public function verify(string $architecture, $resource): array { + $this->architecture = $architecture; + $this->resource = $resource; + + $expectedHashes = $this->getHashesOfResource(); // Compare the list of files which are not identical - $installPath = $this->getInstallPath(); - $currentInstanceHashes = $this->generateHashes($this->getFolderIterator(), $installPath); + $installPath = $this->getInstallPath() . '/' . $this->resource; + $currentInstanceHashes = $this->generateHashes($this->getFolderIterator($installPath), $installPath); $differencesA = array_diff($expectedHashes, $currentInstanceHashes); $differencesB = array_diff($currentInstanceHashes, $expectedHashes); $differences = array_merge($differencesA, $differencesB); @@ -224,9 +261,9 @@ private function getInstallPath(): string { * @return \RecursiveIteratorIterator * @throws \Exception */ - private function getFolderIterator(): \RecursiveIteratorIterator { + private function getFolderIterator(string $folderToIterate): \RecursiveIteratorIterator { $dirItr = new \RecursiveDirectoryIterator( - $this->getInstallPath(), + $folderToIterate, \RecursiveDirectoryIterator::SKIP_DOTS ); diff --git a/tests/Unit/Service/Install/SignSetupServiceTest.php b/tests/Unit/Service/Install/SignSetupServiceTest.php index 5d268f0554..103d7d841c 100644 --- a/tests/Unit/Service/Install/SignSetupServiceTest.php +++ b/tests/Unit/Service/Install/SignSetupServiceTest.php @@ -56,7 +56,7 @@ private function getNewCert(): array { 'private_key_type' => OPENSSL_KEYTYPE_RSA, ]); - $csrNames = ['commonName' => 'LibreSign']; + $csrNames = ['commonName' => 'libresign']; $csr = openssl_csr_new($csrNames, $privateKey, ['digest_alg' => 'sha256']); $x509 = openssl_csr_sign($csr, null, $privateKey, $days = 365, ['digest_alg' => 'sha256']); @@ -115,8 +115,10 @@ private function writeAppSignature(string $architecture): SignSetupService { $structure = [ 'data' => [ 'libresign' => [ - 'fakeFile01' => 'content', - 'fakeFile02' => 'content', + 'java' => [ + 'fakeFile01' => 'content', + 'fakeFile02' => 'content', + ], ], ], 'resources' => [ @@ -134,18 +136,25 @@ private function writeAppSignature(string $architecture): SignSetupService { $this->environmentHelper->method('getServerRoot') ->willReturn('vfs://home'); - $signSetupService = $this->getInstance(['getInternalPathOfFolder']); + $signSetupService = $this->getInstance([ + 'getInternalPathOfFolder', + 'getAppInfoDirectory', + ]); $signSetupService->expects($this->any()) ->method('getInternalPathOfFolder') ->willReturn('libresign'); + $signSetupService->expects($this->any()) + ->method('getAppInfoDirectory') + ->willReturn('vfs://home/appinfo'); + $signSetupService->writeAppSignature($x509, $rsa, $architecture, 'vfs://home/appinfo'); $this->assertFileExists('vfs://home/appinfo/install-' . $architecture . '.json'); $json = file_get_contents('vfs://home/appinfo/install-' . $architecture . '.json'); $signatureContent = json_decode($json, true); $this->assertArrayHasKey('hashes', $signatureContent); $this->assertCount(2, $signatureContent['hashes']); - $expected = hash('sha512', $structure['data']['libresign']['fakeFile01']); - $actual = $signatureContent['hashes']['fakeFile01']; + $expected = hash('sha512', $structure['data']['libresign']['java']['fakeFile01']); + $actual = $signatureContent['hashes']['java/fakeFile01']; $this->assertEquals($expected, $actual); return $signSetupService; } @@ -155,7 +164,9 @@ private function writeAppSignature(string $architecture): SignSetupService { */ public function testWriteAppSignature(string $architecture): void { $signSetupService = $this->writeAppSignature($architecture); - $actual = $signSetupService->verify($architecture, 'vfs://home/appinfo', 'LibreSign'); + $architecture = 'x86_64'; + $resource = 'java'; + $actual = $signSetupService->verify($architecture, $resource); $this->assertCount(0, $actual); } @@ -168,30 +179,30 @@ public static function dataWriteAppSignature(): array { public function testVerify(): void { $architecture = 'x86_64'; $signSetupService = $this->writeAppSignature($architecture); - unlink('vfs://home/data/libresign/fakeFile01'); - file_put_contents('vfs://home/data/libresign/fakeFile02', 'invalidContent'); - file_put_contents('vfs://home/data/libresign/fakeFile03', 'invalidContent'); + unlink('vfs://home/data/libresign/java/fakeFile01'); + file_put_contents('vfs://home/data/libresign/java/fakeFile02', 'invalidContent'); + file_put_contents('vfs://home/data/libresign/java/fakeFile03', 'invalidContent'); $expected = json_encode([ 'FILE_MISSING' => [ - 'fakeFile01' => [ + 'java/fakeFile01' => [ 'expected' => 'b2d1d285b5199c85f988d03649c37e44fd3dde01e5d69c50fef90651962f48110e9340b60d49a479c4c0b53f5f07d690686dd87d2481937a512e8b85ee7c617f', 'current' => '', ], ], 'INVALID_HASH' => [ - 'fakeFile02' => [ + 'java/fakeFile02' => [ 'expected' => 'b2d1d285b5199c85f988d03649c37e44fd3dde01e5d69c50fef90651962f48110e9340b60d49a479c4c0b53f5f07d690686dd87d2481937a512e8b85ee7c617f', 'current' => '827a4e298c978e1eeffebdf09f0fa5a1e1d8b608c8071144f3fffb31f9ed21f6d27f88a63f7409583df7438105f713ff58d55e68e61e01a285125d763045c726', ], ], 'EXTRA_FILE' => [ - 'fakeFile03' => [ + 'java/fakeFile03' => [ 'expected' => '', 'current' => '827a4e298c978e1eeffebdf09f0fa5a1e1d8b608c8071144f3fffb31f9ed21f6d27f88a63f7409583df7438105f713ff58d55e68e61e01a285125d763045c726', ], ], ]); - $actual = $signSetupService->verify($architecture, 'vfs://home/appinfo', 'LibreSign'); + $actual = $signSetupService->verify($architecture, 'java'); $actual = json_encode($actual); $this->assertJsonStringEqualsJsonString($expected, $actual); } diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml index b703fe0f1d..26059fd874 100644 --- a/tests/psalm-baseline.xml +++ b/tests/psalm-baseline.xml @@ -128,6 +128,8 @@ + currentCert]]> + @@ -136,7 +138,6 @@ -