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 secureboot support for Arista devices #4741

Merged
merged 4 commits into from
Jun 22, 2020

Conversation

Staphylo
Copy link
Collaborator

@Staphylo Staphylo commented Jun 9, 2020

This change adds secureboot support for Arista devices.
Some of the pieces can easily be reused by other bootloaders to achieve
the same purpose.

boot0

The default behavior of the boot0 is preserved and should not have any
impact on existing SKUs running in production. This is refered to as
regular boot process.

When secureboot is enabled, boot0 will follow a different code path.
In such cases only a few files get extracted as part of the installation process.
These are expected by SONiC and do not pose a security risk (.imagehash, firstime).

The secureboot mode sets docker_inram by default to force containers to
be extracted in RAM at boot time instead of extracted on the flash where
they could be tampered with.

This mode also adds panic=60 which prevents the initramfs from running
an interactive shell when the boot fails and reboot after 60 seconds.

initrd + union-mount

A new cmdline parameter called loopoffset= is added to the debian
initramfs-tools package. It mostly rely on the existing loop device mount
patch that exists to mount the fs.squashfs file.
When loopoffset= is present, the fs.squashfs can be mounted directly
from a given file at a given offset by using losetup.

In the case of Aboot, fs.squashfs is mounted directly from the SWI file
verified by secureboot in Aboot.

A small refactor was made for the logs in ram workaround.
It is now the logs_inram cmdline option that any vendor can use to store
the logs in RAM instead of using the flash.

build_debian.sh

The squashfs is no longer compressed when added to the zip file.
It was a bit pointless to compress an already compressed file.
It has the advantage of being a bit faster to add to the fs.zip and can now
be mounted directly from the SWI.

NOTES

This change is seemless for regular boot.

In secureboot, this change breaks some assumptions made by some SONiC tools.

  • sonic_installer
  • fast-reboot/warm-reboot

These issues will be addressed separately, as they do not prevent this change from working.

@qiluo-msft qiluo-msft requested review from lguohan, yxieca, qiluo-msft and xumia and removed request for lguohan June 9, 2020 23:06
files/Aboot/boot0.j2 Outdated Show resolved Hide resolved
files/Aboot/boot0.j2 Outdated Show resolved Hide resolved
Copy link
Collaborator

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

As comments

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Jun 10, 2020

Could you please fix build error

06:39:57  shellcheck -e SC1090,SC1091 -s dash hook-functions $(find hooks scripts -type f) $({ find . -maxdepth 1 -type f -executable; find debian -maxdepth 1 -type f; find docs kernel -type f; } | xargs grep -l '^#!/bin/sh')
06:39:57  
06:39:57  In ./init line 120:
06:39:57  		LOOPOFFSET="${x#loopoffset=}"
06:39:57                  ^-- SC2034: LOOPOFFSET appears unused. Verify use (or export if used externally).
06:39:57  
06:39:57  make[3]: *** [debian/rules:25: override_dh_auto_test] Error 1
``` #Closed

files/Aboot/boot0.j2 Outdated Show resolved Hide resolved
files/Aboot/boot0.j2 Outdated Show resolved Hide resolved
@Staphylo Staphylo force-pushed the master-secureboot branch 4 times, most recently from f9b006e to 50ecb13 Compare June 17, 2020 18:22
@Staphylo
Copy link
Collaborator Author

retest broadcom please

@Staphylo Staphylo force-pushed the master-secureboot branch 2 times, most recently from 375d0db to 94c4d56 Compare June 18, 2020 19:56
@Staphylo Staphylo marked this pull request as ready for review June 18, 2020 22:48
qiluo-msft
qiluo-msft previously approved these changes Jun 18, 2020
Copy link
Collaborator

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

LGTM, please also wait for @xumia's review

@Staphylo
Copy link
Collaborator Author

I am also running some more regular boot tests to make sure there is no regression.
Please wait until I have validated these before merging.

@Staphylo
Copy link
Collaborator Author

My testing is looking good on other Arista devices.
This review is ready to go from my perspective.

@Staphylo
Copy link
Collaborator Author

Just rebased on master to fix a conflict on code I removed on union-mount.j2

qiluo-msft
qiluo-msft previously approved these changes Jun 20, 2020
It doesn't make much sense to do so since these files are already
compressed.
Also not compressing the squashfs has the advantage of making it
mountable via a loop device.
@qiluo-msft qiluo-msft merged commit 67987e9 into sonic-net:master Jun 22, 2020
@qiluo-msft
Copy link
Collaborator

@Staphylo Could you clarify the statement that "In secureboot, this change breaks some assumptions made by some SONiC tools" ?

@Staphylo
Copy link
Collaborator Author

Staphylo commented Sep 2, 2021

@qiluo-msft
Essentially any code in SONiC that makes assumption where files are stored on the flash might break.
Ideally code that needs to access the rootfs or other /host/image-XXX/... paths should use the Bootloader abstraction to make there is no problem.
Here are some examples of breakages we had to resolve.

  1. sonic-installer install changes to read the /etc/sonic/sonic_version.yml file of the next image is mounting fs.squashfs. It did a direct mount rather than abstracting the mount operation through the Bootloader class where we already had an helper for this.
    See Fix sonic-installer install for Arista secureboot sonic-utilities#1366

  2. sonic-installer install changes for the sonic package manager expects /var/lib/docker to be on the flash. But because secureboot does not install the containers on the flash the sonic package manager feature had to be disabled for secureboot. This can likely be revisited the day SONiC has a mechanism to trust pre-installed containers. Or we decide to break the trust at the container level.
    See Fix Aboot breakage sonic-installer due to package migration sonic-utilities#1625

I would expect the secureboot related failures to be mostly happening on sonic-installer which is where image related stuff happens. However if other places in the code start assuming installation layout these will break too.

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.

4 participants