Skip to content

WIP: fix(encryption): store key by fileId for CollectiveMountPoint #59343

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.

Sign up for GitHub

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

Draft
mejo- wants to merge 1 commit into
base: master
Choose a base branch
from
+27 −0
Draft

WIP: fix(encryption): store key by fileId for CollectiveMountPoint #59343

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions lib/private/Encryption/Util.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,16 @@
return $result;
}

public function isCollectiveMountPoint(string $path, string $uid) {
$mount = Filesystem::getMountManager()->find('/' . $uid . $path);
if (!$mount) {

Check failure on line 251 in lib/private/Encryption/Util.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainType 

lib/private/Encryption/Util.php:251:8: TypeDoesNotContainType: Type OCP\Files\Mount\IMountPoint for $mount is never falsy (see https://psalm.dev/056)

Check failure on line 251 in lib/private/Encryption/Util.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

TypeDoesNotContainType 

lib/private/Encryption/Util.php:251:7: TypeDoesNotContainType: Operand of type false is always falsy (see https://psalm.dev/056)
return false;
}

// We use class name to avoid hard dependency on collectives app
return strpos(get_class($mount), 'CollectiveMountPoint') !== false;
}

/**
* check if the file is stored on a system wide mount point
* @param string $path relative to /data/user with leading '/'
Expand Down Expand Up @@ -357,6 +367,23 @@
$root = $this->getKeyStorageRoot();

// in case of system-wide mount points the keys are stored directly in the data directory
if ($this->isCollectiveMountPoint($filename, $owner)) {
$fileId === null;

Check failure on line 371 in lib/private/Encryption/Util.php

View workflow job for this annotation

GitHub Actions / static-code-analysis

UndefinedVariable 

lib/private/Encryption/Util.php:371:4: UndefinedVariable: Cannot find referenced variable $fileId (see https://psalm.dev/024)
try {
$fileInfo = $this->rootView->getFileInfo($path);
if ($fileInfo) {
$fileId = $fileInfo['fileid'] ?? null;
}
} catch (\Exception) {
// continue
}

if ($fileId !== null) {
$keyPath = $root . '/files_encryption/keys_by_fileId/' . $fileId . '/';
Copy link
Copy Markdown
Member

@icewind1991 icewind1991 Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will run into problems with the number of files in a single directory. Some form of nested directory structure will need to be used to spread things out a bit

Copy link
Copy Markdown
Member Author

@mejo- mejo- Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an idea how we could nest the the files?

What do you think about a 3-level split of a 12-digit zero-padded version of the fileId? I.e. fileId 123 would go to 0000/0000/0123/123/, 987654321 would go to 0009/8765/4321/987654321/, and so on.

$padded = str_pad((string)$fileId, 12, '0', STR_PAD_LEFT); 
$path = substr($padded, 0, 4) . '/' . substr($padded, 4, 4) . '/' . substr($padded, 8, 4) . '/' . (string)$fileId;

return Filesystem::normalizePath($keyPath . $encryptionModuleId . '/', false);
}
}

if ($this->isSystemWideMountPoint($filename, $owner)) {
$keyPath = $root . '/' . '/files_encryption/keys' . $filename . '/';
} else {
Expand Down
Loading