* [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec.
@ 2015-02-25 22:14 Karoly Kasza
2015-02-25 22:14 ` [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack Karoly Kasza
2015-03-08 21:39 ` [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Romain Naour
0 siblings, 2 replies; 4+ messages in thread
From: Karoly Kasza @ 2015-02-25 22:14 UTC (permalink / raw)
To: buildroot
Add patch to use $(KVERS) instead of shell exec uname.
Originally from Debian.
Signed-off-by: Karoly Kasza <kaszak@gmail.com>
---
package/openvmtools/0008-kvers.patch | 75 ++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 package/openvmtools/0008-kvers.patch
diff --git a/package/openvmtools/0008-kvers.patch b/package/openvmtools/0008-kvers.patch
new file mode 100644
index 0000000..f432dbb
--- /dev/null
+++ b/package/openvmtools/0008-kvers.patch
@@ -0,0 +1,75 @@
+Use KVERS instead of shell exec uname.
+
+Patch from Debian.
+
+Original author: Cyril Brulebois <cyril.brulebois@enst-bretagne.fr>
+Original description: Replacing uname call with KVERS, overwriting seemed not to work.
+
+Signed-off-by: Karoly Kasza <kaszak@gmail.com>
+
+--- a/modules/linux/vmblock/Makefile
++++ b/modules/linux/vmblock/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
+--- a/modules/linux/vmci/Makefile
++++ b/modules/linux/vmci/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
+--- a/modules/linux/vmhgfs/Makefile
++++ b/modules/linux/vmhgfs/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
+--- a/modules/linux/vmsync/Makefile
++++ b/modules/linux/vmsync/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
+--- a/modules/linux/vmxnet/Makefile
++++ b/modules/linux/vmxnet/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
+--- a/modules/linux/vsock/Makefile
++++ b/modules/linux/vsock/Makefile
+@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
+ endif
+
+
+-VM_UNAME = $(shell uname -r)
++VM_UNAME = $(KVERS)
+
+ # Header directory for the running kernel
+ ifdef LINUXINCLUDE
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack
2015-02-25 22:14 [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Karoly Kasza
@ 2015-02-25 22:14 ` Karoly Kasza
2015-03-08 21:24 ` Romain Naour
2015-03-08 21:39 ` [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Romain Naour
1 sibling, 1 reply; 4+ messages in thread
From: Karoly Kasza @ 2015-02-25 22:14 UTC (permalink / raw)
To: buildroot
Add patch to defend lsb from path walking attack.
Originally from Debian.
Signed-off-by: Karoly Kasza <kaszak@gmail.com>
---
package/openvmtools/0009-lsb.patch | 106 ++++++++++++++++++++++++++++++++++++
1 file changed, 106 insertions(+)
create mode 100644 package/openvmtools/0009-lsb.patch
diff --git a/package/openvmtools/0009-lsb.patch b/package/openvmtools/0009-lsb.patch
new file mode 100644
index 0000000..54c3232
--- /dev/null
+++ b/package/openvmtools/0009-lsb.patch
@@ -0,0 +1,106 @@
+Upsteam Debian patch to defend openvmtools against path walking.
+
+Original description:
+
+From 3a9f2297a82b9c109e894b5f8ea17753e68830ac Mon Sep 17 00:00:00 2001
+From: "VMware, Inc" <>
+Date: Tue, 17 Sep 2013 20:39:34 -0700
+Subject: [PATCH] Harden HostinfoOSData against $PATH attacks.
+
+We are doing a popen("lsb_release... ") when attempting to
+determine host details in hostinfoPosix.c. Using popen means that
+$PATH is walked when looking for the lsb_release binary, and that
+may give an attacker the ability to run a malicious version of
+lsb_release.
+
+This change does two things,
+
+a) Hard code the path to lsb_release. I've searched around
+ the web and I believe the path is always "/usr/bin/lsb_release"
+ so let's not leave this up to chance.
+
+b) Stop running HostinfoGetCmdOutput with elevated privileges. Drop
+ to non-root when possible. If someone sneaks in a new call to
+ HostinfoGetCmdOutput and doesn't use a full path, then we will
+ hopefully avoid a firedrill. I'm only applying this to Linux
+ because the Fusion build barfed when I tried to compile with
+ without the vmx86_linux.
+
+I think either (a) or (b) would be enough but I'm doing both,
+because each individually is correct. Also note that in the blog
+post by Tavis Ormandy calls out doing (a) as not enough,
+ http://blog.cmpxchg8b.com/2013/08/security-debianisms.html
+His example uses a bash feature that allows functions to be
+exported. I haven't been able to get that to work on my Ubuntu
+machine.
+
+To test I'm manually run Linux WS and Fusion and verified that
+the logs look correct.
+
+Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
+
+Signed-off-by: Karoly Kasza <kaszak@gmail.com>
+
+---
+ open-vm-tools/lib/misc/hostinfoPosix.c | 23 +++++++++++++++++++----
+ 1 file changed, 19 insertions(+), 4 deletions(-)
+
+--- a/lib/misc/hostinfoPosix.c
++++ b/lib/misc/hostinfoPosix.c
+@@ -800,17 +800,27 @@ out:
+ static char *
+ HostinfoGetCmdOutput(const char *cmd) // IN:
+ {
++ Bool isSuperUser = FALSE;
+ DynBuf db;
+ FILE *stream;
+ char *out = NULL;
+
++ /*
++ * Attempt to lower privs, because we use popen and an attacker
++ * may control $PATH.
++ */
++ if (vmx86_linux && Id_IsSuperUser()) {
++ Id_EndSuperUser(getuid());
++ isSuperUser = TRUE;
++ }
++
+ DynBuf_Init(&db);
+
+ stream = Posix_Popen(cmd, "r");
+ if (stream == NULL) {
+ Warning("Unable to get output of command \"%s\"\n", cmd);
+
+- return NULL;
++ goto exit;
+ }
+
+ for (;;) {
+@@ -844,11 +854,16 @@ HostinfoGetCmdOutput(const char *cmd) /
+ if (DynBuf_Get(&db)) {
+ out = (char *) DynBuf_AllocGet(&db);
+ }
+- closeIt:
+- DynBuf_Destroy(&db);
+
++ closeIt:
+ pclose(stream);
+
++ exit:
++ DynBuf_Destroy(&db);
++
++ if (isSuperUser) {
++ Id_BeginSuperUser();
++ }
+ return out;
+ }
+
+@@ -967,7 +982,7 @@ HostinfoOSData(void)
+ * Try to get OS detailed information from the lsb_release command.
+ */
+
+- lsbOutput = HostinfoGetCmdOutput("lsb_release -sd 2>/dev/null");
++ lsbOutput = HostinfoGetCmdOutput("/usr/bin/lsb_release -sd 2>/dev/null");
+ if (!lsbOutput) {
+ int i;
+
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack
2015-02-25 22:14 ` [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack Karoly Kasza
@ 2015-03-08 21:24 ` Romain Naour
0 siblings, 0 replies; 4+ messages in thread
From: Romain Naour @ 2015-03-08 21:24 UTC (permalink / raw)
To: buildroot
Hi Karoly,
Le 25/02/2015 23:14, Karoly Kasza a ?crit :
> Add patch to defend lsb from path walking attack.
> Originally from Debian.
>
> Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> ---
> package/openvmtools/0009-lsb.patch | 106 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 106 insertions(+)
> create mode 100644 package/openvmtools/0009-lsb.patch
>
> diff --git a/package/openvmtools/0009-lsb.patch b/package/openvmtools/0009-lsb.patch
> new file mode 100644
> index 0000000..54c3232
> --- /dev/null
> +++ b/package/openvmtools/0009-lsb.patch
> @@ -0,0 +1,106 @@
> +Upsteam Debian patch to defend openvmtools against path walking.
> +
> +Original description:
> +
> +From 3a9f2297a82b9c109e894b5f8ea17753e68830ac Mon Sep 17 00:00:00 2001
> +From: "VMware, Inc" <>
> +Date: Tue, 17 Sep 2013 20:39:34 -0700
> +Subject: [PATCH] Harden HostinfoOSData against $PATH attacks.
> +
> +We are doing a popen("lsb_release... ") when attempting to
> +determine host details in hostinfoPosix.c. Using popen means that
> +$PATH is walked when looking for the lsb_release binary, and that
> +may give an attacker the ability to run a malicious version of
> +lsb_release.
> +
> +This change does two things,
> +
> +a) Hard code the path to lsb_release. I've searched around
> + the web and I believe the path is always "/usr/bin/lsb_release"
> + so let's not leave this up to chance.
> +
> +b) Stop running HostinfoGetCmdOutput with elevated privileges. Drop
> + to non-root when possible. If someone sneaks in a new call to
> + HostinfoGetCmdOutput and doesn't use a full path, then we will
> + hopefully avoid a firedrill. I'm only applying this to Linux
> + because the Fusion build barfed when I tried to compile with
> + without the vmx86_linux.
> +
> +I think either (a) or (b) would be enough but I'm doing both,
> +because each individually is correct. Also note that in the blog
> +post by Tavis Ormandy calls out doing (a) as not enough,
> + http://blog.cmpxchg8b.com/2013/08/security-debianisms.html
> +His example uses a bash feature that allows functions to be
> +exported. I haven't been able to get that to work on my Ubuntu
> +machine.
> +
> +To test I'm manually run Linux WS and Fusion and verified that
> +the logs look correct.
> +
> +Signed-off-by: Dmitry Torokhov <dtor@vmware.com>
> +
> +Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> +
> +---
> + open-vm-tools/lib/misc/hostinfoPosix.c | 23 +++++++++++++++++++----
> + 1 file changed, 19 insertions(+), 4 deletions(-)
> +
> +--- a/lib/misc/hostinfoPosix.c
> ++++ b/lib/misc/hostinfoPosix.c
> +@@ -800,17 +800,27 @@ out:
> + static char *
> + HostinfoGetCmdOutput(const char *cmd) // IN:
> + {
> ++ Bool isSuperUser = FALSE;
> + DynBuf db;
> + FILE *stream;
> + char *out = NULL;
> +
> ++ /*
> ++ * Attempt to lower privs, because we use popen and an attacker
> ++ * may control $PATH.
> ++ */
> ++ if (vmx86_linux && Id_IsSuperUser()) {
> ++ Id_EndSuperUser(getuid());
> ++ isSuperUser = TRUE;
> ++ }
> ++
> + DynBuf_Init(&db);
> +
> + stream = Posix_Popen(cmd, "r");
> + if (stream == NULL) {
> + Warning("Unable to get output of command \"%s\"\n", cmd);
> +
> +- return NULL;
> ++ goto exit;
> + }
> +
> + for (;;) {
> +@@ -844,11 +854,16 @@ HostinfoGetCmdOutput(const char *cmd) /
> + if (DynBuf_Get(&db)) {
> + out = (char *) DynBuf_AllocGet(&db);
> + }
> +- closeIt:
> +- DynBuf_Destroy(&db);
> +
> ++ closeIt:
> + pclose(stream);
> +
> ++ exit:
> ++ DynBuf_Destroy(&db);
> ++
> ++ if (isSuperUser) {
> ++ Id_BeginSuperUser();
> ++ }
> + return out;
> + }
> +
> +@@ -967,7 +982,7 @@ HostinfoOSData(void)
> + * Try to get OS detailed information from the lsb_release command.
> + */
> +
> +- lsbOutput = HostinfoGetCmdOutput("lsb_release -sd 2>/dev/null");
> ++ lsbOutput = HostinfoGetCmdOutput("/usr/bin/lsb_release -sd 2>/dev/null");
> + if (!lsbOutput) {
> + int i;
> +
>
Although we don't have a lsb package in Buildroot, this patch looks good.
Reviewed-by: Romain Naour <romain.naour@openwide.fr>
Best regards,
Romain
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec.
2015-02-25 22:14 [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Karoly Kasza
2015-02-25 22:14 ` [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack Karoly Kasza
@ 2015-03-08 21:39 ` Romain Naour
1 sibling, 0 replies; 4+ messages in thread
From: Romain Naour @ 2015-03-08 21:39 UTC (permalink / raw)
To: buildroot
Hi Karoly,
Le 25/02/2015 23:14, Karoly Kasza a ?crit :
> Add patch to use $(KVERS) instead of shell exec uname.
> Originally from Debian.
>
> Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> ---
This patch will be necessary once the kernel module support will be available in
Buildroot.
I have collected several patches which fix some build issues for latest stable
3.2 kernel up to 3.14.
But this requires some invasive fixes that I don't think we want to maintain in
openvmtools package...
At least, openvmtools kernel module build and works fine with 3.2 kernel.
I will do a runtime test with all patches I have and send them to the list.
(probably next weekend)
Best regards,
Romain
> package/openvmtools/0008-kvers.patch | 75 ++++++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 package/openvmtools/0008-kvers.patch
>
> diff --git a/package/openvmtools/0008-kvers.patch b/package/openvmtools/0008-kvers.patch
> new file mode 100644
> index 0000000..f432dbb
> --- /dev/null
> +++ b/package/openvmtools/0008-kvers.patch
> @@ -0,0 +1,75 @@
> +Use KVERS instead of shell exec uname.
> +
> +Patch from Debian.
> +
> +Original author: Cyril Brulebois <cyril.brulebois@enst-bretagne.fr>
> +Original description: Replacing uname call with KVERS, overwriting seemed not to work.
> +
> +Signed-off-by: Karoly Kasza <kaszak@gmail.com>
> +
> +--- a/modules/linux/vmblock/Makefile
> ++++ b/modules/linux/vmblock/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
> +--- a/modules/linux/vmci/Makefile
> ++++ b/modules/linux/vmci/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
> +--- a/modules/linux/vmhgfs/Makefile
> ++++ b/modules/linux/vmhgfs/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
> +--- a/modules/linux/vmsync/Makefile
> ++++ b/modules/linux/vmsync/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
> +--- a/modules/linux/vmxnet/Makefile
> ++++ b/modules/linux/vmxnet/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
> +--- a/modules/linux/vsock/Makefile
> ++++ b/modules/linux/vsock/Makefile
> +@@ -43,7 +43,7 @@ INCLUDE += -I$(SRCROOT)/shared
> + endif
> +
> +
> +-VM_UNAME = $(shell uname -r)
> ++VM_UNAME = $(KVERS)
> +
> + # Header directory for the running kernel
> + ifdef LINUXINCLUDE
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-03-08 21:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-25 22:14 [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Karoly Kasza
2015-02-25 22:14 ` [Buildroot] [PATCH 2/2] openvmtools: add patch to defend from lsb path walking attack Karoly Kasza
2015-03-08 21:24 ` Romain Naour
2015-03-08 21:39 ` [Buildroot] [PATCH 1/2] openvmtools: add patch to use KVERS instead of shell exec Romain Naour
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.