Skip to content
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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

xumia
Copy link
Contributor

@xumia xumia commented Aug 20, 2020

No description provided.

Copy link
Contributor

@Staphylo Staphylo left a 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
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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
Copy link
Contributor

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)

Copy link
Contributor Author

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.
Copy link
Contributor

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 +.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line 73 is not fixed yet.


In reply to: 473782028 [](ancestors = 473782028)

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor issue

@yxieca yxieca force-pushed the master branch 2 times, most recently from 8498931 to 8837dc2 Compare April 15, 2022 16:51
@xumia xumia marked this pull request as draft October 17, 2022 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants