diff mbox series

[isar-cip-core] initramfs-crypt-hook: Optimize disk encryption in combination with expansion of last partition

Message ID 20250120123425.79292-1-alexander.heinisch@siemens.com (mailing list archive)
State New
Headers show
Series [isar-cip-core] initramfs-crypt-hook: Optimize disk encryption in combination with expansion of last partition | expand

Commit Message

Heinisch, Alexander Jan. 20, 2025, 12:34 p.m. UTC 
From: Alexander Heinisch <alexander.heinisch@siemens.com>

In the current implementation (since: 284175c3) disk expansion logic
is done before the actual disk encryption.

This results in reencrypting the full expanded size of the disk, instead
of only reencrypting the sections containing the actual data. This results
in a drastic increase of time (approx 20x on our systems ) needed for reencryption!

Since relevant disk contents are only available in the sections of the disk
available before the expansion, it is sufficient to reencrypt them only.

This patch changes order of encryption and disk expansion resulting in
faster encryption (since only the necessary parts are encrypted).
The resize operation does not reencrypt the added disk parts but just
extends the LUKS container (cryptsetup resize) to reflect the maximum
available disk space.

Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
---
 .../files/local-top-complete                  | 37 +++++++++++++++++--
 1 file changed, 33 insertions(+), 4 deletions(-)

Comments

Heinisch, Alexander Jan. 20, 2025, 12:51 p.m. UTC | #1 
On Mon, 2025-01-20 at 13:34 +0100, alexander.heinisch@siemens.com
wrote:
> From: Alexander Heinisch <alexander.heinisch@siemens.com>
> 
> In the current implementation (since: 284175c3) disk expansion logic
> is done before the actual disk encryption.
> 
> This results in reencrypting the full expanded size of the disk,
> instead
> of only reencrypting the sections containing the actual data. This
> results
> in a drastic increase of time (approx 20x on our systems ) needed for
> reencryption!
> 
> Since relevant disk contents are only available in the sections of
> the disk
> available before the expansion, it is sufficient to reencrypt them
> only.
> 
> This patch changes order of encryption and disk expansion resulting
> in
> faster encryption (since only the necessary parts are encrypted).
> The resize operation does not reencrypt the added disk parts but just
> extends the LUKS container (cryptsetup resize) to reflect the maximum
> available disk space.

The thing I am not sure about this patch is, if executing the disk
expansion here is really the way to go, or if we shall just reuse the
expand-on-first-boot from isar (incl. fixes proposed earlier - [1])?


[1]:https://github.com/ilbers/isar/commit/8b30a4f86cb3ea3369bff3884141872c3a7d9979

BR Alexander

> 
> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
> ---
>  .../files/local-top-complete                  | 37
> +++++++++++++++++--
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-
> complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-
> complete
> index 8adc4e5..c4135c3 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> @@ -153,6 +153,35 @@ expand_partition() {
>         # Inform the kernel about the partitioning change
>         partx -u "${last_part}"
>  
> +       last_part_device_name=${last_part#\/dev/}
> +
> +       mount_point=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX")
> +
> +       mapping_name=$(cat
> /sys/class/block/"$last_part_device_name"/holders/*/dm/name)
> +       cryptsetup resize "$mapping_name"
> +       mount /dev/mapper/"$mapping_name" "${mount_point}"
> +       fs_type=$(findmnt -fno FSTYPE "${mount_point}" )
> +       last_part=/dev/mapper/"$mapping_name"
> +
> +       case ${fs_type} in
> +       ext*)
> +
> +               # Do not fail resize2fs if no mtab entry is found,
> e.g.,
> +               # when using systemd mount units.
> +               export EXT2FS_NO_MTAB_OK=1
> +               resize2fs "${last_part}"
> +               ;;
> +       btrfs)
> +               btrfs filesystem resize max "${mount_point}"
> +               ;;
> +       *)
> +               log_warning_msg "Unrecognized filesystem type
> ${fs_type} - no resize performed"
> +               ;;
> +       esac
> +
> +       umount "${mount_point}"
> +       rmdir "${mount_point}"
> +
>         log_end_msg
>  }
>  
> @@ -215,10 +244,6 @@ for partition_set in $partition_sets; do
>                 echo "ROOT=$decrypted_part" >/conf/param.conf
>         fi
>  
> -       if [ "$partition_expand" = "expand" ]; then
> -               expand_partition $part_device
> -       fi
> -
>         if /usr/sbin/cryptsetup luksDump --batch-mode "$part_device"
> \
>                         | grep -q "luks2"; then
>                 open_tpm2_partition "$part_device"
> "$crypt_mount_name" "$tpm_device"
> @@ -259,6 +284,10 @@ for partition_set in $partition_sets; do
>         esac
>  
>         finalize_tpm2_encryption "$part_device"
> +
> +       if [ "$partition_expand" = "expand" ]; then
> +               expand_partition $part_device
> +       fi
>  done
>  
>  if [ -n "$watchdog_pid" ]; then
Jan Kiszka Jan. 20, 2025, 1:51 p.m. UTC | #2 
On 20.01.25 13:51, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
> On Mon, 2025-01-20 at 13:34 +0100, alexander.heinisch@siemens.com
> wrote:
>> From: Alexander Heinisch <alexander.heinisch@siemens.com>
>>
>> In the current implementation (since: 284175c3) disk expansion logic
>> is done before the actual disk encryption.
>>
>> This results in reencrypting the full expanded size of the disk,
>> instead
>> of only reencrypting the sections containing the actual data. This
>> results
>> in a drastic increase of time (approx 20x on our systems ) needed for
>> reencryption!
>>
>> Since relevant disk contents are only available in the sections of
>> the disk
>> available before the expansion, it is sufficient to reencrypt them
>> only.
>>
>> This patch changes order of encryption and disk expansion resulting
>> in
>> faster encryption (since only the necessary parts are encrypted).
>> The resize operation does not reencrypt the added disk parts but just
>> extends the LUKS container (cryptsetup resize) to reflect the maximum
>> available disk space.
> 
> The thing I am not sure about this patch is, if executing the disk
> expansion here is really the way to go, or if we shall just reuse the
> expand-on-first-boot from isar (incl. fixes proposed earlier - [1])?
> 

If you find a good way to factor out the common code an reuse that
inside the initramfs, I'm open for patches. However, as we discussed on
the isar list as well, using the systemd service later on will no longer
work once we properly hide the keys when initramfs ends.

Jan
Heinisch, Alexander Jan. 20, 2025, 2:09 p.m. UTC | #3 
On Mon, 2025-01-20 at 14:51 +0100, Jan Kiszka wrote:
> On 20.01.25 13:51, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
> > On Mon, 2025-01-20 at 13:34 +0100, alexander.heinisch@siemens.com
> > wrote:
> > > From: Alexander Heinisch <alexander.heinisch@siemens.com>
> > > 
> > > In the current implementation (since: 284175c3) disk expansion
> > > logic
> > > is done before the actual disk encryption.
> > > 
> > > This results in reencrypting the full expanded size of the disk,
> > > instead
> > > of only reencrypting the sections containing the actual data.
> > > This
> > > results
> > > in a drastic increase of time (approx 20x on our systems ) needed
> > > for
> > > reencryption!
> > > 
> > > Since relevant disk contents are only available in the sections
> > > of
> > > the disk
> > > available before the expansion, it is sufficient to reencrypt
> > > them
> > > only.
> > > 
> > > This patch changes order of encryption and disk expansion
> > > resulting
> > > in
> > > faster encryption (since only the necessary parts are encrypted).
> > > The resize operation does not reencrypt the added disk parts but
> > > just
> > > extends the LUKS container (cryptsetup resize) to reflect the
> > > maximum
> > > available disk space.
> > 
> > The thing I am not sure about this patch is, if executing the disk
> > expansion here is really the way to go, or if we shall just reuse
> > the
> > expand-on-first-boot from isar (incl. fixes proposed earlier -
> > [1])?
> > 
> 
> If you find a good way to factor out the common code an reuse that
> inside the initramfs, I'm open for patches. However, as we discussed
> on
> the isar list as well, using the systemd service later on will no
> longer
> work once we properly hide the keys when initramfs ends.
> 

True!

Refactoring that part out, probably introduces more boilerplate, than
the actual refactoring will save. Anyhow, once the new sealing is in
place the systemd service support for crypt has to be
reconsidered/dropped anyways.
Will keep it as proposed in the patch.

BR Alexander

> Jan
>
Jan Kiszka Jan. 20, 2025, 3:10 p.m. UTC | #4 
On 20.01.25 15:09, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
> On Mon, 2025-01-20 at 14:51 +0100, Jan Kiszka wrote:
>> On 20.01.25 13:51, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
>>> On Mon, 2025-01-20 at 13:34 +0100, alexander.heinisch@siemens.com
>>> wrote:
>>>> From: Alexander Heinisch <alexander.heinisch@siemens.com>
>>>>
>>>> In the current implementation (since: 284175c3) disk expansion
>>>> logic
>>>> is done before the actual disk encryption.
>>>>
>>>> This results in reencrypting the full expanded size of the disk,
>>>> instead
>>>> of only reencrypting the sections containing the actual data.
>>>> This
>>>> results
>>>> in a drastic increase of time (approx 20x on our systems ) needed
>>>> for
>>>> reencryption!
>>>>
>>>> Since relevant disk contents are only available in the sections
>>>> of
>>>> the disk
>>>> available before the expansion, it is sufficient to reencrypt
>>>> them
>>>> only.
>>>>
>>>> This patch changes order of encryption and disk expansion
>>>> resulting
>>>> in
>>>> faster encryption (since only the necessary parts are encrypted).
>>>> The resize operation does not reencrypt the added disk parts but
>>>> just
>>>> extends the LUKS container (cryptsetup resize) to reflect the
>>>> maximum
>>>> available disk space.
>>>
>>> The thing I am not sure about this patch is, if executing the disk
>>> expansion here is really the way to go, or if we shall just reuse
>>> the
>>> expand-on-first-boot from isar (incl. fixes proposed earlier -
>>> [1])?
>>>
>>
>> If you find a good way to factor out the common code an reuse that
>> inside the initramfs, I'm open for patches. However, as we discussed
>> on
>> the isar list as well, using the systemd service later on will no
>> longer
>> work once we properly hide the keys when initramfs ends.
>>
> 
> True!
> 
> Refactoring that part out, probably introduces more boilerplate, than
> the actual refactoring will save. Anyhow, once the new sealing is in
> place the systemd service support for crypt has to be
> reconsidered/dropped anyways.

That was already done:

https://github.com/ilbers/isar/commit/004f9935102f81b18a54fc9c0940d23b84be411a

The isar service is for unencrypted partitions only, this one here is 
for fully integrated encrypted ones.

Jan
Heinisch, Alexander Jan. 20, 2025, 3:22 p.m. UTC | #5 
On Mon, 2025-01-20 at 16:10 +0100, Jan Kiszka wrote:
> On 20.01.25 15:09, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
> > On Mon, 2025-01-20 at 14:51 +0100, Jan Kiszka wrote:
> > > On 20.01.25 13:51, Heinisch, Alexander (FT RPD CED SES-AT) wrote:
> > > > On Mon, 2025-01-20 at 13:34 +0100,
> > > > alexander.heinisch@siemens.com
> > > > wrote:
> > > > > From: Alexander Heinisch <alexander.heinisch@siemens.com>
> > > > >
> > > > > In the current implementation (since: 284175c3) disk
> > > > > expansion
> > > > > logic
> > > > > is done before the actual disk encryption.
> > > > >
> > > > > This results in reencrypting the full expanded size of the
> > > > > disk,
> > > > > instead
> > > > > of only reencrypting the sections containing the actual data.
> > > > > This
> > > > > results
> > > > > in a drastic increase of time (approx 20x on our systems )
> > > > > needed
> > > > > for
> > > > > reencryption!
> > > > >
> > > > > Since relevant disk contents are only available in the
> > > > > sections
> > > > > of
> > > > > the disk
> > > > > available before the expansion, it is sufficient to reencrypt
> > > > > them
> > > > > only.
> > > > >
> > > > > This patch changes order of encryption and disk expansion
> > > > > resulting
> > > > > in
> > > > > faster encryption (since only the necessary parts are
> > > > > encrypted).
> > > > > The resize operation does not reencrypt the added disk parts
> > > > > but
> > > > > just
> > > > > extends the LUKS container (cryptsetup resize) to reflect the
> > > > > maximum
> > > > > available disk space.
> > > >
> > > > The thing I am not sure about this patch is, if executing the
> > > > disk
> > > > expansion here is really the way to go, or if we shall just
> > > > reuse
> > > > the
> > > > expand-on-first-boot from isar (incl. fixes proposed earlier -
> > > > [1])?
> > > >
> > >
> > > If you find a good way to factor out the common code an reuse
> > > that
> > > inside the initramfs, I'm open for patches. However, as we
> > > discussed
> > > on
> > > the isar list as well, using the systemd service later on will no
> > > longer
> > > work once we properly hide the keys when initramfs ends.
> > >
> >
> > True!
> >
> > Refactoring that part out, probably introduces more boilerplate,
> > than
> > the actual refactoring will save. Anyhow, once the new sealing is
> > in
> > place the systemd service support for crypt has to be
> > reconsidered/dropped anyways.
>
> That was already done:
>

Somehow managed to not notice the revert, until now. So, please just
ignore the comments :-)

> https://github.com/ilbers/isar/commit/004f9935102f81b18a54fc9c0940d23b84be411a
>
> The isar service is for unencrypted partitions only, this one here is
> for fully integrated encrypted ones.

:-)
>

> Jan
>
Jan Kiszka Jan. 20, 2025, 3:47 p.m. UTC | #6 
On 20.01.25 13:34, alexander.heinisch@siemens.com wrote:
> From: Alexander Heinisch <alexander.heinisch@siemens.com>
> 
> In the current implementation (since: 284175c3) disk expansion logic
> is done before the actual disk encryption.
> 
> This results in reencrypting the full expanded size of the disk, instead
> of only reencrypting the sections containing the actual data. This results
> in a drastic increase of time (approx 20x on our systems ) needed for reencryption!
> 
> Since relevant disk contents are only available in the sections of the disk
> available before the expansion, it is sufficient to reencrypt them only.
> 
> This patch changes order of encryption and disk expansion resulting in
> faster encryption (since only the necessary parts are encrypted).
> The resize operation does not reencrypt the added disk parts but just
> extends the LUKS container (cryptsetup resize) to reflect the maximum
> available disk space.
> 
> Signed-off-by: Alexander Heinisch <alexander.heinisch@siemens.com>
> ---
>  .../files/local-top-complete                  | 37 +++++++++++++++++--
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> index 8adc4e5..c4135c3 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
> @@ -153,6 +153,35 @@ expand_partition() {
>  	# Inform the kernel about the partitioning change
>  	partx -u "${last_part}"
>  
> +	last_part_device_name=${last_part#\/dev/}
> +
> +	mount_point=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX")

New binary (except for clevis) - would have to be listed in
HOOK_COPY_EXECS to ensure its actually available.

But we are working here against a temporary rootfs in RAM, thus could
simply use a static name derived from the hook name as well.

> +
> +	mapping_name=$(cat /sys/class/block/"$last_part_device_name"/holders/*/dm/name)
> +	cryptsetup resize "$mapping_name"
> +	mount /dev/mapper/"$mapping_name" "${mount_point}"
> +	fs_type=$(findmnt -fno FSTYPE "${mount_point}" )

New binary findmnt. But why not using get_fstype like the rest?

> +	last_part=/dev/mapper/"$mapping_name"
> +
> +	case ${fs_type} in
> +	ext*)
> +
> +		# Do not fail resize2fs if no mtab entry is found, e.g.,
> +		# when using systemd mount units.
> +		export EXT2FS_NO_MTAB_OK=1
> +		resize2fs "${last_part}"
> +		;;
> +	btrfs)
> +		btrfs filesystem resize max "${mount_point}"
> +		;;
> +	*)
> +		log_warning_msg "Unrecognized filesystem type ${fs_type} - no resize performed"
> +		;;
> +	esac
> +
> +	umount "${mount_point}"
> +	rmdir "${mount_point}"

rmdir does not exist by default. You could use rm -r instead.

But we don't need that strictly spoken. The initrd rootfs will get
dropped anyway.

> +
>  	log_end_msg
>  }
>  
> @@ -215,10 +244,6 @@ for partition_set in $partition_sets; do
>  		echo "ROOT=$decrypted_part" >/conf/param.conf
>  	fi
>  
> -	if [ "$partition_expand" = "expand" ]; then
> -		expand_partition $part_device
> -	fi
> -
>  	if /usr/sbin/cryptsetup luksDump --batch-mode "$part_device" \
>  			| grep -q "luks2"; then
>  		open_tpm2_partition "$part_device" "$crypt_mount_name" "$tpm_device"
> @@ -259,6 +284,10 @@ for partition_set in $partition_sets; do
>  	esac
>  
>  	finalize_tpm2_encryption "$part_device"
> +
> +	if [ "$partition_expand" = "expand" ]; then
> +		expand_partition $part_device
> +	fi
>  done
>  
>  if [ -n "$watchdog_pid" ]; then

Hehe, I was waiting for something like that from you already. I was too
lazy adding these extra bits without having the pressure myself.

Jan
diff mbox series

Patch

diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
index 8adc4e5..c4135c3 100644
--- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
+++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
@@ -153,6 +153,35 @@  expand_partition() {
 	# Inform the kernel about the partitioning change
 	partx -u "${last_part}"
 
+	last_part_device_name=${last_part#\/dev/}
+
+	mount_point=$(mktemp -d -p "" "$(basename "$0").XXXXXXXXXX")
+
+	mapping_name=$(cat /sys/class/block/"$last_part_device_name"/holders/*/dm/name)
+	cryptsetup resize "$mapping_name"
+	mount /dev/mapper/"$mapping_name" "${mount_point}"
+	fs_type=$(findmnt -fno FSTYPE "${mount_point}" )
+	last_part=/dev/mapper/"$mapping_name"
+
+	case ${fs_type} in
+	ext*)
+
+		# Do not fail resize2fs if no mtab entry is found, e.g.,
+		# when using systemd mount units.
+		export EXT2FS_NO_MTAB_OK=1
+		resize2fs "${last_part}"
+		;;
+	btrfs)
+		btrfs filesystem resize max "${mount_point}"
+		;;
+	*)
+		log_warning_msg "Unrecognized filesystem type ${fs_type} - no resize performed"
+		;;
+	esac
+
+	umount "${mount_point}"
+	rmdir "${mount_point}"
+
 	log_end_msg
 }
 
@@ -215,10 +244,6 @@  for partition_set in $partition_sets; do
 		echo "ROOT=$decrypted_part" >/conf/param.conf
 	fi
 
-	if [ "$partition_expand" = "expand" ]; then
-		expand_partition $part_device
-	fi
-
 	if /usr/sbin/cryptsetup luksDump --batch-mode "$part_device" \
 			| grep -q "luks2"; then
 		open_tpm2_partition "$part_device" "$crypt_mount_name" "$tpm_device"
@@ -259,6 +284,10 @@  for partition_set in $partition_sets; do
 	esac
 
 	finalize_tpm2_encryption "$part_device"
+
+	if [ "$partition_expand" = "expand" ]; then
+		expand_partition $part_device
+	fi
 done
 
 if [ -n "$watchdog_pid" ]; then