mirror of
https://github.com/LibreSign/libresign.git
synced 2025-12-18 05:20:45 +01:00
fix: prevent stale configPath and CA ID exposure in root certificate API
Filter configPath from API response when certificate is not generated to prevent form pre-population with outdated generation numbers that cause validation errors. Filter CA ID (libresign-ca-id:*) from OrganizationalUnit field to prevent users from submitting stale generation values that conflict with certificate validation. Refactor toArray() method by extracting logic into dedicated methods: - getConfigPathForApi(): Returns empty string for non-generated certificates - removeCaIdFromOrganizationalUnit(): Filters CA IDs from OU arrays Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
This commit is contained in:
Vitor Mattos
parent
2d3f820a1f
commit
249f0837b0
2 changed files with 181 additions and 3 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,31 @@ abstract class AEngineHandler implements IEngineHandler {
|
|||
foreach ($names as $name => $value) {
|
||||
$return['rootCert']['names'][] = [
|
||||
'id' => $name,
|
||||
'value' => $value,
|
||||
'value' => $this->filterNameValue($name, $value),
|
||||
];
|
||||
}
|
||||
return $return;
|
||||
}
|
||||
|
||||
private function getConfigPathForApi(bool $generated): string {
|
||||
return $generated ? $this->getCurrentConfigPath() : '';
|
||||
}
|
||||
|
||||
private function filterNameValue(string $name, mixed $value): mixed {
|
||||
if ($name === 'OU' && is_array($value)) {
|
||||
return $this->removeCaIdFromOrganizationalUnit($value);
|
||||
}
|
||||
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', [
|
||||
|
|
|
|||
|
|
@ -232,4 +232,162 @@ 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 testToArrayFiltersCaIdFromOrganizationalUnit(
|
||||
array $organizationalUnits,
|
||||
array $expectedOuValues,
|
||||
string $description,
|
||||
): void {
|
||||
$instance = $this->getInstance();
|
||||
|
||||
$instance->setOrganizationalUnit($organizationalUnits);
|
||||
$instance->setCountry('BR');
|
||||
|
||||
$result = $instance->toArray();
|
||||
|
||||
$ouFound = false;
|
||||
foreach ($result['rootCert']['names'] as $name) {
|
||||
if ($name['id'] === 'OU') {
|
||||
$ouFound = true;
|
||||
$this->assertEquals(
|
||||
$expectedOuValues,
|
||||
$name['value'],
|
||||
"OrganizationalUnit should filter CA IDs: $description"
|
||||
);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (!empty($expectedOuValues)) {
|
||||
$this->assertTrue($ouFound, "OU should be present in names array: $description");
|
||||
} else {
|
||||
$this->assertFalse($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' => [
|
||||
['Engineering', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'normal OU values should pass through',
|
||||
],
|
||||
'OU with CA ID at start' => [
|
||||
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'CA ID at start should be filtered out',
|
||||
],
|
||||
'OU with CA ID in middle' => [
|
||||
['Engineering', 'libresign-ca-id:abc123_g:1_e:openssl', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'CA ID in middle should be filtered out',
|
||||
],
|
||||
'OU with CA ID at end' => [
|
||||
['Engineering', 'Security', 'libresign-ca-id:abc123_g:1_e:openssl'],
|
||||
['Engineering', 'Security'],
|
||||
'CA ID at end should be filtered out',
|
||||
],
|
||||
'OU with multiple CA IDs' => [
|
||||
['libresign-ca-id:abc123_g:1_e:openssl', 'Engineering', 'libresign-ca-id:xyz789_g:2_e:cfssl', 'Security'],
|
||||
['Engineering', 'Security'],
|
||||
'multiple CA IDs should be filtered out',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue