[encryption] reorganize folder structure (second try to make Jenkins happy) #12382

schiessle · 2014-11-24T11:06:59Z

In order to fix #11297 I changed the folder structure for the encryption keys.

We create for every file a corresponding folder in files_encryption/keys. For example for "foo/test.txt" we will create a folder "files_encryption/keys/foo/test.txt/". This folder will contain all share keys, named "userId.shareKey" and the file key named "fileKey". This way we avoid any name collision with file names which are similar to user names, like described in issue #11297.

This makes many file operation easier. If a file gets renamed or moved we just have to rename/move the corresponding folder. This way we can save a lot of overhead to find the correct share-keys and file-keys which should result in some performance improvements and should also make the code more robust.

Additionally this PR changes the naming for public/private keys from "user.public.key" and user.private.key" to "user.publicKey" and user.privateKey" for consistency reasons.

This PR touches all aspects of encryption, so we need to test encryption as complete as possible, including:

sharing, reshares,...
deleting single files or complete folders
restore the deleted file/folder from the trashbin or just single files/sub-folders from the deleted folder
enabling recovery keys
change passwords
recover files
deleted shared files and try to restore them
...

Currently the migration script is missing. But everything should work if you start with a fresh installation or a setup without any encrypted files and encryption keys.

Currently missing:

migration script

all keys are now in files_encryption/key/path_to_file/filename/ share keys are named: user.shareKey file key is named: fileKey

ghost · 2014-11-26T10:56:47Z

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3293/
🚀 Test PASSed. 🚀

PVince81 · 2014-11-26T16:40:03Z

apps/files_encryption/lib/proxy.php


 		// If data is a catfile
 		if (
 			Crypt::mode() === 'server'
+			&& $this->shouldEncrypt($path)


Additional bugfix or needed as per this PR's changes ?

As discussed, obsoletes the need to disable the file proxy

PVince81 · 2014-11-26T16:42:28Z

Wow, glad to see this removes a lot of code.

This will need a full regression test:

all file operations
sharing + all file operations (including move in/out of a shared dir)
recovery keys
add/delete user (keys deletion)

PVince81 · 2014-11-28T14:05:20Z

test with ext storage: system mounts and personal mounts

PVince81 · 2014-11-28T14:30:54Z

test migration with non-initialized users

PVince81 · 2014-11-28T14:34:55Z

Code looks good. Needs testing.

PVince81 · 2014-11-28T15:07:16Z

trashbin (delete, restore files and folders)
versions (download, restore version)

PVince81 · 2014-11-28T15:25:57Z

~~BUG: user1 shares folder with user2. Recover user2's password. user2 cannot read shared files in folder.~~
~~BUG: WebDAV MOVE from root to shared folder as non-owner gives 403 (web UI move works)~~ [master] WebDAV MOVE from root into shared folder as non owner 403 forbidden #12538

PVince81 · 2014-11-28T15:31:10Z

~~BUG: cannot download previous versions from the versions dropdown~~ Versioning + encryption: former versions are not decrypted when downloaded #12221

PVince81 · 2014-11-28T15:40:15Z

~~BUG: share folder with user1, delete user1: share keys for user1 not removed~~ Stray share keys after deleting user #11175

PVince81 · 2014-11-28T15:52:36Z

I'm done testing. Please see the above bugs.

schiessle · 2014-11-28T16:07:32Z

BUG: user1 shares folder with user2. Recover user2's password. user2 cannot read shared files in folder.

Unrelated, this also happens on oc7/master. Currently we only recover the files from the user. Recover shared files can only work if the owner has also enabled the recovery key

share folder with user1, delete user1: share keys for user1 not removed

most likely this also happens on oc7/master, do you remember the issue that not all share keys get cleaned-up in all circumstances?

schiessle · 2014-11-28T16:29:50Z

BUG: cannot download previous versions from the versions dropdown

known issue, unrelated to this bug: #12221

schiessle · 2014-11-28T16:57:19Z

BUG: WebDAV MOVE from root to shared folder as non-owner gives 403 (web UI move works)

This also happens on master without encryption. Probably a sharing permission issue. Please open a seperate issue for it.

ghost · 2014-11-28T18:00:44Z

🚀 Test PASSed. 🚀
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3486/
🚀 Test PASSed. 🚀

PVince81 · 2014-12-02T10:08:31Z

BUG: migration doesn't seem to delete "files_trashbin/{share-keys,keyfiles}" (minor)

PVince81 · 2014-12-02T10:10:51Z

Apart from the minor issue, I'm ok to have this merged 👍

schiessle · 2014-12-02T15:10:55Z

Added migration of trash bin, including unit tests.

This needs a second reviewer... Maybe @LukasReschke @MorrisJobke @nickvergessen, or anyone else ? Thanks!

scrutinizer-notifier · 2014-12-02T15:51:17Z

The inspection completed: 49 new issues, 67 updated code elements

schiessle · 2014-12-02T21:58:09Z

@owncloud-bot retest this please

ghost · 2014-12-02T22:46:04Z

💣 Test FAILed. 💣
Refer to this link for build results (access rights to CI server needed):
https://ci.owncloud.org//job/pull-request-analyser-ng-simple/3641/

Build result: FAILURE

[...truncated 14 lines...] > git config remote.origin.fetch +refs/heads/:refs/remotes/origin/ # timeout=10 > git config remote.origin.url https://github.com/owncloud/core.git # timeout=10Fetching upstream changes from https://github.com/owncloud/core.gitusing GIT_SSH to set credentials using .gitcredentials to set credentials > git config --local credential.helper store --file=/tmp/git18099305855542784.credentials # timeout=10 > git fetch --tags --progress https://github.com/owncloud/core.git +refs/pull/:refs/remotes/origin/pr/ > git config --local --remove-section credential # timeout=10 > git rev-parse origin/pr/12382/merge^{commit} # timeout=10Checking out Revision 0a63738400011b98682db94503f7373626603a6b (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 0a63738400011b98682db94503f7373626603a6b > git rev-list e24a11e378c1d0992f1089e32aa3ea3fcb4d6311 # timeout=10 > git remote # timeout=10 > git submodule init # timeout=10 > git submodule sync # timeout=10 > git config --get remote.origin.url # timeout=10 > git submodule update --init --recursiveTriggering pull-request-analyser-ng-simple » vm-slave-02pull-request-analyser-ng-simple » vm-slave-02 completed with result FAILUREStarted calculate disk usage of buildFinished Calculation of disk usage of build in 0 secondsStarted calculate disk usage of workspaceFinished Calculation of disk usage of workspace in 1 second
💣 Test FAILed. 💣

MorrisJobke · 2014-12-03T08:04:48Z

I guess the "new PR to satisfy jenkins" approach has to be applied here :(

hudson.plugins.git.GitException: Could not checkout null with start point f6fed320c03c0e81b88a44637352dd4b26565af3

schiessle · 2014-12-03T12:20:30Z

Here is a new PR for Jenkins: #12575

MorrisJobke · 2014-12-03T13:40:05Z

I will review this now.

MorrisJobke · 2014-12-03T15:08:23Z

I tested this:

installed master, uploaded some files, shared a folder to admin (without any new users), added to users (one to admin group), added the other user to a share, shared a file/folder by link
checked if all works
updated to this branch
tested all shares, deleted files from shares and restored from owner account, renamed, downloaded
changed password
updated a file. Is downloadable by other sharees

👍 from me

MorrisJobke · 2014-12-03T15:08:38Z

Jenkins is fine with this (see other PR)

[encryption] reorganize folder structure (second try to make Jenkins happy)

LukasReschke · 2014-12-03T15:10:47Z

apps/files_encryption/lib/helper.php

@@ -17,7 +19,7 @@
 * GNU AFFERO GENERAL PUBLIC LICENSE for more details.
 *
 * You should have received a copy of the GNU Affero General Public
- * License along with this library.  If not, see <http://www.gnu.org/licenses/>.
+ * License alon with this library.  If not, see <http://www.gnu.org/licenses/>.


schiessle changed the title ~~Enc reorganize folders2~~ [encryption] reorganize folder structure (second try to make Jenkins happy) Nov 24, 2014

MorrisJobke mentioned this pull request Nov 24, 2014

[encryption] reorganize folder structure #12191

Closed

1 task

schiessle added the 3 - To Review label Nov 24, 2014

schiessle added this to the 8.0-current milestone Nov 24, 2014

Bjoern Schiessle added 3 commits November 26, 2014 10:57

new folder structure for keys

fd86d76

all keys are now in files_encryption/key/path_to_file/filename/ share keys are named: user.shareKey file key is named: fileKey

harmonize copyright notice

266f1a2

change private/public key names for consistency reasons

a90606f

schiessle force-pushed the enc_reorganize_folders2 branch from ab33b88 to 0ecd072 Compare November 26, 2014 09:59

PVince81 reviewed Nov 26, 2014
View reviewed changes

schiessle mentioned this pull request Dec 2, 2014

OCS API for server-to-server sharing #12452

Merged

1 task

Bjoern Schiessle added 2 commits December 2, 2014 16:03

upgrade to new folder structure

49cfc30

delete old keys if file was moved to a different mount point

2e78217

schiessle force-pushed the enc_reorganize_folders2 branch from e6f7ace to 2e78217 Compare December 2, 2014 15:05

small fixes

9ca9acf

This was referenced Dec 3, 2014

Fix namespaces of encryption classes and encryption tests #12568

Merged

[ignore] Enc reorganize folders #12575

Closed

MorrisJobke added a commit that referenced this pull request Dec 3, 2014

Merge pull request #12382 from owncloud/enc_reorganize_folders2

3fdb193

[encryption] reorganize folder structure (second try to make Jenkins happy)

MorrisJobke merged commit 3fdb193 into master Dec 3, 2014

MorrisJobke deleted the enc_reorganize_folders2 branch December 3, 2014 15:08

LukasReschke reviewed Dec 3, 2014
View reviewed changes

schiessle mentioned this pull request Dec 3, 2014

fix typo in license header #12581

Merged

nickvergessen mentioned this pull request Dec 4, 2014

Infinite recursion in lib/trashbin.php scanning the files_trashbin #12594

Closed

lock bot locked as resolved and limited conversation to collaborators Aug 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[encryption] reorganize folder structure (second try to make Jenkins happy) #12382

[encryption] reorganize folder structure (second try to make Jenkins happy) #12382

schiessle commented Nov 24, 2014

ghost commented Nov 26, 2014

PVince81 Nov 26, 2014

PVince81 Nov 28, 2014

PVince81 commented Nov 26, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

schiessle commented Nov 28, 2014

schiessle commented Nov 28, 2014

schiessle commented Nov 28, 2014

ghost commented Nov 28, 2014

PVince81 commented Dec 2, 2014

PVince81 commented Dec 2, 2014

schiessle commented Dec 2, 2014

scrutinizer-notifier commented Dec 2, 2014

schiessle commented Dec 2, 2014

ghost commented Dec 2, 2014

MorrisJobke commented Dec 3, 2014

schiessle commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

LukasReschke Dec 3, 2014

[encryption] reorganize folder structure (second try to make Jenkins happy) #12382

[encryption] reorganize folder structure (second try to make Jenkins happy) #12382

Conversation

schiessle commented Nov 24, 2014

ghost commented Nov 26, 2014

PVince81 Nov 26, 2014

Choose a reason for hiding this comment

PVince81 Nov 28, 2014

Choose a reason for hiding this comment

PVince81 commented Nov 26, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

PVince81 commented Nov 28, 2014

schiessle commented Nov 28, 2014

schiessle commented Nov 28, 2014

schiessle commented Nov 28, 2014

ghost commented Nov 28, 2014

PVince81 commented Dec 2, 2014

PVince81 commented Dec 2, 2014

schiessle commented Dec 2, 2014

scrutinizer-notifier commented Dec 2, 2014

schiessle commented Dec 2, 2014

ghost commented Dec 2, 2014

Build result: FAILURE

MorrisJobke commented Dec 3, 2014

schiessle commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

MorrisJobke commented Dec 3, 2014

LukasReschke Dec 3, 2014

Choose a reason for hiding this comment