diff mbox series

[isar-cip-core] swupdate-img.bbclass: add checksum in non signed case as well

Message ID 20210111154856.6636-1-henning.schild@siemens.com (mailing list archive)
State Not Applicable
Headers show
Series [isar-cip-core] swupdate-img.bbclass: add checksum in non signed case as well | expand

Commit Message

Henning Schild Jan. 11, 2021, 3:48 p.m. UTC 
From: Claudius Heine <ch@denx.de>

Signed-off-by: Claudius Heine <ch@denx.de>
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 classes/swupdate-img.bbclass | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Jan Kiszka Jan. 13, 2021, 10:46 a.m. UTC | #1 
On 11.01.21 16:48, Henning Schild wrote:
> From: Claudius Heine <ch@denx.de>
> 

Can you also provide a reasoning here?

> Signed-off-by: Claudius Heine <ch@denx.de>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  classes/swupdate-img.bbclass | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/classes/swupdate-img.bbclass b/classes/swupdate-img.bbclass
> index a21d6ec..a7a70f6 100644
> --- a/classes/swupdate-img.bbclass
> +++ b/classes/swupdate-img.bbclass
> @@ -39,14 +39,14 @@ do_swupdate_image() {
>          image_do_mounts
>          cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
>          test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}' '${WORKDIR}/dev.crt'
> -
> -        # Fill in file check sums
> -        for file in ${SWU_ADDITIONAL_FILES}; do
> -            sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
> -                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
> -        done
>      fi
>  
> +    # Fill in file check sums
> +    for file in ${SWU_ADDITIONAL_FILES}; do
> +        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
> +            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
> +    done
> +
>      cd "${WORKDIR}/swu"
>      for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES}; do
>          echo "$file"
> 

Jan
Claudius Heine Jan. 13, 2021, 10:52 a.m. UTC | #2 
Hi Jan,

On 2021-01-13 11:46, Jan Kiszka wrote:
> On 11.01.21 16:48, Henning Schild wrote:
>> From: Claudius Heine <ch@denx.de>
>>
> 
> Can you also provide a reasoning here?

I can, but I guess you mean that a commit message should be added here.

Our swupdate has `DISABLE_CPIO_CRC=y` (because our images might be >2GB) 
and we also currently support update via usb stick without any 
signatures. So the only checksum that we have would be the sha256sum 
inside the swdescription.

regards,
Claudius

> 
>> Signed-off-by: Claudius Heine <ch@denx.de>
>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>> ---
>>   classes/swupdate-img.bbclass | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/classes/swupdate-img.bbclass b/classes/swupdate-img.bbclass
>> index a21d6ec..a7a70f6 100644
>> --- a/classes/swupdate-img.bbclass
>> +++ b/classes/swupdate-img.bbclass
>> @@ -39,14 +39,14 @@ do_swupdate_image() {
>>           image_do_mounts
>>           cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
>>           test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}' '${WORKDIR}/dev.crt'
>> -
>> -        # Fill in file check sums
>> -        for file in ${SWU_ADDITIONAL_FILES}; do
>> -            sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
>> -                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>> -        done
>>       fi
>>   
>> +    # Fill in file check sums
>> +    for file in ${SWU_ADDITIONAL_FILES}; do
>> +        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
>> +            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>> +    done
>> +
>>       cd "${WORKDIR}/swu"
>>       for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES}; do
>>           echo "$file"
>>
> 
> Jan
>
Jan Kiszka Jan. 13, 2021, 10:55 a.m. UTC | #3 
On 13.01.21 11:52, Claudius Heine wrote:
> Hi Jan,
> 
> On 2021-01-13 11:46, Jan Kiszka wrote:
>> On 11.01.21 16:48, Henning Schild wrote:
>>> From: Claudius Heine <ch@denx.de>
>>>
>>
>> Can you also provide a reasoning here?
> 
> I can, but I guess you mean that a commit message should be added here.
> 

...or that you provide me a text I can fill in on merge.

Jan

> Our swupdate has `DISABLE_CPIO_CRC=y` (because our images might be >2GB)
> and we also currently support update via usb stick without any
> signatures. So the only checksum that we have would be the sha256sum
> inside the swdescription.
> 
> regards,
> Claudius
> 
>>
>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>>> ---
>>>   classes/swupdate-img.bbclass | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/classes/swupdate-img.bbclass b/classes/swupdate-img.bbclass
>>> index a21d6ec..a7a70f6 100644
>>> --- a/classes/swupdate-img.bbclass
>>> +++ b/classes/swupdate-img.bbclass
>>> @@ -39,14 +39,14 @@ do_swupdate_image() {
>>>           image_do_mounts
>>>           cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
>>>           test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}'
>>> '${WORKDIR}/dev.crt'
>>> -
>>> -        # Fill in file check sums
>>> -        for file in ${SWU_ADDITIONAL_FILES}; do
>>> -            sed -i "s:$file-sha256:$(sha256sum
>>> '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
>>> -                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>>> -        done
>>>       fi
>>>   +    # Fill in file check sums
>>> +    for file in ${SWU_ADDITIONAL_FILES}; do
>>> +        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file |
>>> cut -f 1 -d ' '):g" \
>>> +            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>>> +    done
>>> +
>>>       cd "${WORKDIR}/swu"
>>>       for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES}; do
>>>           echo "$file"
>>>
>>
>> Jan
>>
>
Claudius Heine Jan. 13, 2021, 11:02 a.m. UTC | #4 
On 2021-01-13 11:55, Jan Kiszka wrote:
> On 13.01.21 11:52, Claudius Heine wrote:
>> Hi Jan,
>>
>> On 2021-01-13 11:46, Jan Kiszka wrote:
>>> On 11.01.21 16:48, Henning Schild wrote:
>>>> From: Claudius Heine <ch@denx.de>
>>>>
>>>
>>> Can you also provide a reasoning here?
>>
>> I can, but I guess you mean that a commit message should be added here.
>>
> 
> ...or that you provide me a text I can fill in on merge.

Ok here is my take:

```
Also unsigned images should contain a checksum in the swdescription, for 
instance in case `DISABLE_CPIO_CRC=y` is set in swupdate and a USB stick 
is used for update delivery, only such a checksum would protect against 
file corruption.
```

regards,
Claudius

> 
> Jan
> 
>> Our swupdate has `DISABLE_CPIO_CRC=y` (because our images might be >2GB)
>> and we also currently support update via usb stick without any
>> signatures. So the only checksum that we have would be the sha256sum
>> inside the swdescription.
>>
>> regards,
>> Claudius
>>
>>>
>>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>>> Signed-off-by: Henning Schild <henning.schild@siemens.com>
>>>> ---
>>>>    classes/swupdate-img.bbclass | 12 ++++++------
>>>>    1 file changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/classes/swupdate-img.bbclass b/classes/swupdate-img.bbclass
>>>> index a21d6ec..a7a70f6 100644
>>>> --- a/classes/swupdate-img.bbclass
>>>> +++ b/classes/swupdate-img.bbclass
>>>> @@ -39,14 +39,14 @@ do_swupdate_image() {
>>>>            image_do_mounts
>>>>            cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
>>>>            test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}'
>>>> '${WORKDIR}/dev.crt'
>>>> -
>>>> -        # Fill in file check sums
>>>> -        for file in ${SWU_ADDITIONAL_FILES}; do
>>>> -            sed -i "s:$file-sha256:$(sha256sum
>>>> '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
>>>> -                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>>>> -        done
>>>>        fi
>>>>    +    # Fill in file check sums
>>>> +    for file in ${SWU_ADDITIONAL_FILES}; do
>>>> +        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file |
>>>> cut -f 1 -d ' '):g" \
>>>> +            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
>>>> +    done
>>>> +
>>>>        cd "${WORKDIR}/swu"
>>>>        for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES}; do
>>>>            echo "$file"
>>>>
>>>
>>> Jan
>>>
>>
> 
>
Henning Schild Jan. 13, 2021, 11:09 a.m. UTC | #5 
Am Wed, 13 Jan 2021 11:46:03 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 11.01.21 16:48, Henning Schild wrote:
> > From: Claudius Heine <ch@denx.de>
> >   
> 
> Can you also provide a reasoning here?

This is taken from a layer where we use a postupdate-script as part of
an swu. The whole swu is not signed, still we want basic integrity
checking based on checksums.

My guess is that whenever you have SWU_ADDITIONAL_FILES and do not
sign, you might get a problem because of missing checksums.

But i would wait for Claudius to provide the reasoning. I guess it
should become part of the commit message in a v2.

Henning

> > Signed-off-by: Claudius Heine <ch@denx.de>
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  classes/swupdate-img.bbclass | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/classes/swupdate-img.bbclass
> > b/classes/swupdate-img.bbclass index a21d6ec..a7a70f6 100644
> > --- a/classes/swupdate-img.bbclass
> > +++ b/classes/swupdate-img.bbclass
> > @@ -39,14 +39,14 @@ do_swupdate_image() {
> >          image_do_mounts
> >          cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
> >          test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}'
> > '${WORKDIR}/dev.crt' -
> > -        # Fill in file check sums
> > -        for file in ${SWU_ADDITIONAL_FILES}; do
> > -            sed -i "s:$file-sha256:$(sha256sum
> > '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
> > -                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
> > -        done
> >      fi
> >  
> > +    # Fill in file check sums
> > +    for file in ${SWU_ADDITIONAL_FILES}; do
> > +        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file
> > | cut -f 1 -d ' '):g" \
> > +            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
> > +    done
> > +
> >      cd "${WORKDIR}/swu"
> >      for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES};
> > do echo "$file"
> >   
> 
> Jan
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#6068): https://lists.cip-project.org/g/cip-dev/message/6068
Mute This Topic: https://lists.cip-project.org/mt/79598691/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-
Jan Kiszka Jan. 13, 2021, 11:33 a.m. UTC | #6 
On 13.01.21 12:02, Claudius Heine wrote:
> On 2021-01-13 11:55, Jan Kiszka wrote:
>> On 13.01.21 11:52, Claudius Heine wrote:
>>> Hi Jan,
>>>
>>> On 2021-01-13 11:46, Jan Kiszka wrote:
>>>> On 11.01.21 16:48, Henning Schild wrote:
>>>>> From: Claudius Heine <ch@denx.de>
>>>>>
>>>>
>>>> Can you also provide a reasoning here?
>>>
>>> I can, but I guess you mean that a commit message should be added here.
>>>
>>
>> ...or that you provide me a text I can fill in on merge.
> 
> Ok here is my take:
> 
> ```
> Also unsigned images should contain a checksum in the swdescription, for
> instance in case `DISABLE_CPIO_CRC=y` is set in swupdate and a USB stick
> is used for update delivery, only such a checksum would protect against
> file corruption.
> ```
> 

Thanks, applied with this text.

Jan
Henning Schild Jan. 13, 2021, 12:33 p.m. UTC | #7 
Am Wed, 13 Jan 2021 12:33:12 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 13.01.21 12:02, Claudius Heine wrote:
> > On 2021-01-13 11:55, Jan Kiszka wrote:  
> >> On 13.01.21 11:52, Claudius Heine wrote:  
> >>> Hi Jan,
> >>>
> >>> On 2021-01-13 11:46, Jan Kiszka wrote:  
> >>>> On 11.01.21 16:48, Henning Schild wrote:  
> >>>>> From: Claudius Heine <ch@denx.de>
> >>>>>  
> >>>>
> >>>> Can you also provide a reasoning here?  
> >>>
> >>> I can, but I guess you mean that a commit message should be added
> >>> here. 
> >>
> >> ...or that you provide me a text I can fill in on merge.  
> > 
> > Ok here is my take:
> > 
> > ```
> > Also unsigned images should contain a checksum in the
> > swdescription, for instance in case `DISABLE_CPIO_CRC=y` is set in
> > swupdate and a USB stick is used for update delivery, only such a
> > checksum would protect against file corruption.
> > ```
> >   
> 
> Thanks, applied with this text.

Sweet, thanks!

@claudius this might be ready to pull into our layer anytime soon,
should be patch-free now

Henning

> Jan
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#6069): https://lists.cip-project.org/g/cip-dev/message/6069
Mute This Topic: https://lists.cip-project.org/mt/79598691/4520388
Group Owner: cip-dev+owner@lists.cip-project.org
Unsubscribe: https://lists.cip-project.org/g/cip-dev/leave/8129055/727948398/xyzzy [cip-dev@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-
diff mbox series

Patch

diff --git a/classes/swupdate-img.bbclass b/classes/swupdate-img.bbclass
index a21d6ec..a7a70f6 100644
--- a/classes/swupdate-img.bbclass
+++ b/classes/swupdate-img.bbclass
@@ -39,14 +39,14 @@  do_swupdate_image() {
         image_do_mounts
         cp -f '${SIGN_KEY}' '${WORKDIR}/dev.key'
         test -e '${SIGN_CRT}' && cp -f '${SIGN_CRT}' '${WORKDIR}/dev.crt'
-
-        # Fill in file check sums
-        for file in ${SWU_ADDITIONAL_FILES}; do
-            sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
-                '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
-        done
     fi
 
+    # Fill in file check sums
+    for file in ${SWU_ADDITIONAL_FILES}; do
+        sed -i "s:$file-sha256:$(sha256sum '${WORKDIR}/swu/'$file | cut -f 1 -d ' '):g" \
+            '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
+    done
+
     cd "${WORKDIR}/swu"
     for file in '${SWU_DESCRIPTION_FILE}' ${SWU_ADDITIONAL_FILES}; do
         echo "$file"