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

Improve behavior when editing encrypted devices #959

Merged
merged 2 commits into from
Sep 5, 2019

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Aug 29, 2019

Problem

Editing a block device that already contains an encryption layer has some problems, described at
https://trello.com/c/qeSZSCxQ/1244-5-partitioner-fix-behavior-when-selecting-deselecting-encryption-checkbox

This PR specifically targets the following problems:

  • If the password was already provided, editing again looses it and asks for a new password. That will become a bigger problem soon, since we plan to add more types of encryption (random, pervasive, maybe LUKS2) each of them with their own arguments. Having to re-enter the type and all the arguments would be even a better PITA than it's today.
  • If there is already a existing encryption in the system (e.g. a LUKS2 one created with other tool), it cannot be reused. This will also become a bigger problem soon with pervasive encryption, in which likely the user doesn't want to re-encrypt the device, but simply use the existing one and (re)format the resulting device.

Solution

When encrypting for the first time a device that was not originally encrypted in the system (or encrypting a new device just created using the Partitioner), nothing changes apart from minor adjustments in the labels. The user simply sees a form with an empty "password" field.

enc_new_3

When editing for a second time a device that was already marked for encryption during the current execution of the Partitioner, the form is already prefilled with the password entered before. In the past, the previous encryption layered was ditched (so it's password and other arguments were forgotten) and the user had to define the encryption again from scratch. That will become even more relevant soon, when the form for encryption becomes more than just a "password" field. See #960.

enc_modify_3

Last but not least, when editing a device that is already encrypted in the system (that is, it contains an active encryption layer that was there already when the Partitioner was executed), an option is offered to just use the existing encryption layer instead of replacing it with a (likely more limited) encryption created by the Partitioner.

enc_keep_3

Note the option is only possible if the preexisting encryption device is active (e.g. the user already entered its passphrase during boot or during the initial hardware probing performed by the Partitioner).

Testing

  • Unit test coverage extended
  • Tested manually with the partitioner_testing client
  • Not done yet: manually test a full installation with reused encryption

@coveralls
Copy link

coveralls commented Aug 29, 2019

Coverage Status

Coverage increased (+0.01%) to 97.636% when pulling ded61e0 on ancorgs:refactor_encrypt into 9f359f2 on yast:master.

@ancorgs ancorgs force-pushed the refactor_encrypt branch 3 times, most recently from 5f1a5a8 to f278333 Compare September 3, 2019 17:09
@ancorgs ancorgs changed the title WIP: Extract some code from Controllers::Filesystem WIP: Improve behavior when editing encrypted devices Sep 3, 2019
@ancorgs ancorgs force-pushed the refactor_encrypt branch 3 times, most recently from 6e9b373 to 9277992 Compare September 4, 2019 13:58
Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Although the PR is in draft yet, I left there two minor comments.

src/lib/y2partitioner/dialogs/encryption.rb Outdated Show resolved Hide resolved
test/y2partitioner/actions/controllers/encryption_test.rb Outdated Show resolved Hide resolved
@ancorgs ancorgs force-pushed the refactor_encrypt branch 8 times, most recently from 8f1d395 to 4cb9b44 Compare September 5, 2019 12:06
@ancorgs ancorgs changed the title WIP: Improve behavior when editing encrypted devices Improve behavior when editing encrypted devices Sep 5, 2019
@ancorgs ancorgs force-pushed the refactor_encrypt branch 2 times, most recently from 2d11c39 to 2cdbf4a Compare September 5, 2019 14:17
@ancorgs ancorgs marked this pull request as ready for review September 5, 2019 14:18
Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Despite my two nitpickings, LGTM 👍

test/y2partitioner/dialogs/encryption_test.rb Outdated Show resolved Hide resolved
test/y2partitioner/dialogs/encryption_test.rb Outdated Show resolved Hide resolved
@ancorgs ancorgs merged commit 8d85668 into yast:master Sep 5, 2019
@yast-bot
Copy link

yast-bot commented Sep 5, 2019

✔️ Public Jenkins job #203 successfully finished
✔️ Created OBS submit request #728551

@yast-bot
Copy link

yast-bot commented Sep 5, 2019

✔️ Internal Jenkins job #28 successfully finished
✔️ Created IBS submit request #200273

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants