fixed bootloader method verify_image_sign to have dummy implementation… #2646
Conversation
| @@ -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 | |||
There was a problem hiding this comment.
unsecured normal image is considered not signed, yes
There was a problem hiding this comment.
@ycoheNvidia try/except would be more appropriate?
echo_and_log("Verifing image {} signature...".format(binary_image_version))
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 AttributeError:
echo_and_log("Skip Verifing image {} signature...Not signed image".format(binary_image_version))
pass
There was a problem hiding this comment.
I am not sure which lines you are referring to @prgeor, can you please elaborate?
There was a problem hiding this comment.
@ycoheNvidia @liat-grozovik instead of defining verify_image_sign() as NOP function in the bootloader.py can we try the except/catch block I showed above? So that if a bootloader does not implement verify_image_sign(), it won't crash here https://github.com/sonic-net/sonic-utilities/blob/master/sonic_installer/main.py#L582
|
@qiluo-msft can you approve so we can merge ? |
ycoheNvidia
force-pushed
the
secure_upgrade-bootloader-fix
branch
2 times, most recently
from
c527b5b to
ccdd0c7
Compare
Added try-catch wrapper for verify_image_sign to fix method not implemented in non-grub bootloaders.
ycoheNvidia
force-pushed
the
secure_upgrade-bootloader-fix
branch
from
ccdd0c7 to
c51e6bf
Compare
ycoheNvidia
force-pushed
the
secure_upgrade-bootloader-fix
branch
from
ea44c53 to
514e87d
Compare
ycoheNvidia
force-pushed
the
secure_upgrade-bootloader-fix
branch
from
514e87d to
ed0078e
Compare
| raise click.Abort() | ||
| else: | ||
| echo_and_log('Verification successful') | ||
| except AttributeError: |
There was a problem hiding this comment.
@qiluo-msft whats in your mind? How do you want the fix ?
|
@qiluo-msft can you approve so we can merge ? |
I thought there was an agreement to use #2698 instead, since original PR 2337 was reverted. 2698 includes both Secure upgrade original code and the fixes included in this PR |
|
@ycoheNvidia can you please update the conflicts? |
|
I thought there was an agreement to use #2698 instead, since original PR 2337 was reverted. 2698 includes both Secure upgrade original code and the fixes included in this PR
This PR is irrelevant without 2337, can we move with #2698 instead? it has both original secure upgrade code and the fixes introduced here |
|
This solution is part of a new PR #2698 |
…n in father class
What I did
Fixed bootloader issue in sonic-installer
How I did it
Added dummy implementation of verify_image_sign in bootloader.py
Added tests to check it's available
How to verify it
Run sonic-utilities tests and see that they pass
Previous command output (if the output of a command-line utility has changed)
NONE
New command output (if the output of a command-line utility has changed)
NONE