Merge pull request #45669 from nextcloud/fix/fix-encryption-legacy-reshare

fix: Autodetect legacy filekey instead of trusting the header for legacy header

3 files changed, 23 insertions, 43 deletions

diff --git a/apps/encryption/lib/Crypto/Encryption.php b/apps/encryption/lib/Crypto/Encryption.php
index 1dd1d91c218..dda93e13306 100644
--- a/apps/encryption/lib/Crypto/Encryption.php
+++ b/apps/encryption/lib/Crypto/Encryption.php
@@ -54,8 +54,6 @@ class Encryption implements IEncryptionModule {
 	/** @var int Current version of the file */
 	private int $version = 0;
 
-	private bool $useLegacyFileKey = true;
-
 	/** @var array remember encryption signature version */
 	private static $rememberVersion = [];
 
@@ -112,7 +110,6 @@ class Encryption implements IEncryptionModule {
 		$this->writeCache = '';
 		$this->useLegacyBase64Encoding = true;
 
-		$this->useLegacyFileKey = ($header['useLegacyFileKey'] ?? 'true') !== 'false';
 
 		if (isset($header['encoding'])) {
 			$this->useLegacyBase64Encoding = $header['encoding'] !== Crypt::BINARY_ENCODING_FORMAT;
@@ -126,19 +123,10 @@ class Encryption implements IEncryptionModule {
 			}
 		}
 
-		if ($this->session->decryptAllModeActivated()) {
-			$shareKey = $this->keyManager->getShareKey($this->path, $this->session->getDecryptAllUid());
-			if ($this->useLegacyFileKey) {
-				$encryptedFileKey = $this->keyManager->getEncryptedFileKey($this->path);
-				$this->fileKey = $this->crypt->multiKeyDecryptLegacy($encryptedFileKey,
-					$shareKey,
-					$this->session->getDecryptAllKey());
-			} else {
-				$this->fileKey = $this->crypt->multiKeyDecrypt($shareKey, $this->session->getDecryptAllKey());
-			}
-		} else {
-			$this->fileKey = $this->keyManager->getFileKey($this->path, $this->user, $this->useLegacyFileKey);
-		}
+		/* If useLegacyFileKey is not specified in header, auto-detect, to be safe */
+		$useLegacyFileKey = (($header['useLegacyFileKey'] ?? '') == 'false' ? false : null);
+
+		$this->fileKey = $this->keyManager->getFileKey($this->path, $this->user, $useLegacyFileKey, $this->session->decryptAllModeActivated());
 
 		// always use the version from the original file, also part files
 		// need to have a correct version number if they get moved over to the
diff --git a/apps/encryption/lib/KeyManager.php b/apps/encryption/lib/KeyManager.php
index 6771db7947b..9fd6c7655af 100644
--- a/apps/encryption/lib/KeyManager.php
+++ b/apps/encryption/lib/KeyManager.php
@@ -343,12 +343,9 @@ class KeyManager {
 	}
 
 	/**
-	 * @param string $path
-	 * @param $uid
 	 * @param ?bool $useLegacyFileKey null means try both
-	 * @return string
 	 */
-	public function getFileKey(string $path, ?string $uid, ?bool $useLegacyFileKey): string {
+	public function getFileKey(string $path, ?string $uid, ?bool $useLegacyFileKey, bool $useDecryptAll = false): string {
 		if ($uid === '') {
 			$uid = null;
 		}
@@ -361,8 +358,10 @@ class KeyManager {
 				return '';
 			}
 		}
-
-		if ($this->util->isMasterKeyEnabled()) {
+		if ($useDecryptAll) {
+			$shareKey = $this->getShareKey($path, $this->session->getDecryptAllUid());
+			$privateKey = $this->session->getDecryptAllKey();
+		} elseif ($this->util->isMasterKeyEnabled()) {
 			$uid = $this->getMasterKeyId();
 			$shareKey = $this->getShareKey($path, $uid);
 			if ($publicAccess) {
diff --git a/apps/encryption/tests/Crypto/EncryptionTest.php b/apps/encryption/tests/Crypto/EncryptionTest.php
index 122990da611..4767d22339d 100644
--- a/apps/encryption/tests/Crypto/EncryptionTest.php
+++ b/apps/encryption/tests/Crypto/EncryptionTest.php
@@ -103,6 +103,10 @@ class EncryptionTest extends TestCase {
 	 * test if public key from one of the recipients is missing
 	 */
 	public function testEndUser1() {
+		$this->sessionMock->expects($this->once())
+			->method('decryptAllModeActivated')
+			->willReturn(false);
+
 		$this->instance->begin('/foo/bar', 'user1', 'r', [], ['users' => ['user1', 'user2', 'user3']]);
 		$this->endTest();
 	}
@@ -112,6 +116,10 @@ class EncryptionTest extends TestCase {
 	 *
 	 */
 	public function testEndUser2() {
+		$this->sessionMock->expects($this->once())
+			->method('decryptAllModeActivated')
+			->willReturn(false);
+
 		$this->expectException(\OCA\Encryption\Exceptions\PublicKeyMissingException::class);
 
 		$this->instance->begin('/foo/bar', 'user2', 'r', [], ['users' => ['user1', 'user2', 'user3']]);
@@ -233,35 +241,16 @@ class EncryptionTest extends TestCase {
 	 */
 	public function testBeginDecryptAll() {
 		$path = '/user/files/foo.txt';
-		$recoveryKeyId = 'recoveryKeyId';
-		$recoveryShareKey = 'recoveryShareKey';
-		$decryptAllKey = 'decryptAllKey';
 		$fileKey = 'fileKey';
 
 		$this->sessionMock->expects($this->once())
 			->method('decryptAllModeActivated')
 			->willReturn(true);
-		$this->sessionMock->expects($this->once())
-			->method('getDecryptAllUid')
-			->willReturn($recoveryKeyId);
-		$this->sessionMock->expects($this->once())
-			->method('getDecryptAllKey')
-			->willReturn($decryptAllKey);
-
-		$this->keyManagerMock->expects($this->once())
-			->method('getEncryptedFileKey')
-			->willReturn('encryptedFileKey');
 		$this->keyManagerMock->expects($this->once())
-			->method('getShareKey')
-			->with($path, $recoveryKeyId)
-			->willReturn($recoveryShareKey);
-		$this->cryptMock->expects($this->once())
-			->method('multiKeyDecryptLegacy')
-			->with('encryptedFileKey', $recoveryShareKey, $decryptAllKey)
+			->method('getFileKey')
+			->with($path, 'user', null, true)
 			->willReturn($fileKey);
 
-		$this->keyManagerMock->expects($this->never())->method('getFileKey');
-
 		$this->instance->begin($path, 'user', 'r', [], []);
 
 		$this->assertSame($fileKey,
@@ -275,6 +264,10 @@ class EncryptionTest extends TestCase {
 	 * and continue
 	 */
 	public function testBeginInitMasterKey() {
+		$this->sessionMock->expects($this->once())
+			->method('decryptAllModeActivated')
+			->willReturn(false);
+
 		$this->sessionMock->expects($this->once())->method('isReady')->willReturn(false);
 		$this->utilMock->expects($this->once())->method('isMasterKeyEnabled')
 			->willReturn(true);