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

Add checkbox to not encrypt /boot in GUI #1938

Open
sezanzeb opened this issue May 1, 2022 · 23 comments
Open

Add checkbox to not encrypt /boot in GUI #1938

sezanzeb opened this issue May 1, 2022 · 23 comments

Comments

@sezanzeb
Copy link

sezanzeb commented May 1, 2022

Is your feature request related to a problem? Please describe.

  1. Not encrypting boot requires knowledge of the partitions that need to be created
  2. People might encrypt their disk and then run into the very user unfriendly non-optimized way of decrypting via grub (see Add an option to not encrypt /boot #1311), not knowing that there is a way to improve that.
  3. I'd like to avoid having to do this configuration every time I set up my computer

Describe the solution you'd like

A checkbox to not encrypt /boot with a tooltip that explains the benefits and risks

Describe alternatives you've considered

Having templates for the manual partitioning could also improve it, but would still be not as user friendly as simply a checkbox to tick.

@adriaandegroot
Copy link
Contributor

Please draw a suggestion -- screenshotted from Calamares, and then edited with Kolourpaint, or MSPaint, or whatever, showing where you think such a checkbox would fit.

@sezanzeb
Copy link
Author

sezanzeb commented May 6, 2022

Maybe like this, hovering it opens a tooltip that says "Not encrypting /boot will allow for faster booting, but poses a security risk"

image


or below it, expanding when ticking the "Encrypt system" checkbox. With whatever text you would think works here

image

@sezanzeb
Copy link
Author

sezanzeb commented May 6, 2022

Other text suggestions:

"Not encrypting /boot makes the master-key decryption more user friendly and faster, but makes the kernel vulnerable for malicious modification if physical access to the device is given"

(I think the kernel is in /boot, right?)

"Not encrypting /boot makes the system decrypt the master key faster, but /boot will be vulnerable if physically accessed"

@GloriousEggroll
Copy link

The following patch is not a toggle, however it does disable /boot and swap from being encrypted, enables btrfs as the default, adds a 1gb ext4 boot partition, configures the remaining space as a btrfs partition, and sets "" for the root subvolume (essentially allowing usage with snapper). This more or less aligns with Fedora's default Anaconda installation using btrfs (+ encryption), and while boot and swap are not encrypted, this achieves the nice plymouth password gui rather than forcing the grub prompt.

The only difference between this and Fedora is that

(1) Fedora creates subvolumes for root (/) and home (/home) by default. Those would look something like this in modules/mount.conf:

btrfsSubvolumes:
     - mountPoint: /
       subvolume: "/@root"
     - mountPoint: /home
       subvolume: "/@home"

(2) Fedora does not apply any swap by default because the Fedora-shipped kernel already enables zswap:

https://fedoraproject.org/wiki/Changes/Scale_ZRAM_to_full_memory_size

Therefor if swap is enabled in the installer, you end up with two swaps, one for zswap, one for the configured swap.

If you don't want to allow swap options you can set these in modules/partition.conf (# means to comment these lines):

#userSwapChoices:
#    - none      # Create no swap, use no swap
#    - small     # Up to 4GB
#    - suspend   # At least main memory size
#    # - reuse     # Re-use existing swap, but don't create any (unsupported right now)
#    - file      # To swap file instead of partition
neverCreateSwap:        true
#initialSwapChoice: none
Patch From 7c4ab9c633232f161de3f43abc95cebbe7a7db52 Mon Sep 17 00:00:00 2001 From: Thomas Crider Date: Mon, 21 Nov 2022 22:46:50 -0500 Subject: [PATCH 3/3] Apply Fedora Btrfs encryption defaults

src/modules/mount/mount.conf | 13 ++-----
.../partition/core/PartitionActions.cpp | 32 +++++------------
.../partition/core/PartitionLayout.cpp | 35 ++++++++++++++-----
src/modules/partition/partition.conf | 17 +++++++--
4 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/src/modules/mount/mount.conf b/src/modules/mount/mount.conf
index 84dca05..8c1db6a 100644
--- a/src/modules/mount/mount.conf
+++ b/src/modules/mount/mount.conf
@@ -52,14 +52,5 @@ extraMountsEfi:

for root.

btrfsSubvolumes:

    • mountPoint: /
  •  subvolume: /@
    
  •  # As an alternative:
    
  •  #
    
  •  # subvolume: ""
    
    • mountPoint: /home
  •  subvolume: /@home
    
    • mountPoint: /var/cache
  •  subvolume: /@cache
    
    • mountPoint: /var/log
  •  subvolume: /@log
    
  • - mountPoint: /
    
  •   subvolume: ""
    

diff --git a/src/modules/partition/core/PartitionActions.cpp b/src/modules/partition/core/PartitionActions.cpp
index 0ce9ff4..47aedff 100644
--- a/src/modules/partition/core/PartitionActions.cpp
+++ b/src/modules/partition/core/PartitionActions.cpp
@@ -173,29 +173,15 @@ doAutopartition( PartitionCoreModule* core, Device* dev, Choices::AutoPartitionO
if ( shouldCreateSwap )
{
Partition* swapPartition = nullptr;

  •    if ( o.luksPassphrase.isEmpty() )
    
  •    {
    
  •        swapPartition = KPMHelpers::createNewPartition( dev->partitionTable(),
    
  •                                                        *dev,
    
  •                                                        PartitionRole( PartitionRole::Primary ),
    
  •                                                        FileSystem::LinuxSwap,
    
  •                                                        QStringLiteral( "swap" ),
    
  •                                                        lastSectorForRoot + 1,
    
  •                                                        dev->totalLogical() - 1,
    
  •                                                        KPM_PARTITION_FLAG( None ) );
    
  •    }
    
  •    else
    
  •    {
    
  •        swapPartition = KPMHelpers::createNewEncryptedPartition( dev->partitionTable(),
    
  •                                                                 *dev,
    
  •                                                                 PartitionRole( PartitionRole::Primary ),
    
  •                                                                 FileSystem::LinuxSwap,
    
  •                                                                 QStringLiteral( "swap" ),
    
  •                                                                 lastSectorForRoot + 1,
    
  •                                                                 dev->totalLogical() - 1,
    
  •                                                                 o.luksPassphrase,
    
  •                                                                 KPM_PARTITION_FLAG( None ) );
    
  •    }
    
  •    swapPartition = KPMHelpers::createNewPartition( dev->partitionTable(),
    
  •                                                    *dev,
    
  •                                                    PartitionRole( PartitionRole::Primary ),
    
  •                                                    FileSystem::LinuxSwap,
    
  •                                                    QStringLiteral( "swap" ),
    
  •                                                    lastSectorForRoot + 1,
    
  •                                                    dev->totalLogical() - 1,
    
  •                                                    KPM_PARTITION_FLAG( None ) );
    
  •    PartitionInfo::setFormat( swapPartition, true );
       if ( gs->contains( "swapPartitionName" ) )
       {
    

diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp
index 765d9ff..1bfa64f 100644
--- a/src/modules/partition/core/PartitionLayout.cpp
+++ b/src/modules/partition/core/PartitionLayout.cpp
@@ -314,15 +314,32 @@ PartitionLayout::createPartitions( Device* dev,
}
else
{

  •        part = KPMHelpers::createNewEncryptedPartition( parent,
    
  •                                                        *dev,
    
  •                                                        role,
    
  •                                                        correctFS( entry.partFileSystem ),
    
  •                                                        entry.partLabel,
    
  •                                                        currentSector,
    
  •                                                        currentSector + sectors - 1,
    
  •                                                        luksPassphrase,
    
  •                                                        KPM_PARTITION_FLAG( None ) );
    
  •        if ( !luksPassphrase.isEmpty() )
    
  •        {
    
  •        	if ( entry.partMountPoint == "/boot" || correctFS( entry.partFileSystem ) == FileSystem::LinuxSwap)
    
  •        	{
    
  •        	    part = KPMHelpers::createNewPartition( parent,
    
  •                                                       *dev,
    
  •                                                       role,
    
  •                                                       correctFS( entry.partFileSystem ),
    
  •                                                       entry.partLabel,
    
  •                                                       currentSector,
    
  •                                                       currentSector + sectors - 1,
    
  •                                                       KPM_PARTITION_FLAG( None ) );
    
  •        	}
    
  •        	else
    
  •        	{
    
  •                part = KPMHelpers::createNewEncryptedPartition( parent,
    
  •                                                                *dev,
    
  •                                                                role,
    
  •                                                                correctFS( entry.partFileSystem ),
    
  •                                                                entry.partLabel,
    
  •                                                                currentSector,
    
  •                                                                currentSector + sectors - 1,
    
  •                                                                luksPassphrase,
    
  •                                                                KPM_PARTITION_FLAG( None ) );
    
  •        	}
    
  •        }
       }
    
       // For zfs, we need to make the passphrase available to later modules
    

diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf
index e29c507..02f37a8 100644
--- a/src/modules/partition/partition.conf
+++ b/src/modules/partition/partition.conf
@@ -16,7 +16,7 @@ efiSystemPartition: "/boot/efi"

300MiB is the default, M is treated as MiB, and if you really want

one-million (10^6) bytes, use MB.

-# efiSystemPartitionSize: 300M
+efiSystemPartitionSize: 600M

This optional setting specifies the name of the EFI system partition (see

PARTLABEL; gpt only; requires KPMCore >= 4.2.0).

@@ -65,7 +65,7 @@ efiSystemPartition: "/boot/efi"
neverCreateSwap: true

Correctly draw nested (e.g. logical) partitions as such.

-drawNestedPartitions: false
+drawNestedPartitions: true

Show/hide partition labels on manual partitioning page.

alwaysShowPartitionLabels: true
@@ -142,7 +142,7 @@ initialPartitioningChoice: none

If nothing is specified, Calamares defaults to "ext4".

Names are case-sensitive and defined by KPMCore.

-defaultFileSystemType: "ext4"
+defaultFileSystemType: "btrfs"

Selectable filesystem type, used when "erase" is done.

@@ -253,3 +253,14 @@ defaultFileSystemType: "ext4"

the welcome module, and configure this value in

welcome.conf, not here.

requiredStorage: 3.5

+partitionLayout:

    • name: ""
  •  filesystem: "ext4"
    
  •  mountPoint: "/boot"
    
  •  size: 1G
    
    • name: ""
  •  filesystem: "btrfs"
    
  •  mountPoint: "/"
    
  •  size: 100%
    

--
2.38.1

Hopefully it should be easy for someone to wire up a checkbox to toggle these patch changes (mainly the patches in PartitionActions.cpp and PartitionLayout.cpp are what need toggling)

@ArrayBolt3
Copy link
Contributor

This would be an extremely useful feature for Lubuntu and Kubuntu, especially if we could set a configuration option in the distro config that just enforced that /boot is always unencrypted. Lubuntu has been warned by someone from Canonical that the use of encrypted /boot uses code paths in GRUB that are unsupported and could be a security risk.

I'll probably help add this feature if possible, is this something the Calamares devs are still interested in having if someone's willing to put in the elbow grease to make it happen?

@dalto8
Copy link
Contributor

dalto8 commented Feb 2, 2024

Contributions are always welcome. I think the question would be how to do it in an elegant way.

I guess if it is about Erase disk, Replace Partition or Install Alongside it could be done by adding an element in partitionLayout that described if encryption was allowed.

Manual partitioning I guess we could disable the encryption checkbox with a message explaining why it was disabled?

That would allow the distro to choose if /boot should be unencrypted or not.

It would still require the user to use manual partitioning if the distro allowed it but they didn't want it encrypted but I think that is OK.

@ArrayBolt3
Copy link
Contributor

IMO one good way to do it might be to use the same UI as the second screenshot @sezanzeb posted. But then additionally, in partition.conf perhaps, have an option for luksEncryptBoot that can be set to true, false, or choose (to give some possible example values). true = always encrypt /boot, false = never encrypt /boot, both of those would make the "encrypt /boot" checkbox go away and only the "encrypt system" one would exist. choose would leave it up to the user, and that would cause the "encrypt /boot" checkbox to be shown.

@dalto8
Copy link
Contributor

dalto8 commented Feb 2, 2024

IMO one good way to do it might be to use the same UI as the second screenshot @sezanzeb posted.

Honestly, I think that is too confusing for most users. The average user will not know what to choose there. Also, encrypting /boot or not encrypting it has a bunch of consequences. I think it is better left to manual partitioning if it is a user choice.

On the other hand, if it is a distro choice, that makes it simpler and there is no reason to offer an additional option there.

@ArrayBolt3
Copy link
Contributor

I like that idea a lot. It's already possible to set up unencrypted /boot via manual partitioning, so just adding the distro option should be sufficient. I'll work on getting that added.

Any advice on what the option's name should be and what module it belongs in? I liked the idea of luksEncryptBoot in partition.conf but would like to put it in the right place first time so that it can be accepted easily.

@dalto8
Copy link
Contributor

dalto8 commented Feb 4, 2024

I don't think it needs to be limited to /boot.

In partition.conf, if you put an additional option inside partitionLayout than any partition could be marked as noEncrypt

@ArrayBolt3
Copy link
Contributor

ArrayBolt3 commented Feb 4, 2024

Just to be sure (I can find this out myself but I figure you already know the answer to this), the current default behavior of Calamares is to encrypt all partitions listed in partitionLayout except for /boot/efi if present, correct?

@dalto8
Copy link
Contributor

dalto8 commented Feb 4, 2024

It excludes the ESP, mounted at efi.mountPoint

@ArrayBolt3
Copy link
Contributor

Perfect. If all goes according to plan, I'll probably have a PR tomorrow with a noEncrypt option added. Thanks for all your help!

@ArrayBolt3
Copy link
Contributor

Had a ton of work today and didn't manage to hardly get any time to work on this, but I made some progress. Going to try again tomorrow I guess.

@ArrayBolt3
Copy link
Contributor

Another aspect of this issue I didn't think of until trying to implement this is, if a distro specifies a custom partition layout using partiitonLayout, that layout is going to apply whether LUKS is enabled or not. This creates the slightly awkward scenario of having installs with a separate /boot partition even when non-encrypted, which is strange in the Ubuntu world and possibly in other distros. The easiest way I can think of to work around this is to add support for specifying an entirely separate partition layout to use when LUKS is enabled (this would allow configuring for behavior similar to Ubuntu's Ubiquity and Ubuntu Desktop Installer). This is another feature suggestion that I'd be intending to implement in the PR if this sounds like a good idea. Perhaps if there are other similar conditions that might trigger the need for an alternate partition layout, this mechanism could be made to work in a general way for future reuse?

@dalto8
Copy link
Contributor

dalto8 commented Feb 5, 2024

Another aspect of this issue I didn't think of until trying to implement this is, if a distro specifies a custom partition layout using partiitonLayout, that layout is going to apply whether LUKS is enabled or not. This creates the slightly awkward scenario of having installs with a separate /boot partition even when non-encrypted, which is strange in the Ubuntu world and possibly in other distros.

Does Ubuntu have a separate /boot only when luks is in play? I don't think there are a lot of distros like that. I think most either have a separate /boot or don't. Some achieve this by mounting the ESP on /boot. Others, like Fedora, by always having a separate /boot.

Either way, I would probably treat that as a separate problem. That may be a bit tricky to deal with since luks isn't the only supported encryption method. We also have zfs encryption supported.

@ArrayBolt3
Copy link
Contributor

ArrayBolt3 commented Feb 5, 2024 via email

@dalto8
Copy link
Contributor

dalto8 commented Feb 5, 2024

This raises a second question - does Calamares try to create all partitions specified in the layout when doing an "install alongside" operation?

Yes. It uses the same layout for alongside, entire disk and replace partition.

I can see that having strange effects for multi-boot system users since they'll end up with a slew of extra /boot partitions.

Is there another option here? You can't really share /boot between installs.

I don't see how that would conflict with having the "Encrypt system" checkbox also switch to a second partition layout that the distro uses only on encrypted systems.

The challenge is that since zfs is different than luks you might not want to use the luks layout.

@ArrayBolt3
Copy link
Contributor

Is there another option here? You can't really share /boot between installs.

Right, thus why it might be nice to have different layout types. That being said, it might not be the end of the world. I'll bring it up to the teams I'm a part of and see what they think.

The challenge is that since zfs is different than luks you might not want to use the luks layout.

On top of that you also have the possibility of people mixing ZFS and non-ZFS partitions on the same disk potentially (though hopefully that would be rare).

I think for now I'll just submit the "make encryption optional in a partition layout" patch and then possibly do a downstream patch for adjusting partition layouts based on whether encryption is enabled or not, if that's even desirable (which it might not be).

@ArrayBolt3
Copy link
Contributor

ArrayBolt3 commented Feb 6, 2024 via email

@dalto8
Copy link
Contributor

dalto8 commented Feb 6, 2024

The point of an alternate partition layout would be so that if the unreadable nature of an encrypted partition messed with things, that can be worked around.

But what causes problems with luks and what causes problems with zfs aren't the same.

The main reason I want something like this is because "Replace a partition" will behave poorly if a /boot partition is always made and the user is in the habit of reinstalling their system using it.

But that has nothing to do with encryption, right? You can encrypt in replace partition as well.

From a distro perspective, it seems very strange to me that the result of the installation would be different if you choose replace vs entire disk. I don't think most users would be expecting that they wouldn't get a /boot partition because they chose the replace option.

In Lubuntu, we have at least one prominent tester who frequently "refreshes" their machines by using the "replace partition" feature on an existing Lubuntu partition. If we make an extra /boot partition every time, then an extra /boot partition will be made on every reinstall, meaning that after, say, five reinstalls, you'd have four obsolete /boot partitions and only one useful one.

This seems like a really narrow use case. Couldn't you educate that person to delete the old partitions first?

What about all the people who use replace partition in situation other than re-installing like replacing another distro or OS?

Also, if you aren't worried about not having a /boot in that case, why add it at all?

@ArrayBolt3
Copy link
Contributor

ArrayBolt3 commented Feb 6, 2024

But what causes problems with luks and what causes problems with zfs aren't the same.

Here your experience is greater than mine so I'll defer to you on that one.

But that has nothing to do with encryption, right? You can encrypt in replace partition as well.

True, but... meh. There's only so far one can go. If someone's taking advantage of Calamares' ability to do a "clean install" that keeps user data and applications (which is what this tester oftentimes does), they're probably doing an unencrypted install over an unencrypted install. If the original install was encrypted, Calamares couldn't do a "refresh" operation in this way.

From a distro perspective, it seems very strange to me that the result of the installation would be different if you choose replace vs entire disk. I don't think most users would be expecting that they wouldn't get a /boot partition because they chose the replace option.

Totally agree here. Thus why I'm making it the difference between encrypted vs. unencrypted.

This seems like a really narrow use case. Couldn't you educate that person to delete the old partitions first?

I guess I could, but it seems like the ability to replace a partition while keeping user data and applications is a use case Calamares explicitly supports, so I don't see why its narrowness should be an issue. (Or perhaps this one tester is doing something other than what I think he's doing and Calamares doesn't support this at all.)

Also, if you aren't worried about not having a /boot in that case, why add it at all?

That circles back around to the original problem - /boot has to be separate to allow it to be unencrypted, and in some distros /boot must remain unencrypted for (ironically) security reasons (not to mention that Plymouth has a 10x better disk decryption UI than GRUB does). I explicitly don't mind the creation of extra /boot partitions when doing a replace operation with encryption enabled. It's when encryption is disabled that the extra /boot partition when doing a replace operation is frustrating.

In any event, it sounds like this is probably going to have to be a downstream patch, which I'm happy with.

@ArrayBolt3
Copy link
Contributor

I'm realizing possibly part of the problem here is that my idea of being able to specify a secondary partition layout is way too complex (taking a look at the code also made that plan look like a bad idea).

The ability to enable encryption or not is a binary decision anyway, so... what if I added another option, onlyPresentWithEncryption or something like that? That could be used for specifying that a partition was only to be created if encryption was enabled.

If ZFS could cause complications with that solution also, it would be helpful to know what it might look like if a problem like that arose, since I may be able to work around those issues if I know what they are.

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

No branches or pull requests

6 participants