diff --git a/lib/Handler/CertificateEngine/CfsslHandler.php b/lib/Handler/CertificateEngine/CfsslHandler.php index 82bfd94361..ee247b3104 100644 --- a/lib/Handler/CertificateEngine/CfsslHandler.php +++ b/lib/Handler/CertificateEngine/CfsslHandler.php @@ -87,6 +87,10 @@ public function generateCertificate(): string { $certKeys['private_key'], [ 'friendly_name' => $this->getFriendlyName(), + 'extracerts' => [ + $certKeys['certificate'], + $certKeys['certificate_request'], + ], ], ); } diff --git a/lib/Handler/CertificateEngine/OpenSslHandler.php b/lib/Handler/CertificateEngine/OpenSslHandler.php index ff9603f674..2aea4b716e 100644 --- a/lib/Handler/CertificateEngine/OpenSslHandler.php +++ b/lib/Handler/CertificateEngine/OpenSslHandler.php @@ -69,21 +69,11 @@ public function generateCertificate(): string { 'private_key_bits' => 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA, ]); - $temporaryFile = $this->tempManager->getTemporaryFile('.cfg'); - // More information about x509v3: https://www.openssl.org/docs/manmaster/man5/x509v3_config.html - file_put_contents($temporaryFile, <<getSubjectAltNames()} - authorityKeyIdentifier = keyid - subjectKeyIdentifier = hash - # certificatePolicies = CPS: http://url/with/policy/informations.pdf - CONFIG); + $csr = openssl_csr_new($this->getCsrNames(), $privateKey); + $x509 = openssl_csr_sign($csr, $rootCertificate, $rootPrivateKey, $this->expirity(), [ - 'config' => $temporaryFile, + 'config' => $this->getFilenameToLeafCert(), // This will set "basicConstraints" to CA:FALSE, the default is CA:TRUE // The signer certificate is not a Certificate Authority 'x509_extensions' => 'v3_req', @@ -93,15 +83,56 @@ public function generateCertificate(): string { $privateKey, [ 'friendly_name' => $this->getFriendlyName(), + 'extracerts' => [ + $x509, + $rootCertificate, + ], ], ); } + private function getFilenameToLeafCert(): string { + $temporaryFile = $this->tempManager->getTemporaryFile('.cfg'); + // More information about x509v3: https://www.openssl.org/docs/manmaster/man5/x509v3_config.html + $config = [ + 'v3_req' => [ + 'basicConstraints' => 'CA:FALSE', + 'keyUsage' => 'digitalSignature, keyEncipherment, keyCertSign', + 'extendedKeyUsage' => 'clientAuth, emailProtection', + 'subjectAltName' => $this->getSubjectAltNames(), + 'authorityKeyIdentifier' => 'keyid', + 'subjectKeyIdentifier' => 'hash', + // @todo Implement a feature to define this PDF at Administration Settings + // 'certificatePolicies' => ' CPS: http://url/with/policy/informations.pdf', + ] + ]; + if (empty($config['v3_req']['subjectAltName'])) { + unset($config['v3_req']['subjectAltName']); + } + $config = $this->arrayToIni($config); + file_put_contents($temporaryFile, $config); + return $temporaryFile; + } + + private function arrayToIni(array $config) { + $fileContent = ''; + foreach ($config as $i => $v) { + if (is_array($v)) { + $fileContent .= "\n[$i]\n" . $this->arrayToIni($v); + } else { + $fileContent .= "$i = " . (str_contains($v, "\n") ? '"' . $v . '"' : $v) . "\n"; + } + } + return $fileContent; + } + private function getSubjectAltNames(): string { $hosts = $this->getHosts(); $altNames = []; - foreach ($hosts as $email) { - $altNames[] = 'email:' . $email; + foreach ($hosts as $host) { + if (filter_var($host, FILTER_VALIDATE_EMAIL)) { + $altNames[] = 'email:' . $host; + } } return implode(', ', $altNames); } diff --git a/lib/Handler/CertificateEngine/OrderCertificatesTrait.php b/lib/Handler/CertificateEngine/OrderCertificatesTrait.php index d65d3bab74..ecc9cba894 100644 --- a/lib/Handler/CertificateEngine/OrderCertificatesTrait.php +++ b/lib/Handler/CertificateEngine/OrderCertificatesTrait.php @@ -8,37 +8,70 @@ namespace OCA\Libresign\Handler\CertificateEngine; +use InvalidArgumentException; + trait OrderCertificatesTrait { public function orderCertificates(array $certificates): array { - $ordered = []; - $map = []; + $this->validateCertificateStructure($certificates); + $remainingCerts = []; - $tree = current($certificates); + // Add the root cert at ordered list and collect the remaining certs foreach ($certificates as $cert) { - if ($tree['subject'] === $cert['issuer']) { - $tree = $cert; + if (!$this->arrayDiffCanonicalized($cert['subject'], $cert['issuer'])) { + $ordered = [$cert]; + continue; } - $map[$cert['name']] = $cert; + $remainingCerts[$cert['name']] = $cert; } - if (!$tree) { + if (!isset($ordered)) { return $certificates; } - unset($map[$tree['name']]); - $ordered[] = $tree; - - $current = $tree; - while (!empty($map) && $current) { - if ($current['subject'] === $tree['issuer']) { - $ordered[] = $current; - $tree = $current; - unset($map[$current['name']]); - $current = reset($map); - continue; + + + while (!empty($remainingCerts)) { + $found = false; + foreach ($remainingCerts as $name => $cert) { + $first = reset($ordered); + if (!$this->arrayDiffCanonicalized($first['subject'], $cert['issuer'])) { + array_unshift($ordered, $cert); + unset($remainingCerts[$name]); + $found = true; + break; + } + } + + if (!$found) { + throw new InvalidArgumentException('Certificate chain is incomplete or invalid.'); } - $current = next($map); } return $ordered; } + + private function validateCertificateStructure(array $certificates): void { + if (empty($certificates)) { + throw new InvalidArgumentException('Certificate list cannot be empty'); + } + + foreach ($certificates as $cert) { + if (!isset($cert['subject'], $cert['issuer'], $cert['name'])) { + throw new InvalidArgumentException( + 'Invalid certificate structure. Certificate must have "subject", "issuer", and "name".' + ); + } + } + + $names = array_column($certificates, 'name'); + if (count($names) !== count(array_unique($names))) { + throw new InvalidArgumentException('Duplicate certificate names detected'); + } + } + + private function arrayDiffCanonicalized(array $array1, array $array2): array { + sort($array1); + sort($array2); + + return array_diff($array1, $array2); + } } diff --git a/tests/Unit/Handler/CertificateEngine/OpenSslHandlerTest.php b/tests/Unit/Handler/CertificateEngine/OpenSslHandlerTest.php new file mode 100644 index 0000000000..fcb2f4b21b --- /dev/null +++ b/tests/Unit/Handler/CertificateEngine/OpenSslHandlerTest.php @@ -0,0 +1,217 @@ +config = \OCP\Server::get(IConfig::class); + $this->appConfig = \OCP\Server::get(IAppConfig::class); + $this->appDataFactory = \OCP\Server::get(IAppDataFactory::class); + $this->dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class); + $this->tempManager = \OCP\Server::get(ITempManager::class); + + // The storage can't be modified when create a new instance to + // don't lost the root cert + vfsStream::setup('certificate'); + } + + private function getInstance(): OpenSslHandler { + $this->openSslHandler = new OpenSslHandler( + $this->config, + $this->appConfig, + $this->appDataFactory, + $this->dateTimeFormatter, + $this->tempManager, + ); + $this->openSslHandler->setConfigPath('vfs://certificate/'); + return $this->openSslHandler; + } + + public function testEmptyCertificate(): void { + $signerInstance = $this->getInstance(); + + // Test invalid password + $this->expectException(EmptyCertificateException::class); + $signerInstance->readCertificate('', ''); + } + + public function testInvalidPassword(): void { + // Create root cert + $rootInstance = $this->getInstance(); + $rootInstance->generateRootCert('', []); + + // Create signer cert + $signerInstance = $this->getInstance(); + $signerInstance->setHosts(['user@email.tld']); + $signerInstance->setPassword('right password'); + $certificateContent = $signerInstance->generateCertificate(); + + // Test invalid password + $this->expectException(InvalidPasswordException::class); + $signerInstance->readCertificate($certificateContent, 'invalid password'); + } + + /** + * @dataProvider dataReadCertificate + */ + public function testReadCertificate( + string $commonName, + string $signerName, + array $hosts, + string $password, + array $csrNames, + array $root, + ): void { + // Create root cert + $rootInstance = $this->getInstance(); + if (isset($root['CN'])) { + $rootInstance->setCommonName($root['CN']); + } + if (isset($root['C'])) { + $rootInstance->setCountry($root['C']); + } + if (isset($root['ST'])) { + $rootInstance->setState($root['ST']); + } + if (isset($root['O'])) { + $rootInstance->setOrganization($root['O']); + } + if (isset($root['OU'])) { + $rootInstance->setOrganizationalUnit($root['OU']); + } + $rootInstance->generateRootCert($commonName, $root); + + // Create signer cert + $signerInstance = $this->getInstance(); + $signerInstance->setHosts($hosts); + $signerInstance->setPassword($password); + $signerInstance->setFriendlyName($signerName); + if (isset($csrNames['CN'])) { + $signerInstance->setCommonName($csrNames['CN']); + } + if (isset($csrNames['C'])) { + $signerInstance->setCountry($csrNames['C']); + } + if (isset($csrNames['ST'])) { + $signerInstance->setState($csrNames['ST']); + } + if (isset($csrNames['O'])) { + $signerInstance->setOrganization($csrNames['O']); + } + if (isset($csrNames['OU'])) { + $signerInstance->setOrganizationalUnit($csrNames['OU']); + } + $certificateContent = $signerInstance->generateCertificate(); + + // Parse signer cert + $parsed = $signerInstance->readCertificate($certificateContent, $password); + + // Test total elements of extracerts + // The correct content is: cert signer, intermediate certs (if have), root cert + $this->assertArrayHasKey('extracerts', $parsed); + $this->assertCount(2, $parsed['extracerts']); + + // Test name + $name = $this->csrArrayToString($csrNames); + $this->assertEquals($parsed['name'], $name); + $this->assertJsonStringEqualsJsonString( + json_encode($csrNames), + json_encode($parsed['subject']) + ); + + // Test subject + $this->assertEquals($csrNames, $parsed['subject']); + + // Test issuer ony if was defined root distinguished names + if (count($root) === count($parsed['issuer'])) { + $this->assertEquals($root, $parsed['issuer']); + } + } + + private function csrArrayToString(array $csr): string { + $return = ''; + foreach ($csr as $key => $value) { + $return .= "/$key=$value"; + } + return $return; + } + + public static function dataReadCertificate(): array { + return [ + [ + 'common name', + 'Signer Name', + ['user@domain.tld'], + 'password', + [ + 'C' => 'CT', + 'ST' => 'Some-State', + 'O' => 'Organization Name', + ], + [], + ], + [ + 'common name', + 'Signer Name', + ['account:test'], + 'password', + [ + 'C' => 'CT', + 'ST' => 'Some-State', + 'O' => 'Organization Name', + 'OU' => 'Organization Unit', + 'CN' => 'Common Name', + ], + [ + 'C' => 'RT', + 'ST' => 'Root-State', + 'O' => 'Root Organization Name', + 'OU' => 'Root Organization Unit', + 'CN' => 'Root Common Name', + 'UID' => 'account:test' + ], + ], + [ + 'common name', + 'Signer Name', + ['user@domain.tld'], + 'password', + [ + 'C' => 'CT', + 'ST' => 'Some-State', + 'O' => 'Organization Name', + 'OU' => 'Organization Unit', + 'CN' => 'Common Name', + ], + [ + 'C' => 'RT', + 'ST' => 'Root-State', + 'O' => 'Root Organization Name', + 'OU' => 'Root Organization Unit', + 'CN' => 'Root Common Name', + 'UID' => 'email:user@domain.tld' + ], + ], + ]; + } +} diff --git a/tests/Unit/Handler/CertificateEngine/OrderCertificatesTraitTest.php b/tests/Unit/Handler/CertificateEngine/OrderCertificatesTraitTest.php new file mode 100644 index 0000000000..7ac947ce6b --- /dev/null +++ b/tests/Unit/Handler/CertificateEngine/OrderCertificatesTraitTest.php @@ -0,0 +1,271 @@ +orderCertificates = new Test(); + } + + public function testEmptyCertList(): void { + $this->expectExceptionMessage('Certificate list cannot be empty'); + $this->orderCertificates->orderCertificates([]); + } + + /** + * @dataProvider dataInvalidStructure + */ + public function testInvalidStructure(array $certList): void { + $this->expectExceptionMessage('Invalid certificate structure. Certificate must have "subject", "issuer", and "name".'); + $this->orderCertificates->orderCertificates($certList); + } + + public static function dataInvalidStructure(): array { + return [ + [[['fake']]], + [[['name' => '']]], + [[['name' => '', 'issuer' => '']]], + [[['name' => '', 'subject' => '']]], + [[['issuer' => '', 'subject' => '']]], + ]; + } + + /** + * @dataProvider dataIncompleteCertificateChain + */ + public function testIncompleteCertificateChain($certList): void { + $this->expectExceptionMessage('Certificate chain is incomplete or invalid.'); + $this->orderCertificates->orderCertificates($certList); + } + + public static function dataIncompleteCertificateChain(): array { + return [ + [ + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Invalid'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + [ + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Intermediate'], + ], + [ + 'name' => '/CN=Intermediate', + 'subject' => ['CN' => 'Intermediate'], + 'issuer' => ['CN' => 'Invalid'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + ]; + } + + /** + * @dataProvider dataOrderCertificates + */ + public function testOrderCertificates(array $unordered, array $expected): void { + $actual = $this->orderCertificates->orderCertificates($unordered); + $this->assertEquals($expected, $actual); + } + + public static function dataOrderCertificates(): array { + return [ + 'only one cert, with root' => [ + [ + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + [ + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + 'only one cert, without root' => [ + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + ], + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + 'two certs, unordered' => [ + [ + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + ], + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + 'two certs, orderd' => [ + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + 'tree certs, unordered' => [ + [ + [ + 'name' => '/CN=Intermediate', + 'subject' => ['CN' => 'Intermediate'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Intermediate'], + ], + ], + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Intermediate'], + ], + [ + 'name' => '/CN=Intermediate', + 'subject' => ['CN' => 'Intermediate'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + 'Four certs, unordered' => [ + [ + [ + 'name' => '/CN=Intermediate 2', + 'subject' => ['CN' => 'Intermediate 2'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Intermediate 1', + 'subject' => ['CN' => 'Intermediate 1'], + 'issuer' => ['CN' => 'Intermediate 2'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Intermediate 1'], + ], + ], + [ + [ + 'name' => '/CN=Leaf', + 'subject' => ['CN' => 'Leaf'], + 'issuer' => ['CN' => 'Intermediate 1'], + ], + [ + 'name' => '/CN=Intermediate 1', + 'subject' => ['CN' => 'Intermediate 1'], + 'issuer' => ['CN' => 'Intermediate 2'], + ], + [ + 'name' => '/CN=Intermediate 2', + 'subject' => ['CN' => 'Intermediate 2'], + 'issuer' => ['CN' => 'Root'], + ], + [ + 'name' => '/CN=Root', + 'subject' => ['CN' => 'Root'], + 'issuer' => ['CN' => 'Root'], + ], + ], + ], + ]; + } +} diff --git a/tests/Unit/Handler/OpenSslHandlerTest.php b/tests/Unit/Handler/OpenSslHandlerTest.php deleted file mode 100644 index 1f855d118e..0000000000 --- a/tests/Unit/Handler/OpenSslHandlerTest.php +++ /dev/null @@ -1,109 +0,0 @@ -config = \OCP\Server::get(IConfig::class); - $this->appConfig = \OCP\Server::get(IAppConfig::class); - $this->appDataFactory = \OCP\Server::get(IAppDataFactory::class); - $this->dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class); - $this->tempManager = \OCP\Server::get(ITempManager::class); - $this->openSslHandler = new OpenSslHandler( - $this->config, - $this->appConfig, - $this->appDataFactory, - $this->dateTimeFormatter, - $this->tempManager, - ); - vfsStream::setup('certificate'); - $this->openSslHandler->setConfigPath('vfs://certificate/'); - } - - /** - * @dataProvider dataReadCertificate - */ - public function testReadCertificate(string $commonName, string $signerName, array $hosts, string $password, array $csrNames): void { - if (isset($csrNames['C'])) { - $this->openSslHandler->setCountry($csrNames['C']); - } - if (isset($csrNames['ST'])) { - $this->openSslHandler->setState($csrNames['ST']); - } - if (isset($csrNames['O'])) { - $this->openSslHandler->setOrganization($csrNames['O']); - } - if (isset($csrNames['OU'])) { - $this->openSslHandler->setOrganizationalUnit($csrNames['OU']); - } - $this->openSslHandler->generateRootCert($commonName, $csrNames); - - $this->openSslHandler->setHosts($hosts); - $this->openSslHandler->setPassword($password); - $this->openSslHandler->setFriendlyName($signerName); - $certificateContent = $this->openSslHandler->generateCertificate(); - $parsed = $this->openSslHandler->readCertificate($certificateContent, $password); - - $name = $this->csrArrayToString($csrNames); - $this->assertEquals($parsed['name'], $name); - - $this->assertJsonStringEqualsJsonString( - json_encode($csrNames), - json_encode($parsed['subject']) - ); - } - - private function csrArrayToString(array $csr): string { - $return = ''; - foreach ($csr as $key => $value) { - $return .= "/$key=$value"; - } - return $return; - } - - public static function dataReadCertificate(): array { - return [ - [ - 'common name', - 'Signer Name', - ['user@domain.tld'], - 'password', - [ - 'C' => 'CT', - 'ST' => 'Some-State', - 'O' => 'Organization Name', - ], - ], - [ - 'common name', - 'Signer Name', - ['user@domain.tld'], - 'password', - [ - 'C' => 'CT', - 'ST' => 'Some-State', - 'O' => 'Organization Name', - 'OU' => 'Organization Unit', - ], - ], - ]; - } -} diff --git a/tests/integration/features/file/validate.feature b/tests/integration/features/file/validate.feature index d238b8fdfe..734448f927 100644 --- a/tests/integration/features/file/validate.feature +++ b/tests/integration/features/file/validate.feature @@ -33,7 +33,7 @@ Feature: validate Then the response should be a JSON array with the following mandatory values | key | value | | (jq).ocs.data.signers[0].me | false | - | (jq).ocs.data.signers[0].uid | account:signer1 | + | (jq).ocs.data.signers[0].identifyMethods | [{"method": "account","value": "signer1","mandatory": 1}] | | (jq).ocs.data.signers[0].subject | /C=BR/ST=State of Company/L=City Name/O=Organization/OU=Organization Unit/UID=account:signer1/CN=signer1-displayname | - | (jq).ocs.data.signers[0].signature_validation | {"id":1,"label":"Signature is valid."} | + | (jq).ocs.data.signers[0].signature_validation | {"id":1,"label":"Signature is valid."} | | (jq).ocs.data.signers[0].hash_algorithm | RSA-SHA1 |