Skip to content

Commit b604d52

Browse files
authored
Merge pull request #22218 from nextcloud/enh/sse/make_legacy_cipher_opt_in
SSE: make legacy format opt in
2 parents 4361d65 + 2bbb848 commit b604d52

File tree

19 files changed

+327
-16
lines changed

19 files changed

+327
-16
lines changed

apps/encryption/appinfo/info.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
Please read the documentation to know all implications before you decide
1515
to enable server-side encryption.
1616
</description>
17-
<version>2.8.0</version>
17+
<version>2.8.1</version>
1818
<licence>agpl</licence>
1919
<author>Bjoern Schiessle</author>
2020
<author>Clark Tomlinson</author>
@@ -44,6 +44,7 @@
4444
<command>OCA\Encryption\Command\EnableMasterKey</command>
4545
<command>OCA\Encryption\Command\DisableMasterKey</command>
4646
<command>OCA\Encryption\Command\RecoverUser</command>
47+
<command>OCA\Encryption\Command\ScanLegacyFormat</command>
4748
</commands>
4849

4950
<settings>

apps/encryption/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
'OCA\\Encryption\\Command\\DisableMasterKey' => $baseDir . '/../lib/Command/DisableMasterKey.php',
1111
'OCA\\Encryption\\Command\\EnableMasterKey' => $baseDir . '/../lib/Command/EnableMasterKey.php',
1212
'OCA\\Encryption\\Command\\RecoverUser' => $baseDir . '/../lib/Command/RecoverUser.php',
13+
'OCA\\Encryption\\Command\\ScanLegacyFormat' => $baseDir . '/../lib/Command/ScanLegacyFormat.php',
1314
'OCA\\Encryption\\Controller\\RecoveryController' => $baseDir . '/../lib/Controller/RecoveryController.php',
1415
'OCA\\Encryption\\Controller\\SettingsController' => $baseDir . '/../lib/Controller/SettingsController.php',
1516
'OCA\\Encryption\\Controller\\StatusController' => $baseDir . '/../lib/Controller/StatusController.php',

apps/encryption/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class ComposerStaticInitEncryption
2525
'OCA\\Encryption\\Command\\DisableMasterKey' => __DIR__ . '/..' . '/../lib/Command/DisableMasterKey.php',
2626
'OCA\\Encryption\\Command\\EnableMasterKey' => __DIR__ . '/..' . '/../lib/Command/EnableMasterKey.php',
2727
'OCA\\Encryption\\Command\\RecoverUser' => __DIR__ . '/..' . '/../lib/Command/RecoverUser.php',
28+
'OCA\\Encryption\\Command\\ScanLegacyFormat' => __DIR__ . '/..' . '/../lib/Command/ScanLegacyFormat.php',
2829
'OCA\\Encryption\\Controller\\RecoveryController' => __DIR__ . '/..' . '/../lib/Controller/RecoveryController.php',
2930
'OCA\\Encryption\\Controller\\SettingsController' => __DIR__ . '/..' . '/../lib/Controller/SettingsController.php',
3031
'OCA\\Encryption\\Controller\\StatusController' => __DIR__ . '/..' . '/../lib/Controller/StatusController.php',
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* @copyright Copyright (c) 2020, Roeland Jago Douma <roeland@famdouma.nl>
6+
*
7+
* @author Roeland Jago Douma <roeland@famdouma.nl>
8+
*
9+
* @license GNU AGPL version 3 or any later version
10+
*
11+
* This program is free software: you can redistribute it and/or modify
12+
* it under the terms of the GNU Affero General Public License as
13+
* published by the Free Software Foundation, either version 3 of the
14+
* License, or (at your option) any later version.
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Affero General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Affero General Public License
22+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
23+
*
24+
*/
25+
26+
namespace OCA\Encryption\Command;
27+
28+
use OC\Files\View;
29+
use OCA\Encryption\Util;
30+
use OCP\IConfig;
31+
use OCP\IUserManager;
32+
use Symfony\Component\Console\Command\Command;
33+
use Symfony\Component\Console\Helper\QuestionHelper;
34+
use Symfony\Component\Console\Input\InputInterface;
35+
use Symfony\Component\Console\Output\OutputInterface;
36+
37+
class ScanLegacyFormat extends Command {
38+
39+
/** @var Util */
40+
protected $util;
41+
42+
/** @var IConfig */
43+
protected $config;
44+
45+
/** @var QuestionHelper */
46+
protected $questionHelper;
47+
48+
/** @var IUserManager */
49+
private $userManager;
50+
51+
/** @var View */
52+
private $rootView;
53+
54+
/**
55+
* @param Util $util
56+
* @param IConfig $config
57+
* @param QuestionHelper $questionHelper
58+
*/
59+
public function __construct(Util $util,
60+
IConfig $config,
61+
QuestionHelper $questionHelper,
62+
IUserManager $userManager) {
63+
parent::__construct();
64+
65+
$this->util = $util;
66+
$this->config = $config;
67+
$this->questionHelper = $questionHelper;
68+
$this->userManager = $userManager;
69+
$this->rootView = new View();
70+
}
71+
72+
protected function configure() {
73+
$this
74+
->setName('encryption:scan:legacy-format')
75+
->setDescription('Scan the files for the legacy format');
76+
}
77+
78+
protected function execute(InputInterface $input, OutputInterface $output): int {
79+
$result = true;
80+
81+
$output->writeln('Scanning all files for legacy encryption');
82+
83+
foreach ($this->userManager->getBackends() as $backend) {
84+
$limit = 500;
85+
$offset = 0;
86+
do {
87+
$users = $backend->getUsers('', $limit, $offset);
88+
foreach ($users as $user) {
89+
$output->writeln('Scanning all files for ' . $user);
90+
$this->setupUserFS($user);
91+
$result &= $this->scanFolder($output, '/' . $user);
92+
}
93+
$offset += $limit;
94+
} while (count($users) >= $limit);
95+
}
96+
97+
if ($result) {
98+
$output->writeln('All scanned files are propperly encrypted. You can disable the legacy compatibility mode.');
99+
return 0;
100+
}
101+
102+
return 1;
103+
}
104+
105+
private function scanFolder(OutputInterface $output, string $folder): bool {
106+
$clean = true;
107+
108+
foreach ($this->rootView->getDirectoryContent($folder) as $item) {
109+
$path = $folder . '/' . $item['name'];
110+
if ($this->rootView->is_dir($path)) {
111+
if ($this->scanFolder($output, $path) === false) {
112+
$clean = false;
113+
}
114+
} else {
115+
if (!$item->isEncrypted()) {
116+
// ignore
117+
continue;
118+
}
119+
120+
$stats = $this->rootView->stat($path);
121+
if (!isset($stats['hasHeader']) || $stats['hasHeader'] === false) {
122+
$clean = false;
123+
$output->writeln($path . ' does not have a proper header');
124+
}
125+
}
126+
}
127+
128+
return $clean;
129+
}
130+
131+
/**
132+
* setup user file system
133+
*
134+
* @param string $uid
135+
*/
136+
protected function setupUserFS($uid) {
137+
\OC_Util::tearDownFS();
138+
\OC_Util::setupFS($uid);
139+
}
140+
}

apps/encryption/lib/Crypto/Crypt.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
use OC\Encryption\Exceptions\DecryptionFailedException;
3434
use OC\Encryption\Exceptions\EncryptionFailedException;
35+
use OC\ServerNotAvailableException;
3536
use OCA\Encryption\Exceptions\MultiKeyDecryptException;
3637
use OCA\Encryption\Exceptions\MultiKeyEncryptException;
3738
use OCP\Encryption\Exceptions\GenericEncryptionException;
@@ -89,6 +90,9 @@ class Crypt {
8990
'AES-128-CFB' => 16,
9091
];
9192

93+
/** @var bool */
94+
private $supportLegacy;
95+
9296
/**
9397
* @param ILogger $logger
9498
* @param IUserSession $userSession
@@ -101,6 +105,8 @@ public function __construct(ILogger $logger, IUserSession $userSession, IConfig
101105
$this->config = $config;
102106
$this->l = $l;
103107
$this->supportedKeyFormats = ['hash', 'password'];
108+
109+
$this->supportLegacy = $this->config->getSystemValueBool('encryption.legacy_format_support', false);
104110
}
105111

106112
/**
@@ -299,6 +305,10 @@ protected function getKeySize($cipher) {
299305
* @return string
300306
*/
301307
public function getLegacyCipher() {
308+
if (!$this->supportLegacy) {
309+
throw new ServerNotAvailableException('Legacy cipher is no longer supported!');
310+
}
311+
302312
return self::LEGACY_CIPHER;
303313
}
304314

@@ -391,7 +401,7 @@ public function decryptPrivateKey($privateKey, $password = '', $uid = '') {
391401
if (isset($header['cipher'])) {
392402
$cipher = $header['cipher'];
393403
} else {
394-
$cipher = self::LEGACY_CIPHER;
404+
$cipher = $this->getLegacyCipher();
395405
}
396406

397407
if (isset($header['keyFormat'])) {
@@ -570,6 +580,11 @@ private function hasSignature($catFile, $cipher) {
570580
$meta = substr($catFile, -93);
571581
$signaturePosition = strpos($meta, '00sig00');
572582

583+
// If we no longer support the legacy format then everything needs a signature
584+
if (!$skipSignatureCheck && !$this->supportLegacy && $signaturePosition === false) {
585+
throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature'));
586+
}
587+
573588
// enforce signature for the new 'CTR' ciphers
574589
if (!$skipSignatureCheck && $signaturePosition === false && stripos($cipher, 'ctr') !== false) {
575590
throw new GenericEncryptionException('Missing Signature', $this->l->t('Missing Signature'));

apps/encryption/tests/Crypto/CryptTest.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ public function testConcatIV() {
209209
* @dataProvider dataTestSplitMetaData
210210
*/
211211
public function testSplitMetaData($data, $expected) {
212+
$this->config->method('getSystemValue')
213+
->with('encryption_skip_signature_check', false)
214+
->willReturn(true);
212215
$result = self::invokePrivate($this->crypt, 'splitMetaData', [$data, 'AES-256-CFB']);
213216
$this->assertTrue(is_array($result));
214217
$this->assertSame(3, count($result));
@@ -233,6 +236,9 @@ public function dataTestSplitMetaData() {
233236
* @dataProvider dataTestHasSignature
234237
*/
235238
public function testHasSignature($data, $expected) {
239+
$this->config->method('getSystemValue')
240+
->with('encryption_skip_signature_check', false)
241+
->willReturn(true);
236242
$this->assertSame($expected,
237243
$this->invokePrivate($this->crypt, 'hasSignature', [$data, 'AES-256-CFB'])
238244
);
@@ -385,6 +391,10 @@ public function dataTestGetKeySize() {
385391
* @dataProvider dataTestDecryptPrivateKey
386392
*/
387393
public function testDecryptPrivateKey($header, $privateKey, $expectedCipher, $isValidKey, $expected) {
394+
$this->config->method('getSystemValueBool')
395+
->with('encryption.legacy_format_support', false)
396+
->willReturn(true);
397+
388398
/** @var \OCA\Encryption\Crypto\Crypt | \PHPUnit\Framework\MockObject\MockObject $crypt */
389399
$crypt = $this->getMockBuilder(Crypt::class)
390400
->setConstructorArgs(

apps/encryption/tests/Settings/AdminTest.php

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,20 +74,21 @@ protected function setUp(): void {
7474

7575
public function testGetForm() {
7676
$this->config
77-
->expects($this->at(0))
7877
->method('getAppValue')
79-
->with('encryption', 'recoveryAdminEnabled', '0')
80-
->willReturn(1);
81-
$this->config
82-
->expects($this->at(1))
83-
->method('getAppValue')
84-
->with('encryption', 'encryptHomeStorage', '1')
85-
->willReturn(1);
78+
->will($this->returnCallback(function ($app, $key, $default) {
79+
if ($app === 'encryption' && $key === 'recoveryAdminEnabled' && $default === '0') {
80+
return '1';
81+
}
82+
if ($app === 'encryption' && $key === 'encryptHomeStorage' && $default === '1') {
83+
return '1';
84+
}
85+
return $default;
86+
}));
8687
$params = [
87-
'recoveryEnabled' => 1,
88+
'recoveryEnabled' => '1',
8889
'initStatus' => '0',
89-
'encryptHomeStorage' => false,
90-
'masterKeyEnabled' => false
90+
'encryptHomeStorage' => true,
91+
'masterKeyEnabled' => true
9192
];
9293
$expected = new TemplateResponse('encryption', 'settings-admin', $params, '');
9394
$this->assertEquals($expected, $this->admin->getForm());

apps/settings/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php',
5555
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => $baseDir . '/../lib/Settings/Personal/Security/WebAuthn.php',
5656
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/../lib/Settings/Personal/ServerDevNotice.php',
57+
'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php',
5758
'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php',
5859
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',
5960
);

apps/settings/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ class ComposerStaticInitSettings
6969
'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php',
7070
'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/WebAuthn.php',
7171
'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/..' . '/../lib/Settings/Personal/ServerDevNotice.php',
72+
'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php',
7273
'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php',
7374
'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',
7475
);

apps/settings/lib/Controller/CheckSetupController.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
use OC\IntegrityCheck\Checker;
5454
use OC\Lock\NoopLockingProvider;
5555
use OC\MemoryInfo;
56+
use OCA\Settings\SetupChecks\LegacySSEKeyFormat;
5657
use OCA\Settings\SetupChecks\PhpDefaultCharset;
5758
use OCA\Settings\SetupChecks\PhpOutputBuffering;
5859
use OCP\AppFramework\Controller;
@@ -687,6 +688,7 @@ protected function isEnoughTempSpaceAvailableIfS3PrimaryStorageIsUsed(): bool {
687688
public function check() {
688689
$phpDefaultCharset = new PhpDefaultCharset();
689690
$phpOutputBuffering = new PhpOutputBuffering();
691+
$legacySSEKeyFormat = new LegacySSEKeyFormat($this->l10n, $this->config, $this->urlGenerator);
690692
return new DataResponse(
691693
[
692694
'isGetenvServerWorking' => !empty(getenv('PATH')),
@@ -729,6 +731,7 @@ public function check() {
729731
'reverseProxyGeneratedURL' => $this->urlGenerator->getAbsoluteURL('index.php'),
730732
PhpDefaultCharset::class => ['pass' => $phpDefaultCharset->run(), 'description' => $phpDefaultCharset->description(), 'severity' => $phpDefaultCharset->severity()],
731733
PhpOutputBuffering::class => ['pass' => $phpOutputBuffering->run(), 'description' => $phpOutputBuffering->description(), 'severity' => $phpOutputBuffering->severity()],
734+
LegacySSEKeyFormat::class => ['pass' => $legacySSEKeyFormat->run(), 'description' => $legacySSEKeyFormat->description(), 'severity' => $legacySSEKeyFormat->severity(), 'linkToDocumentation' => $legacySSEKeyFormat->linkToDocumentation()],
732735
]
733736
);
734737
}

0 commit comments

Comments
 (0)