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

Fix user-key encryption with Groups #16332

Closed
MTRichards opened this issue May 13, 2015 · 28 comments
Closed

Fix user-key encryption with Groups #16332

MTRichards opened this issue May 13, 2015 · 28 comments

Comments

@MTRichards
Copy link
Contributor

MTRichards commented May 13, 2015

(for reference, this is fixing an old item #10010)

As an ownCloud user, I want to be able to share my files with other users using groups, regardless of the state of encryption so that I can use ownCloud to collaborate with other users in that group - even when that group membership changes.

Acceptance Criteria:

  • User can share a file with encryption enabled even with groups
  • As group memberships change, the users will be able to access the files appropriately (so even if a user is added to a group, the file can be opened by the new group member - removal already works because of the presentation layer)
  • If a recovery key is enabled, this could be used - but also need to handle the no recovery key situation
@MTRichards MTRichards added this to the 8.2-next milestone May 13, 2015
@schiessle
Copy link
Contributor

If a recovery key is enabled, this could be used - but also need to handle the no recovery key situation

with the restriction that the user who manipulates the groups also know the recovery password, which might not always be the case. Also it can be a performance issue for huge installation if we want to update the share key during group manipulation.

@schiessle
Copy link
Contributor

Another idea is to track the group changes in a separate table. Once a user from such a group log-in we could use his private key to update the group share. In this case we also need to keep the performance in mind for large installation to avoid that the login process will be delayed to much.

This wouldn't give all new group members instantly access to the files. But at least in a reasonable time given that the ownCloud is used regularly by most users. During the time where the user can't decrypt the share we could even hide it and only display it once his share-key is available.

@butonic
Copy link
Member

butonic commented Jul 8, 2015

I just had the idea of creating a key pair for every group. If you share a folder with a group that groups public key is added to the symmetric key. That leaves us with preventing access to the group key for non group members. We could encrypt it with the public keys of all members. A group admin (who, by definintion is a member of the group) can always decrypt the key to reencrypt it when he adds or removes a user)

What about an admin creating a group? At that moment the group is empty and has no members. This is kind of the same problem as with the restore password, muliplied by the number of groups.

@ghost ghost added the green-ticket label Jul 22, 2015
@PVince81
Copy link
Contributor

PVince81 commented Aug 4, 2015

There was an idea here with the recovery key: #10010

@schiessle
Copy link
Contributor

@MTRichards this is partially solved with the key storage implementation we have for 8.2. The key storage consists of two independent parts:

  1. Possibility to move the location of the keys
  2. Encrypt all files with a master key instead of individual keys

If the admin enabled option 2 (no matter if he also moves the storage or not), the problem with group shares is solved. But in this case we use one master key to encrypt/decrypt all file keys instead of individual per-user keys. Don't know if this is already a solution for many users or not, @MTRichards what do you think?

@MTRichards
Copy link
Contributor Author

@schiesbn I like that we can do both per user per file encryption, as well as per system implementation (master key). This is a partial solution certainly. What you are saying is that with 1) above, we can also handle it - but if you use the vanilla encryption app of today, it still doesn't work...right?

@schiessle
Copy link
Contributor

@MTRichards yes, with 2) from above we can handle it, but not with the default settings. If this is really needed we have to think about adding it to the 9.0 roadmap with a high priority, sadly it wasn't possible to implement it for 8.2.

@MTRichards
Copy link
Contributor Author

Understood, will pencil it in for 9.0 and discuss.

@ghost ghost modified the milestones: 9.0-next, 8.2-current Sep 22, 2015
@butonic
Copy link
Member

butonic commented Nov 11, 2015

@schiesbn @PVince81 @MTRichards I still think a group key would correctly solve the problem without having to rely on a master key. Any comments?

@MTRichards
Copy link
Contributor Author

To be honest, the implementation just needs to be such that we are not breaking encryption security levels to solve a UX problem that we experience.
Do groups change? all the time. Does this solve the problem if a group changes? Maybe if a group changes that key may be valid but the file can't be viewed so it becomes not an issue...

@butonic
Copy link
Member

butonic commented Nov 11, 2015

The trick is that the key remains the same when the group changes. It will only be reencrypted with the new users public key.

@bboule
Copy link

bboule commented Jan 4, 2016

Is this milestone correct do we plan to address this issue in 9? is this a green ticket do we have prospects waiting for this?

@MTRichards
Copy link
Contributor Author

The 9.0 milestone is not, I believe, correct at this time.
@schiesbn @cmonteroluque For confirmation and comment please.

This one is partially addressed in 8.2 with the single master key for the entire system. In that case, this is no longer an issue.

However, in the case of using the full encryption mechanism this has not been addressed, and is not planned for 9.0 as far as I know.

@ghost
Copy link

ghost commented Jan 6, 2016

@bboule @MTRichards I agree. Moving to 9.1

@MorrisJobke
Copy link
Contributor

Could we get this somehow implemented?

Implementation would be:

  • if encryption enabled
  • and user get's added to group

We will:

  • write this new group membership into a DB table

We hook into the post_login hook (or something similar):

  • check in the DB if for my groups a new user is added
  • if so: encrypt the group share keys for this new user

@schiesbn @MTRichards Is this correct? Could this be implemented? Or at least been a decision if this is wanted at least to properly communicate if it will be possible in the future.

@schiessle
Copy link
Contributor

schiessle commented May 12, 2016

The trick is that the key remains the same when the group changes. It will only be reencrypted with the new users public key.

You just move the problem to a new file/key. The problem is: "how do we re-encrypt the the file-key with the new users public key if you don't have access to the file-key?" Now you can replace "file-key" with "group-key" in the sentence above and you have the same problem.

One answer could be to give all admins and group admins access to all group keys. Well you could do it. But this would mean the user can no longer rely on that only he and the users he chose to share with can read his files. All (group-)admin now have potentially access to your files which reduces the security and moves you one step further to the master key.
Also the complexity would grow extremely. You would have one group key for each group (let's say we have N groups), encrypted with X public keys which would lead to X share-keys. So we would have N*2X more keys we have to manage. Also the logic would be come more complex. Right now it is relatively simple: You want to access a file, grab the file key, your private key and the share key and go on. That's quite straight forward and works for every file. With group keys we would have different code paths for different file types. Because for a group share the user would have to take his private key, the group key and the share key, decrypt the group key. Then take the group key, and the group-share key for the file key, decrypt the file key in order to finally decrypt the file.

Bottom line: A lot of additional complexity which either don't solve the problem or weaken the security already in a way where you could ask if the user couldn't do the extra step to the master key.

@PVince81 PVince81 modified the milestones: 9.2, 9.1 Jul 6, 2016
@PVince81
Copy link
Contributor

PVince81 commented Jul 6, 2016

Moving enhancement to 9.2

@DeepDiver1975 @dragotin

@PVince81
Copy link
Contributor

PVince81 commented Oct 6, 2016

Good steps to reproduce here #26290

@PVince81
Copy link
Contributor

PVince81 commented Oct 6, 2016

Since 9.1 I heard that the user's passwords are stored encrypted in the database anyway, so it should now be possible to reuse that to give access to the keys that need updating...

@PVince81
Copy link
Contributor

@PVince81
Copy link
Contributor

PVince81 commented Dec 8, 2016

Would move to backlog for now @pmaier1 @felixboehm

@pmaier1 pmaier1 modified the milestones: backlog, 10.0 Dec 21, 2016
@PVince81 PVince81 changed the title Fix Encryption with Groups Fix user-key encryption with Groups Dec 15, 2017
@BrainStone
Copy link

BrainStone commented Jan 8, 2018

This issue has been stalling for over a year now. Are there any updates?
We are aware of resharing. Though if users share shared files themselves, resharing the original file breaks these shares. Also when using the desktop client I cannot delete and recreate the share fast enough so that it doesn't delete and redownload the files.

I mean since with the recovery key and the impersonate plugin you are able to view all files of any user it should be possible to update the share with the key aswell.

The performance heavy tasks could be moved to the cron job. And on demand (so if a file hasn't been migrated but is requested that it then gets migrated)

@PVince81
Copy link
Contributor

PVince81 commented Jan 9, 2018

Unfortunately no. There is no good technical solution for this that wouldn't reduce the security model.

The alternative is to switch to master key encryption which only has a single key for everybody.

@BrainStone
Copy link

Maybe a button to reshare that only gets displayed if such a reshare would make sense?

@PVince81
Copy link
Contributor

PVince81 commented Jan 9, 2018

The problem is not only about reshare but also about adding users to a group for which a share already exists. The user in question cannot access the content already shared because the password of the owner is required for generating a share key for the new recipient.

@BrainStone
Copy link

I meant the reshare button as a conviniece for the file owner. So like it doesn't remove the files for people that already had access to them if that makes sense.
But of course the button will be only available to the owner of the share.

@stale
Copy link

stale bot commented Sep 20, 2021

This issue has been automatically closed.

@stale stale bot closed this as completed Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants