-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add SONiC secure boot test plan #662
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall, just some nitpicking.
|
||
**Steps:** | ||
* Add a new config file in the allowlist | ||
* Rebuild the image and install the image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebuild a signed image and install it
I feel emphasizing on the fact that the build needs to sign the image can be meaningful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
**Steps:** | ||
* Basic boot up test | ||
* Allowlist not cover this scenario. | ||
* Check /proc/cmdline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for secure_boot_enable=y in /proc/cmdline
This might be a bit more precise as to what needs to be checked for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
### 8. Test tempered image | ||
|
||
**Steps:** | ||
* Change a new file into the signed image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add (or flip a bit in the image)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
* Expect the reboot will be failed | ||
|
||
### 9. Test no executable files in rw folder after reboot | ||
If there are any files with -x option in rw folder, the option will be removed after the SONiC switch reboot. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe replace -x
by +x
?
chmod syntax relies on + to add a flag and - to remove it.
Even though what you meant is completely understandable it might be more accurate to use +.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor issue
8498931
to
8837dc2
Compare
No description provided.