Re: [PATCH] meta-isar: add support to verify sha512 checksum for target image

["'cedric.hombourger@siemens.com' via isar-users" <isar-users@googlegroups.com>]

On Thu, 2025-10-09 at 14:46 +0000, Moessbauer, Felix (FT RPD CED OES-
DE) wrote:
> On Thu, 2025-10-09 at 13:59 +0000, Hombourger, Cedric (FT FDS CES LX)
> wrote:
> > On Thu, 2025-10-09 at 18:39 +0530, Arulpandiyan Vadivel wrote:
> > > In current approach, target images from installer is installed
> > > without any
> > > verifications and validations.
> > > Adding support of verifying image with sha512 checksum before
> > > installing image
> > > Currently during the image installation .bmap files also listed
> > > in
> > > the menu.
> > > Update to show only image name instead of showing supported
> > > artifacts
> > > like .bmap and .sha512.
> > > Added a class to support generating sha512 checksum for the
> > > images.
> 
> Hi, is there a particular reason why not rely on the checksums in the
> bmap? These are WAY better than checksums on compressed artifacts and
> are also correctly checked by the bmap tool (instead of an error
> prone
> custom implementation).
> 
> > > 
> > > Signed-off-by: Arulpandiyan Vadivel
> > > <arulpandiyan.vadivel@siemens.com>
> > > ---
> > >  .../classes/installer-add-rootfs.bbclass      |  6 +-
> > >  ...eploy-image_0.1.bb => deploy-image_0.2.bb} |  2 +-
> > >  .../files/usr/bin/deploy-image-wic.sh         | 56
> > > ++++++++++++++++++-
> > >  meta/classes/image-checksum.bbclass           | 14 +++++
> > >  meta/classes/image.bbclass                    |  1 +
> > >  5 files changed, 76 insertions(+), 3 deletions(-)
> > >  rename meta-isar/recipes-installer/deploy-image/{deploy-
> > > image_0.1.bb
> > > => deploy-image_0.2.bb} (96%)
> > >  create mode 100644 meta/classes/image-checksum.bbclass
> > > 
> > > diff --git a/meta-isar/classes/installer-add-rootfs.bbclass
> > > b/meta-
> > > isar/classes/installer-add-rootfs.bbclass
> > > index c738f690..185e4a3c 100644
> > > --- a/meta-isar/classes/installer-add-rootfs.bbclass
> > > +++ b/meta-isar/classes/installer-add-rootfs.bbclass
> > > @@ -19,7 +19,7 @@ IMAGE_DATA_POSTFIX ??= "wic.zst"
> > >  IMAGE_DATA_POSTFIX:buster ??= "wic.xz"
> > >  IMAGE_DATA_POSTFIX:bullseye ??= "wic.xz"
> > >  
> > > -ROOTFS_ADDITIONAL_FILES ??= "installer-target installer-target-
> > > bmap"
> > > +ROOTFS_ADDITIONAL_FILES ??= "installer-target installer-target-
> > > bmap
> > > installer-target-sha512"
> > >  
> > >  def get_installer_source(d, suffix):
> > >      installer_target_image = d.getVar('INSTALLER_TARGET_IMAGE')
> > > or
> > > ""
> > > @@ -49,4 +49,8 @@ ROOTFS_ADDITIONAL_FILE_installer-
> > > target[destination] = "${@ get_installer_destin
> > >  ROOTFS_ADDITIONAL_FILE_installer-target-bmap[source] = "${@
> > > get_installer_source(d, "wic.bmap")}"
> > >  ROOTFS_ADDITIONAL_FILE_installer-target-bmap[destination] = "${@
> > > get_installer_destination(d, "wic.bmap")}"
> > >  
> > > +# Add support for SHA512 checksum files
> > > +ROOTFS_ADDITIONAL_FILE_installer-target-sha512[source] = "${@
> > > get_installer_source(d, d.getVar('IMAGE_DATA_POSTFIX') +
> > > '.sha512')}"
> > > +ROOTFS_ADDITIONAL_FILE_installer-target-sha512[destination] =
> > > "${@
> > > get_installer_destination(d, d.getVar('IMAGE_DATA_POSTFIX') +
> > > '.sha512')}"
> > > +
> > >  do_rootfs_install[mcdepends] += "${@ get_mc_depends(d,
> > > "do_image_wic")}"
> > > diff --git a/meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.1.bb b/meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.2.bb
> > > similarity index 96%
> > > rename from meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.1.bb
> > > rename to meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.2.bb
> > > index b287a8d1..0259a5af 100644
> > > --- a/meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.1.bb
> > > +++ b/meta-isar/recipes-installer/deploy-image/deploy-
> > > image_0.2.bb
> > > @@ -1,5 +1,5 @@
> > >  # This software is a part of ISAR.
> > > -# Copyright (C) Siemens AG, 2024
> > > +# Copyright (C) Siemens AG, 2025
> > >  #
> > >  # SPDX-License-Identifier: MIT
> > >  
> > > diff --git a/meta-isar/recipes-installer/deploy-
> > > image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes-
> > > installer/deploy-image/files/usr/bin/deploy-image-wic.sh
> > > index 333762f1..963f5756 100755
> > > --- a/meta-isar/recipes-installer/deploy-
> > > image/files/usr/bin/deploy-
> > > image-wic.sh
> > > +++ b/meta-isar/recipes-installer/deploy-
> > > image/files/usr/bin/deploy-
> > > image-wic.sh
> > > @@ -10,11 +10,65 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f --
> > > "$0";
> > > )"; )
> > >  
> > >  . "${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh"
> > >  
> > > +verify_checksum() {
> > > +    checksum_file="$1"
> > > +    hash_image_file="$2"
> > > +
> > > +    # Get the extension from the checksum file
> > > +    algorithm=$(echo "$checksum_file" | awk -F. '{print $NF}')
> > > +
> > > +    #Read the expected checksum
> > inconsistency (missing space after #)
> > 
> > > +    expected_checksum=$(cut -d' ' -f1 "$checksum_file")
> > > +
> > > +    # Check if the checksum file was empty
> > > +    if [[ -z "$expected_checksum" ]]; then
> > > +        dialog --msgbox "Error: Checksum file is empty or
> > > unreadable, Installation aborted." 6 60
> > > +        exit 1
> > > +    fi
> > > +
> > > +    # Calculate the current checksum of the file
> > > +    local current_checksum
> > > +    case "$algorithm" in
> > > +        sha512)
> > could easily be changed to sha512|sha256|md5
> > > +            current_checksum=$("${algorithm}sum"
> > > "$hash_image_file"
> > 
> > this may take a while, use dialog to let the user abort the
> > verification while running in the background? or ask upfront if
> > integrity of the image should be checked (only if checksum files
> > were
> > found)
> > 
> > also sha512sum -c may be used and would greatly simply this
> > function
> 
> I'm wondering why you decided to use sha512 which is super slow. The
> checksums anyways just protect against bitflips as the checksum files
> are not signed. By that, a much faster checksum like sha1 or sha256
> can
> be used as well.
> 
> > 
> > > > awk '{print $1}')
> > > +            ;;
> > > +        *)
> > > +            dialog --msgbox "Error: Unsupported
> > > algorithm($algorithm), Installation aborted." 6 60
> > > +            exit 1
> > > +            ;;
> > > +    esac
> > > +
> > > +    # Compare the checksums
> > this comment does not add any value
> > > +    if [[ "$current_checksum" == "$expected_checksum" ]]; then
> > > +        echo "Checksum validation success for $checksum_file and
> > > $hash_image_file"
> > > +    else
> > > +        dialog --msgbox "Error: Checksum validation failure for
> > > $checksum_file and $hash_image_file, Installation aborted." 6 60
> > > +        exit 1
> > I would not mix backend and UI code in the same function. Return
> > well
> > defined error codes and display error messages in your UI code
> > > +    fi
> > > +}
> > > +
> > > +hash_files_uri=$(find "$installdata" -type f -iname "*.sha512")
> > 
> > you have above a mechanism to handle various algorithms but only
> > sha512
> > is considered here
> > 
> > > +if [ -n "$hash_files_uri" ]; then
> > > +    for hash_file in $hash_files_uri; do
> > > +        # extract the checksum / bmap file from signed files
> > > name
> > > +        hash_image_file="${hash_file%.*}"
> > > +        if [ -f "$hash_image_file" ] && [ -f "$hash_file" ];
> > > then
> > > +            verify_checksum "$hash_file" "$hash_image_file"
> > > +        else
> > > +            dialog --msgbox "[ERROR] Checksum file or image file
> > > is
> > > missing! Installation aborted" 6 60
> > > +            exit 1
> > > +        fi
> > > +    done
> > > +else
> > > +    dialog --msgbox "Error: No checksum file(s) found for image
> > > artifacts, Installation aborted." 6 60
> > > +    exit 1
> > 
> > this should only be fatal if the installer was configured to
> > generate
> > checksum files along image artifacts and if there are not there but
> > only in that case!
> 
> What would be valuable is to encode the checksum either in the initrd
> or a dm-verity container to sign this externally. By that, we could
> ensure that only "allowed" artifacts can be deployed. But then the
> question still remains, why not simply use a dm-verity container for
> cryptographic integrity and the bmap checksums to check if the
> artifact
> is written correctly.

I am really liking the idea!

> 
> > 
> > > +fi
> > > +
> > >  if ! $installer_unattended; then
> > >      installer_image_uri=$(find "$installdata" -type f -iname
> > > "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;)
> > >      if [ -z "$installer_image_uri" ] || [ ! -f
> > > "$installdata/$installer_image_uri" ]; then
> > >          pushd "$installdata"
> > > -        for f in $(find . -type f); do
> > > +        for f in $(find . -type f -iname "*.wic.zst" -exec
> > > basename
> > > {} \;); do
> > >              array+=("$f" "$f")
> > >          done
> > >          popd
> > > diff --git a/meta/classes/image-checksum.bbclass
> > > b/meta/classes/image-checksum.bbclass
> > > new file mode 100644
> > > index 00000000..673235a0
> > > --- /dev/null
> > > +++ b/meta/classes/image-checksum.bbclass
> > > @@ -0,0 +1,14 @@
> > > +# This software is a part of ISAR.
> > > +# Copyright (C) 2025 Siemens AG
> > > +#
> > > +# SPDX-License-Identifier: MIT
> > > +
> > > +do_generate_checksum() {
> > > +    cd ${DEPLOY_DIR_IMAGE}
> > > +    for postfix in ${IMAGE_FSTYPES}; do
> > > +        [ -f "${IMAGE_FULLNAME}.$postfix" ] || continue
> > > +        sha512sum "${IMAGE_FULLNAME}.$postfix" >
> > > "${IMAGE_FULLNAME}.$postfix.sha512"
> > > +    done
> > > +}
> > > +
> > > +do_image_wic[postfuncs] += "do_generate_checksum"
> > > diff --git a/meta/classes/image.bbclass
> > > b/meta/classes/image.bbclass
> > > index bd1b8552..57216014 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -141,6 +141,7 @@ IMAGE_CLASSES ??= ""
> > >  IMGCLASSES = "imagetypes imagetypes_wic imagetypes_vm
> > > imagetypes_container squashfs"
> > >  IMGCLASSES += "${IMAGE_CLASSES}"
> > >  inherit ${IMGCLASSES}
> > > +inherit image-checksum
> > not sure we want to always generate checksums (e.g. for development
> > builds, I don't need or want them but would for release builds)
> > >  
> > >  # convenience variables to be used by CMDs
> > >  IMAGE_FILE_HOST =
> > > "${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.${type}"
> > 
> > While I believe the feature being added would be useful, I think we
> > should make it an opt-in and ensure that no changes are introduced
> > in
> > builds that do not require or want the feature
> 
> I would like to clarify the requirements first, mainly by defining a
> threat model.

Agreed. We should carefully document the why (something along the lines
providing a way to only permit installation images from trusted parties
and with a confirmation that they have not been tampered in some
fashion)

> 
> Felix
> 
> > 
> > tests using the Isar test suite are also missing.
> 
> -- 
> Siemens AG
> Linux Expert Center
> Friedrich-Ludwig-Bauer-Str. 3
> 85748 Garching, Germany
> 

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/isar-users/7e02c46a294768fd459208cb0989d91da2e5bc53.camel%40siemens.com.