From 5569d7d2bf15aa6085f8a058efc1b36ef44dfa96 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 7 Nov 2021 16:58:42 +0100 Subject: [PATCH 1/2] new: [security] Store authkeys for servers encrypted --- app/Lib/Tools/EncryptedValue.php | 68 ++++++++++++++++++++++++++++++ app/Model/AppModel.php | 14 +++++-- app/Model/Cerebrate.php | 45 ++++++++++++++++++-- app/Model/Server.php | 46 +++++++++++++++++++-- app/Model/SystemSetting.php | 71 ++++++-------------------------- db_schema.json | 18 ++++---- 6 files changed, 185 insertions(+), 77 deletions(-) create mode 100644 app/Lib/Tools/EncryptedValue.php diff --git a/app/Lib/Tools/EncryptedValue.php b/app/Lib/Tools/EncryptedValue.php new file mode 100644 index 000000000..540e9fc92 --- /dev/null +++ b/app/Lib/Tools/EncryptedValue.php @@ -0,0 +1,68 @@ +value = $value; + $this->isJson = $isJson; + } + + /** + * @return mixed + * @throws JsonException + * @throws Exception + */ + public function decrypt() + { + $decrypt = BetterSecurity::decrypt(substr($this->value, 2), Configure::read('Security.encryption_key')); + return $this->isJson ? JsonTool::decode($decrypt) : $decrypt; + } + + public function __toString() + { + return $this->decrypt(); + } + + public function jsonSerialize() + { + return $this->decrypt(); + } + + /** + * Encrypt provided string if encryption is enabled. If not enabled, input value will be returned. + * @param string $value + * @return string + * @throws Exception + */ + public static function encryptIfEnabled($value) + { + $key = Configure::read('Security.encryption_key'); + if ($key) { + return EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($value, $key); + } + return $value; + } + + /** + * Check if value is encrypted (starts with encrypted magic) + * @param string $value + * @return bool + */ + public static function isEncrypted($value) + { + return substr($value, 0, 2) === EncryptedValue::ENCRYPTED_MAGIC; + } +} diff --git a/app/Model/AppModel.php b/app/Model/AppModel.php index 243c52c2b..c8ac40600 100644 --- a/app/Model/AppModel.php +++ b/app/Model/AppModel.php @@ -1597,6 +1597,8 @@ class AppModel extends Model `value` blob NOT NULL, PRIMARY KEY (`setting`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;"; + $sqlArray[] = "ALTER TABLE `servers` MODIFY COLUMN `authkey` VARBINARY(255) NOT NULL;"; + $sqlArray[] = "ALTER TABLE `cerebrates` MODIFY COLUMN `authkey` VARBINARY(255) NOT NULL;"; break; case 'fixNonEmptySharingGroupID': $sqlArray[] = 'UPDATE `events` SET `sharing_group_id` = 0 WHERE `distribution` != 4;'; @@ -2683,15 +2685,21 @@ class AppModel extends Model { $version = implode('.', $this->checkMISPVersion()); $commit = $this->checkMIPSCommit(); - $request = array( + + $authkey = $server[$model]['authkey']; + App::uses('EncryptedValue', 'Tools'); + if (EncryptedValue::isEncrypted($authkey)) { + $authkey = (string)new EncryptedValue($authkey); + } + + return array( 'header' => array( - 'Authorization' => $server[$model]['authkey'], + 'Authorization' => $authkey, 'Accept' => 'application/json', 'Content-Type' => 'application/json', 'User-Agent' => 'MISP ' . $version . (empty($commit) ? '' : ' - #' . $commit), ) ); - return $request; } /** diff --git a/app/Model/Cerebrate.php b/app/Model/Cerebrate.php index 125bd450d..1b7debdaf 100644 --- a/app/Model/Cerebrate.php +++ b/app/Model/Cerebrate.php @@ -1,6 +1,6 @@ array( - 'className' => 'Organisation', - 'foreignKey' => 'org_id' + 'className' => 'Organisation', + 'foreignKey' => 'org_id' ) ); - public function queryInstance($options) { + public function beforeSave($options = array()) + { + $cerebrate = &$this->data['Server']; + // Encrypt authkey if plain key provided and encryption is enabled + if (!empty($cerebrate['authkey']) && strlen($cerebrate['authkey']) === 40) { + $cerebrate['authkey'] = EncryptedValue::encryptIfEnabled($cerebrate['authkey']); + } + return true; + } + + public function queryInstance($options) + { $url = $options['cerebrate']['Cerebrate']['url'] . $options['path']; $url_params = []; @@ -413,4 +424,30 @@ class Cerebrate extends AppModel } return __('The retrieved data isn\'t a valid sharing group.'); } + + /** + * @param string|null $old Old (or current) encryption key. + * @param string|null $new New encryption key. If empty, encrypted values will be decrypted. + * @throws Exception + */ + public function reencryptAuthKeys($old, $new) + { + $cerebrates = $this->find('list', [ + 'fields' => ['Cerebrate.id', 'Cerebrate.authkey'], + ]); + $toSave = []; + foreach ($cerebrates as $id => $authkey) { + if (EncryptedValue::isEncrypted($authkey)) { + $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + } + if (!empty($new)) { + $authkey = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($authkey, $new); + } + $toSave[] = ['Cerebrate' => [ + 'id' => $id, + 'authkey' => $authkey, + ]]; + } + return $this->saveMany($toSave); + } } diff --git a/app/Model/Server.php b/app/Model/Server.php index ddbc6248a..85d6a3943 100644 --- a/app/Model/Server.php +++ b/app/Model/Server.php @@ -3,6 +3,7 @@ App::uses('AppModel', 'Model'); App::uses('GpgTool', 'Tools'); App::uses('ServerSyncTool', 'Tools'); App::uses('SystemSetting', 'Model'); +App::uses('EncryptedValue', 'Tools'); /** * @property-read array $serverSettings @@ -165,8 +166,11 @@ class Server extends AppModel public function beforeSave($options = array()) { - $this->data['Server']['url'] = rtrim($this->data['Server']['url'], '/'); - if (empty($this->data['Server']['id'])) { + $server = &$this->data['Server']; + if (!empty($server['url'])) { + $server['url'] = rtrim($server['url'], '/'); + } + if (empty($server['id'])) { $max_prio = $this->find('first', array( 'recursive' => -1, 'order' => array('Server.priority' => 'DESC'), @@ -177,7 +181,11 @@ class Server extends AppModel } else { $max_prio = $max_prio['Server']['priority']; } - $this->data['Server']['priority'] = $max_prio + 1; + $server['priority'] = $max_prio + 1; + } + // Encrypt authkey if plain key provided and encryption is enabled + if (!empty($server['authkey']) && strlen($server['authkey']) === 40) { + $server['authkey'] = EncryptedValue::encryptIfEnabled($server['authkey']); } return true; } @@ -4522,6 +4530,32 @@ class Server extends AppModel ]; } + /** + * @param string|null $old Old (or current) encryption key. + * @param string|null $new New encryption key. If empty, encrypted values will be decrypted. + * @throws Exception + */ + public function reencryptAuthKeys($old, $new) + { + $servers = $this->find('list', [ + 'fields' => ['Server.id', 'Server.authkey'], + ]); + $toSave = []; + foreach ($servers as $id => $authkey) { + if (EncryptedValue::isEncrypted($authkey)) { + $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + } + if (!empty($new)) { + $authkey = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($authkey, $new); + } + $toSave[] = ['Server' => [ + 'id' => $id, + 'authkey' => $authkey, + ]]; + } + return $this->saveMany($toSave); + } + /** * Generate just when required * @return array[] @@ -6001,6 +6035,12 @@ class Server extends AppModel /** @var SystemSetting $systemSetting */ $systemSetting = ClassRegistry::init('SystemSetting'); $systemSetting->reencrypt($old, $new); + + $this->reencryptAuthKeys($old, $new); + + /** @var Cerebrate $cerebrate */ + $cerebrate = ClassRegistry::init('Cerebrate'); + $cerebrate->reencryptAuthKeys($old, $new); return true; }, 'type' => 'string', diff --git a/app/Model/SystemSetting.php b/app/Model/SystemSetting.php index b789f1e59..a79812581 100644 --- a/app/Model/SystemSetting.php +++ b/app/Model/SystemSetting.php @@ -1,45 +1,8 @@ value = $value; - } - - /** - * @return mixed - * @throws JsonException - * @throws Exception - */ - public function decrypt() - { - $decrypt = BetterSecurity::decrypt($this->value, Configure::read('Security.encryption_key')); - return JsonTool::decode($decrypt); - } - - public function __toString() - { - return $this->decrypt(); - } - - public function jsonSerialize() - { - return $this->decrypt(); - } -} - class SystemSetting extends AppModel { public $actsAs = [ @@ -91,13 +54,20 @@ class SystemSetting extends AppModel /** * @return array + * @throws JsonException */ public function getSettings() { $settings = $this->find('list', [ 'fields' => ['SystemSetting.setting', 'SystemSetting.value'], ]); - return array_map([$this, 'decode'], $settings); + return array_map(function ($value) { + if (EncryptedValue::isEncrypted($value)) { + return new EncryptedValue($value, true); + } else { + return JsonTool::decode($value); + } + }, $settings); } /** @@ -122,9 +92,8 @@ class SystemSetting extends AppModel $value = JsonTool::encode($value); // If encryption is enabled and setting name contains `password` or `apikey` string, encrypt value to protect it - $key = Configure::read('Security.encryption_key'); - if ($key && self::isSensitive($setting)) { - $value = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($value, $key); + if (self::isSensitive($setting)) { + $value = EncryptedValue::encryptIfEnabled($value); } $valid = $this->save(['SystemSetting' => [ @@ -140,7 +109,7 @@ class SystemSetting extends AppModel /** * @param string|null $old Old (or current) encryption key. * @param string|null $new New encryption key. If empty, encrypted values will be decrypted. - * @throws JsonException + * @throws Exception */ public function reencrypt($old, $new) { @@ -152,7 +121,7 @@ class SystemSetting extends AppModel if (!self::isSensitive($setting)) { continue; } - if (substr($value, 0, 2) === EncryptedValue::ENCRYPTED_MAGIC) { + if (EncryptedValue::isEncrypted($value)) { $value = BetterSecurity::decrypt(substr($value, 2), $old); } if (!empty($new)) { @@ -166,20 +135,6 @@ class SystemSetting extends AppModel return $this->saveMany($toSave); } - /** - * @param string $value - * @return EncryptedValue|mixed - * @throws JsonException - */ - private function decode($value) - { - if (substr($value, 0, 2) === EncryptedValue::ENCRYPTED_MAGIC) { - return new EncryptedValue(substr($value, 2)); - } else { - return JsonTool::decode($value); - } - } - /** * Sensitive setting are passwords or api keys. * @param string $setting Setting name diff --git a/db_schema.json b/db_schema.json index 5a3b3cde5..2991d1883 100644 --- a/db_schema.json +++ b/db_schema.json @@ -769,12 +769,12 @@ }, { "column_name": "authkey", - "is_nullable": "YES", - "data_type": "varchar", - "character_maximum_length": "40", + "is_nullable": "NO", + "data_type": "varbinary", + "character_maximum_length": "255", "numeric_precision": null, - "collation_name": "ascii_general_ci", - "column_type": "varchar(40)", + "collation_name": null, + "column_type": "varbinary(255)", "column_default": null, "extra": "" }, @@ -5063,11 +5063,11 @@ { "column_name": "authkey", "is_nullable": "NO", - "data_type": "varchar", - "character_maximum_length": "40", + "data_type": "varbinary", + "character_maximum_length": "255", "numeric_precision": null, - "collation_name": "utf8_bin", - "column_type": "varchar(40)", + "collation_name": null, + "column_type": "varbinary(255)", "column_default": null, "extra": "" }, From 012b8c4b21ee8bbf58b6e8927ef3577a7baba467 Mon Sep 17 00:00:00 2001 From: Jakub Onderka Date: Sun, 7 Nov 2021 17:21:55 +0100 Subject: [PATCH 2/2] new: [CLI] admin reencrypt command --- app/Console/Command/AdminShell.php | 56 ++++++++++++++++++++++++++++++ app/Model/Cerebrate.php | 11 ++++-- app/Model/Server.php | 14 ++++++-- app/Model/SystemSetting.php | 9 ++++- 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/app/Console/Command/AdminShell.php b/app/Console/Command/AdminShell.php index d520c5de0..14f4d48ca 100644 --- a/app/Console/Command/AdminShell.php +++ b/app/Console/Command/AdminShell.php @@ -45,6 +45,15 @@ class AdminShell extends AppShell ], ], ]); + $parser->addSubcommand('reencrypt', [ + 'help' => __('Reencrypt encrypted values in database (authkeys and sensitive system settings).'), + 'parser' => [ + 'options' => [ + 'old' => ['help' => __('Old key. If not provided, current key will be used.')], + 'new' => ['help' => __('New key. If not provided, new key will be generated.')], + ], + ], + ]); $parser->addSubcommand('removeOrphanedCorrelations', [ 'help' => __('Remove orphaned correlations.'), ]); @@ -891,4 +900,51 @@ class AdminShell extends AppShell $this->out('Redis: ' . ($newStatus !== '0' ? 'True' : 'False')); } } + + public function reencrypt() + { + $old = $this->params['old'] ?? null; + $new = $this->params['new'] ?? null; + + if ($new !== null && strlen($new) < 32) { + $this->error('New key must be at least 32 char long.'); + } + + if ($old === null) { + $old = Configure::read('Security.encryption_key'); + } + + if ($new === null) { + // Generate random new key + $randomTool = new RandomTool(); + $new = $randomTool->random_str(); + } + + $this->Server->getDataSource()->begin(); + + try { + /** @var SystemSetting $systemSetting */ + $systemSetting = ClassRegistry::init('SystemSetting'); + $systemSetting->reencrypt($old, $new); + + $this->Server->reencryptAuthKeys($old, $new); + + /** @var Cerebrate $cerebrate */ + $cerebrate = ClassRegistry::init('Cerebrate'); + $cerebrate->reencryptAuthKeys($old, $new); + + $result = $this->Server->serverSettingsSaveValue('Security.encryption_key', $new, true); + + $this->Server->getDataSource()->commit(); + + if (!$result) { + $this->error('Encrypt key was changed, but it is not possible to save key to config file', __('Please insert new key "%s" to config file manually.', $new)); + } + } catch (Exception $e) { + $this->Server->getDataSource()->rollback(); + throw $e; + } + + $this->out(__('New encryption key "%s" saved into config file.', $new)); + } } diff --git a/app/Model/Cerebrate.php b/app/Model/Cerebrate.php index 1b7debdaf..f61be6d4d 100644 --- a/app/Model/Cerebrate.php +++ b/app/Model/Cerebrate.php @@ -438,7 +438,11 @@ class Cerebrate extends AppModel $toSave = []; foreach ($cerebrates as $id => $authkey) { if (EncryptedValue::isEncrypted($authkey)) { - $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + try { + $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + } catch (Exception $e) { + throw new Exception("Could not decrypt auth key for Cerebrate #$id", 0, $e); + } } if (!empty($new)) { $authkey = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($authkey, $new); @@ -448,6 +452,9 @@ class Cerebrate extends AppModel 'authkey' => $authkey, ]]; } - return $this->saveMany($toSave); + if (empty($toSave)) { + return true; + } + return $this->saveMany($toSave, ['validate' => false, 'fields' => ['authkey']]); } } diff --git a/app/Model/Server.php b/app/Model/Server.php index 85d6a3943..00ca93d2f 100644 --- a/app/Model/Server.php +++ b/app/Model/Server.php @@ -2276,6 +2276,9 @@ class Server extends AppModel /** @var array $config */ require $configFilePath; + if (!isset($config)) { + throw new Exception("Could not load config file `$configFilePath`."); + } $config = Hash::insert($config, $setting, $value); $settingsToSave = array( @@ -4543,7 +4546,11 @@ class Server extends AppModel $toSave = []; foreach ($servers as $id => $authkey) { if (EncryptedValue::isEncrypted($authkey)) { - $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + try { + $authkey = BetterSecurity::decrypt(substr($authkey, 2), $old); + } catch (Exception $e) { + throw new Exception("Could not decrypt auth key for server #$id", 0, $e); + } } if (!empty($new)) { $authkey = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($authkey, $new); @@ -4553,7 +4560,10 @@ class Server extends AppModel 'authkey' => $authkey, ]]; } - return $this->saveMany($toSave); + if (empty($toSave)) { + return true; + } + return $this->saveMany($toSave, ['validate' => false, 'fields' => ['authkey']]); } /** diff --git a/app/Model/SystemSetting.php b/app/Model/SystemSetting.php index a79812581..70ec497a1 100644 --- a/app/Model/SystemSetting.php +++ b/app/Model/SystemSetting.php @@ -122,7 +122,11 @@ class SystemSetting extends AppModel continue; } if (EncryptedValue::isEncrypted($value)) { - $value = BetterSecurity::decrypt(substr($value, 2), $old); + try { + $value = BetterSecurity::decrypt(substr($value, 2), $old); + } catch (Exception $e) { + throw new Exception("Could not decrypt `$setting` setting.", 0, $e); + } } if (!empty($new)) { $value = EncryptedValue::ENCRYPTED_MAGIC . BetterSecurity::encrypt($value, $new); @@ -132,6 +136,9 @@ class SystemSetting extends AppModel 'value' => $value, ]]; } + if (empty($toSave)) { + return true; + } return $this->saveMany($toSave); }