Re: [PATCH v3 4/4] initramfs-crypt-hook: add re-encryption recovery

[Claudius Heine <ch@denx.de>]

Hi Jan,

On 2025-03-04 4:12 pm, Jan Kiszka wrote:
> On 04.03.25 16:10, Quirin Gylstorff wrote:
>>
>>
>> On 3/4/25 15:52, Jan Kiszka wrote:
>>> On 04.03.25 14:07, Claudius Heine wrote:
>>>> Integrate detection and recovery of power failures while a partition is
>>>> being encrypted.
>>>>
>>>
>>> Why is this at the end of the series? It can already help the existing
>>> implementation during encrypt-on-first-boot.

Because that was the order this feature was implemented, to us the 
support for 'noencrypt' was more important than the re-encryption recovery.

I can look into reordering it.

>>>
>>>> There are possible scenarios:
>>>> 1. Power-fail happens while the partition is reencrypted:
>>>>     - The LUKS header contains `online-reencrypt-v2` and needs to be
>>>>       repaired with `cryptsetup repair` before it can continue.
>>>>     - Also no resizing of the file system is necessary
>>>> 2. Power-fail happens before the systemd-tpm2/clevis token can be
>>>> installed
>>>>     - The LUKS header does not contain 'systemd-tpm2'/'clevis', thus it
>>>>       needs to be registered and the temporary encryption key needs to be
>>>>       removed
>>>>
>>>> In both scenarios the system after the reboot needs to have access to
>>>> the temporary encryption key that was initially used. So using a random
>>>> one, generated via `openssl rand` is not possible. Since it is only a
>>>> temporary key and gets removed after the systemd-tpm2/clevis token was
>>>> installed, a known password can be used.
>>>>
>>>> The list of these scenarios is not complete, there might be other
>>>> instances where a sudden power-fail could be fatal to the system, but
>>>> these where the most obvious and risky ones.
>>>
>>> For recovery of the first boot, this might be enough. But when proposing
>>> this for in-field recovery, we should be stricter with thinking the
>>> details through. If we fix them, immediately or later (or never if
>>> unfixable) is on a different page.
>>>
>>>>
>>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>>> ---
>>>>    .../files/local-top-complete                  | 33 +++++++++++++++----
>>>>    .../initramfs-crypt-hook_0.6.bb               |  5 ++-
>>>>    2 files changed, 29 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/local-top-
>>>> complete b/recipes-initramfs/initramfs-crypt-hook/files/local-top-
>>>> complete
>>>> index 47533b5..e67f26f 100644
>>>> --- a/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/local-top-complete
>>>> @@ -72,6 +72,9 @@ reencrypt_existing_partition() {
>>>>        reduced_size="$(expr "$part_size_blocks" - 65536 )"
>>>>        reduced_size_in_byte="$(expr "$reduced_size" \* 512)"
>>>>        reduced_size_in_kb="$(expr "$reduced_size_in_byte" / 1024)K"
>>>> +
>>>> +    CRYPTSETUP_PARAMS="--reduce-device-size ${reduce_device_size}k"
>>>> +
>>>>        case $partition_fstype in
>>>>        ext*)
>>>>            # reduce the filesystem and partition by 32M to fit the
>>>> LUKS header
>>>> @@ -90,14 +93,25 @@ reencrypt_existing_partition() {
>>>>        squashfs|swap|erofs|"")
>>>>            [ "$debug" = "y" ] && echo "skip disk resize as it is not
>>>> supported or unnecessary for fstype: '$partition_fstype'"
>>>>            ;;
>>>> +    luks)
>>>> +        # Check if reencrypt was aborted
>>>> +        if /usr/sbin/cryptsetup luksDump --batch-mode "$1" \
>>>> +                | grep -q "online-reencrypt-v2"; then
>>>> +            /usr/sbin/cryptsetup repair --batch-mode "$1" < "$2" || \
>>>> +                panic "cryptsetup repair was not successful"
>>>> +        fi
>>>> +
>>>> +        # already luks partition, don't resize
>>>> +        CRYPTSETUP_PARAMS=""
>>>> +        ;;
>>>>        *)
>>>>            panic "cannot resize partition, unsupported fstype:
>>>> '$partition_fstype'"
>>>>            ;;
>>>>        esac
>>>>        if [ -x /usr/sbin/cryptsetup-reencrypt ]; then
>>>> -        /usr/sbin/cryptsetup-reencrypt --new --reduce-device-size
>>>> "$reduce_device_size"k "$1" < "$2"
>>>> +        /usr/sbin/cryptsetup-reencrypt --new ${CRYPTSETUP_PARAMS}
>>>> "$1" < "$2"
>>>>        else
>>>> -        /usr/sbin/cryptsetup reencrypt --encrypt --reduce-device-
>>>> size "$reduce_device_size"k "$1" < "$2"
>>>> +        /usr/sbin/cryptsetup reencrypt --encrypt
>>>> ${CRYPTSETUP_PARAMS} "$1" < "$2"
>>>>        fi
>>>>    }
>>>>    @@ -244,7 +258,7 @@ for partition_set in $partition_sets; do
>>>>        # If partition is already encrypted, decrypt and continue with
>>>> next partition:
>>>>        decrypted_part=/dev/mapper/"$crypt_mount_name"
>>>>        if /usr/sbin/cryptsetup luksDump --batch-mode "$part_device" \
>>>> -            | grep -q "luks2"; then
>>>> +            | grep -q "systemd-tpm2\|clevis"; then
>>>>            open_tpm2_partition "$part_device" "$crypt_mount_name"
>>>> "$tpm_device"
>>>>              # check if we are trying to mount root, set ROOT to
>>>> decrypted partition:
>>>> @@ -255,6 +269,12 @@ for partition_set in $partition_sets; do
>>>>            continue
>>>>        fi
>>>>    +    # If partition contains an aborted reencrypt luks header,
>>>> switch to reencrypt mode:
>>>> +    if /usr/sbin/cryptsetup luksDump --batch-mode "${part_device}" \
>>>> +            | grep -q "online-reencrypt-v2"; then
>>>> +        partition_format="reencrypt"
>>>> +    fi
>>>> +
>>>>        # If partition should not be encrypted, continue with next
>>>> partition:
>>>>        if [ "$partition_format" = "noencrypt" ]
>>>>        then
>>>> @@ -272,10 +292,11 @@ for partition_set in $partition_sets; do
>>>>            watchdog_pid=$!
>>>>        fi
>>>>    -    # create random password for initial encryption
>>>> -    # this will be dropped after reboot
>>>> +    # use partuuid of the partition for initial encryption password,
>>>> this key
>>>> +    # will be removed after the reencryption has finished and the
>>>> TPM2 token is
>>>> +    # registered:
>>>>        tmp_key=/tmp/"$(basename "$part_device")-lukskey"
>>>> -    openssl rand -base64 32 > "$tmp_key"
>>>> +    lsblk -no partuuid "$part_device" > "$tmp_key"
>>>
>>> This should be factored out into a separate commit. I don't disagree
>>> about it, it just deserves an own, clear reasoning. Without recovery, it
>>> looks to me like a nice size optimization for the initramfs by dropping
>>> the openssl dependency.
>>
>> Why `lsblk` instead of `blkid -s PARTUUID -o value`, as blkid is already
>> used in initramfs-verity-hook and initramfs-abrootfs-hook this would
>> save more space.

Good idea.

> 
> Or just use "some-long-hardcoded-temporary-password". From a security
> perspective, there should be no difference.

That is was we did initially downstream, but I thought that just 
inserting some fixed hard-coded password string might have even more 
issues being accepted here upstream than using some device parameter.

We could also use `dmidecode -s system-uuid`, that at least would 
require to readout the uuid of the system. Having the storage medium 
alone would not be enough.

regards,
Claudius

> 
> Jan
> 

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de