Skip to content
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.

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

PrivateFile::basic_mv and PrivateFile::cp load full file content in memory #242

Closed
Frando opened this issue Apr 26, 2023 · 3 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Frando
Copy link
Contributor

Frando commented Apr 26, 2023

PrivateFile::attach is called by basic_mv and cp to attach a node to a new location in the directory hierarchy.
Through PrivateNode::update_ancestry it goes to PrivateFile::prepare_key_rotation which contains these lines to reencrypt the file content:

let content = self.get_content(forest, store).await?;
self.header.inumber = utils::get_random_bytes(rng);
self.header.update_bare_name(parent_bare_name);
self.header.reset_ratchet(rng);
let content = Self::prepare_content(&self.header.bare_name, content, forest, store, rng).await?;
self.content.content = content;

This will load the full (decrypted) file content into memory and therefore can easily exhaust available memory for large files and/or constrained devices.

Instead this could use PrivateFile::stream_content and PrivateFile::prepare_content_streaming so that not all of the file content has to be in memory at the same time. Maybe it would need some logic to read with the old key and label and write with the new key and label. Or the file could be cloned maybe.

@matheus23
Copy link
Member

Yep, you're right. This code predates the streaming APIs.

Instead this could use PrivateFile::stream_contenty and PrivateFile::prepare_content_streaming

That's the way to do it.

Maybe it would need some logic to read with the old key and label and write with the new key and label. Or the file could be cloned maybe.

For now we should make sure the key changes (and not clone the `PrivateFile). We've thought about adding deduplication in the future though. But that's future.

@matheus23 matheus23 added enhancement New feature or request good first issue Good for newcomers labels Apr 28, 2023
@matheus23
Copy link
Member

Now due to some changes from #306 we can actually implement this in a way that wouldn't require us to re-encrypt at all.

@matheus23
Copy link
Member

This was fixed in #319

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants