Skip to content

Commit

Permalink
Merge pull request #39065 from nextcloud/fix/remove-ilogger-uses-in-e…
Browse files Browse the repository at this point in the history 
…ncryption

Migrate away from ILogger in encryption
  • Loading branch information
@come-nc
come-nc authored Aug 8, 2023
2 parents bbd0deb + 3e176f5 commit e232cd8
Show file tree
Hide file tree
Showing 16 changed files with 144 additions and 496 deletions.
21 changes: 11 additions & 10 deletions apps/encryption/lib/AppInfo/Application.php
View file
Original file line number Diff line number Diff line change
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 @@ public function registerHooks(IConfig $config) {
$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 @@ public function registerEncryptionModule(IManager $encryptionManager) {
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),
$container->query(KeyManager::class),
$container->query(Util::class),
$container->query(Session::class),
$container->query(EncryptAll::class),
$container->query(DecryptAll::class),
$container->getServer()->get(LoggerInterface::class),
$container->getServer()->getL10N($container->getAppName())
);
});
}
}
43 changes: 9 additions & 34 deletions apps/encryption/lib/Command/FixEncryptedVersion.php
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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 @@ class Crypt {
// 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"';
$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 @@ public function __construct(ILogger $logger, IUserSession $userSession, IConfig
* @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 @@ public function createKeyPair() {
'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 @@ public function getOpenSSLPKey() {
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 @@ public function generateHeader($keyFormat = self::DEFAULT_KEY_FORMAT) {
}

/**
* @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 @@ private function encrypt($plainContent, $iv, $passPhrase = '', $cipher = self::D
/**
* 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 @@ public function getLegacyCipher() {
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 @@ private function createSignature(string $data, string $passPhrase): string {


/**
* 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 @@ private function removePadding($padded, $hasSignature = false) {
* 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 @@ private function splitMetaData($catFile, $cipher) {
/**
* 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 @@ private function hasSignature($catFile, $cipher) {


/**
* @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 @@ protected function parseHeader($data) {
/**
* generate initialization vector
*
* @return string
* @throws GenericEncryptionException
*/
private function generateIv() {
private function generateIv(): string {
return random_bytes(16);
}

Expand Down
Loading

0 comments on commit e232cd8

Please sign in to comment.