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.

Sign up for GitHub

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

Use type safe method to get secret and instanceid #35587

Closed
wants to merge 3 commits into from
Closed

Use type safe method to get secret and instanceid #35587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13692bd
Use getSystemValueString to fetch secret
J0WI Dec 4, 2022
ef04f86
Use getSystemValueString to fetch instanceid
J0WI Dec 4, 2022
352c212
Clarify return type documentation
J0WI Dec 4, 2022
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
4 changes: 2 additions & 2 deletions apps/encryption/lib/Crypto/Crypt.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,8 +372,8 @@ private function addPadding($data) {
* @return string
*/
protected function generatePasswordHash($password, $cipher, $uid = '') {
$instanceId = $this->config->getSystemValue('instanceid');
$instanceSecret = $this->config->getSystemValue('secret');
$instanceId = $this->config->getSystemValueString('instanceid');
$instanceSecret = $this->config->getSystemValueString('secret');
$salt = hash('sha256', $uid . $instanceId . $instanceSecret, true);
$keySize = $this->getKeySize($cipher);

Expand Down
4 changes: 2 additions & 2 deletions apps/encryption/lib/KeyManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,8 @@ public function addSystemKeys(array $accessList, array $publicKeys, $uid) {
* @throws \Exception
*/
public function getMasterKeyPassword() {
$password = $this->config->getSystemValue('secret');
if (empty($password)) {
$password = $this->config->getSystemValueString('secret');
if ($password === '') {
throw new \Exception('Can not get secret from Nextcloud instance');
}

Expand Down
6 changes: 3 additions & 3 deletions apps/encryption/tests/KeyManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ public function testGetFileKey($uid, $isMasterKeyEnabled, $privateKey, $encrypte

if ($isMasterKeyEnabled) {
$expectedUid = 'masterKeyId';
$this->configMock->expects($this->any())->method('getSystemValue')->with('secret')
$this->configMock->expects($this->any())->method('getSystemValueString')->with('secret')
->willReturn('password');
} elseif (!$uid) {
$expectedUid = 'systemKeyId';
Expand Down Expand Up @@ -560,7 +560,7 @@ public function testGetPublicMasterKey() {
}

public function testGetMasterKeyPassword() {
$this->configMock->expects($this->once())->method('getSystemValue')->with('secret')
$this->configMock->expects($this->once())->method('getSystemValueString')->with('secret')
->willReturn('password');

$this->assertSame('password',
Expand All @@ -572,7 +572,7 @@ public function testGetMasterKeyPassword() {
public function testGetMasterKeyPasswordException() {
$this->expectException(\Exception::class);

$this->configMock->expects($this->once())->method('getSystemValue')->with('secret')
$this->configMock->expects($this->once())->method('getSystemValueString')->with('secret')
->willReturn('');

$this->invokePrivate($this->instance, 'getMasterKeyPassword', []);
Expand Down
4 changes: 2 additions & 2 deletions apps/files/lib/Command/ScanAppData.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,9 @@ protected function reconnectToDatabase(OutputInterface $output): Connection {
* @throws NotFoundException
*/
private function getAppDataFolder() {
$instanceId = $this->config->getSystemValue('instanceid', null);
$instanceId = $this->config->getSystemValueString('instanceid');

if ($instanceId === null) {
if ($instanceId === '') {
throw new NotFoundException();
}

Expand Down
9 changes: 6 additions & 3 deletions apps/files_external/lib/Lib/Auth/PublicKey/RSA.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
*
Expand Down Expand Up @@ -56,9 +59,9 @@ public function __construct(IL10N $l, IConfig $config) {
;
}

public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) {
public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null): void {
$auth = new RSACrypt();
$auth->setPassword($this->config->getSystemValue('secret', ''));
$auth->setPassword($this->config->getSystemValueString('secret'));
if (!$auth->loadKey($storage->getBackendOption('private_key'))) {
// Add fallback routine for a time where secret was not enforced to be exists
$auth->setPassword('');
Expand All @@ -78,7 +81,7 @@ public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = n
public function createKey($keyLength) {
$rsa = new RSACrypt();
$rsa->setPublicKeyFormat(RSACrypt::PUBLIC_FORMAT_OPENSSH);
$rsa->setPassword($this->config->getSystemValue('secret', ''));
$rsa->setPassword($this->config->getSystemValueString('secret'));

if ($keyLength !== 1024 && $keyLength !== 2048 && $keyLength !== 4096) {
$keyLength = 1024;
Expand Down
7 changes: 5 additions & 2 deletions apps/files_external/lib/Lib/Auth/PublicKey/RSAPrivateKey.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php

declare(strict_types=1);

/**
* @copyright 2018, Roeland Jago Douma <roeland@famdouma.nl>
*
Expand Down Expand Up @@ -54,9 +57,9 @@ public function __construct(IL10N $l, IConfig $config) {
]);
}

public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null) {
public function manipulateStorageConfig(StorageConfig &$storage, IUser $user = null): void {
$auth = new RSACrypt();
$auth->setPassword($this->config->getSystemValue('secret', ''));
$auth->setPassword($this->config->getSystemValueString('secret'));
if (!$auth->loadKey($storage->getBackendOption('private_key'))) {
// Add fallback routine for a time where secret was not enforced to be exists
$auth->setPassword('');
Expand Down
4 changes: 2 additions & 2 deletions apps/settings/lib/Mailer/NewUserMailHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ public function generateTemplate(IUser $user, $generatePasswordResetToken = fals
ISecureRandom::CHAR_ALPHANUMERIC
);
$tokenValue = $this->timeFactory->getTime() . ':' . $token;
$mailAddress = (null !== $user->getEMailAddress()) ? $user->getEMailAddress() : '';
$encryptedValue = $this->crypto->encrypt($tokenValue, $mailAddress . $this->config->getSystemValue('secret'));
$mailAddress = $user->getEMailAddress() ?? '';
$encryptedValue = $this->crypto->encrypt($tokenValue, $mailAddress . $this->config->getSystemValueString('secret'));
Fixed Show fixed Hide fixed
$this->config->setUserValue($user->getUID(), 'core', 'lostpassword', $encryptedValue);
$link = $this->urlGenerator->linkToRouteAbsolute('core.lost.resetform', ['userId' => $user->getUID(), 'token' => $token]);
} else {
Expand Down
4 changes: 2 additions & 2 deletions core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ public function __construct(IConfig $config, LoggerInterface $log, ITimeFactory
*/
public function run($arguments) {
$updateDir = $this->config->getSystemValue('updatedirectory', null) ?? $this->config->getSystemValue('datadirectory', \OC::$SERVERROOT . '/data');
$instanceId = $this->config->getSystemValue('instanceid', null);
$instanceId = $this->config->getSystemValueString('instanceid');

if (!is_string($instanceId) || empty($instanceId)) {
if ($instanceId === '') {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion core/Service/LoginFlowV2Service.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public function createTokens(string $userAgent): LoginFlowV2Tokens {
}

private function hashToken(string $token): string {
$secret = $this->config->getSystemValue('secret');
$secret = $this->config->getSystemValueString('secret');
return hash('sha512', $token . $secret);
}

Expand Down
4 changes: 2 additions & 2 deletions lib/private/Repair/NC25/AddMissingSecretJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function getName(): string {
}

public function run(IOutput $output): void {
$passwordSalt = $this->config->getSystemValueString('passwordsalt', '');
$passwordSalt = $this->config->getSystemValueString('passwordsalt');
if ($passwordSalt === '') {
try {
$this->config->setSystemValue('passwordsalt', $this->random->generate(30));
Expand All @@ -51,7 +51,7 @@ public function run(IOutput $output): void {
}
}

$secret = $this->config->getSystemValueString('secret', '');
$secret = $this->config->getSystemValueString('secret');
if ($secret === '') {
try {
$this->config->setSystemValue('secret', $this->random->generate(48));
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Security/Crypto.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public function encrypt(string $plaintext, string $password = ''): string {
* @throws Exception If the decryption failed
*/
public function decrypt(string $authenticatedCiphertext, string $password = ''): string {
$secret = $this->config->getSystemValue('secret');
$secret = $this->config->getSystemValueString('secret');
try {
if ($password === '') {
return $this->decryptWithoutSecret($authenticatedCiphertext, $secret);
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Security/IdentityProof/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ public function getKey(IUser $user): Key {
* @throws \RuntimeException
*/
public function getSystemKey(): Key {
$instanceId = $this->config->getSystemValue('instanceid', null);
if ($instanceId === null) {
$instanceId = $this->config->getSystemValueString('instanceid');
if ($instanceId === '') {
throw new \RuntimeException('no instance id!');
}
return $this->retrieveKey('system-' . $instanceId);
Expand Down
4 changes: 2 additions & 2 deletions lib/private/legacy/OC_Util.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ public static function copySkeleton($userId, \OCP\Files\Folder $userDirectory) {
}
}

$instanceId = \OC::$server->getConfig()->getSystemValue('instanceid', '');
$instanceId = \OC::$server->getConfig()->getSystemValueString('instanceid');

if ($instanceId === null) {
if ($instanceId === '') {
throw new \RuntimeException('no instance id!');
}
$appdata = 'appdata_' . $instanceId;
Expand Down
6 changes: 3 additions & 3 deletions lib/public/IConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function getSystemValue($key, $default = '');
*
* @param string $key the key of the value, under which it was saved
* @param bool $default the default value to be returned if the value isn't set
* @return bool the value or $default
* @return bool the value or $default, must not be null
* @since 16.0.0
*/
public function getSystemValueBool(string $key, bool $default = false): bool;
Expand All @@ -87,7 +87,7 @@ public function getSystemValueBool(string $key, bool $default = false): bool;
*
* @param string $key the key of the value, under which it was saved
* @param int $default the default value to be returned if the value isn't set
* @return int the value or $default
* @return int the value or $default, must not be null
* @since 16.0.0
*/
public function getSystemValueInt(string $key, int $default = 0): int;
Expand All @@ -97,7 +97,7 @@ public function getSystemValueInt(string $key, int $default = 0): int;
*
* @param string $key the key of the value, under which it was saved
* @param string $default the default value to be returned if the value isn't set
* @return string the value or $default
* @return string the value or $default, must not be null
* @since 16.0.0
*/
public function getSystemValueString(string $key, string $default = ''): string;
Expand Down
4 changes: 2 additions & 2 deletions tests/lib/Security/IdentityProof/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public function testGetSystemKey() {
/** @var Key|\PHPUnit\Framework\MockObject\MockObject $key */
$key = $this->createMock(Key::class);

$this->config->expects($this->once())->method('getSystemValue')
$this->config->expects($this->once())->method('getSystemValueString')
->with('instanceid', null)->willReturn('instanceId');

$manager->expects($this->once())->method('retrieveKey')->with('system-instanceId')
Expand All @@ -229,7 +229,7 @@ public function testGetSystemKeyFailure() {
/** @var Key|\PHPUnit\Framework\MockObject\MockObject $key */
$key = $this->createMock(Key::class);

$this->config->expects($this->once())->method('getSystemValue')
$this->config->expects($this->once())->method('getSystemValueString')
->with('instanceid', null)->willReturn(null);

$manager->getSystemKey();
Expand Down