Skip to content

vmware: add image.yaml toggle for secure boot#2971

Merged
cgwalters merged 1 commit intocoreos:mainfrom
mike-nguyen:unsecure_rhcos
Jul 12, 2022
Merged

vmware: add image.yaml toggle for secure boot#2971
cgwalters merged 1 commit intocoreos:mainfrom
mike-nguyen:unsecure_rhcos

Conversation

@mike-nguyen
Copy link
Copy Markdown
Member

@mike-nguyen mike-nguyen commented Jul 11, 2022

Add toggle for secure boot in the OVA.

ref: https://bugzilla.redhat.com/show_bug.cgi?id=2106055

Comment thread src/cosalib/ova.py Outdated
@cgwalters
Copy link
Copy Markdown
Member

IOW something more like:

diff --git a/src/cosalib/ova.py b/src/cosalib/ova.py
index b11a202db..18a1bb88d 100644
--- a/src/cosalib/ova.py
+++ b/src/cosalib/ova.py
@@ -102,6 +102,7 @@ class OVA(QemuVariantImage):
         params = {
             'ovf_cpu_count':                    cpu,
             'ovf_memory_mb':                    memory,
+            'secure_boot':                      image_json['vmware-secure-boot'],
             'vsphere_image_name':               image,
             'vsphere_product_name':             product,
             'vsphere_product_vendor_name':      vendor,
diff --git a/src/image-default.yaml b/src/image-default.yaml
index c4bc05798..37abe4e58 100644
--- a/src/image-default.yaml
+++ b/src/image-default.yaml
@@ -22,3 +22,5 @@ squashfs-compression: zstd
 # Defaults for VMware OVA, matching historical behavior
 vmware-hw-version: 13
 vmware-os-type: rhel7_64Guest
+# xref https://bugzilla.redhat.com/show_bug.cgi?id=2106055
+vmware-secure-boot: true

@dustymabe
Copy link
Copy Markdown
Member

hmm. How common is it for users who use unsigned kernel modules? It would be nice if we could not go backwards with our defaults here.

Is it possible for the user to change this knob if needed (similar to https://docs.fedoraproject.org/en-US/fedora-coreos/provisioning-vmware/#_modifying_ovf_metadata)?

@mike-nguyen
Copy link
Copy Markdown
Member Author

IOW something more like:

diff --git a/src/cosalib/ova.py b/src/cosalib/ova.py
index b11a202db..18a1bb88d 100644
--- a/src/cosalib/ova.py
+++ b/src/cosalib/ova.py
@@ -102,6 +102,7 @@ class OVA(QemuVariantImage):
         params = {
             'ovf_cpu_count':                    cpu,
             'ovf_memory_mb':                    memory,
+            'secure_boot':                      image_json['vmware-secure-boot'],
             'vsphere_image_name':               image,
             'vsphere_product_name':             product,
             'vsphere_product_vendor_name':      vendor,
diff --git a/src/image-default.yaml b/src/image-default.yaml
index c4bc05798..37abe4e58 100644
--- a/src/image-default.yaml
+++ b/src/image-default.yaml
@@ -22,3 +22,5 @@ squashfs-compression: zstd
 # Defaults for VMware OVA, matching historical behavior
 vmware-hw-version: 13
 vmware-os-type: rhel7_64Guest
+# xref https://bugzilla.redhat.com/show_bug.cgi?id=2106055
+vmware-secure-boot: true

Yes, I'm testing this right now. Micah mentioned we did this templating for hw-version but I didn't connect it to the image.yaml file. I'm testing this out and will update this PR shortly.

@mike-nguyen mike-nguyen changed the title vmware: turn off secure boot for rhcos vmware: add image.yaml toggle for secure boot Jul 11, 2022
@miabbott
Copy link
Copy Markdown
Member

hmm. How common is it for users who use unsigned kernel modules? It would be nice if we could not go backwards with our defaults here.

For RHCOS/OCP, apparently pretty darn common. ¯_(ツ)_/¯

The short list of vendors/partners that may be affected by something like this includes Calico, Intel, Nvidia, Portworx, Spectrumscale, Veritas...

Is it possible for the user to change this knob if needed (similar to https://docs.fedoraproject.org/en-US/fedora-coreos/provisioning-vmware/#_modifying_ovf_metadata)?

In the case of customers doing IPI installs of OCP on vSphere, modifying the OVF metadata before installation is currently not possible.

miabbott
miabbott previously approved these changes Jul 11, 2022
@cgwalters
Copy link
Copy Markdown
Member

In the case of customers doing IPI installs of OCP on vSphere, modifying the OVF metadata before installation is currently not possible.

FWIW, I am pretty sure it wouldn't be hard to add a knob to openshift/installer to configure this; it already uses terraform to do a ton of stuff against their API. So that's a followup we can make; allow the users who do want Secure Boot to opt in.

(OTOH, a counterargument is that Secure Boot for us currently is mostly a security theater, and we should revisit this when we actually support verifying at least all privileged userspace code too)

Comment thread src/cosalib/ova.py Outdated
@bgilbert
Copy link
Copy Markdown
Contributor

(OTOH, a counterargument is that Secure Boot for us currently is mostly a security theater, and we should revisit this when we actually support verifying at least all privileged userspace code too)

How so? The purpose of Secure Boot is to protect the integrity of the kernel, and I'm not aware of anything that makes that less effective for FCOS. It's not trying to perform a fully verified boot.

@mike-nguyen
Copy link
Copy Markdown
Member Author

mike-nguyen commented Jul 12, 2022

This now requires coreos/fedora-coreos-config#1837 and openshift/os#888

@travier
Copy link
Copy Markdown
Member

travier commented Jul 12, 2022

Which behavior do we want as default? For other parameters we chose to use the values that we used before and overrode things in RHCOS/FCOS. Here we've already changed the default to enabled for all cases. I'd say we keep enabled the default?

@mike-nguyen
Copy link
Copy Markdown
Member Author

+1 to being secure by default

@mike-nguyen
Copy link
Copy Markdown
Member Author

I tested locally with FCOS (no override with default vmware-secure-boot: "true" taken from image-default.yaml) and RHCOS (with image.yaml and vmware-secure-boot: "false"). coreos.ovf from the extracted OVAs are showing the correct values for secure boot.

@mike-nguyen
Copy link
Copy Markdown
Member Author

/retest

@travier
Copy link
Copy Markdown
Member

travier commented Jul 12, 2022

RHCOS will keep failing until we have openshift/os#890

@cgwalters
Copy link
Copy Markdown
Member

Let's not block though when we're reasonably confident (as we are in this case) that the FCOS CI covers things.
/override ci/prow/rhcos

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jul 12, 2022

@cgwalters: Overrode contexts on behalf of cgwalters: ci/prow/rhcos

Details

In response to this:

Let's not block though when we're reasonably confident (as we are in this case) that the FCOS CI covers things.
/override ci/prow/rhcos

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@cgwalters cgwalters enabled auto-merge (rebase) July 12, 2022 16:31
@cgwalters cgwalters merged commit 37df486 into coreos:main Jul 12, 2022
Comment thread src/image-default.yaml
@openshift-cherrypick-robot
Copy link
Copy Markdown

@mike-nguyen: new pull request created: #2976

Details

In response to this:

/cherry-pick rhcos-4.11

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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.

7 participants