From b972cde9528e14083f7763a626af28708cdd1bcd Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Mon, 15 Aug 2022 10:58:11 +0300
Subject: [PATCH 01/14] implemented secure upgrade

---
 scripts/create_mock_image.sh                 | 40 +++++++++
 scripts/create_sign_and_verify_test_files.sh | 93 ++++++++++++++++++++
 scripts/verify_image_sign.sh                 | 75 ++++++++++++++++
 scripts/verify_image_sign_common.sh          | 34 +++++++
 scripts/verify_image_sign_test.sh            | 29 ++++++
 setup.py                                     |  2 +
 sonic_installer/main.py                      | 25 ++++++
 tests/sign_and_verify_test.py                | 70 +++++++++++++++
 8 files changed, 368 insertions(+)
 create mode 100755 scripts/create_mock_image.sh
 create mode 100755 scripts/create_sign_and_verify_test_files.sh
 create mode 100644 scripts/verify_image_sign.sh
 create mode 100755 scripts/verify_image_sign_common.sh
 create mode 100755 scripts/verify_image_sign_test.sh
 create mode 100644 tests/sign_and_verify_test.py

diff --git a/scripts/create_mock_image.sh b/scripts/create_mock_image.sh
new file mode 100755
index 0000000000..f23032af0d
--- /dev/null
+++ b/scripts/create_mock_image.sh
@@ -0,0 +1,40 @@
+repo_dir=$1
+input_image=$2
+output_file=$3
+cert_file=$4
+key_file=$5
+tmp_dir=
+clean_up()
+{
+    sudo rm -rf $tmp_dir
+    sudo rm -rf $output_file
+    exit $1
+}
+
+DIR="$(dirname "$0")"
+
+tmp_dir=$(mktemp -d)
+sha1=$(cat $input_image | sha1sum | awk '{print $1}')
+echo -n "."
+cp $repo_dir/installer/sharch_body.sh $output_file || {
+    echo "Error: Problems copying sharch_body.sh"
+    clean_up 1
+}
+# Replace variables in the sharch template
+sed -i -e "s/%%IMAGE_SHA1%%/$sha1/" $output_file
+echo -n "."
+tar_size="$(wc -c < "${input_image}")"
+cat $input_image >> $output_file
+sed -i -e "s|%%PAYLOAD_IMAGE_SIZE%%|${tar_size}|" ${output_file}
+CMS_SIG="${tmp_dir}/signature.sig"
+
+echo "$0 CMS signing ${input_image} with ${key_file}. Output file ${output_file}"
+. $repo_dir/scripts/sign_image_dev.sh
+sign_image_dev  ${cert_file} ${key_file} $output_file ${CMS_SIG} || clean_up 1
+    
+cat ${CMS_SIG} >> ${output_file}
+echo "Signature done."
+# append signature to binary
+sudo rm -rf ${CMS_SIG}
+sudo rm -rf $tmp_dir
+exit 0
diff --git a/scripts/create_sign_and_verify_test_files.sh b/scripts/create_sign_and_verify_test_files.sh
new file mode 100755
index 0000000000..33f8b12fcd
--- /dev/null
+++ b/scripts/create_sign_and_verify_test_files.sh
@@ -0,0 +1,93 @@
+repo_dir=$1
+out_dir=$2
+mock_image="mock_img.bin"
+output_file=$out_dir/output_file.bin
+cert_file=$3
+other_cert_file=$4
+tmp_dir=
+clean_up()
+{
+    sudo rm -rf $tmp_dir
+    sudo rm -rf $mock_image
+    exit $1
+}
+DIR="$(dirname "$0")"
+[ -d $out_dir ] || rm -rf $out_dir
+mkdir $out_dir
+tmp_dir=$(mktemp -d)
+#generate self signed keys and certificate
+key_file=$tmp_dir/private-key.pem
+pub_key_file=$tmp_dir/public-key.pem
+openssl ecparam -name secp256r1 -genkey -noout -out $key_file
+openssl ec -in $key_file -pubout -out $pub_key_file
+openssl req -new -x509 -key $key_file -out $cert_file -days 360 -subj "/C=US/ST=Test/L=Test/O=Test/CN=Test"
+alt_key_file=$tmp_dir/alt-private-key.pem
+alt_pub_key_file=$tmp_dir/alt-public-key.pem
+openssl ecparam -name secp256r1 -genkey -noout -out $alt_key_file
+openssl ec -in $alt_key_file -pubout -out $alt_pub_key_file
+openssl req -new -x509 -key $alt_key_file -out $other_cert_file -days 360 -subj "/C=US/ST=Test/L=Test/O=Test/CN=Test"
+
+echo "this is a mock image\nThis is another line !2#4%6\n" > $mock_image
+echo "Created a mock image with following text:"
+cat $mock_image
+# create signed mock image
+
+sh $DIR/create_mock_image.sh $repo_dir $mock_image $output_file $cert_file $key_file || {
+    echo "Error: unable to create mock image"   
+    clean_up 1
+}
+
+[ -f "$output_file" ] || {
+    echo "signed mock image not created - exiting without testing"
+    clean_up 1
+}
+
+test_image_1=$out_dir/test_image_1.bin
+cp -v $output_file $test_image_1 || {
+    echo "Error: Problems copying image"
+    clean_up 1
+}
+
+# test_image_1 = modified image size to something else - should fail on signature verification
+image_size=$(sed -n 's/^payload_image_size=\(.*\)/\1/p' < $test_image_1)
+sed -i "/payload_image_size=/c\payload_image_size=$(($image_size - 5))" $test_image_1
+
+test_image_2=$out_dir/test_image_2.bin
+cp -v $output_file $test_image_2 || {
+    echo "Error: Problems copying image"
+    clean_up 1
+}
+
+# test_image_2 = modified image sha1 to other sha1 value - should fail on signature verification
+im_sha=$(sed -n 's/^payload_sha1=\(.*\)/\1/p' < $test_image_2)
+sed -i "/payload_sha1=/c\payload_sha1=2f1bbd5a0d411253103e688e4e66c00c94bedd40" $test_image_2
+
+tmp_image=$tmp_dir/"tmp_image.bin"
+echo "this is a different image now" >> $mock_image
+sh $DIR/create_mock_image.sh $repo_dir $mock_image $tmp_image $cert_file $key_file || { #TODO modify mock image instead of adding new signature
+    echo "Error: unable to create mock image"   
+    clean_up 1
+}
+# test_image_3 = original mock image with wrong signature
+# Extract cms signature from signed file
+test_image_3=$out_dir/"test_image_3.bin"
+tmp_sig="${tmp_dir}/tmp_sig.sig"
+TMP_TAR_SIZE=$(head -n 50 $tmp_image | grep "payload_image_size=" | cut -d"=" -f2- )
+sed -e '1,/^exit_marker$/d' $tmp_image | tail -c +$(( $TMP_TAR_SIZE + 1 )) > $tmp_sig
+
+TAR_SIZE=$(head -n 50 $output_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $output_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+head -c $SIG_PAYLOAD_SIZE $output_file > $test_image_3
+sudo rm -rf $tmp_image
+
+cat ${tmp_sig} >> ${test_image_3}
+
+# test_image_4 = modified image with original mock image signature
+test_image_4=$out_dir/"test_image_4.bin"
+tmp_sig2="${tmp_dir}/tmp_sig2.sig"
+head -c $SIG_PAYLOAD_SIZE $output_file > $test_image_4
+echo "this is additional line" >> $test_image_4
+sed -e '1,/^exit_marker$/d' $output_file | tail -c +$(( $TAR_SIZE + 1 )) > $tmp_sig2
+cat ${tmp_sig} >> ${test_image_4}
+clean_up 0
\ No newline at end of file
diff --git a/scripts/verify_image_sign.sh b/scripts/verify_image_sign.sh
new file mode 100644
index 0000000000..4599abe148
--- /dev/null
+++ b/scripts/verify_image_sign.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+image_file="${1}"
+cms_sig_file="sig.cms"
+lines_for_lookup=50
+SECURE_UPGRADE_ENABLED=0
+DIR="$(dirname "$0")"
+if [ -d "/sys/firmware/efi/efivars" ]; then
+    if ! [ -n "$(ls -A /sys/firmware/efi/efivars 2>/dev/null)" ]; then
+        mount -t efivarfs none /sys/firmware/efi/efivars 2>/dev/null
+    fi
+    SECURE_UPGRADE_ENABLED=$(bootctl status 2>/dev/null | grep -c "Secure Boot: enabled")
+else
+    echo "efi not supported - exiting without verification"
+    exit 0
+fi
+
+. /usr/local/bin/verify_image_common.sh
+
+if [ ${SECURE_UPGRADE_ENABLED} -eq 0 ]; then
+    echo "secure boot not enabled - exiting without image verification"
+    exit 0
+fi
+
+clean_up ()
+{
+    if  [ -d ${EFI_CERTS_DIR} ]; then rm -rf ${EFI_CERTS_DIR}; fi
+    if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+    exit $?
+}
+
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file
+# Add extra byte for payload
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+# verify signature with certificate fetched with efi tools
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+efi-readvar -v db -o $EFI_CERTS_DIR/db_efi >/dev/null || 
+{
+    echo "Error: unable to read certs from efi db: $?"
+    clean_up 1
+}
+# Convert one file to der certificates
+sig-list-to-certs $EFI_CERTS_DIR/db_efi $EFI_CERTS_DIR/db >/dev/null||
+{
+    echo "Error: convert sig list to certs: $?"
+    clean_up 1
+}
+for file in $(ls $EFI_CERTS_DIR | grep "db-"); do
+    LOG=$(openssl  x509 -in $EFI_CERTS_DIR/$file -inform der -out $EFI_CERTS_DIR/cert.pem 2>&1)
+    if [ $? -ne 0 ]; then
+        logger "cms_validation: $LOG"
+    fi
+    # Verify detached signature
+    LOG=$(verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE)
+    VALIDATION_RES=$?
+    if [ $VALIDATION_RES -eq 0 ]; then
+        RESULT="CMS Verified OK this is using efi keys"
+        echo "verification ok:$RESULT"
+        # No need to continue.
+        # Exit without error if any success signature verification.
+        clean_up 0
+    fi
+done
+echo "Error: image not verified $LOG"
+
+clean_up 1
\ No newline at end of file
diff --git a/scripts/verify_image_sign_common.sh b/scripts/verify_image_sign_common.sh
new file mode 100755
index 0000000000..cf58875b62
--- /dev/null
+++ b/scripts/verify_image_sign_common.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+verify_image_sign_common() {
+image_file="${1}"
+cms_sig_file="sig.cms"
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${2}"
+CMS_SIG_FILE="${3}"
+
+openssl version | awk '$2 ~ /(^0\.)|(^1\.(0\.|1\.0))/ { exit 1 }'
+if [ $? -eq 0 ]; then
+    # for version 1.1.1 and later
+    no_check_time="-no_check_time"
+else
+    # for version older than 1.1.1 use noattr
+    no_check_time="-noattr"
+fi
+
+# making sure image verification is supported
+EFI_CERTS_DIR=/tmp/efi_certs
+RESULT="CMS Verification Failure"
+LOG=$(openssl cms -verify $no_check_time -noout -CAfile $EFI_CERTS_DIR/cert.pem -binary -in ${CMS_SIG_FILE} -content ${DATA_FILE} -inform pem 2>&1 > /dev/null )
+VALIDATION_RES=$?
+if [ $VALIDATION_RES -eq 0 ]; then
+    RESULT="CMS Verified OK this is using efi keys"
+    if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+    echo "verification ok:$RESULT"
+    # No need to continue.
+    # Exit without error if any success signature verification.
+    return 0
+fi
+
+if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+return 1
+}
\ No newline at end of file
diff --git a/scripts/verify_image_sign_test.sh b/scripts/verify_image_sign_test.sh
new file mode 100755
index 0000000000..f4abd2584f
--- /dev/null
+++ b/scripts/verify_image_sign_test.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+image_file="${1}"
+cert_path="${2}"
+cms_sig_file="sig.cms"
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+lines_for_lookup=50
+
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file - exit marker marks last sharch prefix + number of image lines + 1 for next linel
+# Add extra byte for payload - extracting image signature from line after data file
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+cp $cert_path $EFI_CERTS_DIR/cert.pem
+
+DIR="$(dirname "$0")"
+. $DIR/verify_image_sign_common.sh
+verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE
+VERIFICATION_RES=$?
+if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+exit $VERIFICATION_RES
\ No newline at end of file
diff --git a/setup.py b/setup.py
index 7f617905da..8f9414a7f0 100644
--- a/setup.py
+++ b/setup.py
@@ -151,6 +151,8 @@
         'scripts/memory_threshold_check_handler.py',
         'scripts/techsupport_cleanup.py',
         'scripts/storm_control.py',
+        'scripts/verify_image_sign.sh',
+        'scripts/verify_image_sign_common.sh',
         'scripts/check_db_integrity.py',
         'scripts/sysreadyshow'
     ],
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index db3fe49827..b59ae9055d 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -567,6 +567,14 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
                 "Aborting...", LOG_ERR)
             raise click.Abort()
 
+        # Verify image signature by default (in sonic there will be a flag here)
+        echo_and_log("Verifing image {} signature...".format(binary_image_version))
+        if not _verify_signature(image_path):
+            echo_and_log('Error: Failed verify image signature', LOG_ERR)
+            raise click.Abort()
+        else:
+            echo_and_log('Verification successful')
+
         echo_and_log("Installing image {} and setting it as default...".format(binary_image_version))
         with SWAPAllocator(not skip_setup_swap, swap_mem_size, total_mem_threshold, available_mem_threshold):
             bootloader.install_image(image_path)
@@ -949,5 +957,22 @@ def verify_next_image():
         sys.exit(1)
     click.echo('Image successfully verified')
 
+
+def _verify_signature(image_path):
+    script_path = os.path.join('usr', 'local', 'bin', 'verify_image_sign.sh')
+    if not os.path.exists(script_path):
+        echo_and_log("No need to verify mock image")
+        return True
+    verification_result = subprocess.Popen([script_path, image_path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    stdout, stderr = verification_result.communicate()
+    if verification_result.returncode != 0:
+        echo_and_log(stdout, LOG_ERR)
+        echo_and_log(stderr, LOG_ERR)
+    else:
+        echo_and_log(stdout)
+        echo_and_log(stderr)
+    return verification_result.returncode == 0
+
+
 if __name__ == '__main__':
     sonic_installer()
diff --git a/tests/sign_and_verify_test.py b/tests/sign_and_verify_test.py
new file mode 100644
index 0000000000..2b95c9d4eb
--- /dev/null
+++ b/tests/sign_and_verify_test.py
@@ -0,0 +1,70 @@
+
+import subprocess
+import os
+import sys
+import shutil
+
+
+class TestSignVerify(object):
+    def _run_verification_script_and_check(self, image, cert_file_path, success_str, expected_value=0):
+        res = subprocess.run(['sh', self._verification_script, image, cert_file_path])
+        assert res.returncode == expected_value
+        print(success_str)
+
+    def test_basic_signature_verification(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'output_file.bin'),
+                                                self._cert_file_path, "test case 1 - basic verify signature - SUCCESS")
+
+    # change image size to something else - should fail on signature verification
+    def test_modified_image_size(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_1.bin'),
+                                                self._cert_file_path, "test case 2 - modified image size - SUCCESS", 1)
+
+    def test_modified_image_sha1(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_2.bin'),
+                                                self._cert_file_path, "test case 3 - modified image sha1 - SUCCESS", 1)
+
+    def test_modified_image_data(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_3.bin'),
+                                                self._cert_file_path, "test case 4 - modified image data - SUCCESS", 1)
+
+    def test_modified_image_signature(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_4.bin'),
+                                                self._cert_file_path, "test case 5 - modified image data - SUCCESS", 1)
+
+    def test_verify_image_with_wrong_certificate(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'output_file.bin'),
+                                                self._alt_cert_path, "test case 6 - verify with wrong signature - SUCCESS", 1)
+
+    def __init__(self):
+        self._test_path = os.path.dirname(os.path.abspath(__file__))
+        self._modules_path = os.path.dirname(self._test_path)
+        self._repo_path = os.path.join(self._modules_path, '../..')
+        self._scripts_path = os.path.join(self._modules_path, "scripts")
+        sys.path.insert(0, self._test_path)
+        sys.path.insert(0, self._modules_path)
+        sys.path.insert(0, self._scripts_path)
+        script_path = os.path.join(self._scripts_path, 'create_sign_and_verify_test_files.sh')
+        self._verification_script = os.path.join(self._scripts_path, 'verify_image_sign_test.sh')
+        self._out_dir_path = '/tmp/sign_verify_test'
+        self._cert_file_path = os.path.join(self._out_dir_path, 'self_certificate.pem')
+        self._alt_cert_path = os.path.join(self._out_dir_path, 'alt_self_certificate.pem')
+        create_files_result = subprocess.run(['sh', script_path, self._repo_path, self._out_dir_path,
+                                              self._cert_file_path,
+                                              self._alt_cert_path])
+        print(create_files_result)
+        assert create_files_result.returncode == 0
+
+    def __del__(self):
+        shutil.rmtree(self._out_dir_path)
+
+
+if __name__ == '__main__':
+    t = TestSignVerify()
+    t.test_basic_signature_verification()
+    subprocess.run(['ls', '/tmp/sign_verify_test'])
+    t.test_modified_image_data()
+    t.test_modified_image_sha1()
+    t.test_modified_image_signature()
+    t.test_modified_image_size()
+    t.test_verify_image_with_wrong_certificate()

From 6504dd89b7fb754c138fc9e8bff3cc8a5c085f53 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Tue, 23 Aug 2022 11:06:56 +0300
Subject: [PATCH 02/14] fixed minor issues

---
 scripts/verify_image_sign.sh |  2 +-
 sonic_installer/main.py      | 15 ++++++++-------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/scripts/verify_image_sign.sh b/scripts/verify_image_sign.sh
index 4599abe148..40c1db5728 100644
--- a/scripts/verify_image_sign.sh
+++ b/scripts/verify_image_sign.sh
@@ -14,7 +14,7 @@ else
     exit 0
 fi
 
-. /usr/local/bin/verify_image_common.sh
+. /usr/local/bin/verify_image_sign_common.sh
 
 if [ ${SECURE_UPGRADE_ENABLED} -eq 0 ]; then
     echo "secure boot not enabled - exiting without image verification"
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index b59ae9055d..ebbf4966ea 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -959,18 +959,19 @@ def verify_next_image():
 
 
 def _verify_signature(image_path):
-    script_path = os.path.join('usr', 'local', 'bin', 'verify_image_sign.sh')
+    verification_script_name = 'verify_image_sign.sh'
+    script_path = os.path.join('/usr', 'local', 'bin', verification_script_name)
     if not os.path.exists(script_path):
-        echo_and_log("No need to verify mock image")
-        return True
+        script_path = os.path.join(os.path.dirname(__file__), '..', 'scripts', verification_script_name)
+        if not os.path.exists(script_path):
+            echo_and_log("Unable to find verification script")
+            return False
     verification_result = subprocess.Popen([script_path, image_path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     stdout, stderr = verification_result.communicate()
     if verification_result.returncode != 0:
-        echo_and_log(stdout, LOG_ERR)
-        echo_and_log(stderr, LOG_ERR)
+        echo_and_log(str(stdout) + " " + str(stderr), LOG_ERR)
     else:
-        echo_and_log(stdout)
-        echo_and_log(stderr)
+        echo_and_log(str(stdout) + " " + str(stderr))
     return verification_result.returncode == 0
 
 

From 3efcc950a9b763c4a96555bc581f5c9db46b63d7 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Tue, 23 Aug 2022 15:10:30 +0300
Subject: [PATCH 03/14] minor fixes after review

---
 scripts/create_sign_and_verify_test_files.sh | 4 +---
 scripts/verify_image_sign_common.sh          | 2 +-
 sonic_installer/main.py                      | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/create_sign_and_verify_test_files.sh b/scripts/create_sign_and_verify_test_files.sh
index 33f8b12fcd..0040c04a7a 100755
--- a/scripts/create_sign_and_verify_test_files.sh
+++ b/scripts/create_sign_and_verify_test_files.sh
@@ -64,7 +64,7 @@ sed -i "/payload_sha1=/c\payload_sha1=2f1bbd5a0d411253103e688e4e66c00c94bedd40"
 
 tmp_image=$tmp_dir/"tmp_image.bin"
 echo "this is a different image now" >> $mock_image
-sh $DIR/create_mock_image.sh $repo_dir $mock_image $tmp_image $cert_file $key_file || { #TODO modify mock image instead of adding new signature
+sh $DIR/create_mock_image.sh $repo_dir $mock_image $tmp_image $cert_file $key_file || {
     echo "Error: unable to create mock image"   
     clean_up 1
 }
@@ -85,9 +85,7 @@ cat ${tmp_sig} >> ${test_image_3}
 
 # test_image_4 = modified image with original mock image signature
 test_image_4=$out_dir/"test_image_4.bin"
-tmp_sig2="${tmp_dir}/tmp_sig2.sig"
 head -c $SIG_PAYLOAD_SIZE $output_file > $test_image_4
 echo "this is additional line" >> $test_image_4
-sed -e '1,/^exit_marker$/d' $output_file | tail -c +$(( $TAR_SIZE + 1 )) > $tmp_sig2
 cat ${tmp_sig} >> ${test_image_4}
 clean_up 0
\ No newline at end of file
diff --git a/scripts/verify_image_sign_common.sh b/scripts/verify_image_sign_common.sh
index cf58875b62..09af38291e 100755
--- a/scripts/verify_image_sign_common.sh
+++ b/scripts/verify_image_sign_common.sh
@@ -31,4 +31,4 @@ fi
 
 if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
 return 1
-}
\ No newline at end of file
+}
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index ebbf4966ea..c20fa658a6 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -567,7 +567,7 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
                 "Aborting...", LOG_ERR)
             raise click.Abort()
 
-        # Verify image signature by default (in sonic there will be a flag here)
+        # Calling verification script by default - signature will be checked if enabled in bios
         echo_and_log("Verifing image {} signature...".format(binary_image_version))
         if not _verify_signature(image_path):
             echo_and_log('Error: Failed verify image signature', LOG_ERR)

From 22d197c72af57f800098da9d7ff36d57ca6b5380 Mon Sep 17 00:00:00 2001
From: ycoheNvidia <99744138+ycoheNvidia@users.noreply.github.com>
Date: Thu, 25 Aug 2022 16:49:56 +0300
Subject: [PATCH 04/14] Update main.py

---
 sonic_installer/main.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index c20fa658a6..656d99f972 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -502,7 +502,8 @@ def sonic_installer():
 @click.option('-y', '--yes', is_flag=True, callback=abort_if_false,
               expose_value=False, prompt='New image will be installed, continue?')
 @click.option('-f', '--force', '--skip-secure-check', is_flag=True,
-              help="Force installation of an image of a non-secure type than secure running image")
+              help="Force installation of an image of a non-secure type than secure running image,
+              this flag does not affect secure upgrade image verification")
 @click.option('--skip-platform-check', is_flag=True,
               help="Force installation of an image of a type which is not of the same platform")
 @click.option('--skip_migration', is_flag=True,

From 311b70224ed14f9d1592508af70b1e3be16a7e0f Mon Sep 17 00:00:00 2001
From: ycoheNvidia <99744138+ycoheNvidia@users.noreply.github.com>
Date: Sun, 28 Aug 2022 11:24:33 +0300
Subject: [PATCH 05/14] Minor syntax issue fixed

---
 sonic_installer/main.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index 656d99f972..789378ef95 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -502,8 +502,8 @@ def sonic_installer():
 @click.option('-y', '--yes', is_flag=True, callback=abort_if_false,
               expose_value=False, prompt='New image will be installed, continue?')
 @click.option('-f', '--force', '--skip-secure-check', is_flag=True,
-              help="Force installation of an image of a non-secure type than secure running image,
-              this flag does not affect secure upgrade image verification")
+              help="Force installation of an image of a non-secure type than secure running " +
+              " image, this flag does not affect secure upgrade image verification")
 @click.option('--skip-platform-check', is_flag=True,
               help="Force installation of an image of a type which is not of the same platform")
 @click.option('--skip_migration', is_flag=True,

From baa6918d5e4275e248885e61e5cc4f92c95a2ccc Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Tue, 30 Aug 2022 12:07:44 +0300
Subject: [PATCH 06/14] fixed sonic installer test issues

---
 sonic_installer/bootloader/grub.py | 10 ++++++++++
 sonic_installer/main.py            | 19 +------------------
 tests/test_sonic_installer.py      |  1 +
 3 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/sonic_installer/bootloader/grub.py b/sonic_installer/bootloader/grub.py
index 7ab5c6c0bc..cb18a74d90 100644
--- a/sonic_installer/bootloader/grub.py
+++ b/sonic_installer/bootloader/grub.py
@@ -153,6 +153,16 @@ def verify_image_platform(self, image_path):
         # Check if platform is inside image's target platforms
         return self.platform_in_platforms_asic(platform, image_path)
 
+    def verify_image_sign(self, image_path):
+        verification_script_name = 'verify_image_sign.sh'
+        script_path = os.path.join('/usr', 'local', 'bin', verification_script_name)
+        if not os.path.exists(script_path):
+            click.echo("Unable to find verification script in path " + script_path)
+            return False
+        verification_result = subprocess.run([script_path, image_path], capture_output=True)
+        click.echo(str(verification_result.stdout) + " " + str(verification_result.stderr))
+        return verification_result.returncode == 0
+
     @classmethod
     def detect(cls):
         return os.path.isfile(os.path.join(HOST_PATH, 'grub/grub.cfg'))
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index 789378ef95..f62bfda3b2 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -570,7 +570,7 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
 
         # Calling verification script by default - signature will be checked if enabled in bios
         echo_and_log("Verifing image {} signature...".format(binary_image_version))
-        if not _verify_signature(image_path):
+        if not bootloader.verify_image_sign(image_path):
             echo_and_log('Error: Failed verify image signature', LOG_ERR)
             raise click.Abort()
         else:
@@ -959,22 +959,5 @@ def verify_next_image():
     click.echo('Image successfully verified')
 
 
-def _verify_signature(image_path):
-    verification_script_name = 'verify_image_sign.sh'
-    script_path = os.path.join('/usr', 'local', 'bin', verification_script_name)
-    if not os.path.exists(script_path):
-        script_path = os.path.join(os.path.dirname(__file__), '..', 'scripts', verification_script_name)
-        if not os.path.exists(script_path):
-            echo_and_log("Unable to find verification script")
-            return False
-    verification_result = subprocess.Popen([script_path, image_path], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    stdout, stderr = verification_result.communicate()
-    if verification_result.returncode != 0:
-        echo_and_log(str(stdout) + " " + str(stderr), LOG_ERR)
-    else:
-        echo_and_log(str(stdout) + " " + str(stderr))
-    return verification_result.returncode == 0
-
-
 if __name__ == '__main__':
     sonic_installer()
diff --git a/tests/test_sonic_installer.py b/tests/test_sonic_installer.py
index c004bba9dc..2361679bb0 100644
--- a/tests/test_sonic_installer.py
+++ b/tests/test_sonic_installer.py
@@ -31,6 +31,7 @@ def test_install(run_command, run_command_or_raise, get_bootloader, swap, fs):
     mock_bootloader.get_binary_image_version = Mock(return_value=new_image_version)
     mock_bootloader.get_installed_images = Mock(return_value=[current_image_version])
     mock_bootloader.get_image_path = Mock(return_value=new_image_folder)
+    mock_bootloader.verify_image_sign = Mock(return_value=True)
 
     @contextmanager
     def rootfs_path_mock(path):

From 3cf593500b48470690c033771ea7bd090ea020ff Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Thu, 20 Oct 2022 11:56:56 +0300
Subject: [PATCH 07/14] Fixed coverage issues

---
 sonic_installer/bootloader/grub.py      | 1 +
 tests/installer_bootloader_grub_test.py | 8 ++++++++
 tests/test_sonic_installer.py           | 8 +++++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/sonic_installer/bootloader/grub.py b/sonic_installer/bootloader/grub.py
index cb18a74d90..dcafc3f840 100644
--- a/sonic_installer/bootloader/grub.py
+++ b/sonic_installer/bootloader/grub.py
@@ -154,6 +154,7 @@ def verify_image_platform(self, image_path):
         return self.platform_in_platforms_asic(platform, image_path)
 
     def verify_image_sign(self, image_path):
+        click.echo('Verifying image signature')
         verification_script_name = 'verify_image_sign.sh'
         script_path = os.path.join('/usr', 'local', 'bin', verification_script_name)
         if not os.path.exists(script_path):
diff --git a/tests/installer_bootloader_grub_test.py b/tests/installer_bootloader_grub_test.py
index ff35e13b37..10c9dc5ba7 100644
--- a/tests/installer_bootloader_grub_test.py
+++ b/tests/installer_bootloader_grub_test.py
@@ -53,3 +53,11 @@ def test_set_fips_grub():
 
     # Cleanup the _tmp_host folder
     shutil.rmtree(tmp_host_path)
+
+def test_verify_image():
+
+    bootloader = grub.GrubBootloader()
+    image = f'{grub.IMAGE_PREFIX}expeliarmus-{grub.IMAGE_PREFIX}abcde'
+
+    # command should fail
+    assert not bootloader.verify_image_sign(image)
diff --git a/tests/test_sonic_installer.py b/tests/test_sonic_installer.py
index 2361679bb0..7bf09836df 100644
--- a/tests/test_sonic_installer.py
+++ b/tests/test_sonic_installer.py
@@ -3,6 +3,7 @@
 from sonic_installer.main import sonic_installer
 from click.testing import CliRunner
 from unittest.mock import patch, Mock, call
+from sonic_installer.bootloader import GrubBootloader
 
 @patch("sonic_installer.main.SWAPAllocator")
 @patch("sonic_installer.main.get_bootloader")
@@ -32,7 +33,6 @@ def test_install(run_command, run_command_or_raise, get_bootloader, swap, fs):
     mock_bootloader.get_installed_images = Mock(return_value=[current_image_version])
     mock_bootloader.get_image_path = Mock(return_value=new_image_folder)
     mock_bootloader.verify_image_sign = Mock(return_value=True)
-
     @contextmanager
     def rootfs_path_mock(path):
         yield mounted_image_folder
@@ -46,7 +46,13 @@ def rootfs_path_mock(path):
     print(result.output)
 
     assert result.exit_code == 0
+    mock_bootloader_verify_image_sign_fail = mock_bootloader
+    mock_bootloader_verify_image_sign_fail.verify_image_sign = Mock(return_value=False)
+    get_bootloader.return_value=mock_bootloader_verify_image_sign_fail
+    result = runner.invoke(sonic_installer.commands["install"], [sonic_image_filename, "-y"])
+    print(result.output)
 
+    assert result.exit_code != 0
     # Assert bootloader install API was called
     mock_bootloader.install_image.assert_called_with(f"./{sonic_image_filename}")
     # Assert all below commands were called, so we ensure that

From e5f3d6f6d195f48f6286b0b954fbc112fdf67c30 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Wed, 9 Nov 2022 14:53:06 +0200
Subject: [PATCH 08/14] Fixed a bug regarding secure upgrade verify

Fixed return value from cms verification script used for secure upgrade.
Also modified and added prints to be similar to ONIE verification
---
 scripts/verify_image_sign.sh        | 6 +++---
 scripts/verify_image_sign_common.sh | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/verify_image_sign.sh b/scripts/verify_image_sign.sh
index 40c1db5728..d66148d597 100644
--- a/scripts/verify_image_sign.sh
+++ b/scripts/verify_image_sign.sh
@@ -25,7 +25,7 @@ clean_up ()
 {
     if  [ -d ${EFI_CERTS_DIR} ]; then rm -rf ${EFI_CERTS_DIR}; fi
     if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
-    exit $?
+    exit $1
 }
 
 TMP_DIR=$(mktemp -d)
@@ -63,13 +63,13 @@ for file in $(ls $EFI_CERTS_DIR | grep "db-"); do
     LOG=$(verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE)
     VALIDATION_RES=$?
     if [ $VALIDATION_RES -eq 0 ]; then
-        RESULT="CMS Verified OK this is using efi keys"
+        RESULT="CMS Verified OK using efi keys"
         echo "verification ok:$RESULT"
         # No need to continue.
         # Exit without error if any success signature verification.
         clean_up 0
     fi
 done
-echo "Error: image not verified $LOG"
+echo "Failure: CMS signature Verification Failed: $LOG"
 
 clean_up 1
\ No newline at end of file
diff --git a/scripts/verify_image_sign_common.sh b/scripts/verify_image_sign_common.sh
index 09af38291e..7e181103b0 100755
--- a/scripts/verify_image_sign_common.sh
+++ b/scripts/verify_image_sign_common.sh
@@ -21,7 +21,7 @@ RESULT="CMS Verification Failure"
 LOG=$(openssl cms -verify $no_check_time -noout -CAfile $EFI_CERTS_DIR/cert.pem -binary -in ${CMS_SIG_FILE} -content ${DATA_FILE} -inform pem 2>&1 > /dev/null )
 VALIDATION_RES=$?
 if [ $VALIDATION_RES -eq 0 ]; then
-    RESULT="CMS Verified OK this is using efi keys"
+    RESULT="CMS Verified OK"
     if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
     echo "verification ok:$RESULT"
     # No need to continue.

From f747fd1de58b2d6c7d6cf370184c500a833167fe Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Sun, 15 Jan 2023 15:50:07 +0200
Subject: [PATCH 09/14] Fixed issues raised in review

---
 scripts/verify_image_sign_common.sh           | 60 +++++++++----------
 .../scripts}/create_mock_image.sh             |  0
 .../create_sign_and_verify_test_files.sh      |  0
 .../scripts}/verify_image_sign_test.sh        |  0
 tests/sign_and_verify_test.py                 |  8 +--
 tests/verify_image_sign_test.sh               | 29 +++++++++
 6 files changed, 63 insertions(+), 34 deletions(-)
 rename {scripts => tests/scripts}/create_mock_image.sh (100%)
 rename {scripts => tests/scripts}/create_sign_and_verify_test_files.sh (100%)
 rename {scripts => tests/scripts}/verify_image_sign_test.sh (100%)
 create mode 100755 tests/verify_image_sign_test.sh

diff --git a/scripts/verify_image_sign_common.sh b/scripts/verify_image_sign_common.sh
index 7e181103b0..ec6511bc6d 100755
--- a/scripts/verify_image_sign_common.sh
+++ b/scripts/verify_image_sign_common.sh
@@ -1,34 +1,34 @@
 #!/bin/bash
 verify_image_sign_common() {
-image_file="${1}"
-cms_sig_file="sig.cms"
-TMP_DIR=$(mktemp -d)
-DATA_FILE="${2}"
-CMS_SIG_FILE="${3}"
-
-openssl version | awk '$2 ~ /(^0\.)|(^1\.(0\.|1\.0))/ { exit 1 }'
-if [ $? -eq 0 ]; then
-    # for version 1.1.1 and later
-    no_check_time="-no_check_time"
-else
-    # for version older than 1.1.1 use noattr
-    no_check_time="-noattr"
-fi
-
-# making sure image verification is supported
-EFI_CERTS_DIR=/tmp/efi_certs
-RESULT="CMS Verification Failure"
-LOG=$(openssl cms -verify $no_check_time -noout -CAfile $EFI_CERTS_DIR/cert.pem -binary -in ${CMS_SIG_FILE} -content ${DATA_FILE} -inform pem 2>&1 > /dev/null )
-VALIDATION_RES=$?
-if [ $VALIDATION_RES -eq 0 ]; then
-    RESULT="CMS Verified OK"
+    image_file="${1}"
+    cms_sig_file="sig.cms"
+    TMP_DIR=$(mktemp -d)
+    DATA_FILE="${2}"
+    CMS_SIG_FILE="${3}"
+    
+    openssl version | awk '$2 ~ /(^0\.)|(^1\.(0\.|1\.0))/ { exit 1 }'
+    if [ $? -eq 0 ]; then
+        # for version 1.1.1 and later
+        no_check_time="-no_check_time"
+    else
+        # for version older than 1.1.1 use noattr
+        no_check_time="-noattr"
+    fi
+    
+    # making sure image verification is supported
+    EFI_CERTS_DIR=/tmp/efi_certs
+    RESULT="CMS Verification Failure"
+    LOG=$(openssl cms -verify $no_check_time -noout -CAfile $EFI_CERTS_DIR/cert.pem -binary -in ${CMS_SIG_FILE} -content ${DATA_FILE} -inform pem 2>&1 > /dev/null )
+    VALIDATION_RES=$?
+    if [ $VALIDATION_RES -eq 0 ]; then
+        RESULT="CMS Verified OK"
+        if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+        echo "verification ok:$RESULT"
+        # No need to continue.
+        # Exit without error if any success signature verification.
+        return 0
+    fi
+    
     if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
-    echo "verification ok:$RESULT"
-    # No need to continue.
-    # Exit without error if any success signature verification.
-    return 0
-fi
-
-if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
-return 1
+    return 1
 }
diff --git a/scripts/create_mock_image.sh b/tests/scripts/create_mock_image.sh
similarity index 100%
rename from scripts/create_mock_image.sh
rename to tests/scripts/create_mock_image.sh
diff --git a/scripts/create_sign_and_verify_test_files.sh b/tests/scripts/create_sign_and_verify_test_files.sh
similarity index 100%
rename from scripts/create_sign_and_verify_test_files.sh
rename to tests/scripts/create_sign_and_verify_test_files.sh
diff --git a/scripts/verify_image_sign_test.sh b/tests/scripts/verify_image_sign_test.sh
similarity index 100%
rename from scripts/verify_image_sign_test.sh
rename to tests/scripts/verify_image_sign_test.sh
diff --git a/tests/sign_and_verify_test.py b/tests/sign_and_verify_test.py
index 2b95c9d4eb..77d58a4ac9 100644
--- a/tests/sign_and_verify_test.py
+++ b/tests/sign_and_verify_test.py
@@ -40,12 +40,12 @@ def __init__(self):
         self._test_path = os.path.dirname(os.path.abspath(__file__))
         self._modules_path = os.path.dirname(self._test_path)
         self._repo_path = os.path.join(self._modules_path, '../..')
-        self._scripts_path = os.path.join(self._modules_path, "scripts")
+        self._test_scripts_path = os.path.join(self._test_path, "scripts")
         sys.path.insert(0, self._test_path)
         sys.path.insert(0, self._modules_path)
-        sys.path.insert(0, self._scripts_path)
-        script_path = os.path.join(self._scripts_path, 'create_sign_and_verify_test_files.sh')
-        self._verification_script = os.path.join(self._scripts_path, 'verify_image_sign_test.sh')
+        sys.path.insert(0, self._test_scripts_path)
+        script_path = os.path.join(self._test_scripts_path, 'create_sign_and_verify_test_files.sh')
+        self._verification_script = os.path.join(self._test_scripts_path, 'verify_image_sign_test.sh')
         self._out_dir_path = '/tmp/sign_verify_test'
         self._cert_file_path = os.path.join(self._out_dir_path, 'self_certificate.pem')
         self._alt_cert_path = os.path.join(self._out_dir_path, 'alt_self_certificate.pem')
diff --git a/tests/verify_image_sign_test.sh b/tests/verify_image_sign_test.sh
new file mode 100755
index 0000000000..f4abd2584f
--- /dev/null
+++ b/tests/verify_image_sign_test.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+image_file="${1}"
+cert_path="${2}"
+cms_sig_file="sig.cms"
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+lines_for_lookup=50
+
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file - exit marker marks last sharch prefix + number of image lines + 1 for next linel
+# Add extra byte for payload - extracting image signature from line after data file
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+cp $cert_path $EFI_CERTS_DIR/cert.pem
+
+DIR="$(dirname "$0")"
+. $DIR/verify_image_sign_common.sh
+verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE
+VERIFICATION_RES=$?
+if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+exit $VERIFICATION_RES
\ No newline at end of file

From 30aaa864905ee8f98b947a034910d79cca0c6840 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Tue, 28 Feb 2023 09:55:48 +0200
Subject: [PATCH 10/14] Added bootloader fix for image verification

---
 sonic_installer/bootloader/bootloader.py | 4 ++++
 tests/installer_bootloader_aboot_test.py | 5 +++++
 tests/installer_bootloader_onie_test.py  | 4 ++++
 tests/installer_bootloader_uboot_test.py | 6 ++++++
 4 files changed, 19 insertions(+)

diff --git a/sonic_installer/bootloader/bootloader.py b/sonic_installer/bootloader/bootloader.py
index 91dcaf4665..3eeeb0b6af 100644
--- a/sonic_installer/bootloader/bootloader.py
+++ b/sonic_installer/bootloader/bootloader.py
@@ -75,6 +75,10 @@ def supports_package_migration(self, image):
         """tells if the image supports package migration"""
         return True
 
+    def verify_image_sign(self, image_path):
+        """verify image signature is valid"""
+        return True
+
     @classmethod
     def detect(cls):
         """returns True if the bootloader is in use"""
diff --git a/tests/installer_bootloader_aboot_test.py b/tests/installer_bootloader_aboot_test.py
index 56eee4872e..1bc1db91df 100644
--- a/tests/installer_bootloader_aboot_test.py
+++ b/tests/installer_bootloader_aboot_test.py
@@ -73,3 +73,8 @@ def test_set_fips_aboot():
 
     # Cleanup
     shutil.rmtree(dirpath)
+
+def test_verify_image_sign():
+    bootloader = aboot.AbootBootloader()
+
+    assert bootloader.verify_image_sign(exp_image) == True
diff --git a/tests/installer_bootloader_onie_test.py b/tests/installer_bootloader_onie_test.py
index f5c2d96f7d..f863a278be 100644
--- a/tests/installer_bootloader_onie_test.py
+++ b/tests/installer_bootloader_onie_test.py
@@ -15,3 +15,7 @@ def test_get_current_image(re_search):
     # Test image dir conversion
     onie.re.search().group = Mock(return_value=image)
     assert bootloader.get_current_image() == exp_image
+
+def test_verify_image_sign():
+    bootloader = onie.OnieInstallerBootloader()
+    assert bootloader.verify_image_sign('some_path.path') == True
diff --git a/tests/installer_bootloader_uboot_test.py b/tests/installer_bootloader_uboot_test.py
index b0fc7c61a7..30a1071866 100644
--- a/tests/installer_bootloader_uboot_test.py
+++ b/tests/installer_bootloader_uboot_test.py
@@ -63,3 +63,9 @@ def mock_run_command(cmd):
     # Test fips disabled
     bootloader.set_fips(image, False)
     assert not bootloader.get_fips(image)
+
+def test_verify_image_sign():
+    bootloader = uboot.UbootBootloader()
+    image = 'test-image'
+    # Test convertion image dir to image name
+    assert bootloader.verify_image_sign(image) == True

From d9b0543b4e16c4057f393bf2b7ac2112986b7948 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Thu, 16 Mar 2023 16:27:40 +0200
Subject: [PATCH 11/14] Revert "Revert "Secure upgrade (#2337)" (#2675)"

This reverts commit e98011f8d5bfa6ae20433e6ac54fc1597dae61de.
---
 scripts/verify_image_sign.sh                  | 75 +++++++++++++++
 scripts/verify_image_sign_common.sh           | 34 +++++++
 setup.py                                      |  2 +
 sonic_installer/bootloader/grub.py            | 11 +++
 sonic_installer/main.py                       | 12 ++-
 tests/installer_bootloader_grub_test.py       |  8 ++
 tests/scripts/create_mock_image.sh            | 40 ++++++++
 .../create_sign_and_verify_test_files.sh      | 91 +++++++++++++++++++
 tests/scripts/verify_image_sign_test.sh       | 29 ++++++
 tests/sign_and_verify_test.py                 | 70 ++++++++++++++
 tests/test_sonic_installer.py                 |  9 +-
 tests/verify_image_sign_test.sh               | 29 ++++++
 12 files changed, 408 insertions(+), 2 deletions(-)
 create mode 100644 scripts/verify_image_sign.sh
 create mode 100755 scripts/verify_image_sign_common.sh
 create mode 100755 tests/scripts/create_mock_image.sh
 create mode 100755 tests/scripts/create_sign_and_verify_test_files.sh
 create mode 100755 tests/scripts/verify_image_sign_test.sh
 create mode 100644 tests/sign_and_verify_test.py
 create mode 100755 tests/verify_image_sign_test.sh

diff --git a/scripts/verify_image_sign.sh b/scripts/verify_image_sign.sh
new file mode 100644
index 0000000000..d66148d597
--- /dev/null
+++ b/scripts/verify_image_sign.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+image_file="${1}"
+cms_sig_file="sig.cms"
+lines_for_lookup=50
+SECURE_UPGRADE_ENABLED=0
+DIR="$(dirname "$0")"
+if [ -d "/sys/firmware/efi/efivars" ]; then
+    if ! [ -n "$(ls -A /sys/firmware/efi/efivars 2>/dev/null)" ]; then
+        mount -t efivarfs none /sys/firmware/efi/efivars 2>/dev/null
+    fi
+    SECURE_UPGRADE_ENABLED=$(bootctl status 2>/dev/null | grep -c "Secure Boot: enabled")
+else
+    echo "efi not supported - exiting without verification"
+    exit 0
+fi
+
+. /usr/local/bin/verify_image_sign_common.sh
+
+if [ ${SECURE_UPGRADE_ENABLED} -eq 0 ]; then
+    echo "secure boot not enabled - exiting without image verification"
+    exit 0
+fi
+
+clean_up ()
+{
+    if  [ -d ${EFI_CERTS_DIR} ]; then rm -rf ${EFI_CERTS_DIR}; fi
+    if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+    exit $1
+}
+
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file
+# Add extra byte for payload
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+# verify signature with certificate fetched with efi tools
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+efi-readvar -v db -o $EFI_CERTS_DIR/db_efi >/dev/null || 
+{
+    echo "Error: unable to read certs from efi db: $?"
+    clean_up 1
+}
+# Convert one file to der certificates
+sig-list-to-certs $EFI_CERTS_DIR/db_efi $EFI_CERTS_DIR/db >/dev/null||
+{
+    echo "Error: convert sig list to certs: $?"
+    clean_up 1
+}
+for file in $(ls $EFI_CERTS_DIR | grep "db-"); do
+    LOG=$(openssl  x509 -in $EFI_CERTS_DIR/$file -inform der -out $EFI_CERTS_DIR/cert.pem 2>&1)
+    if [ $? -ne 0 ]; then
+        logger "cms_validation: $LOG"
+    fi
+    # Verify detached signature
+    LOG=$(verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE)
+    VALIDATION_RES=$?
+    if [ $VALIDATION_RES -eq 0 ]; then
+        RESULT="CMS Verified OK using efi keys"
+        echo "verification ok:$RESULT"
+        # No need to continue.
+        # Exit without error if any success signature verification.
+        clean_up 0
+    fi
+done
+echo "Failure: CMS signature Verification Failed: $LOG"
+
+clean_up 1
\ No newline at end of file
diff --git a/scripts/verify_image_sign_common.sh b/scripts/verify_image_sign_common.sh
new file mode 100755
index 0000000000..ec6511bc6d
--- /dev/null
+++ b/scripts/verify_image_sign_common.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+verify_image_sign_common() {
+    image_file="${1}"
+    cms_sig_file="sig.cms"
+    TMP_DIR=$(mktemp -d)
+    DATA_FILE="${2}"
+    CMS_SIG_FILE="${3}"
+    
+    openssl version | awk '$2 ~ /(^0\.)|(^1\.(0\.|1\.0))/ { exit 1 }'
+    if [ $? -eq 0 ]; then
+        # for version 1.1.1 and later
+        no_check_time="-no_check_time"
+    else
+        # for version older than 1.1.1 use noattr
+        no_check_time="-noattr"
+    fi
+    
+    # making sure image verification is supported
+    EFI_CERTS_DIR=/tmp/efi_certs
+    RESULT="CMS Verification Failure"
+    LOG=$(openssl cms -verify $no_check_time -noout -CAfile $EFI_CERTS_DIR/cert.pem -binary -in ${CMS_SIG_FILE} -content ${DATA_FILE} -inform pem 2>&1 > /dev/null )
+    VALIDATION_RES=$?
+    if [ $VALIDATION_RES -eq 0 ]; then
+        RESULT="CMS Verified OK"
+        if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+        echo "verification ok:$RESULT"
+        # No need to continue.
+        # Exit without error if any success signature verification.
+        return 0
+    fi
+    
+    if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+    return 1
+}
diff --git a/setup.py b/setup.py
index 70d7473bd7..231b80c8ed 100644
--- a/setup.py
+++ b/setup.py
@@ -154,6 +154,8 @@
         'scripts/memory_threshold_check_handler.py',
         'scripts/techsupport_cleanup.py',
         'scripts/storm_control.py',
+        'scripts/verify_image_sign.sh',
+        'scripts/verify_image_sign_common.sh',
         'scripts/check_db_integrity.py',
         'scripts/sysreadyshow'
     ],
diff --git a/sonic_installer/bootloader/grub.py b/sonic_installer/bootloader/grub.py
index 7ab5c6c0bc..dcafc3f840 100644
--- a/sonic_installer/bootloader/grub.py
+++ b/sonic_installer/bootloader/grub.py
@@ -153,6 +153,17 @@ def verify_image_platform(self, image_path):
         # Check if platform is inside image's target platforms
         return self.platform_in_platforms_asic(platform, image_path)
 
+    def verify_image_sign(self, image_path):
+        click.echo('Verifying image signature')
+        verification_script_name = 'verify_image_sign.sh'
+        script_path = os.path.join('/usr', 'local', 'bin', verification_script_name)
+        if not os.path.exists(script_path):
+            click.echo("Unable to find verification script in path " + script_path)
+            return False
+        verification_result = subprocess.run([script_path, image_path], capture_output=True)
+        click.echo(str(verification_result.stdout) + " " + str(verification_result.stderr))
+        return verification_result.returncode == 0
+
     @classmethod
     def detect(cls):
         return os.path.isfile(os.path.join(HOST_PATH, 'grub/grub.cfg'))
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index ce1c15866d..d78259317e 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -511,7 +511,8 @@ def sonic_installer():
 @click.option('-y', '--yes', is_flag=True, callback=abort_if_false,
               expose_value=False, prompt='New image will be installed, continue?')
 @click.option('-f', '--force', '--skip-secure-check', is_flag=True,
-              help="Force installation of an image of a non-secure type than secure running image")
+              help="Force installation of an image of a non-secure type than secure running " +
+              " image, this flag does not affect secure upgrade image verification")
 @click.option('--skip-platform-check', is_flag=True,
               help="Force installation of an image of a type which is not of the same platform")
 @click.option('--skip_migration', is_flag=True,
@@ -576,6 +577,14 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
                 "Aborting...", LOG_ERR)
             raise click.Abort()
 
+        # Calling verification script by default - signature will be checked if enabled in bios
+        echo_and_log("Verifing image {} signature...".format(binary_image_version))
+        if not bootloader.verify_image_sign(image_path):
+            echo_and_log('Error: Failed verify image signature', LOG_ERR)
+            raise click.Abort()
+        else:
+            echo_and_log('Verification successful')
+
         echo_and_log("Installing image {} and setting it as default...".format(binary_image_version))
         with SWAPAllocator(not skip_setup_swap, swap_mem_size, total_mem_threshold, available_mem_threshold):
             bootloader.install_image(image_path)
@@ -958,5 +967,6 @@ def verify_next_image():
         sys.exit(1)
     click.echo('Image successfully verified')
 
+
 if __name__ == '__main__':
     sonic_installer()
diff --git a/tests/installer_bootloader_grub_test.py b/tests/installer_bootloader_grub_test.py
index ff35e13b37..10c9dc5ba7 100644
--- a/tests/installer_bootloader_grub_test.py
+++ b/tests/installer_bootloader_grub_test.py
@@ -53,3 +53,11 @@ def test_set_fips_grub():
 
     # Cleanup the _tmp_host folder
     shutil.rmtree(tmp_host_path)
+
+def test_verify_image():
+
+    bootloader = grub.GrubBootloader()
+    image = f'{grub.IMAGE_PREFIX}expeliarmus-{grub.IMAGE_PREFIX}abcde'
+
+    # command should fail
+    assert not bootloader.verify_image_sign(image)
diff --git a/tests/scripts/create_mock_image.sh b/tests/scripts/create_mock_image.sh
new file mode 100755
index 0000000000..f23032af0d
--- /dev/null
+++ b/tests/scripts/create_mock_image.sh
@@ -0,0 +1,40 @@
+repo_dir=$1
+input_image=$2
+output_file=$3
+cert_file=$4
+key_file=$5
+tmp_dir=
+clean_up()
+{
+    sudo rm -rf $tmp_dir
+    sudo rm -rf $output_file
+    exit $1
+}
+
+DIR="$(dirname "$0")"
+
+tmp_dir=$(mktemp -d)
+sha1=$(cat $input_image | sha1sum | awk '{print $1}')
+echo -n "."
+cp $repo_dir/installer/sharch_body.sh $output_file || {
+    echo "Error: Problems copying sharch_body.sh"
+    clean_up 1
+}
+# Replace variables in the sharch template
+sed -i -e "s/%%IMAGE_SHA1%%/$sha1/" $output_file
+echo -n "."
+tar_size="$(wc -c < "${input_image}")"
+cat $input_image >> $output_file
+sed -i -e "s|%%PAYLOAD_IMAGE_SIZE%%|${tar_size}|" ${output_file}
+CMS_SIG="${tmp_dir}/signature.sig"
+
+echo "$0 CMS signing ${input_image} with ${key_file}. Output file ${output_file}"
+. $repo_dir/scripts/sign_image_dev.sh
+sign_image_dev  ${cert_file} ${key_file} $output_file ${CMS_SIG} || clean_up 1
+    
+cat ${CMS_SIG} >> ${output_file}
+echo "Signature done."
+# append signature to binary
+sudo rm -rf ${CMS_SIG}
+sudo rm -rf $tmp_dir
+exit 0
diff --git a/tests/scripts/create_sign_and_verify_test_files.sh b/tests/scripts/create_sign_and_verify_test_files.sh
new file mode 100755
index 0000000000..0040c04a7a
--- /dev/null
+++ b/tests/scripts/create_sign_and_verify_test_files.sh
@@ -0,0 +1,91 @@
+repo_dir=$1
+out_dir=$2
+mock_image="mock_img.bin"
+output_file=$out_dir/output_file.bin
+cert_file=$3
+other_cert_file=$4
+tmp_dir=
+clean_up()
+{
+    sudo rm -rf $tmp_dir
+    sudo rm -rf $mock_image
+    exit $1
+}
+DIR="$(dirname "$0")"
+[ -d $out_dir ] || rm -rf $out_dir
+mkdir $out_dir
+tmp_dir=$(mktemp -d)
+#generate self signed keys and certificate
+key_file=$tmp_dir/private-key.pem
+pub_key_file=$tmp_dir/public-key.pem
+openssl ecparam -name secp256r1 -genkey -noout -out $key_file
+openssl ec -in $key_file -pubout -out $pub_key_file
+openssl req -new -x509 -key $key_file -out $cert_file -days 360 -subj "/C=US/ST=Test/L=Test/O=Test/CN=Test"
+alt_key_file=$tmp_dir/alt-private-key.pem
+alt_pub_key_file=$tmp_dir/alt-public-key.pem
+openssl ecparam -name secp256r1 -genkey -noout -out $alt_key_file
+openssl ec -in $alt_key_file -pubout -out $alt_pub_key_file
+openssl req -new -x509 -key $alt_key_file -out $other_cert_file -days 360 -subj "/C=US/ST=Test/L=Test/O=Test/CN=Test"
+
+echo "this is a mock image\nThis is another line !2#4%6\n" > $mock_image
+echo "Created a mock image with following text:"
+cat $mock_image
+# create signed mock image
+
+sh $DIR/create_mock_image.sh $repo_dir $mock_image $output_file $cert_file $key_file || {
+    echo "Error: unable to create mock image"   
+    clean_up 1
+}
+
+[ -f "$output_file" ] || {
+    echo "signed mock image not created - exiting without testing"
+    clean_up 1
+}
+
+test_image_1=$out_dir/test_image_1.bin
+cp -v $output_file $test_image_1 || {
+    echo "Error: Problems copying image"
+    clean_up 1
+}
+
+# test_image_1 = modified image size to something else - should fail on signature verification
+image_size=$(sed -n 's/^payload_image_size=\(.*\)/\1/p' < $test_image_1)
+sed -i "/payload_image_size=/c\payload_image_size=$(($image_size - 5))" $test_image_1
+
+test_image_2=$out_dir/test_image_2.bin
+cp -v $output_file $test_image_2 || {
+    echo "Error: Problems copying image"
+    clean_up 1
+}
+
+# test_image_2 = modified image sha1 to other sha1 value - should fail on signature verification
+im_sha=$(sed -n 's/^payload_sha1=\(.*\)/\1/p' < $test_image_2)
+sed -i "/payload_sha1=/c\payload_sha1=2f1bbd5a0d411253103e688e4e66c00c94bedd40" $test_image_2
+
+tmp_image=$tmp_dir/"tmp_image.bin"
+echo "this is a different image now" >> $mock_image
+sh $DIR/create_mock_image.sh $repo_dir $mock_image $tmp_image $cert_file $key_file || {
+    echo "Error: unable to create mock image"   
+    clean_up 1
+}
+# test_image_3 = original mock image with wrong signature
+# Extract cms signature from signed file
+test_image_3=$out_dir/"test_image_3.bin"
+tmp_sig="${tmp_dir}/tmp_sig.sig"
+TMP_TAR_SIZE=$(head -n 50 $tmp_image | grep "payload_image_size=" | cut -d"=" -f2- )
+sed -e '1,/^exit_marker$/d' $tmp_image | tail -c +$(( $TMP_TAR_SIZE + 1 )) > $tmp_sig
+
+TAR_SIZE=$(head -n 50 $output_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $output_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+head -c $SIG_PAYLOAD_SIZE $output_file > $test_image_3
+sudo rm -rf $tmp_image
+
+cat ${tmp_sig} >> ${test_image_3}
+
+# test_image_4 = modified image with original mock image signature
+test_image_4=$out_dir/"test_image_4.bin"
+head -c $SIG_PAYLOAD_SIZE $output_file > $test_image_4
+echo "this is additional line" >> $test_image_4
+cat ${tmp_sig} >> ${test_image_4}
+clean_up 0
\ No newline at end of file
diff --git a/tests/scripts/verify_image_sign_test.sh b/tests/scripts/verify_image_sign_test.sh
new file mode 100755
index 0000000000..f4abd2584f
--- /dev/null
+++ b/tests/scripts/verify_image_sign_test.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+image_file="${1}"
+cert_path="${2}"
+cms_sig_file="sig.cms"
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+lines_for_lookup=50
+
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file - exit marker marks last sharch prefix + number of image lines + 1 for next linel
+# Add extra byte for payload - extracting image signature from line after data file
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+cp $cert_path $EFI_CERTS_DIR/cert.pem
+
+DIR="$(dirname "$0")"
+. $DIR/verify_image_sign_common.sh
+verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE
+VERIFICATION_RES=$?
+if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+exit $VERIFICATION_RES
\ No newline at end of file
diff --git a/tests/sign_and_verify_test.py b/tests/sign_and_verify_test.py
new file mode 100644
index 0000000000..77d58a4ac9
--- /dev/null
+++ b/tests/sign_and_verify_test.py
@@ -0,0 +1,70 @@
+
+import subprocess
+import os
+import sys
+import shutil
+
+
+class TestSignVerify(object):
+    def _run_verification_script_and_check(self, image, cert_file_path, success_str, expected_value=0):
+        res = subprocess.run(['sh', self._verification_script, image, cert_file_path])
+        assert res.returncode == expected_value
+        print(success_str)
+
+    def test_basic_signature_verification(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'output_file.bin'),
+                                                self._cert_file_path, "test case 1 - basic verify signature - SUCCESS")
+
+    # change image size to something else - should fail on signature verification
+    def test_modified_image_size(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_1.bin'),
+                                                self._cert_file_path, "test case 2 - modified image size - SUCCESS", 1)
+
+    def test_modified_image_sha1(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_2.bin'),
+                                                self._cert_file_path, "test case 3 - modified image sha1 - SUCCESS", 1)
+
+    def test_modified_image_data(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_3.bin'),
+                                                self._cert_file_path, "test case 4 - modified image data - SUCCESS", 1)
+
+    def test_modified_image_signature(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'test_image_4.bin'),
+                                                self._cert_file_path, "test case 5 - modified image data - SUCCESS", 1)
+
+    def test_verify_image_with_wrong_certificate(self):
+        self._run_verification_script_and_check(os.path.join(self._out_dir_path, 'output_file.bin'),
+                                                self._alt_cert_path, "test case 6 - verify with wrong signature - SUCCESS", 1)
+
+    def __init__(self):
+        self._test_path = os.path.dirname(os.path.abspath(__file__))
+        self._modules_path = os.path.dirname(self._test_path)
+        self._repo_path = os.path.join(self._modules_path, '../..')
+        self._test_scripts_path = os.path.join(self._test_path, "scripts")
+        sys.path.insert(0, self._test_path)
+        sys.path.insert(0, self._modules_path)
+        sys.path.insert(0, self._test_scripts_path)
+        script_path = os.path.join(self._test_scripts_path, 'create_sign_and_verify_test_files.sh')
+        self._verification_script = os.path.join(self._test_scripts_path, 'verify_image_sign_test.sh')
+        self._out_dir_path = '/tmp/sign_verify_test'
+        self._cert_file_path = os.path.join(self._out_dir_path, 'self_certificate.pem')
+        self._alt_cert_path = os.path.join(self._out_dir_path, 'alt_self_certificate.pem')
+        create_files_result = subprocess.run(['sh', script_path, self._repo_path, self._out_dir_path,
+                                              self._cert_file_path,
+                                              self._alt_cert_path])
+        print(create_files_result)
+        assert create_files_result.returncode == 0
+
+    def __del__(self):
+        shutil.rmtree(self._out_dir_path)
+
+
+if __name__ == '__main__':
+    t = TestSignVerify()
+    t.test_basic_signature_verification()
+    subprocess.run(['ls', '/tmp/sign_verify_test'])
+    t.test_modified_image_data()
+    t.test_modified_image_sha1()
+    t.test_modified_image_signature()
+    t.test_modified_image_size()
+    t.test_verify_image_with_wrong_certificate()
diff --git a/tests/test_sonic_installer.py b/tests/test_sonic_installer.py
index c445dfb6e3..0f8fcdb8ca 100644
--- a/tests/test_sonic_installer.py
+++ b/tests/test_sonic_installer.py
@@ -3,6 +3,7 @@
 from sonic_installer.main import sonic_installer
 from click.testing import CliRunner
 from unittest.mock import patch, Mock, call
+from sonic_installer.bootloader import GrubBootloader
 
 @patch("sonic_installer.main.SWAPAllocator")
 @patch("sonic_installer.main.get_bootloader")
@@ -31,7 +32,7 @@ def test_install(run_command, run_command_or_raise, get_bootloader, swap, fs):
     mock_bootloader.get_binary_image_version = Mock(return_value=new_image_version)
     mock_bootloader.get_installed_images = Mock(return_value=[current_image_version])
     mock_bootloader.get_image_path = Mock(return_value=new_image_folder)
-
+    mock_bootloader.verify_image_sign = Mock(return_value=True)
     @contextmanager
     def rootfs_path_mock(path):
         yield mounted_image_folder
@@ -45,7 +46,13 @@ def rootfs_path_mock(path):
     print(result.output)
 
     assert result.exit_code == 0
+    mock_bootloader_verify_image_sign_fail = mock_bootloader
+    mock_bootloader_verify_image_sign_fail.verify_image_sign = Mock(return_value=False)
+    get_bootloader.return_value=mock_bootloader_verify_image_sign_fail
+    result = runner.invoke(sonic_installer.commands["install"], [sonic_image_filename, "-y"])
+    print(result.output)
 
+    assert result.exit_code != 0
     # Assert bootloader install API was called
     mock_bootloader.install_image.assert_called_with(f"./{sonic_image_filename}")
     # Assert all below commands were called, so we ensure that
diff --git a/tests/verify_image_sign_test.sh b/tests/verify_image_sign_test.sh
new file mode 100755
index 0000000000..f4abd2584f
--- /dev/null
+++ b/tests/verify_image_sign_test.sh
@@ -0,0 +1,29 @@
+#!/bin/bash
+image_file="${1}"
+cert_path="${2}"
+cms_sig_file="sig.cms"
+TMP_DIR=$(mktemp -d)
+DATA_FILE="${TMP_DIR}/data.bin"
+CMS_SIG_FILE="${TMP_DIR}/${cms_sig_file}"
+lines_for_lookup=50
+
+TAR_SIZE=$(head -n $lines_for_lookup $image_file | grep "payload_image_size=" | cut -d"=" -f2- )
+SHARCH_SIZE=$(sed '/^exit_marker$/q' $image_file | wc -c)
+SIG_PAYLOAD_SIZE=$(($TAR_SIZE + $SHARCH_SIZE ))
+# Extract cms signature from signed file - exit marker marks last sharch prefix + number of image lines + 1 for next linel
+# Add extra byte for payload - extracting image signature from line after data file
+sed -e '1,/^exit_marker$/d' $image_file | tail -c +$(( $TAR_SIZE + 1 )) > $CMS_SIG_FILE
+# Extract image from signed file
+head -c $SIG_PAYLOAD_SIZE $image_file > $DATA_FILE
+EFI_CERTS_DIR=/tmp/efi_certs
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+mkdir $EFI_CERTS_DIR
+cp $cert_path $EFI_CERTS_DIR/cert.pem
+
+DIR="$(dirname "$0")"
+. $DIR/verify_image_sign_common.sh
+verify_image_sign_common $image_file $DATA_FILE $CMS_SIG_FILE
+VERIFICATION_RES=$?
+if  [ -d "${TMP_DIR}" ]; then rm -rf ${TMP_DIR}; fi
+[ -d $EFI_CERTS_DIR ] && rm  -rf $EFI_CERTS_DIR
+exit $VERIFICATION_RES
\ No newline at end of file

From f36bfe53b31ca74c746803823ac01caf8486c760 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Sun, 11 Jun 2023 14:57:26 +0300
Subject: [PATCH 12/14] Modified bootloader default image verification

Default bootloader image verification to raise NotImplementedError
Modified relevant tests as well
---
 sonic_installer/bootloader/bootloader.py |  2 +-
 sonic_installer/main.py                  | 14 ++++++++------
 tests/installer_bootloader_aboot_test.py |  9 +++++++--
 tests/installer_bootloader_onie_test.py  |  8 +++++++-
 tests/installer_bootloader_uboot_test.py |  9 +++++++--
 5 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/sonic_installer/bootloader/bootloader.py b/sonic_installer/bootloader/bootloader.py
index 3eeeb0b6af..f73e0a712d 100644
--- a/sonic_installer/bootloader/bootloader.py
+++ b/sonic_installer/bootloader/bootloader.py
@@ -77,7 +77,7 @@ def supports_package_migration(self, image):
 
     def verify_image_sign(self, image_path):
         """verify image signature is valid"""
-        return True
+        raise NotImplementedError
 
     @classmethod
     def detect(cls):
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index d78259317e..99f793f8c2 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -579,12 +579,14 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
 
         # Calling verification script by default - signature will be checked if enabled in bios
         echo_and_log("Verifing image {} signature...".format(binary_image_version))
-        if not bootloader.verify_image_sign(image_path):
-            echo_and_log('Error: Failed verify image signature', LOG_ERR)
-            raise click.Abort()
-        else:
-            echo_and_log('Verification successful')
-
+        try:
+            if not bootloader.verify_image_sign(image_path):
+                echo_and_log('Error: Failed verify image signature', LOG_ERR)
+                raise click.Abort()
+            else:
+                echo_and_log('Verification successful')
+        except NotImplementedError:
+            echo_and_log('Image verification not impelmented, continue image install without it')
         echo_and_log("Installing image {} and setting it as default...".format(binary_image_version))
         with SWAPAllocator(not skip_setup_swap, swap_mem_size, total_mem_threshold, available_mem_threshold):
             bootloader.install_image(image_path)
diff --git a/tests/installer_bootloader_aboot_test.py b/tests/installer_bootloader_aboot_test.py
index 1bc1db91df..2025e4fa1f 100644
--- a/tests/installer_bootloader_aboot_test.py
+++ b/tests/installer_bootloader_aboot_test.py
@@ -76,5 +76,10 @@ def test_set_fips_aboot():
 
 def test_verify_image_sign():
     bootloader = aboot.AbootBootloader()
-
-    assert bootloader.verify_image_sign(exp_image) == True
+    return_value = None
+    try:
+        return_value = bootloader.verify_image_sign(exp_image)
+    except NotImplementedError:
+        pass
+    else:
+        assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)
diff --git a/tests/installer_bootloader_onie_test.py b/tests/installer_bootloader_onie_test.py
index f863a278be..d132566103 100644
--- a/tests/installer_bootloader_onie_test.py
+++ b/tests/installer_bootloader_onie_test.py
@@ -18,4 +18,10 @@ def test_get_current_image(re_search):
 
 def test_verify_image_sign():
     bootloader = onie.OnieInstallerBootloader()
-    assert bootloader.verify_image_sign('some_path.path') == True
+    return_value = None
+    try:
+        return_value = bootloader.verify_image_sign('some_path.path')
+    except NotImplementedError:
+        pass
+    else:
+        assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)
\ No newline at end of file
diff --git a/tests/installer_bootloader_uboot_test.py b/tests/installer_bootloader_uboot_test.py
index 487609b0d9..77b5b03a2c 100644
--- a/tests/installer_bootloader_uboot_test.py
+++ b/tests/installer_bootloader_uboot_test.py
@@ -98,5 +98,10 @@ def mock_run_command(cmd):
 def test_verify_image_sign():
     bootloader = uboot.UbootBootloader()
     image = 'test-image'
-    # Test convertion image dir to image name
-    assert bootloader.verify_image_sign(image) == True
+    return_value =
+    try:
+        return_value = bootloader.verify_image_sign(image)
+    except NotImplementedError:
+        pass
+    else:
+        assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)

From 64dab4a3fca7d00115c5863065594cefeae45d4a Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Wed, 21 Jun 2023 16:12:01 +0300
Subject: [PATCH 13/14] Added check for image verification support

---
 scripts/verify_image_sign.sh             | 15 ---------------
 sonic_installer/bootloader/bootloader.py |  3 +++
 sonic_installer/bootloader/grub.py       | 24 ++++++++++++++++++++++++
 sonic_installer/main.py                  |  8 +++-----
 tests/installer_bootloader_aboot_test.py |  3 ++-
 tests/installer_bootloader_grub_test.py  |  2 +-
 tests/installer_bootloader_onie_test.py  |  3 ++-
 tests/installer_bootloader_uboot_test.py |  3 ++-
 8 files changed, 37 insertions(+), 24 deletions(-)

diff --git a/scripts/verify_image_sign.sh b/scripts/verify_image_sign.sh
index d66148d597..c23d4b711d 100644
--- a/scripts/verify_image_sign.sh
+++ b/scripts/verify_image_sign.sh
@@ -2,25 +2,10 @@
 image_file="${1}"
 cms_sig_file="sig.cms"
 lines_for_lookup=50
-SECURE_UPGRADE_ENABLED=0
 DIR="$(dirname "$0")"
-if [ -d "/sys/firmware/efi/efivars" ]; then
-    if ! [ -n "$(ls -A /sys/firmware/efi/efivars 2>/dev/null)" ]; then
-        mount -t efivarfs none /sys/firmware/efi/efivars 2>/dev/null
-    fi
-    SECURE_UPGRADE_ENABLED=$(bootctl status 2>/dev/null | grep -c "Secure Boot: enabled")
-else
-    echo "efi not supported - exiting without verification"
-    exit 0
-fi
 
 . /usr/local/bin/verify_image_sign_common.sh
 
-if [ ${SECURE_UPGRADE_ENABLED} -eq 0 ]; then
-    echo "secure boot not enabled - exiting without image verification"
-    exit 0
-fi
-
 clean_up ()
 {
     if  [ -d ${EFI_CERTS_DIR} ]; then rm -rf ${EFI_CERTS_DIR}; fi
diff --git a/sonic_installer/bootloader/bootloader.py b/sonic_installer/bootloader/bootloader.py
index f73e0a712d..3b9e548352 100644
--- a/sonic_installer/bootloader/bootloader.py
+++ b/sonic_installer/bootloader/bootloader.py
@@ -79,6 +79,9 @@ def verify_image_sign(self, image_path):
         """verify image signature is valid"""
         raise NotImplementedError
 
+    def is_secure_upgrade_image_verification_supported(self):
+        return False
+
     @classmethod
     def detect(cls):
         """returns True if the bootloader is in use"""
diff --git a/sonic_installer/bootloader/grub.py b/sonic_installer/bootloader/grub.py
index 8856f095b3..4782c7d4d6 100644
--- a/sonic_installer/bootloader/grub.py
+++ b/sonic_installer/bootloader/grub.py
@@ -153,6 +153,30 @@ def verify_image_platform(self, image_path):
         # Check if platform is inside image's target platforms
         return self.platform_in_platforms_asic(platform, image_path)
 
+    def is_secure_upgrade_image_verification_supported(self):
+
+        check_if_verification_is_enabled_and_supported_code = '''
+        SECURE_UPGRADE_ENABLED=0
+        if [ -d "/sys/firmware/efi/efivars" ]; then
+            if ! [ -n "$(ls -A /sys/firmware/efi/efivars 2>/dev/null)" ]; then
+                mount -t efivarfs none /sys/firmware/efi/efivars 2>/dev/null
+            fi
+            SECURE_UPGRADE_ENABLED=$(bootctl status 2>/dev/null | grep -c "Secure Boot: enabled")
+        else
+            echo "efi not supported - exiting without verification"
+            exit 1
+        fi
+
+        if [ ${SECURE_UPGRADE_ENABLED} -eq 0 ]; then
+            echo "secure boot not enabled - exiting without image verification"
+            exit 1
+        fi
+        exit 0
+        '''
+        verification_result = subprocess.run(['bash', '-c', check_if_verification_is_enabled_and_supported_code], check=True, capture_output=True)
+        click.echo(str(verification_result.stdout) + " " + str(verification_result.stderr))
+        return verification_result.returncode == 0
+
     def verify_image_sign(self, image_path):
         click.echo('Verifying image signature')
         verification_script_name = 'verify_image_sign.sh'
diff --git a/sonic_installer/main.py b/sonic_installer/main.py
index 6dec472f6e..9737e57de7 100644
--- a/sonic_installer/main.py
+++ b/sonic_installer/main.py
@@ -577,16 +577,14 @@ def install(url, force, skip_platform_check=False, skip_migration=False, skip_pa
                 "Aborting...", LOG_ERR)
             raise click.Abort()
 
-        # Calling verification script by default - signature will be checked if enabled in bios
-        echo_and_log("Verifing image {} signature...".format(binary_image_version))
-        try:
+        if bootloader.is_secure_upgrade_image_verification_supported():
+            echo_and_log("Verifing image {} signature...".format(binary_image_version))
             if not bootloader.verify_image_sign(image_path):
                 echo_and_log('Error: Failed verify image signature', LOG_ERR)
                 raise click.Abort()
             else:
                 echo_and_log('Verification successful')
-        except NotImplementedError:
-            echo_and_log('Image verification not impelmented, continue image install without it')
+
         echo_and_log("Installing image {} and setting it as default...".format(binary_image_version))
         with SWAPAllocator(not skip_setup_swap, swap_mem_size, total_mem_threshold, available_mem_threshold):
             bootloader.install_image(image_path)
diff --git a/tests/installer_bootloader_aboot_test.py b/tests/installer_bootloader_aboot_test.py
index 53187cfa14..fbe580a638 100644
--- a/tests/installer_bootloader_aboot_test.py
+++ b/tests/installer_bootloader_aboot_test.py
@@ -96,9 +96,10 @@ def test_set_fips_aboot():
 def test_verify_image_sign():
     bootloader = aboot.AbootBootloader()
     return_value = None
+    is_supported = bootloader.is_secure_upgrade_image_verification_supported()
     try:
         return_value = bootloader.verify_image_sign(exp_image)
     except NotImplementedError:
-        pass
+        assert not is_supported
     else:
         assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)
diff --git a/tests/installer_bootloader_grub_test.py b/tests/installer_bootloader_grub_test.py
index 2817db64b9..bef6e63ce7 100644
--- a/tests/installer_bootloader_grub_test.py
+++ b/tests/installer_bootloader_grub_test.py
@@ -93,6 +93,6 @@ def test_verify_image():
 
     bootloader = grub.GrubBootloader()
     image = f'{grub.IMAGE_PREFIX}expeliarmus-{grub.IMAGE_PREFIX}abcde'
-
+    assert bootloader.is_secure_upgrade_image_verification_supported()
     # command should fail
     assert not bootloader.verify_image_sign(image)
diff --git a/tests/installer_bootloader_onie_test.py b/tests/installer_bootloader_onie_test.py
index d132566103..5e63cc867d 100644
--- a/tests/installer_bootloader_onie_test.py
+++ b/tests/installer_bootloader_onie_test.py
@@ -19,9 +19,10 @@ def test_get_current_image(re_search):
 def test_verify_image_sign():
     bootloader = onie.OnieInstallerBootloader()
     return_value = None
+    is_supported = bootloader.is_secure_upgrade_image_verification_supported()
     try:
         return_value = bootloader.verify_image_sign('some_path.path')
     except NotImplementedError:
-        pass
+        assert not is_supported
     else:
         assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)
\ No newline at end of file
diff --git a/tests/installer_bootloader_uboot_test.py b/tests/installer_bootloader_uboot_test.py
index bc89a9d288..0097df1bc3 100644
--- a/tests/installer_bootloader_uboot_test.py
+++ b/tests/installer_bootloader_uboot_test.py
@@ -146,9 +146,10 @@ def test_verify_image_sign():
     bootloader = uboot.UbootBootloader()
     image = 'test-image'
     return_value = None
+    is_supported = bootloader.is_secure_upgrade_image_verification_supported()
     try:
         return_value = bootloader.verify_image_sign(image)
     except NotImplementedError:
-        pass
+        assert not is_supported
     else:
         assert False, "Wrong return value from verify_image_sign, returned" + str(return_value)

From 7877b3aa73dfa12151790119c09283390edaef25 Mon Sep 17 00:00:00 2001
From: Yona Cohen <ycohen@nvidia.com>
Date: Thu, 22 Jun 2023 13:48:37 +0300
Subject: [PATCH 14/14] Fixed grub verify_image_sign print

---
 sonic_installer/bootloader/grub.py      | 6 +++---
 tests/installer_bootloader_grub_test.py | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sonic_installer/bootloader/grub.py b/sonic_installer/bootloader/grub.py
index 4782c7d4d6..c2bfe8d534 100644
--- a/sonic_installer/bootloader/grub.py
+++ b/sonic_installer/bootloader/grub.py
@@ -173,8 +173,8 @@ def is_secure_upgrade_image_verification_supported(self):
         fi
         exit 0
         '''
-        verification_result = subprocess.run(['bash', '-c', check_if_verification_is_enabled_and_supported_code], check=True, capture_output=True)
-        click.echo(str(verification_result.stdout) + " " + str(verification_result.stderr))
+        verification_result = subprocess.run(['bash', '-c', check_if_verification_is_enabled_and_supported_code], capture_output=True)
+        click.echo(verification_result.stdout.decode())
         return verification_result.returncode == 0
 
     def verify_image_sign(self, image_path):
@@ -185,7 +185,7 @@ def verify_image_sign(self, image_path):
             click.echo("Unable to find verification script in path " + script_path)
             return False
         verification_result = subprocess.run([script_path, image_path], capture_output=True)
-        click.echo(str(verification_result.stdout) + " " + str(verification_result.stderr))
+        click.echo(verification_result.stdout.decode())
         return verification_result.returncode == 0
 
     @classmethod
diff --git a/tests/installer_bootloader_grub_test.py b/tests/installer_bootloader_grub_test.py
index bef6e63ce7..d418b76e55 100644
--- a/tests/installer_bootloader_grub_test.py
+++ b/tests/installer_bootloader_grub_test.py
@@ -93,6 +93,6 @@ def test_verify_image():
 
     bootloader = grub.GrubBootloader()
     image = f'{grub.IMAGE_PREFIX}expeliarmus-{grub.IMAGE_PREFIX}abcde'
-    assert bootloader.is_secure_upgrade_image_verification_supported()
+    assert not bootloader.is_secure_upgrade_image_verification_supported()
     # command should fail
     assert not bootloader.verify_image_sign(image)