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

Migrate away from ILogger in encryption #39065

Merged
merged 3 commits into from Aug 8, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 11 additions & 10 deletions apps/encryption/lib/AppInfo/Application.php
Expand Up @@ -39,6 +39,7 @@
use OCA\Encryption\Util;
use OCP\Encryption\IManager;
use OCP\IConfig;
use Psr\Log\LoggerInterface;

class Application extends \OCP\AppFramework\App {
/**
Expand Down Expand Up @@ -69,7 +70,7 @@
$hookManager->registerHook([
new UserHooks($container->query(KeyManager::class),
$server->getUserManager(),
$server->getLogger(),
$server->get(LoggerInterface::class),
$container->query(Setup::class),
$server->getUserSession(),
$container->query(Util::class),
Expand All @@ -93,15 +94,15 @@
Encryption::DISPLAY_NAME,
function () use ($container) {
return new Encryption(
$container->query(Crypt::class),
$container->query(KeyManager::class),
$container->query(Util::class),
$container->query(Session::class),
$container->query(EncryptAll::class),
$container->query(DecryptAll::class),
$container->getServer()->getLogger(),
$container->getServer()->getL10N($container->getAppName())
);
$container->query(Crypt::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->query(KeyManager::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->query(Util::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->query(Session::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->query(EncryptAll::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->query(DecryptAll::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IContainer::query has been marked as deprecated
$container->getServer()->get(LoggerInterface::class),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\AppFramework\IAppContainer::getServer has been marked as deprecated
$container->getServer()->getL10N($container->getAppName())

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\AppFramework\IAppContainer::getServer has been marked as deprecated

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IServerContainer::getL10N has been marked as deprecated

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\AppFramework\IAppContainer::getAppName has been marked as deprecated
);
});
}
}
43 changes: 9 additions & 34 deletions apps/encryption/lib/Command/FixEncryptedVersion.php
Expand Up @@ -29,51 +29,26 @@
use OCP\Files\IRootFolder;
use OCP\HintException;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\IUserManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

class FixEncryptedVersion extends Command {
/** @var IConfig */
private $config;

/** @var ILogger */
private $logger;

/** @var IRootFolder */
private $rootFolder;

/** @var IUserManager */
private $userManager;

/** @var Util */
private $util;

/** @var View */
private $view;

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

public function __construct(
IConfig $config,
ILogger $logger,
IRootFolder $rootFolder,
IUserManager $userManager,
Util $util,
View $view
private IConfig $config,
private LoggerInterface $logger,
private IRootFolder $rootFolder,
private IUserManager $userManager,
private Util $util,
private View $view,
) {
$this->config = $config;
$this->logger = $logger;
$this->rootFolder = $rootFolder;
$this->userManager = $userManager;
$this->util = $util;
$this->view = $view;
$this->supportLegacy = false;

parent::__construct();
Expand Down Expand Up @@ -134,7 +109,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
return $this->runForUser($user, $pathOption, $output);
} elseif ($all) {
$result = 0;
$this->userManager->callForSeenUsers(function(IUser $user) use ($pathOption, $output, &$result) {
$this->userManager->callForSeenUsers(function (IUser $user) use ($pathOption, $output, &$result) {
$output->writeln("Processing files for " . $user->getUID());
$result = $this->runForUser($user->getUID(), $pathOption, $output);
return $result === 0;
Expand Down
113 changes: 25 additions & 88 deletions apps/encryption/lib/Crypto/Crypt.php
Expand Up @@ -40,8 +40,8 @@
use OCP\Encryption\Exceptions\GenericEncryptionException;
use OCP\IConfig;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;
use phpseclib\Crypt\RC4;

/**
Expand Down Expand Up @@ -83,40 +83,24 @@
// default encoding format, old Nextcloud versions used base64
public const BINARY_ENCODING_FORMAT = 'binary';

/** @var ILogger */
private $logger;
private string $user;

/** @var string */
private $user;
private ?string $currentCipher = null;

/** @var IConfig */
private $config;

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

/** @var string|null */
private $currentCipher;

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

/**
* Use the legacy base64 encoding instead of the more space-efficient binary encoding.
*/
private bool $useLegacyBase64Encoding;

/**
* @param ILogger $logger
* @param IUserSession $userSession
* @param IConfig $config
* @param IL10N $l
*/
public function __construct(ILogger $logger, IUserSession $userSession, IConfig $config, IL10N $l) {
$this->logger = $logger;
$this->user = $userSession && $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"';
$this->config = $config;
$this->l = $l;
public function __construct(
private LoggerInterface $logger,
IUserSession $userSession,
private IConfig $config,
private IL10N $l,
) {
$this->user = $userSession->isLoggedIn() ? $userSession->getUser()->getUID() : '"no user given"';

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method getUID on possibly null value
$this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', false);
$this->useLegacyBase64Encoding = $this->config->getSystemValueBool('encryption.use_legacy_base64_encoding', false);
}
Expand All @@ -127,15 +111,14 @@
* @return array|bool
*/
public function createKeyPair() {
$log = $this->logger;
$res = $this->getOpenSSLPKey();

if (!$res) {
$log->error("Encryption Library couldn't generate users key-pair for {$this->user}",
$this->logger->error("Encryption Library couldn't generate users key-pair for {$this->user}",
['app' => 'encryption']);

if (openssl_error_string()) {
$log->error('Encryption library openssl_pkey_new() fails: ' . openssl_error_string(),
$this->logger->error('Encryption library openssl_pkey_new() fails: ' . openssl_error_string(),
['app' => 'encryption']);
}
} elseif (openssl_pkey_export($res,
Expand All @@ -150,10 +133,10 @@
'privateKey' => $privateKey
];
}
$log->error('Encryption library couldn\'t export users private key, please check your servers OpenSSL configuration.' . $this->user,
$this->logger->error('Encryption library couldn\'t export users private key, please check your servers OpenSSL configuration.' . $this->user,
['app' => 'encryption']);
if (openssl_error_string()) {
$log->error('Encryption Library:' . openssl_error_string(),
$this->logger->error('Encryption Library:' . openssl_error_string(),
['app' => 'encryption']);
}

Expand All @@ -170,12 +153,7 @@
return openssl_pkey_new($config);
}

/**
* get openSSL Config
*
* @return array
*/
private function getOpenSSLConfig() {
private function getOpenSSLConfig(): array {
$config = ['private_key_bits' => 4096];
$config = array_merge(
$config,
Expand Down Expand Up @@ -236,14 +214,9 @@
}

/**
* @param string $plainContent
* @param string $iv
* @param string $passPhrase
* @param string $cipher
* @return string
* @throws EncryptionFailedException
*/
private function encrypt($plainContent, $iv, $passPhrase = '', $cipher = self::DEFAULT_CIPHER) {
private function encrypt(string $plainContent, string $iv, string $passPhrase = '', string $cipher = self::DEFAULT_CIPHER): string {
$options = $this->useLegacyBase64Encoding ? 0 : OPENSSL_RAW_DATA;
$encryptedContent = openssl_encrypt($plainContent,
$cipher,
Expand All @@ -264,10 +237,8 @@
/**
* return cipher either from config.php or the default cipher defined in
* this class
*
* @return string
*/
private function getCachedCipher() {
private function getCachedCipher(): string {
if (isset($this->currentCipher)) {
return $this->currentCipher;
}
Expand Down Expand Up @@ -333,43 +304,27 @@
return self::LEGACY_CIPHER;
}

/**
* @param string $encryptedContent
* @param string $iv
* @return string
*/
private function concatIV($encryptedContent, $iv) {
private function concatIV(string $encryptedContent, string $iv): string {
return $encryptedContent . '00iv00' . $iv;
}

/**
* @param string $encryptedContent
* @param string $signature
* @return string
*/
private function concatSig($encryptedContent, $signature) {
private function concatSig(string $encryptedContent, string $signature): string {
return $encryptedContent . '00sig00' . $signature;
}

/**
* Note: This is _NOT_ a padding used for encryption purposes. It is solely
* used to achieve the PHP stream size. It has _NOTHING_ to do with the
* encrypted content and is not used in any crypto primitive.
*
* @param string $data
* @return string
*/
private function addPadding($data) {
private function addPadding(string $data): string {
return $data . 'xxx';
}

/**
* generate password hash used to encrypt the users private key
*
* @param string $password
* @param string $cipher
* @param string $uid only used for user keys
* @return string
*/
protected function generatePasswordHash(string $password, string $cipher, string $uid = '', int $iterations = 600000): string {
$instanceId = $this->config->getSystemValue('instanceid');
Expand Down Expand Up @@ -542,13 +497,9 @@


/**
* remove padding
*
* @param string $padded
* @param bool $hasSignature did the block contain a signature, in this case we use a different padding
* @return string|false
*/
private function removePadding($padded, $hasSignature = false) {
private function removePadding(string $padded, bool $hasSignature = false): string|false {
if ($hasSignature === false && substr($padded, -2) === 'xx') {
return substr($padded, 0, -2);
} elseif ($hasSignature === true && substr($padded, -3) === 'xxx') {
Expand All @@ -561,12 +512,8 @@
* split meta data from encrypted file
* Note: for now, we assume that the meta data always start with the iv
* followed by the signature, if available
*
* @param string $catFile
* @param string $cipher
* @return array
*/
private function splitMetaData($catFile, $cipher) {
private function splitMetaData(string $catFile, string $cipher): array {
if ($this->hasSignature($catFile, $cipher)) {
$catFile = $this->removePadding($catFile, true);
$meta = substr($catFile, -93);
Expand All @@ -591,12 +538,9 @@
/**
* check if encrypted block is signed
*
* @param string $catFile
* @param string $cipher
* @return bool
* @throws GenericEncryptionException
*/
private function hasSignature($catFile, $cipher) {
private function hasSignature(string $catFile, string $cipher): bool {
$skipSignatureCheck = $this->config->getSystemValueBool('encryption_skip_signature_check', false);

$meta = substr($catFile, -93);
Expand All @@ -617,12 +561,6 @@


/**
* @param string $encryptedContent
* @param string $iv
* @param string $passPhrase
* @param string $cipher
* @param boolean $binaryEncoding
* @return string
* @throws DecryptionFailedException
*/
private function decrypt(string $encryptedContent, string $iv, string $passPhrase = '', string $cipher = self::DEFAULT_CIPHER, bool $binaryEncoding = false): string {
Expand Down Expand Up @@ -669,10 +607,9 @@
/**
* generate initialization vector
*
* @return string
* @throws GenericEncryptionException
*/
private function generateIv() {
private function generateIv(): string {
return random_bytes(16);
}

Expand Down