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

support testing secureboot in Qemu #556

Merged
merged 7 commits into from
Sep 11, 2024

Conversation

jepio
Copy link
Member

@jepio jepio commented Sep 10, 2024

@sayanchowdhury + @jepio

  • add new kola command line flags: --qemu-ovmf-vars and --enable-secureboot. --enable-secureboot will eventually be useful for other platforms as well (AWS/GCP/Azure).
  • --qemu-bios has been renamed to --qemu-firmware, but --qemu-bios is kept around so that we don't break old releases if we only backport kola changes.
  • ovmf vars file is copied into instance folder per test case execution
  • skip falco and zfs tests when secure boot: unsigned kernel modules can't be loaded when secure boot is enabled, enforced by lockdown integrity mode.
  • force boot from primary disk using bootindex

Tested here: http://jenkins.infra.kinvolk.io:8080/job/container/job/test_dispatcher/5120/cldsv/ with ghcr.io/flatcar/mantle:pr-554

kola/harness.go Outdated Show resolved Hide resolved
cmd/kola/options.go Outdated Show resolved Hide resolved
kola/tests/sysext/zfs.go Outdated Show resolved Hide resolved
@jepio jepio force-pushed the sayan/secureboot-implement-ci+jepio branch from d7b7a14 to ff74ce5 Compare September 10, 2024 09:32
build Outdated
@@ -30,7 +30,7 @@ cross_build() {
echo "Building $a/$1"
mkdir -p "bin/$a"
CGO_ENABLED=0 GOARCH=$a \
go build -mod=vendor -ldflags "${ldflags}" \
go build -mod=vendor -ldflags "${ldflags}" -gcflags="all=-N -l" \
Copy link
Member

Choose a reason for hiding this comment

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

This disables optimizations and inlining. Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

removed this again

platform/qemu.go Outdated
Comment on lines 301 to 315
ovmfVarsSrc, err := os.Open(ovmfVars)
if err != nil {
return "", err
}
defer ovmfVarsSrc.Close()

ovmfVarsCopy, err := os.Create(ovmfVarsDst)
if err != nil {
return "", err
}
defer ovmfVarsCopy.Close()

if _, err := io.Copy(ovmfVarsCopy, ovmfVarsSrc); err != nil {
os.Remove(ovmfVarsCopy.Name())
return "", err
Copy link
Member

Choose a reason for hiding this comment

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

There is a CopyRegularFile function in github.com/flatcar/mantle/system package. Maybe you could use it instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

i was looking for something like this. Switched to CopyRegularFile

)
if enableSecureboot {
qmCmd = append(qmCmd,
"-global", "ICH9-LPC.disable_s3=1",
Copy link
Member

Choose a reason for hiding this comment

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

This one could use a comment, really.

Copy link
Member Author

Choose a reason for hiding this comment

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

added a comment with what i know

sayanchowdhury and others added 6 commits September 10, 2024 17:45
Signed-off-by: Sayan Chowdhury <[email protected]>
Continue supporting BIOS by passing `-bios` and only enable `smm=on` when
secure boot is requested, as it requires build of OVMF code. This special build
is required for secure boot support, but non-sboot OVMFs won't support it.
and cleanup on shutdown.

Signed-off-by: Jeremi Piotrowski <[email protected]>
To make this change easier to apply to all channels.

Signed-off-by: Jeremi Piotrowski <[email protected]>
Kernel lockdown blocks loading unsigned kernel modules, so these tests need to
be disabled. Eventually the zfs sysext should ship a signed kernel modules, but
falco is built on the running system and won't work the same way. Falco
suggests running in eBPF mode instead.
@jepio jepio force-pushed the sayan/secureboot-implement-ci+jepio branch from ff74ce5 to 6dc4d85 Compare September 10, 2024 15:45
Copy link
Contributor

@tormath1 tormath1 left a comment

Choose a reason for hiding this comment

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

Is this still accurate:

###### Run tests for AMD64
? (except the renaming from qemu-bios to qemu-firmware ?)

The previous instructions still work, but update to show the newly added cli
options.

Signed-off-by: Jeremi Piotrowski <[email protected]>
@jepio
Copy link
Member Author

jepio commented Sep 11, 2024

Is this still accurate:

###### Run tests for AMD64

? (except the renaming from qemu-bios to qemu-firmware ?)

Yes, the instructions are still accurate. --qemu-ovmf-vars is optional and --qemu-bios is still supported. I've pushed a commit updating the README though, so that we have an example of the newest options. I tested both old and new versions.

@jepio jepio merged commit cf464dd into flatcar-master Sep 11, 2024
2 checks passed
@jepio jepio deleted the sayan/secureboot-implement-ci+jepio branch September 11, 2024 08:56
@jepio jepio mentioned this pull request Sep 11, 2024
2 tasks
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