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

Possible patch to allow hardened build to boot VMs #30

Open
laniakea64 opened this issue Jun 19, 2024 · 1 comment
Open

Possible patch to allow hardened build to boot VMs #30

laniakea64 opened this issue Jun 19, 2024 · 1 comment

Comments

@laniakea64
Copy link

First, thank you very much for this KVM backend for VirtualBox! It works well for me, no more micro-managing kernel versions 🎉

Saw that building without --disable-hardening is now an option with the latest patch set, so tried it out in case a hardened build might be more secure. I was able to get the hardened build to run, but it would not boot any VM: the hardened VirtualBoxVM binary expects to be SUID root, but with KVM backend it is not SUID and doesn't need root at all.

The VM host and build machine are both running Xubuntu 22.04 with kernel 6.8.0-76060800daily20240311-generic from the System76 Driver PPA. Non-hardened build works without issue on this system (VirtualBox 7.0.14a with KVM backend patches from 9d202d4). A guest OS is not required to reproduce the issue with hardened builds: so far I have been testing this with new, blank VMs with no guest OS.

To enable the hardened build to boot VMs, this patch seems to do the job -

diff --git a/src/VBox/HostDrivers/Support/SUPLibInternal.h b/src/VBox/HostDrivers/Support/SUPLibInternal.h
index 4c788fac..caef2de4 100644
--- a/src/VBox/HostDrivers/Support/SUPLibInternal.h
+++ b/src/VBox/HostDrivers/Support/SUPLibInternal.h
@@ -77,7 +77,7 @@
 /** @def SUP_HARDENED_SUID
  * Whether we're employing set-user-ID-on-execute in the hardening.
  */
-#if (!defined(RT_OS_OS2) && !defined(RT_OS_WINDOWS) && !defined(RT_OS_L4)) || defined(DOXYGEN_RUNNING)
+#if (!defined(VBOX_WITH_KVM) && !defined(RT_OS_OS2) && !defined(RT_OS_WINDOWS) && !defined(RT_OS_L4)) || defined(DOXYGEN_RUNNING)
 # define SUP_HARDENED_SUID
 #else
 # undef  SUP_HARDENED_SUID
diff --git a/src/VBox/HostDrivers/Support/SUPR3HardenedMain.cpp b/src/VBox/HostDrivers/Support/SUPR3HardenedMain.cpp
index 25c6b4c5..879d5ba5 100644
--- a/src/VBox/HostDrivers/Support/SUPR3HardenedMain.cpp
+++ b/src/VBox/HostDrivers/Support/SUPR3HardenedMain.cpp
@@ -470,7 +470,7 @@
 *   Defined Constants And Macros                                                                                                 *
 *********************************************************************************************************************************/
 /* This mess is temporary after eliminating a define duplicated in SUPLibInternal.h. */
-#if !defined(RT_OS_OS2) && !defined(RT_OS_WINDOWS) && !defined(RT_OS_L4)
+#if !defined(VBOX_WITH_KVM) && !defined(RT_OS_OS2) && !defined(RT_OS_WINDOWS) && !defined(RT_OS_L4)
 # ifndef SUP_HARDENED_SUID
 #  error "SUP_HARDENED_SUID is NOT defined?!?"
 # endif

Is this a reasonable fix that would be safe to use in production? If so, could you please add it to the patches in this repo? Or is there a better way to get this working?

Thanks!

@tpressure
Copy link
Contributor

Not having to set SUID certainly makes sense as we do not have any kernel module to load. I will look into this.

ping @eugenesan @dionorgua

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

No branches or pull requests

2 participants