diff mbox series

[isar-cip-core,v2,1/4] swupdate: check output of sign-swu

Message ID 20240305110311.2073425-2-Quirin.Gylstorff@siemens.com (mailing list archive)
State Superseded
Headers show
Series Make swupdate signing more robust | expand

Commit Message

Gylstorff Quirin March 5, 2024, 11:02 a.m. UTC 
From: Quirin Gylstorff <quirin.gylstorff@siemens.com>

Check for signing errors to avoid an unusable swu file.

This also moves the siging out of the loop to generate
the cpio archive *.swu as the Messages from the signing
can lead to errors in the archive generation. The cpio
options are no longer using the short form.

Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
---
 classes/swupdate.bbclass | 43 ++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 13 deletions(-)

Comments

Jan Kiszka March 5, 2024, 1:50 p.m. UTC | #1 
On 05.03.24 12:02, Quirin Gylstorff wrote:
> From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> 
> Check for signing errors to avoid an unusable swu file.
> 
> This also moves the siging out of the loop to generate
> the cpio archive *.swu as the Messages from the signing
> can lead to errors in the archive generation. The cpio
> options are no longer using the short form.
> 
> Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
> ---
>  classes/swupdate.bbclass | 43 ++++++++++++++++++++++++++++------------
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
> index aaff072..c62f43f 100644
> --- a/classes/swupdate.bbclass
> +++ b/classes/swupdate.bbclass
> @@ -191,24 +191,41 @@ IMAGE_CMD:swu() {
>                      "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"
>              done
>              cd "${PP_WORK}/$swu_file_base"
> -            for file in "${SWU_DESCRIPTION_FILE}" ${SWU_ADDITIONAL_FILES}; do
> -                if [ "$file" = "${SWU_DESCRIPTION_FILE}" ] || \
> -                    grep -q "$file" "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
> +            cpio_files="${SWU_DESCRIPTION_FILE}"
> +
> +            if [ -n "$sign" ]; then
> +                if ! /usr/bin/sign-swu \
> +                    "${SWU_DESCRIPTION_FILE}" "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" \
> +                    > /dev/null 2>&1 || \
> +                    [ ! -f "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" ]; then
> +                    echo "Could not create swupdate signature file '${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}'" 1>&2
> +                    exit 1
> +                fi
> +                cpio_files="$cpio_files ${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}"

Can we use a local short-hand variable for
"${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}"? One that makes the lines
shorter and more readable?

> +            fi
> +
> +            # sw-description must be first file in *.swu
> +            for cpio_file in $cpio_files ${SWU_ADDITIONAL_FILES}; do
> +                if [ -f "$cpio_file" ]; then
>                      # Set file timestamps for reproducible builds
>                      if [ -n "${SOURCE_DATE_EPOCH}" ]; then
>                          touch -d@"${SOURCE_DATE_EPOCH}" "$file"
>                      fi
> -                    echo "$file"
> -                    if [ -n "$sign" -a "${SWU_DESCRIPTION_FILE}" = "$file" ]; then
> -                        sign-swu "$file" "$file.${SWU_SIGNATURE_EXT}"
> -                        # Set file timestamps for reproducible builds
> -                        if [ -n "${SOURCE_DATE_EPOCH}" ]; then
> -                            touch -d@"${SOURCE_DATE_EPOCH}" "$file.${SWU_SIGNATURE_EXT}"
> -                        fi

Why is this timestamp tuning no longer needed with the new approach?

> -                        echo "$file.${SWU_SIGNATURE_EXT}"
> -                    fi
> +                    case "$cpio_file" in
> +                        sw-description*)
> +                            echo "$cpio_file"
> +                            ;;
> +                        *)
> +                            if grep -q "$cpio_file" \
> +                                "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
> +                                echo "$cpio_file"
> +                            fi
> +                            ;;
> +                    esac
>                  fi
> -            done | cpio -ovL --reproducible -H crc > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
> +            done | cpio \
> +                --verbose --dereference --create --reproducible --format=crc \
> +                > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
>  EOIMAGER
>      done
>  }

Jan
Gylstorff Quirin March 5, 2024, 2:33 p.m. UTC | #2 
On 3/5/24 2:50 PM, Jan Kiszka wrote:
> On 05.03.24 12:02, Quirin Gylstorff wrote:
>> From: Quirin Gylstorff <quirin.gylstorff@siemens.com>
>>
>> Check for signing errors to avoid an unusable swu file.
>>
>> This also moves the siging out of the loop to generate
>> the cpio archive *.swu as the Messages from the signing
>> can lead to errors in the archive generation. The cpio
>> options are no longer using the short form.
>>
>> Signed-off-by: Quirin Gylstorff <quirin.gylstorff@siemens.com>
>> ---
>>   classes/swupdate.bbclass | 43 ++++++++++++++++++++++++++++------------
>>   1 file changed, 30 insertions(+), 13 deletions(-)
>>
>> diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
>> index aaff072..c62f43f 100644
>> --- a/classes/swupdate.bbclass
>> +++ b/classes/swupdate.bbclass
>> @@ -191,24 +191,41 @@ IMAGE_CMD:swu() {
>>                       "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"
>>               done
>>               cd "${PP_WORK}/$swu_file_base"
>> -            for file in "${SWU_DESCRIPTION_FILE}" ${SWU_ADDITIONAL_FILES}; do
>> -                if [ "$file" = "${SWU_DESCRIPTION_FILE}" ] || \
>> -                    grep -q "$file" "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
>> +            cpio_files="${SWU_DESCRIPTION_FILE}"
>> +
>> +            if [ -n "$sign" ]; then
>> +                if ! /usr/bin/sign-swu \
>> +                    "${SWU_DESCRIPTION_FILE}" "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" \
>> +                    > /dev/null 2>&1 || \
>> +                    [ ! -f "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" ]; then
>> +                    echo "Could not create swupdate signature file '${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}'" 1>&2
>> +                    exit 1
>> +                fi
>> +                cpio_files="$cpio_files ${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}"
> 
> Can we use a local short-hand variable for
> "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}"? One that makes the lines
> shorter and more readable?

Sure will add in a v3.
> 
>> +            fi
>> +
>> +            # sw-description must be first file in *.swu
>> +            for cpio_file in $cpio_files ${SWU_ADDITIONAL_FILES}; do
>> +                if [ -f "$cpio_file" ]; then
>>                       # Set file timestamps for reproducible builds
>>                       if [ -n "${SOURCE_DATE_EPOCH}" ]; then
>>                           touch -d@"${SOURCE_DATE_EPOCH}" "$file"
>>                       fi
>> -                    echo "$file"
>> -                    if [ -n "$sign" -a "${SWU_DESCRIPTION_FILE}" = "$file" ]; then
>> -                        sign-swu "$file" "$file.${SWU_SIGNATURE_EXT}"
>> -                        # Set file timestamps for reproducible builds
>> -                        if [ -n "${SOURCE_DATE_EPOCH}" ]; then
>> -                            touch -d@"${SOURCE_DATE_EPOCH}" "$file.${SWU_SIGNATURE_EXT}"
>> -                        fi
> 
> Why is this timestamp tuning no longer needed with the new approach?
I moved it in the for loop for creating the cpio container. That way the 
timestamp tuning is in one location.

Quirin

> 
>> -                        echo "$file.${SWU_SIGNATURE_EXT}"
>> -                    fi
>> +                    case "$cpio_file" in
>> +                        sw-description*)
>> +                            echo "$cpio_file"
>> +                            ;;
>> +                        *)
>> +                            if grep -q "$cpio_file" \
>> +                                "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
>> +                                echo "$cpio_file"
>> +                            fi
>> +                            ;;
>> +                    esac
>>                   fi
>> -            done | cpio -ovL --reproducible -H crc > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
>> +            done | cpio \
>> +                --verbose --dereference --create --reproducible --format=crc \
>> +                > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
>>   EOIMAGER
>>       done
>>   }
> 
> Jan
>
diff mbox series

Patch

diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
index aaff072..c62f43f 100644
--- a/classes/swupdate.bbclass
+++ b/classes/swupdate.bbclass
@@ -191,24 +191,41 @@  IMAGE_CMD:swu() {
                     "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"
             done
             cd "${PP_WORK}/$swu_file_base"
-            for file in "${SWU_DESCRIPTION_FILE}" ${SWU_ADDITIONAL_FILES}; do
-                if [ "$file" = "${SWU_DESCRIPTION_FILE}" ] || \
-                    grep -q "$file" "${PP_WORK}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
+            cpio_files="${SWU_DESCRIPTION_FILE}"
+
+            if [ -n "$sign" ]; then
+                if ! /usr/bin/sign-swu \
+                    "${SWU_DESCRIPTION_FILE}" "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" \
+                    > /dev/null 2>&1 || \
+                    [ ! -f "${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}" ]; then
+                    echo "Could not create swupdate signature file '${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}'" 1>&2
+                    exit 1
+                fi
+                cpio_files="$cpio_files ${SWU_DESCRIPTION_FILE}.${SWU_SIGNATURE_EXT}"
+            fi
+
+            # sw-description must be first file in *.swu
+            for cpio_file in $cpio_files ${SWU_ADDITIONAL_FILES}; do
+                if [ -f "$cpio_file" ]; then
                     # Set file timestamps for reproducible builds
                     if [ -n "${SOURCE_DATE_EPOCH}" ]; then
                         touch -d@"${SOURCE_DATE_EPOCH}" "$file"
                     fi
-                    echo "$file"
-                    if [ -n "$sign" -a "${SWU_DESCRIPTION_FILE}" = "$file" ]; then
-                        sign-swu "$file" "$file.${SWU_SIGNATURE_EXT}"
-                        # Set file timestamps for reproducible builds
-                        if [ -n "${SOURCE_DATE_EPOCH}" ]; then
-                            touch -d@"${SOURCE_DATE_EPOCH}" "$file.${SWU_SIGNATURE_EXT}"
-                        fi
-                        echo "$file.${SWU_SIGNATURE_EXT}"
-                    fi
+                    case "$cpio_file" in
+                        sw-description*)
+                            echo "$cpio_file"
+                            ;;
+                        *)
+                            if grep -q "$cpio_file" \
+                                "${WORKDIR}/$swu_file_base/${SWU_DESCRIPTION_FILE}"; then
+                                echo "$cpio_file"
+                            fi
+                            ;;
+                    esac
                 fi
-            done | cpio -ovL --reproducible -H crc > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
+            done | cpio \
+                --verbose --dereference --create --reproducible --format=crc \
+                > "${PP_DEPLOY}/${SWU_IMAGE_FILE}$swu_file_extension.swu"
 EOIMAGER
     done
 }