Skip to content

Commit 663563a

Browse files
SebSeptSebSept
authored
Add warnings when GLPI encryption key is invalid (#21614)
1 parent fe1ea3f commit 663563a

File tree

8 files changed

+202
-3
lines changed

8 files changed

+202
-3
lines changed

‎phpunit/functional/GLPIKeyTest.php‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -538,4 +538,105 @@ public function testIsConfigSecured()
538538
$this->assertFalse($is_myplugin_href_secured);
539539
$this->assertFalse($is_someplugin_conf_secured);
540540
}
541+
542+
public function testGetKeyFileReadErrorsWithMissingFile(): void
543+
{
544+
// arrange : create directory structure without key file
545+
vfsStream::setup('glpi', null, ['config' => []]);
546+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
547+
548+
// act
549+
$errors = $glpikey->getKeyFileReadErrors();
550+
551+
// assert
552+
$this->assertStringContainsString('The security key file does not exist.', implode(" ", $errors));
553+
}
554+
555+
public function testGetKeyFileReadErrorsWithUnreadableFile(): void
556+
{
557+
// arrange : create unreadable key file
558+
$structure = vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => 'unreadable file']]);
559+
$structure->getChild('config/glpicrypt.key')->chmod(0222);
560+
561+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
562+
563+
// act
564+
$errors = $glpikey->getKeyFileReadErrors();
565+
566+
// assert
567+
$this->assertStringContainsString('Unable to get security key file contents.', implode(" ", $errors));
568+
}
569+
570+
public function testGetKeyFileReadErrorsWithInvalidKey(): void
571+
{
572+
// arrange : key file exists but has invalid contents/length
573+
vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => 'not a valid key']]);
574+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
575+
576+
// act
577+
$errors = $glpikey->getKeyFileReadErrors();
578+
579+
// assert
580+
$this->assertStringContainsString('Invalid security key file contents.', implode(" ", $errors));
581+
}
582+
583+
public function testGetKeyFileReadErrorsWithValidKey(): void
584+
{
585+
// arrange : key file exists and is valid => no errors
586+
$valid_key = 'abcdefghijklmnopqrstuvwxyz123456';
587+
vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => $valid_key]]);
588+
589+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
590+
591+
// act
592+
$errors = $glpikey->getKeyFileReadErrors();
593+
594+
// assert
595+
$this->assertEmpty($errors);
596+
}
597+
598+
public function testHasKeyFileReadErrorsWithMissingFile(): void
599+
{
600+
// arrange : create directory structure without key file
601+
vfsStream::setup('glpi', null, ['config' => []]);
602+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
603+
604+
// act + assert
605+
$this->assertTrue($glpikey->hasReadErrors());
606+
}
607+
608+
public function testHasKeyFileReadErrorsWithUnreadableFile(): void
609+
{
610+
// arrange : create unreadable key file
611+
$structure = vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => 'unreadable file']]);
612+
$structure->getChild('config/glpicrypt.key')->chmod(0222);
613+
614+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
615+
616+
// act + assert
617+
$this->assertTrue($glpikey->hasReadErrors());
618+
619+
}
620+
621+
public function testHasKeyFileReadErrorsWithInvalidKey(): void
622+
{
623+
// arrange : key file exists but has invalid contents/length
624+
vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => 'not a valid key']]);
625+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
626+
627+
// act + assert
628+
$this->assertTrue($glpikey->hasReadErrors());
629+
}
630+
631+
public function testHasKeyFileReadErrorsWithValidKey(): void
632+
{
633+
// arrange : key file exists and is valid => no errors
634+
$valid_key = 'abcdefghijklmnopqrstuvwxyz123456';
635+
vfsStream::setup('glpi', null, ['config' => ['glpicrypt.key' => $valid_key]]);
636+
637+
$glpikey = new \GLPIKey(vfsStream::url('glpi/config'));
638+
639+
// act + assert
640+
$this->assertFalse($glpikey->hasReadErrors());
641+
}
541642
}

‎src/AuthLDAP.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,15 @@ public function showForm($ID, array $options = [])
431431
if (!Config::canUpdate()) {
432432
return false;
433433
}
434+
435+
// warning and no form if can't read keyfile
436+
$glpi_encryption_key = new GLPIKey();
437+
if ($glpi_encryption_key->hasReadErrors()) {
438+
$glpi_encryption_key->showReadErrors();
439+
440+
return false;
441+
}
442+
434443
if (empty($ID)) {
435444
$this->getEmpty();
436445
if (isset($options['preconfig'])) {
@@ -593,6 +602,13 @@ public function showForm($ID, array $options = [])
593602
*/
594603
public function showFormAdvancedConfig()
595604
{
605+
// warning and no form if can't read keyfile
606+
$glpi_encryption_key = new GLPIKey();
607+
if ($glpi_encryption_key->hasReadErrors()) {
608+
$glpi_encryption_key->showReadErrors();
609+
610+
return;
611+
}
596612

597613
$ID = $this->getField('id');
598614
$hidden = '';

‎src/Central.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,9 @@ private static function getMessages(): array
512512
. sprintf(__('Run the "%1$s" command to migrate them.'), 'php bin/console migration:unsigned_keys');
513513
}
514514

515+
// encrypt/decrypt key problems
516+
$messages['errors'] = (new GLPIKey())->getKeyFileReadErrors();
517+
515518
$security_requirements = [
516519
new PhpSupportedVersion(),
517520
new SafeDocumentRoot(),

‎src/GLPIKey.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,51 @@ public function keyExists()
118118
return file_exists($this->keyfile);
119119
}
120120

121+
/**
122+
* Check if key is valid
123+
*
124+
* @return string[]
125+
*/
126+
public function getKeyFileReadErrors(): array
127+
{
128+
$errors = [];
129+
if (!file_exists($this->keyfile)) {
130+
$errors[] = sprintf(__s('The security key file does not exist. You have to run the "%s" command to generate a key.'), 'php bin/console security:change_key');
131+
132+
return $errors; // early return, as, if file does not exist, no need to check further
133+
}
134+
if (false === ($key = @file_get_contents($this->keyfile))) {
135+
$errors[] = sprintf(__s("Unable to get security key file contents. Fix file permissions of %s."), $this->keyfile);
136+
137+
return $errors; // early return, as, if file does not exist, no need to check further
138+
}
139+
if (strlen($key) !== SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_KEYBYTES) {
140+
$errors[] = sprintf(__s('Invalid security key file contents. You have to run the "%s" command to regenerate a key.'), 'php bin/console security:change_key');
141+
}
142+
143+
return $errors;
144+
}
145+
146+
public function hasReadErrors(): bool
147+
{
148+
return !empty($this->getKeyFileReadErrors());
149+
}
150+
151+
public function showReadErrors(): void
152+
{
153+
$glpi_key_read_errors = $this->getKeyFileReadErrors();
154+
if (!empty($glpi_key_read_errors)) {
155+
\Glpi\Application\View\TemplateRenderer::getInstance()->display(
156+
'/central/messages.html.twig',
157+
[
158+
'messages' => [
159+
'errors' => $glpi_key_read_errors,
160+
],
161+
]
162+
);
163+
}
164+
}
165+
121166
/**
122167
* Get GLPI security key used for decryptable passwords
123168
*

‎src/GLPINetwork.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ public function getTabNameForItem(CommonGLPI $item, $withtemplate = 0)
4646
public static function displayTabContentForItem(CommonGLPI $item, $tabnum = 1, $withtemplate = 0)
4747
{
4848
if ($item->getType() == 'Config') {
49-
$glpiNetwork = new self();
50-
$glpiNetwork->showForConfig();
49+
self::showForConfig();
5150
}
5251
return true;
5352
}
@@ -58,9 +57,17 @@ public static function showForConfig()
5857
return;
5958
}
6059

61-
$registration_key = self::getRegistrationKey();
60+
// warning and no form if can't read keyfile
61+
$glpi_encryption_key = new GLPIKey();
62+
if ($glpi_encryption_key->hasReadErrors()) {
63+
$glpi_encryption_key->showReadErrors();
64+
65+
return;
66+
}
6267

6368
$canedit = Config::canUpdate();
69+
$registration_key = self::getRegistrationKey();
70+
6471
if ($canedit) {
6572
echo "<form name='form' action=\"" . Toolbox::getItemTypeFormURL(Config::class) . "\" method='post'>";
6673
}

‎src/MailCollector.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,14 @@ public function showForm($ID, array $options = [])
271271
/** @var array $CFG_GLPI */
272272
global $CFG_GLPI;
273273

274+
// warning and no form if can't read keyfile
275+
$glpi_encryption_key = new GLPIKey();
276+
if ($glpi_encryption_key->hasReadErrors()) {
277+
$glpi_encryption_key->showReadErrors();
278+
279+
return false;
280+
}
281+
274282
$this->initForm($ID, $options);
275283
$options['colspan'] = 1;
276284
$this->showFormHeader($options);

‎src/NotificationMailingSetting.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,16 @@ public function showFormConfig($options = [])
124124
/** @var array $CFG_GLPI */
125125
global $CFG_GLPI;
126126

127+
// warning and no form if can't read keyfile
128+
// always display no matter what $options['display']
129+
// see comment at the end of this function
130+
$glpi_encryption_key = new GLPIKey();
131+
if ($glpi_encryption_key->hasReadErrors()) {
132+
$glpi_encryption_key->showReadErrors();
133+
134+
return;
135+
}
136+
127137
if (!isset($options['display'])) {
128138
$options['display'] = true;
129139
}

‎src/SNMPCredential.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ public function defineTabs($options = [])
113113

114114
public function showForm($ID, array $options = [])
115115
{
116+
// warning and no form if can't read keyfile,
117+
// only version 3 is impacted but it's better to always show the warning & forbid form display
118+
$glpi_encryption_key = new GLPIKey();
119+
if ($glpi_encryption_key->hasReadErrors()) {
120+
$glpi_encryption_key->showReadErrors();
121+
122+
return false;
123+
}
124+
116125
$this->initForm($ID, $options);
117126
TemplateRenderer::getInstance()->display('components/form/snmpcredential.html.twig', [
118127
'item' => $this,

0 commit comments

Comments
 (0)