diff --git a/lib/private/Encryption/Keys/Storage.php b/lib/private/Encryption/Keys/Storage.php index cce22b9138a21..0ee24d8a52caa 100644 --- a/lib/private/Encryption/Keys/Storage.php +++ b/lib/private/Encryption/Keys/Storage.php @@ -235,104 +235,98 @@ private function getKeyWithUid(string $path, ?string $uid): string { } /** - * read key from hard disk + * Read the key from disk. * - * @param string $path to key - * @return array containing key as base64encoded key, and possible the uid + * @param string $path Path to key file. + * @return array Containing key as base64encoded key, and possibly the uid. + * @throws ServerNotAvailableException */ private function getKey($path): array { - $key = [ - 'key' => '', - ]; + if (!$this->view->file_exists($path)) { + return = [ + 'key' => '', + ]; + } - if ($this->view->file_exists($path)) { - if (isset($this->keyCache[$path])) { - $key = $this->keyCache[$path]; - } else { - $data = $this->view->file_get_contents($path); - - // Version <20.0.0.1 doesn't have this - $versionFromBeforeUpdate = $this->config->getSystemValueString('version', '0.0.0.0'); - if (version_compare($versionFromBeforeUpdate, '20.0.0.1', '<=')) { - $key = [ - 'key' => base64_encode($data), - ]; - } else { - if ($this->config->getSystemValueBool('encryption.key_storage_migrated', true)) { - try { - $clearData = $this->crypto->decrypt($data); - } catch (\Exception $e) { - throw new ServerNotAvailableException('Could not decrypt key', 0, $e); - } - - $dataArray = json_decode($clearData, true); - if ($dataArray === null) { - throw new ServerNotAvailableException('Invalid encryption key'); - } - - $key = $dataArray; - } else { - /* - * Even if not all keys are migrated we should still try to decrypt it (in case some have moved). - * However it is only a failure now if it is an array and decryption fails - */ - $fallback = false; - try { - $clearData = $this->crypto->decrypt($data); - } catch (\Throwable $e) { - $fallback = true; - } - - if (!$fallback) { - $dataArray = json_decode($clearData, true); - if ($dataArray === null) { - throw new ServerNotAvailableException('Invalid encryption key'); - } - $key = $dataArray; - } else { - $key = [ - 'key' => base64_encode($data), - ]; - } - } + if (isset($this->keyCache[$path])) { + return $this->keyCache[$path]; + } + + $data = $this->view->file_get_contents($path); + $migrated = $this->config->getSystemValueBool('encryption.key_storage_migrated', true); + + if ($migrated) { + try { + $clearData = $this->crypto->decrypt($data); + $dataArray = json_decode($clearData, true); + if ($dataArray === null) { + throw new ServerNotAvailableException('Invalid encryption key'); } - - $this->keyCache[$path] = $key; + $key = $dataArray; + } catch (\Exception $e) { + // Config indicates migration completed, but decrypted data is invalid. + throw new ServerNotAvailableException('Could not decrypt key', 0, $e); + } + } else { + // If key storage migration isn't indicated as being complete, + // attempt to decrypt the key as some may have already migrated. + // Otherwise, fall back to returning the (non-migrated) base64-encoded key. + try { + $clearData = $this->crypto->decrypt($data); + $dataArray = json_decode($clearData, true); + if ($dataArray === null) { + // used only to trigger fallback + throw new ServerNotAvailableException('Invalid encryption key'); + } + $key = $dataArray; + } catch (\Throwable $e) { + // Fallback: base64-encoded blob if decryption fails + $key = [ + 'key' => base64_encode($data) + ]; } } + $this->keyCache[$path] = $key; return $key; } /** - * write key to disk + * Write the given key to disk. * - * - * @param string $path path to key directory - * @param array $key key - * @return bool + * @param string $path Destination file path for the key + * @param array $key Associative array with encryption key data (must have 'key') + * @return bool True if key is persisted; throws on error. + * @throws \UnexpectedValueException if $key structure is invalid (rare) + * @throws \RuntimeException if encode, encrypt, or write fails or is incomplete */ - private function setKey($path, $key) { + private function setKey(string $path, array $key): bool { $this->keySetPreparation(dirname($path)); - $versionFromBeforeUpdate = $this->config->getSystemValueString('version', '0.0.0.0'); - if (version_compare($versionFromBeforeUpdate, '20.0.0.1', '<=')) { - // Only store old format if this happens during the migration. - // TODO: Remove for 21 - $data = base64_decode($key['key']); - } else { - // Wrap the data - $data = $this->crypto->encrypt(json_encode($key)); + if (!isset($key['key'])) { + throw new \UnexpectedValueException('Provided $key missing required "key" entry'); } - $result = $this->view->file_put_contents($path, $data); + try { + $json = json_encode($key, JSON_THROW_ON_ERROR); + $data = $this->crypto->encrypt($json); + } catch (\JsonException $e) { + throw new \RuntimeException('Failed to JSON encode key for storage: ' . $e->getMessage(), 0, $e); + } catch (\Throwable $e) { + throw new \RuntimeException('Failed to encrypt key for storage: ' . $e->getMessage(), 0, $e); + } - if (is_int($result) && $result > 0) { - $this->keyCache[$path] = $key; - return true; + $result = $this->view->file_put_contents($path, $data); + $expected = \strlen($data); + if ($result === false || $result !== $expected) { + throw new \RuntimeException( + "Failed to write encryption key to {$path}: " + . ($result === false ? 'file_put_contents returned false' : "wrote {$result} bytes, expected ".\strlen($data)) + ); } - return false; + $this->keyCache[$path] = $key; + return true; } /** diff --git a/tests/lib/Encryption/Keys/StorageTest.php b/tests/lib/Encryption/Keys/StorageTest.php index 333d8d8ce2199..0441e95b37f52 100644 --- a/tests/lib/Encryption/Keys/StorageTest.php +++ b/tests/lib/Encryption/Keys/StorageTest.php @@ -89,32 +89,6 @@ public function testSetFileKey(): void { ); } - public function testSetFileOld(): void { - $this->config->method('getSystemValueString') - ->with('version') - ->willReturn('20.0.0.0'); - $this->util->expects($this->any()) - ->method('getUidAndFilename') - ->willReturn(['user1', '/files/foo.txt']); - $this->util->expects($this->any()) - ->method('stripPartialFileExtension') - ->willReturnArgument(0); - $this->util->expects($this->any()) - ->method('isSystemWideMountPoint') - ->willReturn(false); - $this->crypto->expects($this->never()) - ->method('encrypt'); - $this->view->expects($this->once()) - ->method('file_put_contents') - ->with($this->equalTo('/user1/files_encryption/keys/files/foo.txt/encModule/fileKey'), - $this->equalTo('key')) - ->willReturn(strlen('key')); - - $this->assertTrue( - $this->storage->setFileKey('user1/files/foo.txt', 'fileKey', 'key', 'encModule') - ); - } - public static function dataTestGetFileKey() { return [ ['/files/foo.txt', '/files/foo.txt', true, 'key'],