diff mbox series

[isar-cip-core,3/4] initramfs-crypt-hook: Avoid calling of tiny mke2fs from busybox

Message ID 20230728143320.3891194-4-stefan-koch@siemens.com (mailing list archive)
State Superseded
Headers show
Series initramfs-crypt-hook: Fix disk encryption | expand

Commit Message

Stefan Koch July 28, 2023, 2:33 p.m. UTC
The tiny mke2fs does not support the used -t option.

Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
---
 .../initramfs-crypt-hook/files/encrypt_partition.systemd.script | 2 +-
 .../initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb            | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Jan Kiszka Aug. 10, 2023, 10:36 a.m. UTC | #1
On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
> The tiny mke2fs does not support the used -t option.
> 
> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> ---
>  .../initramfs-crypt-hook/files/encrypt_partition.systemd.script | 2 +-
>  .../initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb            | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
> index b32b2f2..4e04d37 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
> +++ b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
> @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
>  create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
>  
>  if [ -z "${create_file_system_cmd}" ]; then
> -	create_file_system_cmd="mke2fs -t ext4"
> +	create_file_system_cmd="/usr/sbin/mke2fs"
>  fi
>  
>  service_watchdog() {
> diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
> index 1c1bf3d..3c3f6bb 100644
> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
> @@ -32,7 +32,7 @@ SRC_URI += "file://encrypt_partition.env.tmpl \
>  CRYPT_PARTITIONS ??= "home:/home:reencrypt var:/var:reencrypt"
>  # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to create the filesystem
>  # in a newly formatted LUKS Partition
> -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
> +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>  # Timeout for creating / re-encrypting partitions on first boot
>  CRYPT_SETUP_TIMEOUT ??= "600"
>  # Watchdog to service during the initial setup of the crypto partitions

Do we have both already? Also in case of the isar-cip-core generated
images, or only with additional customizations of the initramfs? Just to
understand when this issue is currently visible.

Jan
Gylstorff Quirin Aug. 10, 2023, 11:17 a.m. UTC | #2
On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
> On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
>> The tiny mke2fs does not support the used -t option.
>>
>> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
>> ---
>>   .../initramfs-crypt-hook/files/encrypt_partition.systemd.script | 2 +-
>>   .../initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb            | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>> index b32b2f2..4e04d37 100644
>> --- a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>> +++ b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>> @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
>>   create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
>>   
>>   if [ -z "${create_file_system_cmd}" ]; then
>> -	create_file_system_cmd="mke2fs -t ext4"
>> +	create_file_system_cmd="/usr/sbin/mke2fs"
>>   fi
>>   
>>   service_watchdog() {
>> diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>> index 1c1bf3d..3c3f6bb 100644
>> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>> @@ -32,7 +32,7 @@ SRC_URI += "file://encrypt_partition.env.tmpl \
>>   CRYPT_PARTITIONS ??= "home:/home:reencrypt var:/var:reencrypt"
>>   # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to create the filesystem
>>   # in a newly formatted LUKS Partition
>> -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
>> +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>>   # Timeout for creating / re-encrypting partitions on first boot
>>   CRYPT_SETUP_TIMEOUT ??= "600"
>>   # Watchdog to service during the initial setup of the crypto partitions
> 
> Do we have both already? Also in case of the isar-cip-core generated
> images, or only with additional customizations of the initramfs? Just to
> understand when this issue is currently visible.

cip-core does not include busybox in the initramfs - therefore this was 
overlooked.

Quirin

> 
> Jan
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#12591): https://lists.cip-project.org/g/cip-dev/message/12591
> Mute This Topic: https://lists.cip-project.org/mt/100411235/1753640
> Group Owner: cip-dev+owner@lists.cip-project.org
> Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129121/1753640/1405269326/xyzzy [quirin.gylstorff@siemens.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Jan Kiszka Aug. 11, 2023, 12:53 p.m. UTC | #3
On 10.08.23 13:17, Gylstorff Quirin wrote:
> 
> 
> On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
>> On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
>>> The tiny mke2fs does not support the used -t option.
>>>
>>> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
>>> ---
>>>   .../initramfs-crypt-hook/files/encrypt_partition.systemd.script | 2 +-
>>>   .../initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb            | 2 +-
>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git
>>> a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>>> index b32b2f2..4e04d37 100644
>>> ---
>>> a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>>> +++
>>> b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
>>> @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
>>>   create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
>>>     if [ -z "${create_file_system_cmd}" ]; then
>>> -    create_file_system_cmd="mke2fs -t ext4"
>>> +    create_file_system_cmd="/usr/sbin/mke2fs"

This is not correct.

>>>   fi
>>>     service_watchdog() {
>>> diff --git
>>> a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>>> b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>>> index 1c1bf3d..3c3f6bb 100644
>>> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>>> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
>>> @@ -32,7 +32,7 @@ SRC_URI += "file://encrypt_partition.env.tmpl \
>>>   CRYPT_PARTITIONS ??= "home:/home:reencrypt var:/var:reencrypt"
>>>   # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to create
>>> the filesystem
>>>   # in a newly formatted LUKS Partition
>>> -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
>>> +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>>>   # Timeout for creating / re-encrypting partitions on first boot
>>>   CRYPT_SETUP_TIMEOUT ??= "600"
>>>   # Watchdog to service during the initial setup of the crypto
>>> partitions
>>
>> Do we have both already? Also in case of the isar-cip-core generated
>> images, or only with additional customizations of the initramfs? Just to
>> understand when this issue is currently visible.
> 
> cip-core does not include busybox in the initramfs - therefore this was
> overlooked.

Ok, so the path should be adjusted.

But now I'm wondering why to spots have to be changed. The script checks
for create_file_system_cmd, means CRYPT_CREATE_FILE_SYSTEM_CMD, being
zero and falls back to the standard creation. But then we have the
standard command also in the recipe. One should go, also to avoid
mistakes like done by this patch.

Jan
Stefan Koch Aug. 14, 2023, 11:25 a.m. UTC | #4
On Fri, 2023-08-11 at 14:53 +0200, Jan Kiszka wrote:
> On 10.08.23 13:17, Gylstorff Quirin wrote:
> > 
> > 
> > On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
> > > On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
> > > > The tiny mke2fs does not support the used -t option.
> > > > 
> > > > Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> > > > ---
> > > >   .../initramfs-crypt-
> > > > hook/files/encrypt_partition.systemd.script | 2 +-
> > > >   .../initramfs-crypt-hook/initramfs-crypt-
> > > > hook_0.1.bb            | 2 +-
> > > >   2 files changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git
> > > > a/recipes-initramfs/initramfs-crypt-
> > > > hook/files/encrypt_partition.systemd.script b/recipes-
> > > > initramfs/initramfs-crypt-
> > > > hook/files/encrypt_partition.systemd.script
> > > > index b32b2f2..4e04d37 100644
> > > > ---
> > > > a/recipes-initramfs/initramfs-crypt-
> > > > hook/files/encrypt_partition.systemd.script
> > > > +++
> > > > b/recipes-initramfs/initramfs-crypt-
> > > > hook/files/encrypt_partition.systemd.script
> > > > @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
> > > >   create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
> > > >     if [ -z "${create_file_system_cmd}" ]; then
> > > > -    create_file_system_cmd="mke2fs -t ext4"
> > > > +    create_file_system_cmd="/usr/sbin/mke2fs"
> 
> This is not correct.

Oops... Params lost :( Then default should be ext2.

But the condition
	if [ -z "${create_file_system_cmd}" ]; then

is never true, because of "??=" in the initramfs-crypt-hook_0.1.bb
file:
CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"

> 
> > > >   fi
> > > >     service_watchdog() {
> > > > diff --git
> > > > a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > hook_0.1.bb
> > > > b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > hook_0.1.bb
> > > > index 1c1bf3d..3c3f6bb 100644
> > > > --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > hook_0.1.bb
> > > > +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > hook_0.1.bb
> > > > @@ -32,7 +32,7 @@ SRC_URI +=
> > > > "file://encrypt_partition.env.tmpl \
> > > >   CRYPT_PARTITIONS ??= "home:/home:reencrypt
> > > > var:/var:reencrypt"
> > > >   # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to
> > > > create
> > > > the filesystem
> > > >   # in a newly formatted LUKS Partition
> > > > -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
> > > > +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
> > > >   # Timeout for creating / re-encrypting partitions on first
> > > > boot
> > > >   CRYPT_SETUP_TIMEOUT ??= "600"
> > > >   # Watchdog to service during the initial setup of the crypto
> > > > partitions
> > > 
> > > Do we have both already? Also in case of the isar-cip-core
> > > generated
> > > images, or only with additional customizations of the initramfs?
> > > Just to
> > > understand when this issue is currently visible.
> > 
> > cip-core does not include busybox in the initramfs - therefore this
> > was
> > overlooked.
> 
> Ok, so the path should be adjusted.
> 
> But now I'm wondering why to spots have to be changed. The script
> checks
> for create_file_system_cmd, means CRYPT_CREATE_FILE_SYSTEM_CMD, being
> zero and falls back to the standard creation. But then we have the
> standard command also in the recipe. One should go, also to avoid
> mistakes like done by this patch.

Should we just delete the condition above within
encrypt_partition.systemd.script - since the default value is always
set? Or are ther cases, that do not use the initramfs-crypt-hook_0.1.bb
and bring in own customizations that might fail after dropping it?

> 
> Jan
>
Jan Kiszka Aug. 14, 2023, 11:43 a.m. UTC | #5
On 14.08.23 13:25, Koch, Stefan (DI PA DCP R&D 3) wrote:
> On Fri, 2023-08-11 at 14:53 +0200, Jan Kiszka wrote:
>> On 10.08.23 13:17, Gylstorff Quirin wrote:
>>>
>>>
>>> On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
>>>> On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
>>>>> The tiny mke2fs does not support the used -t option.
>>>>>
>>>>> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
>>>>> ---
>>>>>   .../initramfs-crypt-
>>>>> hook/files/encrypt_partition.systemd.script | 2 +-
>>>>>   .../initramfs-crypt-hook/initramfs-crypt-
>>>>> hook_0.1.bb            | 2 +-
>>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git
>>>>> a/recipes-initramfs/initramfs-crypt-
>>>>> hook/files/encrypt_partition.systemd.script b/recipes-
>>>>> initramfs/initramfs-crypt-
>>>>> hook/files/encrypt_partition.systemd.script
>>>>> index b32b2f2..4e04d37 100644
>>>>> ---
>>>>> a/recipes-initramfs/initramfs-crypt-
>>>>> hook/files/encrypt_partition.systemd.script
>>>>> +++
>>>>> b/recipes-initramfs/initramfs-crypt-
>>>>> hook/files/encrypt_partition.systemd.script
>>>>> @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
>>>>>   create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
>>>>>     if [ -z "${create_file_system_cmd}" ]; then
>>>>> -    create_file_system_cmd="mke2fs -t ext4"
>>>>> +    create_file_system_cmd="/usr/sbin/mke2fs"
>>
>> This is not correct.
> 
> Oops... Params lost :( Then default should be ext2.
> 
> But the condition
> 	if [ -z "${create_file_system_cmd}" ]; then
> 
> is never true, because of "??=" in the initramfs-crypt-hook_0.1.bb
> file:
> CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
> 
>>
>>>>>   fi
>>>>>     service_watchdog() {
>>>>> diff --git
>>>>> a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>> hook_0.1.bb
>>>>> b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>> hook_0.1.bb
>>>>> index 1c1bf3d..3c3f6bb 100644
>>>>> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>> hook_0.1.bb
>>>>> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>> hook_0.1.bb
>>>>> @@ -32,7 +32,7 @@ SRC_URI +=
>>>>> "file://encrypt_partition.env.tmpl \
>>>>>   CRYPT_PARTITIONS ??= "home:/home:reencrypt
>>>>> var:/var:reencrypt"
>>>>>   # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to
>>>>> create
>>>>> the filesystem
>>>>>   # in a newly formatted LUKS Partition
>>>>> -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
>>>>> +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>>>>>   # Timeout for creating / re-encrypting partitions on first
>>>>> boot
>>>>>   CRYPT_SETUP_TIMEOUT ??= "600"
>>>>>   # Watchdog to service during the initial setup of the crypto
>>>>> partitions
>>>>
>>>> Do we have both already? Also in case of the isar-cip-core
>>>> generated
>>>> images, or only with additional customizations of the initramfs?
>>>> Just to
>>>> understand when this issue is currently visible.
>>>
>>> cip-core does not include busybox in the initramfs - therefore this
>>> was
>>> overlooked.
>>
>> Ok, so the path should be adjusted.
>>
>> But now I'm wondering why to spots have to be changed. The script
>> checks
>> for create_file_system_cmd, means CRYPT_CREATE_FILE_SYSTEM_CMD, being
>> zero and falls back to the standard creation. But then we have the
>> standard command also in the recipe. One should go, also to avoid
>> mistakes like done by this patch.
> 
> Should we just delete the condition above within
> encrypt_partition.systemd.script - since the default value is always
> set? Or are ther cases, that do not use the initramfs-crypt-hook_0.1.bb
> and bring in own customizations that might fail after dropping it?
> 

I'm inclined to delete the script test, given that we have a default via
the variable. Or what was the idea behind that, Quirin?

Jan
Gylstorff Quirin Aug. 14, 2023, 11:51 a.m. UTC | #6
On 8/14/23 13:43, Jan Kiszka wrote:
> On 14.08.23 13:25, Koch, Stefan (DI PA DCP R&D 3) wrote:
>> On Fri, 2023-08-11 at 14:53 +0200, Jan Kiszka wrote:
>>> On 10.08.23 13:17, Gylstorff Quirin wrote:
>>>>
>>>>
>>>> On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
>>>>> On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
>>>>>> The tiny mke2fs does not support the used -t option.
>>>>>>
>>>>>> Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
>>>>>> ---
>>>>>>    .../initramfs-crypt-
>>>>>> hook/files/encrypt_partition.systemd.script | 2 +-
>>>>>>    .../initramfs-crypt-hook/initramfs-crypt-
>>>>>> hook_0.1.bb            | 2 +-
>>>>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/recipes-initramfs/initramfs-crypt-
>>>>>> hook/files/encrypt_partition.systemd.script b/recipes-
>>>>>> initramfs/initramfs-crypt-
>>>>>> hook/files/encrypt_partition.systemd.script
>>>>>> index b32b2f2..4e04d37 100644
>>>>>> ---
>>>>>> a/recipes-initramfs/initramfs-crypt-
>>>>>> hook/files/encrypt_partition.systemd.script
>>>>>> +++
>>>>>> b/recipes-initramfs/initramfs-crypt-
>>>>>> hook/files/encrypt_partition.systemd.script
>>>>>> @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
>>>>>>    create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
>>>>>>      if [ -z "${create_file_system_cmd}" ]; then
>>>>>> -    create_file_system_cmd="mke2fs -t ext4"
>>>>>> +    create_file_system_cmd="/usr/sbin/mke2fs"
>>>
>>> This is not correct.
>>
>> Oops... Params lost :( Then default should be ext2.
>>
>> But the condition
>> 	if [ -z "${create_file_system_cmd}" ]; then
>>
>> is never true, because of "??=" in the initramfs-crypt-hook_0.1.bb
>> file:
>> CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>>
>>>
>>>>>>    fi
>>>>>>      service_watchdog() {
>>>>>> diff --git
>>>>>> a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>>> hook_0.1.bb
>>>>>> b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>>> hook_0.1.bb
>>>>>> index 1c1bf3d..3c3f6bb 100644
>>>>>> --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>>> hook_0.1.bb
>>>>>> +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
>>>>>> hook_0.1.bb
>>>>>> @@ -32,7 +32,7 @@ SRC_URI +=
>>>>>> "file://encrypt_partition.env.tmpl \
>>>>>>    CRYPT_PARTITIONS ??= "home:/home:reencrypt
>>>>>> var:/var:reencrypt"
>>>>>>    # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to
>>>>>> create
>>>>>> the filesystem
>>>>>>    # in a newly formatted LUKS Partition
>>>>>> -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
>>>>>> +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
>>>>>>    # Timeout for creating / re-encrypting partitions on first
>>>>>> boot
>>>>>>    CRYPT_SETUP_TIMEOUT ??= "600"
>>>>>>    # Watchdog to service during the initial setup of the crypto
>>>>>> partitions
>>>>>
>>>>> Do we have both already? Also in case of the isar-cip-core
>>>>> generated
>>>>> images, or only with additional customizations of the initramfs?
>>>>> Just to
>>>>> understand when this issue is currently visible.
>>>>
>>>> cip-core does not include busybox in the initramfs - therefore this
>>>> was
>>>> overlooked.
>>>
>>> Ok, so the path should be adjusted.
>>>
>>> But now I'm wondering why to spots have to be changed. The script
>>> checks
>>> for create_file_system_cmd, means CRYPT_CREATE_FILE_SYSTEM_CMD, being
>>> zero and falls back to the standard creation. But then we have the
>>> standard command also in the recipe. One should go, also to avoid
>>> mistakes like done by this patch.
>>
>> Should we just delete the condition above within
>> encrypt_partition.systemd.script - since the default value is always
>> set? Or are ther cases, that do not use the initramfs-crypt-hook_0.1.bb
>> and bring in own customizations that might fail after dropping it?
>>
> 
> I'm inclined to delete the script test, given that we have a default via
> the variable. Or what was the idea behind that, Quirin?

The script test can be deleted - It was more or less a fallback during 
development.

Quirin
Stefan Koch Aug. 14, 2023, 11:54 a.m. UTC | #7
On Mon, 2023-08-14 at 13:51 +0200, Gylstorff Quirin wrote:
> 
> 
> On 8/14/23 13:43, Jan Kiszka wrote:
> > On 14.08.23 13:25, Koch, Stefan (DI PA DCP R&D 3) wrote:
> > > On Fri, 2023-08-11 at 14:53 +0200, Jan Kiszka wrote:
> > > > On 10.08.23 13:17, Gylstorff Quirin wrote:
> > > > > 
> > > > > 
> > > > > On 8/10/23 12:36, Jan Kiszka via lists.cip-project.org wrote:
> > > > > > On 28.07.23 16:33, Koch, Stefan (DI PA DCP R&D 3) wrote:
> > > > > > > The tiny mke2fs does not support the used -t option.
> > > > > > > 
> > > > > > > Signed-off-by: Stefan Koch <stefan-koch@siemens.com>
> > > > > > > ---
> > > > > > >    .../initramfs-crypt-
> > > > > > > hook/files/encrypt_partition.systemd.script | 2 +-
> > > > > > >    .../initramfs-crypt-hook/initramfs-crypt-
> > > > > > > hook_0.1.bb            | 2 +-
> > > > > > >    2 files changed, 2 insertions(+), 2 deletions(-)
> > > > > > > 
> > > > > > > diff --git
> > > > > > > a/recipes-initramfs/initramfs-crypt-
> > > > > > > hook/files/encrypt_partition.systemd.script b/recipes-
> > > > > > > initramfs/initramfs-crypt-
> > > > > > > hook/files/encrypt_partition.systemd.script
> > > > > > > index b32b2f2..4e04d37 100644
> > > > > > > ---
> > > > > > > a/recipes-initramfs/initramfs-crypt-
> > > > > > > hook/files/encrypt_partition.systemd.script
> > > > > > > +++
> > > > > > > b/recipes-initramfs/initramfs-crypt-
> > > > > > > hook/files/encrypt_partition.systemd.script
> > > > > > > @@ -42,7 +42,7 @@ partition_sets="$PARTITIONS"
> > > > > > >    create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
> > > > > > >      if [ -z "${create_file_system_cmd}" ]; then
> > > > > > > -    create_file_system_cmd="mke2fs -t ext4"
> > > > > > > +    create_file_system_cmd="/usr/sbin/mke2fs"
> > > > 
> > > > This is not correct.
> > > 
> > > Oops... Params lost :( Then default should be ext2.
> > > 
> > > But the condition
> > >         if [ -z "${create_file_system_cmd}" ]; then
> > > 
> > > is never true, because of "??=" in the initramfs-crypt-
> > > hook_0.1.bb
> > > file:
> > > CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
> > > 
> > > > 
> > > > > > >    fi
> > > > > > >      service_watchdog() {
> > > > > > > diff --git
> > > > > > > a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > > > > hook_0.1.bb
> > > > > > > b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-
> > > > > > > hook_0.1.bb
> > > > > > > index 1c1bf3d..3c3f6bb 100644
> > > > > > > --- a/recipes-initramfs/initramfs-crypt-hook/initramfs-
> > > > > > > crypt-
> > > > > > > hook_0.1.bb
> > > > > > > +++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-
> > > > > > > crypt-
> > > > > > > hook_0.1.bb
> > > > > > > @@ -32,7 +32,7 @@ SRC_URI +=
> > > > > > > "file://encrypt_partition.env.tmpl \
> > > > > > >    CRYPT_PARTITIONS ??= "home:/home:reencrypt
> > > > > > > var:/var:reencrypt"
> > > > > > >    # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell
> > > > > > > command to
> > > > > > > create
> > > > > > > the filesystem
> > > > > > >    # in a newly formatted LUKS Partition
> > > > > > > -CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
> > > > > > > +CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t
> > > > > > > ext4"
> > > > > > >    # Timeout for creating / re-encrypting partitions on
> > > > > > > first
> > > > > > > boot
> > > > > > >    CRYPT_SETUP_TIMEOUT ??= "600"
> > > > > > >    # Watchdog to service during the initial setup of the
> > > > > > > crypto
> > > > > > > partitions
> > > > > > 
> > > > > > Do we have both already? Also in case of the isar-cip-core
> > > > > > generated
> > > > > > images, or only with additional customizations of the
> > > > > > initramfs?
> > > > > > Just to
> > > > > > understand when this issue is currently visible.
> > > > > 
> > > > > cip-core does not include busybox in the initramfs -
> > > > > therefore this
> > > > > was
> > > > > overlooked.
> > > > 
> > > > Ok, so the path should be adjusted.
> > > > 
> > > > But now I'm wondering why to spots have to be changed. The
> > > > script
> > > > checks
> > > > for create_file_system_cmd, means CRYPT_CREATE_FILE_SYSTEM_CMD,
> > > > being
> > > > zero and falls back to the standard creation. But then we have
> > > > the
> > > > standard command also in the recipe. One should go, also to
> > > > avoid
> > > > mistakes like done by this patch.
> > > 
> > > Should we just delete the condition above within
> > > encrypt_partition.systemd.script - since the default value is
> > > always
> > > set? Or are ther cases, that do not use the initramfs-crypt-
> > > hook_0.1.bb
> > > and bring in own customizations that might fail after dropping
> > > it?
> > > 
> > 
> > I'm inclined to delete the script test, given that we have a
> > default via
> > the variable. Or what was the idea behind that, Quirin?
> 
> The script test can be deleted - It was more or less a fallback
> during 
> development.
I'll update that single patch and resend it.
> 
> Quirin
diff mbox series

Patch

diff --git a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
index b32b2f2..4e04d37 100644
--- a/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
+++ b/recipes-initramfs/initramfs-crypt-hook/files/encrypt_partition.systemd.script
@@ -42,7 +42,7 @@  partition_sets="$PARTITIONS"
 create_file_system_cmd="$CREATE_FILE_SYSTEM_CMD"
 
 if [ -z "${create_file_system_cmd}" ]; then
-	create_file_system_cmd="mke2fs -t ext4"
+	create_file_system_cmd="/usr/sbin/mke2fs"
 fi
 
 service_watchdog() {
diff --git a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
index 1c1bf3d..3c3f6bb 100644
--- a/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
+++ b/recipes-initramfs/initramfs-crypt-hook/initramfs-crypt-hook_0.1.bb
@@ -32,7 +32,7 @@  SRC_URI += "file://encrypt_partition.env.tmpl \
 CRYPT_PARTITIONS ??= "home:/home:reencrypt var:/var:reencrypt"
 # CRYPT_CREATE_FILE_SYSTEM_CMD contains the shell command to create the filesystem
 # in a newly formatted LUKS Partition
-CRYPT_CREATE_FILE_SYSTEM_CMD ??= "mke2fs -t ext4"
+CRYPT_CREATE_FILE_SYSTEM_CMD ??= "/usr/sbin/mke2fs -t ext4"
 # Timeout for creating / re-encrypting partitions on first boot
 CRYPT_SETUP_TIMEOUT ??= "600"
 # Watchdog to service during the initial setup of the crypto partitions