| 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 |
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
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
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 >
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
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 >
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 --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