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 constant for supported formats #26323

Merged
merged 1 commit into from
Jun 23, 2021
Merged

Use constant for supported formats #26323

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 24 additions & 26 deletions apps/encryption/lib/Crypto/Crypt.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,20 @@
* @package OCA\Encryption\Crypto
*/
class Crypt {
public const SUPPORTED_CIPHERS_AND_KEY_SIZE = [
Copy link
Member

Choose a reason for hiding this comment

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

why does their visibility change all of a sudden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEFAULT_CIPHER and LEGACY_CIPHER are also public, so IMHO it makes sense to have the same visibility for all available ciphers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rullzer should I revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#26383 depends on this

'AES-256-CTR' => 32,
'AES-128-CTR' => 16,
'AES-256-CFB' => 32,
'AES-128-CFB' => 16,
];
// one out of SUPPORTED_CIPHERS_AND_KEY_SIZE
public const DEFAULT_CIPHER = 'AES-256-CTR';
// default cipher from old Nextcloud versions
public const LEGACY_CIPHER = 'AES-128-CFB';

public const SUPPORTED_KEY_FORMATS = ['hash', 'password'];
// one out of SUPPORTED_KEY_FORMATS
public const DEFAULT_KEY_FORMAT = 'hash';
// default key format, old Nextcloud version encrypted the private key directly
// with the user password
public const LEGACY_KEY_FORMAT = 'password';
Expand All @@ -77,20 +87,9 @@ class Crypt {
/** @var IConfig */
private $config;

/** @var array */
private $supportedKeyFormats;

/** @var IL10N */
private $l;

/** @var array */
private $supportedCiphersAndKeySize = [
'AES-256-CTR' => 32,
'AES-128-CTR' => 16,
'AES-256-CFB' => 32,
'AES-128-CFB' => 16,
];

/** @var bool */
private $supportLegacy;

Expand All @@ -105,8 +104,6 @@ public function __construct(ILogger $logger, IUserSession $userSession, IConfig
$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"';
$this->config = $config;
$this->l = $l;
$this->supportedKeyFormats = ['hash', 'password'];

$this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', false);
}

Expand Down Expand Up @@ -207,12 +204,12 @@ public function symmetricEncryptFileContent($plainContent, $passPhrase, $version
/**
* generate header for encrypted file
*
* @param string $keyFormat (can be 'hash' or 'password')
* @param string $keyFormat see SUPPORTED_KEY_FORMATS
* @return string
* @throws \InvalidArgumentException
*/
public function generateHeader($keyFormat = 'hash') {
if (in_array($keyFormat, $this->supportedKeyFormats, true) === false) {
public function generateHeader($keyFormat = self::DEFAULT_KEY_FORMAT) {
if (in_array($keyFormat, self::SUPPORTED_KEY_FORMATS, true) === false) {
throw new \InvalidArgumentException('key format "' . $keyFormat . '" is not supported');
}

Expand Down Expand Up @@ -259,14 +256,15 @@ private function encrypt($plainContent, $iv, $passPhrase = '', $cipher = self::D
*/
public function getCipher() {
$cipher = $this->config->getSystemValue('cipher', self::DEFAULT_CIPHER);
if (!isset($this->supportedCiphersAndKeySize[$cipher])) {
if (!isset(self::SUPPORTED_CIPHERS_AND_KEY_SIZE[$cipher])) {
$this->logger->warning(
sprintf(
'Unsupported cipher (%s) defined in config.php supported. Falling back to %s',
$cipher,
self::DEFAULT_CIPHER
),
['app' => 'encryption']);
sprintf(
'Unsupported cipher (%s) defined in config.php supported. Falling back to %s',
$cipher,
self::DEFAULT_CIPHER
),
['app' => 'encryption']
);
$cipher = self::DEFAULT_CIPHER;
}

Expand All @@ -288,8 +286,8 @@ public function getCipher() {
* @throws \InvalidArgumentException
*/
protected function getKeySize($cipher) {
if (isset($this->supportedCiphersAndKeySize[$cipher])) {
return $this->supportedCiphersAndKeySize[$cipher];
if (isset(self::SUPPORTED_CIPHERS_AND_KEY_SIZE[$cipher])) {
return self::SUPPORTED_CIPHERS_AND_KEY_SIZE[$cipher];
}

throw new \InvalidArgumentException(
Expand Down Expand Up @@ -411,7 +409,7 @@ public function decryptPrivateKey($privateKey, $password = '', $uid = '') {
$keyFormat = self::LEGACY_KEY_FORMAT;
}

if ($keyFormat === 'hash') {
if ($keyFormat === self::DEFAULT_KEY_FORMAT) {
$password = $this->generatePasswordHash($password, $cipher, $uid);
}

Expand Down