Fix recreate masterkey for hsm

[sharidas]

Fix recreate masterkey for hsm

Signed-off-by: Sujith H sharidasan@owncloud.com

[sharidas]

Description

When masterkey is enabled with hsm, the recreate mastekey command was failing. The reason for this was Crypt was pointing to wrong class. Meaning instead of CryptHSM, it was pointing to Crypt. This caused the keys to be generated in the oC instance and hence things were not working. Once the app is re-enabled, the keymanager requires to be pointed to CryptHSM if hsm is enabled. The switch to the Crypt is decided here https://github.com/owncloud/encryption/blob/master/lib/AppInfo/Application.php#L136-L149. This pr tries to address this issue.

Issue

#125

Test done

• Enable encryption with hsm
• Login as admin user
• Verify the OC_DEFAULT_MODULE as shown below:

 sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ls -lt data/files_encryption/OC_DEFAULT_MODULE
total 16
-rw-r--r-- 1 sujith sujith 189 May 27 12:35 master_4d9273b1.privateKey
-rw-r--r-- 1 sujith sujith 451 May 27 12:35 master_4d9273b1.publicKey
-rw-rw-r-- 1 sujith sujith 189 May 27 12:35 pubShare_4d9273b1.privateKey
-rw-rw-r-- 1 sujith sujith 451 May 27 12:35 pubShare_4d9273b1.publicKey
 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

• Execute the recreate master key command:

✘ sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ./occ encryption:recreate-master-key -y
Cannot load Xdebug - it was already loaded
Decryption started

    1 [->--------------------------]prepare encryption modules...
 done.


 decrypt files for user admin (1 of 1): /admin/files/welcome.txt 
 [-->-------------------------]

 starting to decrypt files... finished 
 [============================]


all files could be decrypted successfully!
    1 [============================]
Decryption completed

Encryption started

Waiting for creating new masterkey

New masterkey created successfully



Encrypt all files with the Default encryption module
====================================================


Use master key to encrypt all files.


Start to encrypt users files
----------------------------



 all files encrypted 
 [============================]


Encryption completed successfully


Note: All users are required to relogin.

 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

• Verify the keys have been updated

sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ls -lt data/files_encryption/OC_DEFAULT_MODULE
total 16
-rw-rw-r-- 1 sujith sujith 189 May 27 12:36 master_51aea79b.privateKey
-rw-rw-r-- 1 sujith sujith 451 May 27 12:36 master_51aea79b.publicKey
-rw-rw-r-- 1 sujith sujith 189 May 27 12:36 pubShare_51aea79b.privateKey
-rw-rw-r-- 1 sujith sujith 451 May 27 12:36 pubShare_51aea79b.publicKey
 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

• Verified that files are accessible
• Re-run the recreate masterkey command

 sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ./occ encryption:recreate-master-key -y
Cannot load Xdebug - it was already loaded
Decryption started

    1 [->--------------------------]prepare encryption modules...
 done.


 decrypt files for user admin (1 of 1): /admin/files/welcome.txt 
 [-->-------------------------]

 starting to decrypt files... finished 
 [============================]


all files could be decrypted successfully!
    1 [============================]
Decryption completed

Encryption started

Waiting for creating new masterkey

New masterkey created successfully



Encrypt all files with the Default encryption module
====================================================


Use master key to encrypt all files.


Start to encrypt users files
----------------------------



 all files encrypted 
 [============================]


Encryption completed successfully


Note: All users are required to relogin.

 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

• Verified the file welcome.txt was accesible
• Verified the new keys have been generated:

 sujith@sujith-ownCloud  ~/test/owncloud3   master ●  ls -lt data/files_encryption/OC_DEFAULT_MODULE
total 16
-rw-rw-r-- 1 sujith sujith 189 May 27 12:36 master_293b1f5e.privateKey
-rw-rw-r-- 1 sujith sujith 451 May 27 12:36 master_293b1f5e.publicKey
-rw-rw-r-- 1 sujith sujith 189 May 27 12:36 pubShare_293b1f5e.privateKey
-rw-rw-r-- 1 sujith sujith 451 May 27 12:36 pubShare_293b1f5e.publicKey
 sujith@sujith-ownCloud  ~/test/owncloud3   master ● 

[jvillafanez]

There are several things to fix in the class, indirectly related with the work in this PR. I don't know if we can fix them in the PR or not.

• The commands depends on 16 components. This is bad, specially when only 6-8 are directly used by this class. The rest of the components are being passed through. This has several related consequences:
  • The command creates several components by itself. Those components won't be mocked, so unit tests are impossible to do. The tests currently made are more in the direction of integration tests.
  • The more components the command relies on, the more probable is that a bug happens. In addition, even if the command itself is simple (in the sense of "I just call this method is we're done"), having more components raises the complexity unnecessarily
• The command overwrites one of the injected components. While the command can be easy to follow, in general we can expect to use the same dependency across the class. It's quite easy to have surprises when you're injecting an object but then you're using a different object.

I'd need to dig into the code to come up with refactor plan, and I'm not sure how much time it could take. For now, I'd recommend to check the possiblity of using at least one factory class to take care of whatever the command needs to create

[codecov]

Codecov Report

Merging #128 into master will increase coverage by 0.87%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master     #128      +/-   ##
============================================
+ Coverage     69.79%   70.67%   +0.87%     
- Complexity      620      627       +7     
============================================
  Files            34       35       +1     
  Lines          2291     2312      +21     
============================================
+ Hits           1599     1634      +35     
+ Misses          692      678      -14

Impacted Files	Coverage Δ	Complexity Δ
lib/Util.php	81.03% <100%> (+2.6%)	17 <1> (+1)	⬆️
lib/Command/RecreateMasterKey.php	98.24% <100%> (+12.16%)	9 <1> (-1)	⬇️
lib/Crypto/EncryptAll.php	52.77% <100%> (+1.11%)	48 <2> (+2)	⬆️
lib/Factory/EncDecAllFactory.php	100% <100%> (ø)	5 <5> (?)
lib/KeyManager.php	78.77% <0%> (+1.63%)	83% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e68099...b41566c. Read the comment docs.

[codecov]

Codecov Report

Merging #128 into master will increase coverage by 0.47%.
The diff coverage is 83.92%.

@@             Coverage Diff              @@
##             master     #128      +/-   ##
============================================
+ Coverage     69.79%   70.27%   +0.47%     
- Complexity      620      627       +7     
============================================
  Files            34       35       +1     
  Lines          2291     2311      +20     
============================================
+ Hits           1599     1624      +25     
+ Misses          692      687       -5

Impacted Files	Coverage Δ	Complexity Δ
lib/Crypto/EncryptAll.php	49.77% <0%> (-1.89%)	48 <2> (+2)
lib/Util.php	81.03% <100%> (+2.6%)	17 <1> (+1)	⬆️
lib/Factory/EncDecAllFactory.php	100% <100%> (ø)	5 <5> (?)
lib/Command/RecreateMasterKey.php	96.29% <90.9%> (+10.22%)	9 <1> (-1)	⬇️
lib/KeyManager.php	78.77% <0%> (+1.63%)	83% <0%> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f18bdc...84db7bc. Read the comment docs.

[sharidas]

Backport PR owncloud/core#35380