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

Design improvement for encryption + reset password #2920

Open
GitHubUser4234 opened this issue Jan 3, 2017 · 8 comments
Open

Design improvement for encryption + reset password #2920

GitHubUser4234 opened this issue Jan 3, 2017 · 8 comments
Labels

Comments

@GitHubUser4234
Copy link
Contributor

GitHubUser4234 commented Jan 3, 2017

Hi,

There are 2 major issues when a password reset is performed with encryption enabled. Especially the first one is actually well known, and has been reported from time to time, but I would like to discuss and contribute (if necessary) to finally provide a solution that isn't only a half-hearted one.

1) Existing files cannot be opened anymore after a password reset

Previously uploaded files can only be recovered when all these conditions are met:

  • the user has enabled the password recovery option before the incident
  • he reaches out to a system or group administrator for help to reset the password
  • the new password is communicated in a (hopefully) secure way between the administrator and the user

To have to go through all this, doesn't really promote user-friendliness, to say the least. In a project of a larger scale, this would additionally lead to administrative overhead as a non-negligible consequence.

Obviously, an ideal solution would be to provide the same user experience as when encryption was disabled.

2) There is only one global recovery password

As there is only one global recovery password to recover files after a password reset, this password needs to be shared among all relevant system and group administrators. The more groups and administrators exist in an environment, the more communicating the recovery password (e.g. after a renewal) becomes a security concern.

Here, it would be more preferable, if every administrator could maintain his own recovery password.

Solutions?

If issue 1) could be resolved, it would obviously at the same time solve issue 2), so the focus would be on 1). Last but not least, there have been quite a few reports about it (e.g. #427 etc.) and there are examples (including my project) where the reset password function with the current design would prove completely useless and would have to be themed out completely.

Solution A

One solution that currently comes to my mind is to use a similar method like the master key approach. With the master key, the application is able to decrypt user files automatically, so how about doing the same with the recovery key? Just additionally it would be helpful to add a feature enabling the renewal of the recovery key through occ (for reference see here).

An activation of this feature could be an optional setting like "Automatic recovery". It would leave the current approach untouched by default and only comes to action if activated. If activated, recovery is enabled for all users by default and there won't be any indications of manual password recovery in the Web UI any more.

Solution B, C, D...

Any idea? :D


@schiessle @LukasReschke Your opinion is highly appreciated :)

@schiessle
Copy link
Member

Here, it would be more preferable, if every administrator could maintain his own recovery password.

Would be possible in theory. A few things to consider. This would (depending on the number of administrators) add significant overhead. Also every time a (new) admin would enable his recovery key Nextcloud would need to re-encrypt all file-keys which could become a serious performance issue on large systems. Another problem could be to decide which admin/subadmin is allowed to hold a recovery key for which group of users. The initial idea was that the recovery key is really only a fall back which doesn't reduce security. So it is one key which is only accessible to this one responsible and trustworthy person, either one administrator or even someone like the chief data protection officer of the organization.

One solution that currently comes to my mind is to use a similar method like the master key approach. With the master key, the application is able to decrypt user files automatically, so how about doing the same with the recovery key?

The recovery key is a quite different thing than the master key. The recovery key is a normal "user key" where one person knows the password only. On the other hand the master key is a key, where the server knows the password. This is quite different from a security point of view. If the recovery key would work the same way as the master key, then enabling the recovery key would have the same effects as switching from per-user keys to the master key from a security point of view.
So for people who want this behavior I would suggest to simply use the master key, this has the same effect with less overhead and without making the "user key setup" less secure.

@GitHubUser4234
Copy link
Contributor Author

GitHubUser4234 commented Jan 3, 2017

Thanks @schiessle for the valuable thoughts.

Would be possible in theory. A few things to consider. This would (depending on the number of administrators) add significant overhead. Also every time a (new) admin would enable his recovery key Nextcloud would need to re-encrypt all file-keys which could become a serious performance issue on large systems. Another problem could be to decide which admin/subadmin is allowed to hold a recovery key for which group of users. The initial idea was that the recovery key is really only a fall back which doesn't reduce security. So it is one key which is only accessible to this one responsible and trustworthy person, either one administrator or even someone like the chief data protection officer of the organization.

Definitely getting your point. Yeah, thing is with a growing number of users, incidents like user passwords getting "lost" grow, too. Then, delegating the burden of helping them regain access to their files from the chief data protection officer to the group administrators would be a desirable feature. Well, it can in fact be done now, just the problem is the password needs to be delegated, too :D

A personal recovery password feature would certainly have to be activated application-wide first as it might not always be desirable to allow group administrators to perform a recovery.

Yeah, and possible performance issues would have to be considered then, when considering to enable this feature.

So basically, an enhancement of this kind would be acceptable from your point of view?

The recovery key is a quite different thing than the master key. The recovery key is a normal "user key" where one person knows the password only. On the other hand the master key is a key, where the server knows the password. This is quite different from a security point of view. If the recovery key would work the same way as the master key, then enabling the recovery key would have the same effects as switching from per-user keys to the master key from a security point of view.
So for people who want this behavior I would suggest to simply use the master key, this has the same effect with less overhead and without making the "user key setup" less secure.

I see, so user experience would be the same for encryption with master key and without encryption? Let me discuss this with my project team tomorrow. I would probably re-create this old ownCloud issue in nextCloud and contribute - in case I'd have your mentorship for it :D

@schiessle
Copy link
Member

schiessle commented Jan 3, 2017

So basically, this enhancement would be acceptable from your point of view?

I'm not sure yet regarding the multiple recovery keys for different admins. First I would like to change the recovery key behavior from recovering all file keys of the user to only recovering the users private key. Once this is done we can reconsider the idea of multiple recovery keys, because then also the performance issue should be reduced significantly.

I see, so user experience would be the same for encryption with master key and without encryption?

Exactly, the master key solves both the "password recovery issue" and the problem people have by accessing group shares if new users join a group. It reduces the security but provides a encryption more seamlessly integrated in the users/groups work flow. Since one of the core assumption of Nextclouds server-side encryption is that you have to trust your Nextcloud administrator anyway, the security impact is probably acceptable for many use cases.

I would probably re-create this old ownCloud issue in nextCloud and contribute - in case I'd have your mentorship for it

Having a occ command to replace the master key would be awesome. I'm happy to help if you want to work on it! 😃

@GitHubUser4234
Copy link
Contributor Author

I'm not sure yet regarding the multiple recovery keys for different admins. First I would like to change the recovery key behavior from recovering all file keys of the user to only recovering the users private key. Once this is done we can reconsider the idea of multiple recovery keys, because then also the performance issue should be reduced significantly.

Wow, great that performance can in fact be improved. Is there already an GitHub issue for recovering only the users private key? :D

Having a occ command to replace the master key would be awesome. I'm happy to help if you want to work on it!

Thanks :) There is one more hurdle: to limit the "blast radius" of a potential master key compromise, we would need the master key feature enhanced in a way so that files are not encrypted by only one single master key, but multiple master keys in a partitioning manner. Meaning each of the master keys are used for a specific range of users whereas a simple partition rule would suffice. Similar (though more sophisticated) security guidelines are for example used by Google Cloud Storage. Do you think a feature like this would be an acceptable enhancement in Nextcloud?

@schiessle
Copy link
Member

Wow, great that performance can in fact be improved. Is there already an GitHub issue for recovering only the users private key? :D

Yes, just copied it over from ownCloud #3016

@schiessle
Copy link
Member

schiessle commented Jan 10, 2017

There is one more hurdle: to limit the "blast radius" of a potential master key compromise, we would need the master key feature enhanced in a way so that files are not encrypted by only one single master key, but multiple master keys in a partitioning manner. Meaning each of the master keys are used for a specific range of users whereas a simple partition rule would suffice. Similar (though more sophisticated) security guidelines are for example used by Google Cloud Storage. Do you think a feature like this would be an acceptable enhancement in Nextcloud?

Sounds interesting. I'm just not sure if we should add it to the "default encryption module". I have on my todo list to write a second encryption module which uses only the master key approach and with PHP 7.1 dependency because there we can use a openssl cipher with integrated authentication so we don't need to create signatures manually. Maybe such a feature would fit better into this new encryption module instead of adding more options to the default encryption module.

@Ninos
Copy link

Ninos commented Nov 21, 2017

Hey there, isn't this change merged into nc13? I cannot find the related issue or merge request..

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 20, 2018
@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Jun 12, 2019
@ghost ghost removed the stale Ticket or PR with no recent activity label Jun 12, 2019
@szaimen

This comment has been minimized.

@szaimen szaimen closed this as completed May 21, 2021
@szaimen szaimen reopened this May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants