Skip to content

[stable10] Backport of Fix recreate masterkey for hsm#35380

Merged
phil-davis merged 1 commit intostable10from
fix-recreate-masterkey-hsm-stable10
May 30, 2019
Merged

[stable10] Backport of Fix recreate masterkey for hsm#35380
phil-davis merged 1 commit intostable10from
fix-recreate-masterkey-hsm-stable10

Conversation

@sharidas
Copy link
Copy Markdown
Contributor

Fix recreate masterkey for hsm

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

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.

This change also brings usage of Factory for the EncryptAll and DecryptAll. Because the number of arguments to the constructor for the RecreateMasterKey command crossed beyond 16. And that's not something correct. The code is recactored, to bring down the arguments.

Related Issue

Motivation and Context

Fix the RecreateMasterKey command to work with HSM.

How Has This Been Tested?

  • Tested with HSM. And verified that new masterkey is created/generated and the files are encrypted with the newly generated masterkey.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • Backport (if applicable set "backport-request" label and remove when the backport was done)

Fix recreate masterkey for hsm

Signed-off-by: Sujith H <sharidasan@owncloud.com>
@sharidas sharidas added this to the development milestone May 30, 2019
@sharidas sharidas requested a review from jvillafanez May 30, 2019 09:20
@sharidas sharidas self-assigned this May 30, 2019
@sharidas
Copy link
Copy Markdown
Contributor Author

Original PR: owncloud/encryption#128

@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2019

Codecov Report

Merging #35380 into stable10 will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #35380      +/-   ##
==============================================
+ Coverage       64.64%   64.66%   +0.02%     
- Complexity      20093    20100       +7     
==============================================
  Files            1289     1290       +1     
  Lines           76947    76968      +21     
  Branches         1300     1300              
==============================================
+ Hits            49739    49770      +31     
+ Misses          26824    26814      -10     
  Partials          384      384
Flag Coverage Δ Complexity Δ
#javascript 53.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.8% <100%> (+0.02%) 20100 <9> (+7) ⬆️
Impacted Files Coverage Δ Complexity Δ
apps/encryption/lib/Util.php 81.03% <100%> (+2.6%) 17 <1> (+1) ⬆️
apps/encryption/lib/Command/RecreateMasterKey.php 98.24% <100%> (+12.16%) 9 <1> (-1) ⬇️
apps/encryption/lib/Factory/EncDecAllFactory.php 100% <100%> (ø) 5 <5> (?)
apps/encryption/lib/Crypto/EncryptAll.php 93.51% <100%> (+0.15%) 48 <2> (+2) ⬆️

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 655d4aa...9e6fd0b. Read the comment docs.

1 similar comment
@codecov
Copy link
Copy Markdown

codecov bot commented May 30, 2019

Codecov Report

Merging #35380 into stable10 will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff               @@
##             stable10   #35380      +/-   ##
==============================================
+ Coverage       64.64%   64.66%   +0.02%     
- Complexity      20093    20100       +7     
==============================================
  Files            1289     1290       +1     
  Lines           76947    76968      +21     
  Branches         1300     1300              
==============================================
+ Hits            49739    49770      +31     
+ Misses          26824    26814      -10     
  Partials          384      384
Flag Coverage Δ Complexity Δ
#javascript 53.85% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.8% <100%> (+0.02%) 20100 <9> (+7) ⬆️
Impacted Files Coverage Δ Complexity Δ
apps/encryption/lib/Util.php 81.03% <100%> (+2.6%) 17 <1> (+1) ⬆️
apps/encryption/lib/Command/RecreateMasterKey.php 98.24% <100%> (+12.16%) 9 <1> (-1) ⬇️
apps/encryption/lib/Factory/EncDecAllFactory.php 100% <100%> (ø) 5 <5> (?)
apps/encryption/lib/Crypto/EncryptAll.php 93.51% <100%> (+0.15%) 48 <2> (+2) ⬆️

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 655d4aa...9e6fd0b. Read the comment docs.

Copy link
Copy Markdown
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code matches the diffs of the original encryption PR

@phil-davis phil-davis merged commit a5ddc53 into stable10 May 30, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix-recreate-masterkey-hsm-stable10 branch May 30, 2019 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants