allow binary encoding for written files to reduce file size #224

karakayasemi · 2020-12-20T22:26:41Z

Fixes #210, owncloud/core#10831
Needs owncloud/core#38249

This PR makes binary encoding default and writes the encoding info to the file metadata. After this PR, all the newly created files will be encrypted with binary encoding.
For decryption, if no metadata presents we keep continue to decrypt these files with legacy base64 encoding, if header is present we use binary encoding to decrypt those files.

karakayasemi · 2020-12-21T09:52:09Z

@phil-davis @micbar looks like we have common problem in web ui tests independent from this pr. Daily buils are also failing. Probably, it is related with the web ui changes of oc10. (tags, open with etc.)

phil-davis · 2020-12-21T09:54:04Z

PR #223
is currently running CI. I am trying to get a pass! When that is merged, then other PRs can be rebased.

karakayasemi · 2020-12-21T09:56:50Z

@phil-davis thank you for information.

phil-davis · 2020-12-22T03:06:33Z

@micbar the repo is still set to require codecov/patch and codecov/project - those should no longer be required.

micbar · 2020-12-22T07:38:30Z

removed the check

mmattel · 2020-12-22T13:23:06Z

@karakayasemi How does this work with existing encrypted files?
Just thinking about that the reduction is as written in the referenced PR ~35%, I would like to see that old files get the reduction too. What about backwards compatibility for existing setups? See also my comment in the referenced issue.
BTW, needs a rebase 😄

karakayasemi · 2020-12-22T20:24:25Z

@mmattel I already read your comments on the issue. In this pr's current implementation, the code checks encryption.binary_encode system config value for binary encoding. If it is not exist, the app works like before. But, I guess this config value more proper as app config. We may move this config value to app config and add a checkbox to admin settings to be able to enable.

For the migration, as you said we can add an occ command.

karakayasemi · 2020-12-22T20:55:26Z

@mmattel I thought about the migration command again. Actually, we have encryption:decrypt-all and encryption:encrypt-all commands. We may just suggest decrypt-all the files, enable the binary encode and re-encrypt all for existing setups.

What do you think? I am open to suggestions.

mmattel · 2020-12-22T21:09:09Z

Just reading the occ command description:
encryption:encrypt-all encrypts all data files for all users (no single user encryption... good)
encryption:decrypt-all decrypts all user data files, or optionally a single user (I can live with it)

Yes, this would work
decrypt
enable new config setting
encrypt

I would thing that it would be good to have the choice, at least to see which (old or new) method will be used. That was the reason for my thoughts of a new config setting...

karakayasemi · 2020-12-22T21:09:16Z

Another option would be making binary encoding default and writing the encoding info to the file metadata. I guess no reason to keep base64 encoding format for new files. In this way, all the newly created files encrypted with binary encoding.
For decryption, if no metadata presents we can keep continue to decrypt these files with legacy base64 encoding, if header is present we can use binary encoding to decrypt.

mmattel · 2020-12-22T21:21:53Z

making binary encoding default

as long you can still reade the old format for existing files... 😄
(means you can have both types in parallel (old: still readable, new: rw) )
The rest is a matter of documentation.

mmattel · 2020-12-23T08:15:08Z

old: still readable, new: rw

Idea: as old files are still readable and new files will have the new format...
What about the idea of an occ command which rewrites existing files? Somthing like:
occ encryption:rewrite (in single user mode only)
To think about possible options, at least -v, or a --skip= option, or a --path= option
Such a command could be interrupted and restarted without harm!

mmattel · 2020-12-27T14:12:20Z

It is interresting that the blocksize was set to

old: private $unencryptedBlockSizeSigned = 6072;
new: private $unencryptedBlockSizeSigned = 8096;

I just think about that on the filesystem level, blocks are usually chunks in 4k. Where is 6k coming from, because that allone is producing a overhead and misaglingment ...?

karakayasemi · 2020-12-27T21:18:46Z

@mmattel It is really not easy to understand these calculations for me either. Still I could not exactly understand the reason of magic number 6126 also. As far as I understand, ownCloud filesystem blocksize is 8192. With current base64 encoding format, we can only use 6072 bytes in an 8192 bytes block for storing data. Its the cause of %33 file overhead. Each signed encrypted block also stores initialization vector, signature and extra padding.

Seems like storing signature of each block takes (6126-6072) = 54 bytes and if a block signed, the available bytes reduces to 6072. Since I only touched encoded block, I just adjusted 6072 to 8096 for binary encoded data by multiplying it with coefficient 1.33.

phil-davis · 2021-02-04T04:10:30Z

@karakayasemi we were forced to switch drone CI to use .drone.yml in PR #237

@dpakach is ~~now getting the tests sorted out~~ in PR #236 - he has rebased that. (done)

~~But you will need to rebase this PR anyway so that it uses the "new" .drone.yml. So you may as well rebase it now. Then we can easily cherry-pick tests onto it later today or tomorrow.~~ - I have rebased just now. ~~And soon we will push the commit with the tests.~~ And a commit with tests has been cherry-picked into this PR. Let's see what CI thinks.

sq

phil-davis · 2021-02-05T10:00:09Z

If everyone is happy, IMO this PR is OK to merge.

Default to the more space-efficient binary encoding for newly encrypted files instead of the traditional base64 encoding, eliminating the 33% overhead. The new option 'encryption.use_legacy_encoding' allows to force the legacy encoding format if needed. Files encoded in the old format remain readable. Based on owncloud/encryption#224 and owncloud/core#38249 by karakayasemi.

Default to the more space-efficient binary encoding for newly encrypted files instead of the traditional base64 encoding, eliminating the 33% overhead. The new option 'encryption.use_legacy_encoding' allows to force the legacy encoding format if needed. Files encoded in the old format remain readable. Based on owncloud/encryption#224 and owncloud/core#38249 by karakayasemi. Signed-off-by: plumbeo <plumbeo@users.noreply.github.com>

karakayasemi self-assigned this Dec 20, 2020

karakayasemi force-pushed the binary-encode branch from e3396bf to 7130676 Compare December 20, 2020 22:29

karakayasemi force-pushed the binary-encode branch from 7130676 to 58227a6 Compare December 21, 2020 20:34

karakayasemi force-pushed the binary-encode branch from 58227a6 to 154d047 Compare December 22, 2020 20:26

karakayasemi force-pushed the binary-encode branch 9 times, most recently from 0c41a5b to bde9d10 Compare December 27, 2020 13:26

karakayasemi mentioned this pull request Dec 27, 2020

determine unencrypted block size after begin operation owncloud/core#38249

Merged

11 tasks

karakayasemi force-pushed the binary-encode branch 2 times, most recently from 628c5d5 to 747492a Compare January 14, 2021 17:57

karakayasemi added 2 commits February 4, 2021 17:35

allow binary encoding for written files to reduce file size

557618c

explain unencrypted block size calculation logic in phpdoc

c74099d

sq

phil-davis force-pushed the binary-encode branch from bb82610 to c74099d Compare February 4, 2021 11:50

[Tests-Only] add api test different encoding with encryption

c4f04e7

phil-davis mentioned this pull request Feb 5, 2021

[strictly 10.7.0] Release Encryption 1.5.0 #220

Closed

36 tasks

micbar merged commit b10151d into master Feb 9, 2021

delete-merged-branch bot deleted the binary-encode branch February 9, 2021 13:01

karakayasemi mentioned this pull request Feb 9, 2021

Use binary files for encryption instead of base64 encoded ones owncloud/core#10831

Closed

mmattel mentioned this pull request Feb 16, 2021

Encrypted files are not self-contained #211

Open

butonic requested review from micbar, pmaier1 and dragotin and removed request for dragotin, micbar and pmaier1 March 19, 2021 16:02

LukasReschke mentioned this pull request Jun 14, 2021

Save space by storing encrypted files in raw binary, not in base64 nextcloud/server#22156

Closed

plumbeo mentioned this pull request Apr 24, 2022

Save encrypted files in binary format nextcloud/server#32110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow binary encoding for written files to reduce file size #224

allow binary encoding for written files to reduce file size #224

karakayasemi commented Dec 20, 2020 •

edited

karakayasemi commented Dec 21, 2020

phil-davis commented Dec 21, 2020

karakayasemi commented Dec 21, 2020

phil-davis commented Dec 22, 2020

micbar commented Dec 22, 2020

mmattel commented Dec 22, 2020 •

edited

karakayasemi commented Dec 22, 2020

karakayasemi commented Dec 22, 2020 •

edited

mmattel commented Dec 22, 2020

karakayasemi commented Dec 22, 2020

mmattel commented Dec 22, 2020

mmattel commented Dec 23, 2020

mmattel commented Dec 27, 2020

karakayasemi commented Dec 27, 2020 •

edited

phil-davis commented Feb 4, 2021 •

edited

phil-davis commented Feb 5, 2021

allow binary encoding for written files to reduce file size #224

allow binary encoding for written files to reduce file size #224

Conversation

karakayasemi commented Dec 20, 2020 • edited

karakayasemi commented Dec 21, 2020

phil-davis commented Dec 21, 2020

karakayasemi commented Dec 21, 2020

phil-davis commented Dec 22, 2020

micbar commented Dec 22, 2020

mmattel commented Dec 22, 2020 • edited

karakayasemi commented Dec 22, 2020

karakayasemi commented Dec 22, 2020 • edited

mmattel commented Dec 22, 2020

karakayasemi commented Dec 22, 2020

mmattel commented Dec 22, 2020

mmattel commented Dec 23, 2020

mmattel commented Dec 27, 2020

karakayasemi commented Dec 27, 2020 • edited

phil-davis commented Feb 4, 2021 • edited

phil-davis commented Feb 5, 2021

karakayasemi commented Dec 20, 2020 •

edited

mmattel commented Dec 22, 2020 •

edited

karakayasemi commented Dec 22, 2020 •

edited

karakayasemi commented Dec 27, 2020 •

edited

phil-davis commented Feb 4, 2021 •

edited