Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(fix): Encrypt sensitive values(haproxy_password) in the DB #451

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/Controller/DaemonConfigController.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\Security\ICrypto;

/**
* DaemonConfig actions (for UI)
Expand All @@ -33,6 +34,7 @@ public function __construct(
private readonly AppAPIService $service,
private readonly ExAppService $exAppService,
private readonly IL10N $l10n,
private readonly ICrypto $crypto,
) {
parent::__construct(Application::APP_ID, $request);
}
Expand Down Expand Up @@ -66,6 +68,9 @@ public function updateDaemonConfig(string $name, array $daemonConfigParams): Res
// Restore the original password if "dummySecret123" is provided
if ($haproxyPassword === 'dummySecret123') {
$daemonConfigParams['deploy_config']['haproxy_password'] = $daemonConfig->getDeployConfig()['haproxy_password'] ?? "";
} elseif (!empty($haproxyPassword)) {
// New password provided, encrypt it
$daemonConfigParams['deploy_config']['haproxy_password'] = $this->crypto->encrypt($haproxyPassword);
}

// Create and update DaemonConfig instance
Expand Down Expand Up @@ -129,16 +134,22 @@ public function checkDaemonConnection(array $daemonParams): Response {
$haproxyPassword = $daemonParams['deploy_config']['haproxy_password'] ?? null;

if ($haproxyPassword === 'dummySecret123') {
// If the secret is "dummySecret123" we check if such record is present in DB
// For cases when the password itself is 'dummySecret123'
$daemonParams['deploy_config']['haproxy_password'] = $this->crypto->encrypt($haproxyPassword);

// Check if such record is present in the DB
$daemonConfig = $this->daemonConfigService->getDaemonConfigByName($daemonParams['name']);
if ($daemonConfig !== null) {
// such Daemon config already present in the DB
$haproxyPasswordDB = $daemonConfig->getDeployConfig()['haproxy_password'] ?? "";
if ($haproxyPasswordDB) {
// if there is a record in the DB and there is a password,
// then we request it from the DB instead of the “masked” one
// get password from the DB instead of the “masked” one
$daemonParams['deploy_config']['haproxy_password'] = $haproxyPasswordDB;
}
}
} elseif (!empty($haproxyPassword)) {
// New password provided, encrypt it, as "initGuzzleClient" expects to receive encrypted password
$daemonParams['deploy_config']['haproxy_password'] = $this->crypto->encrypt($haproxyPassword);
}

$daemonConfig = new DaemonConfig([
Expand Down
8 changes: 6 additions & 2 deletions lib/DeployActions/DockerActions.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
use OCP\ICertificateManager;
use OCP\IConfig;
use OCP\IURLGenerator;
use OCP\Security\ICrypto;
use Psr\Log\LoggerInterface;

class DockerActions implements IDeployActions {
Expand All @@ -36,6 +37,7 @@ public function __construct(
private readonly IURLGenerator $urlGenerator,
private readonly AppAPICommonService $service,
private readonly ExAppService $exAppService,
private readonly ICrypto $crypto,
) {
}

Expand Down Expand Up @@ -556,7 +558,8 @@ public function resolveExAppUrl(
}
if ($protocol == 'https' && isset($deployConfig['haproxy_password']) && $deployConfig['haproxy_password'] !== '') {
// we only set haproxy auth for remote installations, when all requests come through HaProxy.
$auth = [self::APP_API_HAPROXY_USER, $deployConfig['haproxy_password']];
$haproxyPass = $this->crypto->decrypt($deployConfig['haproxy_password']);
$auth = [self::APP_API_HAPROXY_USER, $haproxyPass];
}
return sprintf('%s://%s:%s', $protocol, $exAppHost, $port);
}
Expand Down Expand Up @@ -620,7 +623,8 @@ public function initGuzzleClient(DaemonConfig $daemonConfig): void {
$guzzleParams = $this->setupCerts($guzzleParams);
}
if (isset($daemonConfig->getDeployConfig()['haproxy_password']) && $daemonConfig->getDeployConfig()['haproxy_password'] !== '') {
$guzzleParams['auth'] = [self::APP_API_HAPROXY_USER, $daemonConfig->getDeployConfig()['haproxy_password']];
$haproxyPass = $this->crypto->decrypt($daemonConfig->getDeployConfig()['haproxy_password']);
$guzzleParams['auth'] = [self::APP_API_HAPROXY_USER, $haproxyPass];
}
$this->guzzleClient = new Client($guzzleParams);
}
Expand Down
59 changes: 59 additions & 0 deletions lib/Migration/Version5000Date20241120135411.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php
/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

declare(strict_types=1);

namespace OCA\AppAPI\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;
use OCP\Security\ICrypto;

class Version5000Date20241120135411 extends SimpleMigrationStep {

public function __construct(
private IDBConnection $connection,
private ICrypto $crypto,
) {
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*
* @return null|ISchemaWrapper
*/
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
// encrypt "haproxy_password" in the "ex_apps_daemons" table
$qbSelect = $this->connection->getQueryBuilder();
$qbSelect->select(['id', 'deploy_config'])
->from('ex_apps_daemons')
->where(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks postgres due to type mismatch.

$req = $qbSelect->executeQuery();

while ($row = $req->fetch()) {
$deployConfig = $row['deploy_config'];
$deployConfig = json_decode($deployConfig, true);
if (!empty($deployConfig['haproxy_password'])) {
$deployConfig['haproxy_password'] = $this->crypto->encrypt($deployConfig['haproxy_password']);
$encodedDeployConfig = json_encode($deployConfig);
$qbUpdate = $this->connection->getQueryBuilder();
$qbUpdate->update('ex_apps_daemons')
->set('deploy_config', $qbUpdate->createNamedParameter($encodedDeployConfig))
->where(
$qbUpdate->expr()->eq('id', $qbUpdate->createNamedParameter($row['id']))
);
$qbUpdate->executeStatement();
oleksandr-nc marked this conversation as resolved.
Show resolved Hide resolved
}
}
$req->closeCursor();
return null;
}
}
5 changes: 5 additions & 0 deletions lib/Service/DaemonConfigService.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Db\MultipleObjectsReturnedException;
use OCP\DB\Exception;
use OCP\Security\ICrypto;
use Psr\Log\LoggerInterface;

/**
Expand All @@ -20,6 +21,7 @@ public function __construct(
private readonly LoggerInterface $logger,
private readonly DaemonConfigMapper $mapper,
private readonly ExAppService $exAppService,
private readonly ICrypto $crypto,
) {
}

Expand All @@ -38,6 +40,9 @@ public function registerDaemonConfig(array $params): ?DaemonConfig {
}
$params['deploy_config']['nextcloud_url'] = rtrim($params['deploy_config']['nextcloud_url'], '/');
try {
if (isset($params['deploy_config']['haproxy_password']) && $params['deploy_config']['haproxy_password'] !== '') {
$params['deploy_config']['haproxy_password'] = $this->crypto->encrypt($params['deploy_config']['haproxy_password']);
}
return $this->mapper->insert(new DaemonConfig([
'name' => $params['name'],
'display_name' => $params['display_name'],
Expand Down
Loading