Proceed encrypt-all after one of the encryption mode is selected #31598
Conversation
Proceed encrypt-all command only after user had selected one of the encryption mode. Signed-off-by: Sujith H <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #31598 +/- ##
============================================
+ Coverage 62.83% 62.83% +<.01%
- Complexity 18369 18371 +2
============================================
Files 1151 1151
Lines 69051 69055 +4
Branches 1260 1260
============================================
+ Hits 43389 43393 +4
Misses 25293 25293
Partials 369 369
Continue to review full report at Codecov.
|
| @@ -106,6 +106,12 @@ protected function execute(InputInterface $input, OutputInterface $output) { | |||
| throw new \Exception('Server side encryption is not enabled'); | |||
| } | |||
|
|
|||
| $masterKeyEnabled = $this->config->getAppValue('encryption', 'useMasterKey', ''); | |||
| $userKeyEnabled = $this->config->getAppValue('encryption', 'userSpecificKey', ''); | |||
| if (($masterKeyEnabled === '') && ($userKeyEnabled === '')) { | |||
There was a problem hiding this comment.
Hm, there is $this->encryptionManager->isEnabled() a few lines above. That already checks encryption_enabled and useMasterKey. Maybe it also needs to check userSpecificKey.
What is the bug you are trying to solve? It seems that when user based encryption was enabled user individual or master key based encryption should have been selected.
So, to properly enable encryption you now need to
occ app:enable encryption
occ encryption:enable # this is a core commend
occ encryption:select-encryption-type # this is an encryption app command
before you can encryption:encryptall ... shouldn't encryption:enable also set the encryption type? Currently, it neither sets useMasterKey, nor userSpecificKey. Presumably, because core shouldn't know anything about the inner workings of the encryption app, such as which encryption type is used. The encryption login hook will initialize user individual keys unless useMasterKey is set, so implicitly user individual keys are used.
-
in the login hook check if
useMasterKeyoruserSpecificKeyare set. If not setuserSpecificKey. That will make user individual keys explicit. do it in a separat PR, as it will change behavior. Currently you can just use user individual keys and then switch to master key based encryption, even if some users use user individual keys. -
add an issue to add an API to explicitly enable an encryption module. That will allow us to call eg
\OC::$server->getEncryptionManager()->enable()to tell an encryption that it has been enabled and it con do things like create eg the master key. We might pass input and output but that remains to be discussed in the new issue. -
the IEncryptionModule API should have an enable(), isEnabled() and disable() method. Maybe add a new interface in a new PR so that existing code can work, but we can call these methods on an app when they are being enabled with
occ app:enable. Best would be ifenable()could take options as a parameter, so the app can configure itself. Otherwise configuration is a separate step before an app can be used. encryption, ldap and user_shibboleth all need configuration before they are really enalbled. -
with that api core must not know about config variables used in apps
core/lib/private/Encryption/Manager.php
Line 91 in 5f0af49
-
We need to check if the documentation contains all three steps to enable the encryption and make sure
userSpecificKeyis set. -
maybe related: https://github.com/owncloud/encryption/blob/9f576ab02af0578265134c1e728b6703f14d5a7d/lib/Command/RecreateMasterKey.php#L197-L202
does not delete theuserSpecificKeyconfig variable. open a new PR if it is not. -
can you add a new issue to add a proper method to the encryption api to check what encryption is used? checking the config bit by bit is tedious and error prone. There are other places where the same functionality is needed:
https://github.com/owncloud/encryption/blob/407c630e03605cf5050a64d6cf3a2b8ac58c7a7d/lib/Command/SelectEncryptionType.php#L91-L95
https://github.com/owncloud/encryption/blob/407c630e03605cf5050a64d6cf3a2b8ac58c7a7d/appinfo/Migrations/Version20170913113840.php#L38-L41
https://github.com/owncloud/encryption/blob/407c630e03605cf5050a64d6cf3a2b8ac58c7a7d/templates/settings-admin.php#L15-L20
https://github.com/owncloud/encryption/blob/407c630e03605cf5050a64d6cf3a2b8ac58c7a7d/lib/Session.php#L63-L65
https://github.com/owncloud/encryption/blob/4992583c5a1f75067a6ccecb54644d39373af47d/templates/settings-admin.php#L15-L20
Arg what a mess ... why did I even look at this ... in light of all this, this PR is good enough.
|
#31597 makes more sense. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Proceed encrypt-all command only after user had selected one
of the encryption mode.
Signed-off-by: Sujith H [email protected]
Description
If user had not selected user-keys or masterkey, then stop execution of encrypt-all command. Throw an exception.
Related Issue
Motivation and Context
If user had not selected user-keys or masterkey, then stop execution of encrypt-all command. Throw an exception.
How Has This Been Tested?
adminScreenshots (if appropriate):
Types of changes
Checklist: