mirror of
https://github.com/LibreSign/libresign.git
synced 2025-12-17 21:12:16 +01:00
Merge pull request #6201 from LibreSign/fix/filter-ca-id-and-config-path-in-api
fix: prevent stale configPath and CA ID exposure in root certificate API
This commit is contained in:
commit
2f1e10bfc2
21 changed files with 233 additions and 24 deletions
|
|
@ -721,9 +721,10 @@ abstract class AEngineHandler implements IEngineHandler {
|
|||
|
||||
#[\Override]
|
||||
public function toArray(): array {
|
||||
$generated = $this->isSetupOk();
|
||||
$return = [
|
||||
'configPath' => $this->getCurrentConfigPath(),
|
||||
'generated' => $this->isSetupOk(),
|
||||
'configPath' => $this->getConfigPathForApi($generated),
|
||||
'generated' => $generated,
|
||||
'rootCert' => [
|
||||
'commonName' => $this->getCommonName(),
|
||||
'names' => [],
|
||||
|
|
@ -737,12 +738,32 @@ abstract class AEngineHandler implements IEngineHandler {
|
|||
foreach ($names as $name => $value) {
|
||||
$return['rootCert']['names'][] = [
|
||||
'id' => $name,
|
||||
'value' => $value,
|
||||
'value' => $this->filterNameValue($name, $value, $generated),
|
||||
];
|
||||
}
|
||||
return $return;
|
||||
}
|
||||
|
||||
private function getConfigPathForApi(bool $generated): string {
|
||||
return $generated ? $this->getCurrentConfigPath() : '';
|
||||
}
|
||||
|
||||
private function filterNameValue(string $name, mixed $value, bool $generated): mixed {
|
||||
if ($name === 'OU' && is_array($value) && !$generated) {
|
||||
$filtered = $this->removeCaIdFromOrganizationalUnit($value);
|
||||
return empty($filtered) ? null : $filtered;
|
||||
}
|
||||
return $value;
|
||||
}
|
||||
|
||||
private function removeCaIdFromOrganizationalUnit(array $organizationalUnits): array {
|
||||
$filtered = array_filter(
|
||||
$organizationalUnits,
|
||||
fn ($item) => !str_starts_with($item, 'libresign-ca-id:')
|
||||
);
|
||||
return array_values($filtered);
|
||||
}
|
||||
|
||||
protected function getCrlDistributionUrl(): string {
|
||||
$caIdParsed = $this->caIdentifierService->getCaIdParsed();
|
||||
return $this->urlGenerator->linkToRouteAbsolute('libresign.crl.getRevocationList', [
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ final class AEngineHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
|
||||
public function setUp(): void {
|
||||
$this->config = \OCP\Server::get(IConfig::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->appDataFactory = \OCP\Server::get(IAppDataFactory::class);
|
||||
$this->dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class);
|
||||
$this->tempManager = \OCP\Server::get(ITempManager::class);
|
||||
|
|
@ -40,9 +40,6 @@ final class AEngineHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
$this->urlGenerator = \OCP\Server::get(IURLGenerator::class);
|
||||
$this->caIdentifierService = \OCP\Server::get(CaIdentifierService::class);
|
||||
$this->logger = \OCP\Server::get(LoggerInterface::class);
|
||||
|
||||
$this->appConfig->deleteKey(Application::APP_ID, 'certificate_engine');
|
||||
$this->appConfig->deleteKey(Application::APP_ID, 'identify_methods');
|
||||
}
|
||||
|
||||
private function getInstance(): OpenSslHandler {
|
||||
|
|
@ -232,4 +229,181 @@ final class AEngineHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
'First time cfssl' => ['', 'cfssl', null, 'no previous config'],
|
||||
];
|
||||
}
|
||||
|
||||
#[DataProvider('dataProviderToArray')]
|
||||
public function testToArrayReturnsExpectedStructure(
|
||||
bool $isSetupOk,
|
||||
array $certificateData,
|
||||
array $expectedKeys,
|
||||
string $description,
|
||||
): void {
|
||||
$instance = $this->getInstance();
|
||||
|
||||
foreach ($certificateData as $setter => $value) {
|
||||
$method = 'set' . ucfirst($setter);
|
||||
if (method_exists($instance, $method)) {
|
||||
$instance->$method($value);
|
||||
}
|
||||
}
|
||||
|
||||
if (!$isSetupOk) {
|
||||
$this->appConfig->deleteKey(Application::APP_ID, 'config_path');
|
||||
}
|
||||
|
||||
$result = $instance->toArray();
|
||||
|
||||
foreach ($expectedKeys as $key) {
|
||||
$this->assertArrayHasKey($key, $result, "toArray() should contain '$key': $description");
|
||||
}
|
||||
|
||||
$this->assertIsBool($result['generated'], "generated should be boolean: $description");
|
||||
}
|
||||
|
||||
#[DataProvider('dataProviderToArrayConfigPath')]
|
||||
public function testToArrayFiltersConfigPathWhenNotGenerated(
|
||||
bool $certificateGenerated,
|
||||
string $expectedConfigPath,
|
||||
string $description,
|
||||
): void {
|
||||
$instance = $this->getInstance();
|
||||
|
||||
$tempPath = $this->tempManager->getTemporaryFolder('test-config');
|
||||
$instance->setConfigPath($tempPath);
|
||||
|
||||
if ($certificateGenerated) {
|
||||
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca.pem', 'fake-cert');
|
||||
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca-key.pem', 'fake-key');
|
||||
}
|
||||
|
||||
$result = $instance->toArray();
|
||||
|
||||
if ($expectedConfigPath === '') {
|
||||
$this->assertEmpty($result['configPath'], "configPath should be empty: $description");
|
||||
} else {
|
||||
$this->assertNotEmpty($result['configPath'], "configPath should not be empty: $description");
|
||||
}
|
||||
|
||||
$this->assertEquals($certificateGenerated, $result['generated'], "generated flag: $description");
|
||||
}
|
||||
|
||||
#[DataProvider('dataProviderToArrayCaIdFiltering')]
|
||||
public function testToArrayFiltersCaIdFromOrganizationalUnitWhenNotGenerated(
|
||||
bool $certificateGenerated,
|
||||
array $organizationalUnits,
|
||||
array $expectedOuValues,
|
||||
string $description,
|
||||
): void {
|
||||
$instance = $this->getInstance();
|
||||
|
||||
$tempPath = $this->tempManager->getTemporaryFolder('test-config');
|
||||
$instance->setConfigPath($tempPath);
|
||||
$instance->setOrganizationalUnit($organizationalUnits);
|
||||
$instance->setCountry('BR');
|
||||
|
||||
if ($certificateGenerated) {
|
||||
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca.pem', 'fake-cert');
|
||||
file_put_contents($tempPath . DIRECTORY_SEPARATOR . 'ca-key.pem', 'fake-key');
|
||||
}
|
||||
|
||||
$result = $instance->toArray();
|
||||
|
||||
$ouFound = null;
|
||||
foreach ($result['rootCert']['names'] as $name) {
|
||||
if ($name['id'] === 'OU') {
|
||||
$ouFound = $name['value'];
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (!empty($expectedOuValues)) {
|
||||
$this->assertNotNull($ouFound, "OU should be present in names array: $description");
|
||||
$this->assertEquals(
|
||||
$expectedOuValues,
|
||||
$ouFound,
|
||||
"OrganizationalUnit filtering: $description"
|
||||
);
|
||||
} else {
|
||||
$this->assertNull($ouFound, "OU should not be present when filtered to empty: $description");
|
||||
}
|
||||
}
|
||||
|
||||
public static function dataProviderToArray(): array {
|
||||
return [
|
||||
'Basic structure with minimal data' => [
|
||||
false,
|
||||
['commonName' => 'Test CA'],
|
||||
['configPath', 'generated', 'rootCert', 'policySection'],
|
||||
'minimal certificate data',
|
||||
],
|
||||
'Complete certificate data' => [
|
||||
false,
|
||||
[
|
||||
'commonName' => 'LibreSign CA',
|
||||
'country' => 'BR',
|
||||
'state' => 'Santa Catarina',
|
||||
'locality' => 'Florianópolis',
|
||||
'organization' => 'LibreCode',
|
||||
'organizationalUnit' => ['Engineering', 'Security'],
|
||||
],
|
||||
['configPath', 'generated', 'rootCert', 'policySection'],
|
||||
'full certificate data',
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
public static function dataProviderToArrayConfigPath(): array {
|
||||
return [
|
||||
'Certificate not generated' => [
|
||||
false,
|
||||
'',
|
||||
'configPath should be empty when certificate not generated',
|
||||
],
|
||||
'Certificate generated' => [
|
||||
true,
|
||||
'/path/to/config',
|
||||
'configPath should be set when certificate is generated',
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
public static function dataProviderToArrayCaIdFiltering(): array {
|
||||
return [
|
||||
'OU without CA ID - not generated' => [
|
||||
false,
|
||||
['Engineering', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'normal OU values should pass through when not generated',
|
||||
],
|
||||
'OU without CA ID - generated' => [
|
||||
true,
|
||||
['Engineering', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'normal OU values should pass through when generated',
|
||||
],
|
||||
'OU with CA ID - not generated (filtered)' => [
|
||||
false,
|
||||
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'CA ID should be filtered when certificate not generated',
|
||||
],
|
||||
'OU with CA ID - generated (kept)' => [
|
||||
true,
|
||||
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
|
||||
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
|
||||
'CA ID should be kept when certificate is generated',
|
||||
],
|
||||
'OU with only CA ID - not generated' => [
|
||||
false,
|
||||
['libresign-ca-id:abc123_g:1_e:openssl'],
|
||||
[],
|
||||
'OU should be empty when only CA ID and not generated',
|
||||
],
|
||||
'OU with only CA ID - generated' => [
|
||||
true,
|
||||
['libresign-ca-id:abc123_g:1_e:openssl'],
|
||||
['libresign-ca-id:abc123_g:1_e:openssl'],
|
||||
'OU with only CA ID should be kept when generated',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ class CertificateEngineFactoryTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
}
|
||||
|
||||
public function setUp(): void {
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->openSslHandler = $this->createMock(OpenSslHandler::class);
|
||||
$this->cfsslHandler = $this->createMock(CfsslHandler::class);
|
||||
$this->noneHandler = $this->createMock(NoneHandler::class);
|
||||
|
|
|
|||
|
|
@ -39,7 +39,7 @@ final class OpenSslHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
private string $tempDir;
|
||||
public function setUp(): void {
|
||||
$this->config = \OCP\Server::get(IConfig::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->appDataFactory = \OCP\Server::get(IAppDataFactory::class);
|
||||
$this->dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class);
|
||||
$this->tempManager = \OCP\Server::get(ITempManager::class);
|
||||
|
|
|
|||
|
|
@ -27,7 +27,7 @@ final class FooterHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
private ITempManager $tempManager;
|
||||
private FooterHandler $footerHandler;
|
||||
public function setUp(): void {
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->pdfParserService = $this->createMock(PdfParserService::class);
|
||||
$this->urlGenerator = $this->createMock(IURLGenerator::class);
|
||||
$this->tempManager = \OCP\Server::get(ITempManager::class);
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ final class PdfTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
public function setUp(): void {
|
||||
parent::setUp();
|
||||
$this->javaHelper = $this->createMock(JavaHelper::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -63,7 +63,7 @@ final class JSignPdfHandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
}
|
||||
}
|
||||
public function setUp(): void {
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->loggerInterface = $this->createMock(LoggerInterface::class);
|
||||
$this->tempManager = \OCP\Server::get(ITempManager::class);
|
||||
$this->signatureBackgroundService = $this->createMock(SignatureBackgroundService::class);
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ final class Pkcs12HandlerTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
|
||||
public function setUp(): void {
|
||||
$this->folderService = $this->createMock(FolderService::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class);
|
||||
$this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID);
|
||||
$this->footerHandler = $this->createMock(FooterHandler::class);
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ class JavaHelperTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
public function setUp(): void {
|
||||
parent::setUp();
|
||||
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->l10n = $this->createMock(IL10N::class);
|
||||
$this->logger = $this->createMock(LoggerInterface::class);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -33,7 +33,7 @@ final class CertificatePolicyServiceTest extends \OCA\Libresign\Tests\Unit\TestC
|
|||
public function setUp(): void {
|
||||
$this->appData = $this->createMock(IAppData::class);
|
||||
$this->urlGenerator = $this->createMock(IURLGenerator::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -106,7 +106,7 @@ final class FileServiceTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
$this->accountManager = $this->createMock(IAccountManager::class);
|
||||
$this->client = \OCP\Server::get(IClientService::class);
|
||||
$this->dateTimeFormatter = \OCP\Server::get(IDateTimeFormatter::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->urlGenerator = \OCP\Server::get(IURLGenerator::class);
|
||||
$this->mimeTypeDetector = \OCP\Server::get(IMimeTypeDetector::class);
|
||||
$this->pkcs12Handler = \OCP\Server::get(Pkcs12Handler::class);
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ class FooterServiceTest extends TestCase {
|
|||
|
||||
public function setUp(): void {
|
||||
parent::setUp();
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->footerHandler = $this->createMock(FooterHandler::class);
|
||||
$this->service = new FooterService($this->appConfig, $this->footerHandler);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ final class PasswordTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
|
||||
public function setUp(): void {
|
||||
$this->identifyService = $this->createMock(IdentifyService::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->folderService = $this->createMock(FolderService::class);
|
||||
$this->certificateEngineFactory = $this->createMock(CertificateEngineFactory::class);
|
||||
$this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID);
|
||||
|
|
|
|||
|
|
@ -48,7 +48,7 @@ final class ConfigureCheckServiceTest extends \OCA\Libresign\Tests\Unit\TestCase
|
|||
|
||||
public function setUp(): void {
|
||||
self::$mockExtensionLoaded = [];
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->systemConfig = $this->createMock(SystemConfig::class);
|
||||
$this->ocAppConfig = $this->createMock(AppConfig::class);
|
||||
$this->appManager = $this->createMock(IAppManager::class);
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ final class SignSetupServiceTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
$this->fileAccessHelper = new FileAccessHelper();
|
||||
$this->appManager = $this->createMock(IAppManager::class);
|
||||
$this->config = $this->createMock(IConfig::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->appDataFactory = \OCP\Server::get(IAppDataFactory::class);
|
||||
$this->tempManager = \OCP\Server::get(ITempManager::class);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ final class ReminderServiceTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
|
||||
public function setUp(): void {
|
||||
$this->jobList = $this->createMock(IJobList::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->dateTimeZone = Server::get(IDateTimeZone::class);
|
||||
$this->time = $this->createMock(ITimeFactory::class);
|
||||
$this->signRequestMapper = $this->createMock(SignRequestMapper::class);
|
||||
|
|
|
|||
|
|
@ -105,7 +105,7 @@ final class SignFileServiceTest extends \OCA\Libresign\Tests\Unit\TestCase {
|
|||
$this->userManager = $this->createMock(IUserManager::class);
|
||||
$this->folderService = $this->createMock(FolderService::class);
|
||||
$this->logger = $this->createMock(LoggerInterface::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->validateHelper = $this->createMock(\OCA\Libresign\Helper\ValidateHelper::class);
|
||||
$this->signerElementsService = $this->createMock(SignerElementsService::class);
|
||||
$this->root = $this->createMock(\OCP\Files\IRootFolder::class);
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ final class SignatureBackgroundServiceTest extends \OCA\Libresign\Tests\Unit\Tes
|
|||
|
||||
public function setUp(): void {
|
||||
$this->appData = $this->createMock(IAppData::class);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->config = $this->createMock(IConfig::class);
|
||||
$this->tempManager = $this->createMock(ITempManager::class);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ final class SignatureTextServiceTest extends \OCA\Libresign\Tests\Unit\TestCase
|
|||
|
||||
public function setUp(): void {
|
||||
$this->l10n = \OCP\Server::get(IL10NFactory::class)->get(Application::APP_ID);
|
||||
$this->appConfig = $this->getMockAppConfig();
|
||||
$this->appConfig = $this->getMockAppConfigWithReset();
|
||||
$this->dateTimeZone = \OCP\Server::get(IDateTimeZone::class);
|
||||
$this->request = $this->createMock(IRequest::class);
|
||||
$this->userSession = $this->createMock(IUserSession::class);
|
||||
|
|
|
|||
|
|
@ -82,6 +82,14 @@ class TestCase extends \Test\TestCase {
|
|||
return $service;
|
||||
}
|
||||
|
||||
public static function getMockAppConfigWithReset(): IAppConfig {
|
||||
$appConfig = self::getMockAppConfig();
|
||||
if ($appConfig instanceof AppConfigOverwrite) {
|
||||
$appConfig->reset();
|
||||
}
|
||||
return $appConfig;
|
||||
}
|
||||
|
||||
public function mockConfig($config):void {
|
||||
$service = \OCP\Server::get(\OCP\IConfig::class);
|
||||
if (!$service instanceof AllConfigOverwrite) {
|
||||
|
|
|
|||
|
|
@ -138,6 +138,12 @@ class AppConfigOverwrite extends AppConfig {
|
|||
$this->markDeleted($app, $key);
|
||||
}
|
||||
|
||||
public function reset(): self {
|
||||
$this->overWrite = [];
|
||||
$this->deleted = [];
|
||||
return $this;
|
||||
}
|
||||
|
||||
private function isDeleted(string $app, string $key): bool {
|
||||
return isset($this->deleted[$app][$key]);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue