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.

Sign up for GitHub

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

Jump to bottom

Add persistent disk encryption #2219

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add persistent disk encryption #2219

wants to merge 2 commits into from

Conversation

frelon
Copy link
Contributor

@frelon frelon commented Oct 29, 2024 •
edited
Loading

Add flags to encrypt persistent partition on install:

  • encrypt-persistent: flag to enable luks encryption on persistent partition.
  • enroll-passphrase: string to enroll as passphrase to unlock partition.
  • enroll-key-file: key-file to enroll as key to unlock partition.

During install this will invoke cryptsetup to create the LUKS partition and during mount we use systemd-cryptsetup to attach the partition before mounting the contained filesystem.

This also introduces some changes in the grub configuration, the encrypted_volumes variable can be set in grub_oem_env during install to configure which volumes are actually encrypted.

This change is tested in the new test-encryption target that runs an elemental install with a random passphrase and tests that the persistent partition and any extra-partitions can be unlocked when booted from the recovery system.

In the future the hope is to extend this to include state and oem partition, but that will need more changes to the bootloader configuration (most notably using the cryptomount command)

Part of #1782

@frelon frelon force-pushed the disk-encryption branch 4 times, most recently from 432251c to f830c4e Compare October 29, 2024 13:29
@jmpolom
Copy link

jmpolom commented Nov 5, 2024

Are you considering supporting binding the LUKS volume to the TPM (or FIDO2 device) via systemd-cryptenroll with this? I would recommend either of these options in addition to password and key files.

@frelon
Copy link
Contributor Author

frelon commented Nov 5, 2024

Are you considering supporting binding the LUKS volume to the TPM (or FIDO2 device) via systemd-cryptenroll with this? I would recommend either of these options in addition to password and key files.

Yep, this will be low hanging fruit for a follow up pr! ;)

@frelon
Copy link
Contributor Author

frelon commented Nov 5, 2024 •
edited
Loading

I have been testing this by booting an installer with make prepare-installer-test and then running elemental install /dev/vda --debug --encrypt-persistent --enroll-passphrase <passphrase> and then rebooting.

It should also work to add a install yaml like the following:

install:
  target: /dev/vda
  encrypt-persistent: true
  enroll-passphrase:
    - "passphrase"

This config expands to:

install:
  target: /dev/vda
  partitions:
    persistent:
      encryption:
        name: cr_persistent # full path is /dev/mapper/cr_persistent
        key_slots:
        - slot: 0
          passphrase: "passphrase"

which should also work, but is a bit cumbersome.

@frelon frelon force-pushed the disk-encryption branch 3 times, most recently from 674c1c5 to dd5dbf0 Compare November 12, 2024 14:14
@frelon frelon marked this pull request as ready for review November 12, 2024 14:15
@frelon frelon requested a review from a team as a code owner November 12, 2024 14:15
davidcassany
davidcassany reviewed Nov 13, 2024
View reviewed changes
Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

Looks good to me. This is cool!

I miss unit tests and including the encryption configuration in state file. So we can reset and get the persistent encrypted again as it was configured at a time or eventually re-encrypt with different parameters.

Dockerfile Outdated Show resolved Hide resolved
cmd/install.go
@@ -123,6 +123,7 @@ func NewInstallCmd(root *cobra.Command, addCheckRoot bool) *cobra.Command {
addSharedInstallUpgradeFlags(c)
addLocalImageFlag(c)
addPlatformFlags(c)
addEncryptionFlags(c)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think those would also make sense on reset command.

pkg/features/embedded/grub-default-bootargs/etc/elemental/bootargs.cfg Outdated Show resolved Hide resolved
pkg/features/embedded/grub-default-bootargs/etc/elemental/bootargs.cfg Show resolved Hide resolved
pkg/types/config.go
@@ -261,6 +264,36 @@ func (i *InstallSpec) Sanitize() error {
}
}

// Setup disk encryption
if i.EncryptPersistent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn´t error out if EncryptPersistent is set to true and no secret is provided?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn´t error out if EncryptPersistent is set to true and no secret is provided?

I actually didn't test this edge-case.. I would expect an empty keyslot to allow anything to unlock it, but I will give it a try!

@frelon
Add persistent disk encryption
053c08f 
Add flags to encrypt persistent partition on install:

* encrypt-persistent: flag to enable luks encryption on persistent
  partition.
* enroll-passphrase: string to enroll as passphrase to unlock partition.
* enroll-key-file: key-file to enroll as key to unlock partition.

During install this will invoke cryptsetup to create the LUKS partition
and during mount we use systemd-cryptsetup to attach the partition
before mounting the contained filesystem.

This also introduces some changes in the grub configuration, the
encrypted_volumes variable can be set in grub_oem_env during install to
configure which volumes are actually encrypted.

Using a config-file it is also possible to encrypt any extra-partitions
using the following syntax:

```yaml
install:
  extra-partitions:
    - Name: extra
      size: 100
      fs: ext4
      label: extra
      encryption:
        name: cr_extra
        key_slots:
        - slot: 1
          passphrase: "extrapass"
```

Signed-off-by: Fredrik Lönnegren <[email protected]>
@frelon
Copy link
Contributor Author

frelon commented Nov 14, 2024

Looks good to me. This is cool!

I miss unit tests and including the encryption configuration in state file. So we can reset and get the persistent encrypted again as it was configured at a time or eventually re-encrypt with different parameters.

Good point, I will have an extra look at the reset functionality with this in mind!

@frelon
Minor nit fixes
7069604 
Signed-off-by: Fredrik Lönnegren <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidcassany davidcassany davidcassany left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@frelon @jmpolom @davidcassany