rpms/open-vm-tools
7
0
Fork 0
Code Issues Pull Requests Releases Activity

- File descriptor vulnerability in the open-vm-tools

Browse Source 
vmware-user-suid-wrapperx on Linux
- Don't accept tokens with unrelated certs
This commit is contained in:
eabdullin 2023-11-16 11:19:46 +03:00
parent 808725f60d
commit 3a1b8841f0
4 changed files with 463 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

255
SOURCES/Dont-accept-tokens-with-unrelated-certs.patch Normal file
View File

@ -0,0 +1,255 @@
From 1bfe23d728b74e08f4f65cd9b0093ca73937003a Mon Sep 17 00:00:00 2001
From: Katy Feng <fkaty@vmware.com>
Date: Tue, 17 Oct 2023 15:24:48 -0700
Subject: [PATCH] Don't accept tokens with unrelated certs
If a SAML token has a cert that's not a part of a chain,
fail the token as invalid.
---
open-vm-tools/vgauth/common/certverify.c | 147 +++++++++++++++++-
open-vm-tools/vgauth/common/certverify.h | 6 +-
open-vm-tools/vgauth/common/prefs.h | 4 +-
.../vgauth/serviceImpl/saml-xmlsec1.c | 14 ++
4 files changed, 168 insertions(+), 3 deletions(-)
diff --git a/open-vm-tools/vgauth/common/certverify.c b/open-vm-tools/vgauth/common/certverify.c
index 0ed78edb0..845f59b91 100644
--- a/open-vm-tools/vgauth/common/certverify.c
+++ b/open-vm-tools/vgauth/common/certverify.c
@@ -1,5 +1,5 @@
/*********************************************************
- * Copyright (C) 2011-2016, 2018-2019, 2021-2022 VMware, Inc. All rights reserved.
+ * Copyright (c) 2011-2016, 2018-2019, 2021-2023 VMware, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published
@@ -914,3 +914,148 @@ done:
return err;
}
+
+
+/*
+ * Finds a cert with a subject (if checkSubj is set) or issuer (if
+ * checkSUbj is unset), matching 'val' in the list
+ * of certs. Returns a match or NULL.
+ */
+
+static X509 *
+FindCert(GList *cList,
+ X509_NAME *val,
+ int checkSubj)
+{
+ GList *l;
+ X509 *c;
+ X509_NAME *v;
+
+ l = cList;
+ while (l != NULL) {
+ c = (X509 *) l->data;
+ if (checkSubj) {
+ v = X509_get_subject_name(c);
+ } else {
+ v = X509_get_issuer_name(c);
+ }
+ if (X509_NAME_cmp(val, v) == 0) {
+ return c;
+ }
+ l = l->next;
+ }
+ return NULL;
+}
+
+
+/*
+ ******************************************************************************
+ * CertVerify_CheckForUnrelatedCerts -- */ /**
+ *
+ * Looks over a list of certs. If it finds that they are not all
+ * part of the same chain, returns failure.
+ *
+ * @param[in] numCerts The number of certs in the chain.
+ * @param[in] pemCerts The chain of certificates to verify.
+ *
+ * @return VGAUTH_E_OK on success, VGAUTH_E_FAIL if unrelated certs are found.
+ *
+ ******************************************************************************
+ */
+
+VGAuthError
+CertVerify_CheckForUnrelatedCerts(int numCerts,
+ const char **pemCerts)
+{
+ VGAuthError err = VGAUTH_E_FAIL;
+ int chainLen = 0;
+ int i;
+ X509 **certs = NULL;
+ GList *rawList = NULL;
+ X509 *baseCert;
+ X509 *curCert;
+ X509_NAME *subject;
+ X509_NAME *issuer;
+
+ /* common single cert case; nothing to do */
+ if (numCerts == 1) {
+ return VGAUTH_E_OK;
+ }
+
+ /* convert all PEM to X509 objects */
+ certs = g_malloc0(numCerts * sizeof(X509 *));
+ for (i = 0; i < numCerts; i++) {
+ certs[i] = CertStringToX509(pemCerts[i]);
+ if (NULL == certs[i]) {
+ g_warning("%s: failed to convert cert to X509\n", __FUNCTION__);
+ goto done;
+ }
+ }
+
+ /* choose the cert to start the chain. shouldn't matter which */
+ baseCert = certs[0];
+
+ /* put the rest into a list */
+ for (i = 1; i < numCerts; i++) {
+ rawList = g_list_append(rawList, certs[i]);
+ }
+
+ /* now chase down to a leaf, looking for certs the baseCert issued */
+ subject = X509_get_subject_name(baseCert);
+ while ((curCert = FindCert(rawList, subject, 0)) != NULL) {
+ /* pull it from the list */
+ rawList = g_list_remove(rawList, curCert);
+ /* set up the next find */
+ subject = X509_get_subject_name(curCert);
+ }
+
+ /*
+ * walk up to the root cert, by finding a cert where the
+ * issuer equals the subject of the current
+ */
+ issuer = X509_get_issuer_name(baseCert);
+ while ((curCert = FindCert(rawList, issuer, 1)) != NULL) {
+ /* pull it from the list */
+ rawList = g_list_remove(rawList, curCert);
+ /* set up the next find */
+ issuer = X509_get_issuer_name(curCert);
+ }
+
+ /*
+ * At this point, anything on the list should be certs that are not part
+ * of the chain that includes the original 'baseCert'.
+ *
+ * For a valid token, the list should be empty.
+ */
+ chainLen = g_list_length(rawList);
+ if (chainLen != 0 ) {
+ GList *l;
+
+ g_warning("%s: %d unrelated certs found in list\n",
+ __FUNCTION__, chainLen);
+
+ /* debug helper */
+ l = rawList;
+ while (l != NULL) {
+ X509* c = (X509 *) l->data;
+ char *s = X509_NAME_oneline(X509_get_subject_name(c), NULL, 0);
+
+ g_debug("%s: unrelated cert subject: %s\n", __FUNCTION__, s);
+ free(s);
+ l = l->next;
+ }
+
+ goto done;
+ }
+
+ g_debug("%s: Success! no unrelated certs found\n", __FUNCTION__);
+ err = VGAUTH_E_OK;
+
+done:
+ g_list_free(rawList);
+ for (i = 0; i < numCerts; i++) {
+ X509_free(certs[i]);
+ }
+ g_free(certs);
+ return err;
+}
diff --git a/open-vm-tools/vgauth/common/certverify.h b/open-vm-tools/vgauth/common/certverify.h
index d7c6410b6..89ec97a10 100644
--- a/open-vm-tools/vgauth/common/certverify.h
+++ b/open-vm-tools/vgauth/common/certverify.h
@@ -1,5 +1,5 @@
/*********************************************************
- * Copyright (C) 2011-2016, 2020 VMware, Inc. All rights reserved.
+ * Copyright (C) 2011-2016, 2020, 2023 VMware, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published
@@ -67,6 +67,10 @@ VGAuthError CertVerify_CheckSignatureUsingCert(VGAuthHashAlg hash,
size_t signatureLen,
const unsigned char *signature);
+
+VGAuthError CertVerify_CheckForUnrelatedCerts(int numCerts,
+ const char **pemCerts);
+
gchar * CertVerify_StripPEMCert(const gchar *pemCert);
gchar * CertVerify_CertToX509String(const gchar *pemCert);
diff --git a/open-vm-tools/vgauth/common/prefs.h b/open-vm-tools/vgauth/common/prefs.h
index ff116928c..6c58f3f4b 100644
--- a/open-vm-tools/vgauth/common/prefs.h
+++ b/open-vm-tools/vgauth/common/prefs.h
@@ -1,5 +1,5 @@
/*********************************************************
- * Copyright (C) 2011-2019 VMware, Inc. All rights reserved.
+ * Copyright (C) 2011-2019,2023 VMware, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published
@@ -136,6 +136,8 @@ msgCatalog = /etc/vmware-tools/vgauth/messages
#define VGAUTH_PREF_ALIASSTORE_DIR "aliasStoreDir"
/** The number of seconds slack allowed in either direction in SAML token date checks. */
#define VGAUTH_PREF_CLOCK_SKEW_SECS "clockSkewAdjustment"
+/** If unrelated certificates are allowed in a SAML token */
+#define VGAUTH_PREF_ALLOW_UNRELATED_CERTS "allowUnrelatedCerts"
/** Ticket group name. */
#define VGAUTH_PREF_GROUP_NAME_TICKET "ticket"
diff --git a/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c b/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c
index 14cba1b5b..57e931626 100644
--- a/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c
+++ b/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c
@@ -49,6 +49,7 @@
#include "vmxlog.h"
static int gClockSkewAdjustment = VGAUTH_PREF_DEFAULT_CLOCK_SKEW_SECS;
+static gboolean gAllowUnrelatedCerts = FALSE;
static xmlSchemaPtr gParsedSchemas = NULL;
static xmlSchemaValidCtxtPtr gSchemaValidateCtx = NULL;
@@ -369,6 +370,10 @@ LoadPrefs(void)
VGAUTH_PREF_DEFAULT_CLOCK_SKEW_SECS);
Log("%s: Allowing %d of clock skew for SAML date validation\n",
__FUNCTION__, gClockSkewAdjustment);
+ gAllowUnrelatedCerts = Pref_GetBool(gPrefs,
+ VGAUTH_PREF_ALLOW_UNRELATED_CERTS,
+ VGAUTH_PREF_GROUP_NAME_SERVICE,
+ FALSE);
}
@@ -1589,6 +1594,15 @@ SAML_VerifyBearerTokenAndChain(const char *xmlText,
return VGAUTH_E_AUTHENTICATION_DENIED;
}
+ if (!gAllowUnrelatedCerts) {
+ err = CertVerify_CheckForUnrelatedCerts(num, (const char **) certChain);
+ if (err != VGAUTH_E_OK) {
+ VMXLog_Log(VMXLOG_LEVEL_WARNING,
+ "Unrelated certs found in SAML token, failing\n");
+ return VGAUTH_E_AUTHENTICATION_DENIED;
+ }
+ }
+
subj.type = SUBJECT_TYPE_NAMED;
subj.name = *subjNameOut;
err = ServiceVerifyAndCheckTrustCertChainForSubject(num,

196
SOURCES/File-descriptor-vulnerability-in-the-open-vm-tools.patch Normal file
View File

@ -0,0 +1,196 @@
From 63f7c79c4aecb14d37cc4ce9da509419e31d394f Mon Sep 17 00:00:00 2001
From: Katy Feng <fkaty@vmware.com>
Date: Tue, 17 Oct 2023 15:24:48 -0700
Subject: [PATCH] File descriptor vulnerability in the open-vm-tools
vmware-user-suid-wrapperx on Linux
Moving the privilege drop logic (dropping privilege to the real uid and
gid of the process for the vmusr service) from suidWrapper to vmtoolsd code.
Now the vmtoolsd is not executed with dropped privileges (started as setuid
program) and the dumpable attribute of the process is not reset.
The unprivileged user will not have access to the privileged file descriptors
in the vmtoolsd vmusr process.
Also, setting the FD_CLOEXEC flag for both uinputFd and blockFd preventing
the file descriptors being inherited any further from the vmtoolsd.
---
open-vm-tools/services/vmtoolsd/mainPosix.c | 78 ++++++++++++++++++-
open-vm-tools/vmware-user-suid-wrapper/main.c | 28 +------
2 files changed, 81 insertions(+), 25 deletions(-)
diff --git a/open-vm-tools/services/vmtoolsd/mainPosix.c b/open-vm-tools/services/vmtoolsd/mainPosix.c
index fd2667cd5..6c52156bc 100644
--- a/open-vm-tools/services/vmtoolsd/mainPosix.c
+++ b/open-vm-tools/services/vmtoolsd/mainPosix.c
@@ -1,5 +1,5 @@
/*********************************************************
- * Copyright (C) 2008-2020,2022 VMware, Inc. All rights reserved.
+ * Copyright (c) 2008-2020,2022-2023 VMware, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published
@@ -28,10 +28,12 @@
#include <signal.h>
#include <string.h>
#include <unistd.h>
+#include <fcntl.h>
#include <glib/gstdio.h>
#include "file.h"
#include "guestApp.h"
#include "hostinfo.h"
+#include "su.h"
#include "system.h"
#include "unicode.h"
#include "util.h"
@@ -154,6 +156,59 @@ ToolsCoreWorkAroundLoop(ToolsServiceState *state,
}
+/**
+ * Tools function to set close-on-exec flg for the fd.
+ *
+ * @param[in] fd open file descriptor.
+ *
+ * @return TRUE on success, FALSE otherwise.
+ */
+
+static gboolean
+ToolsSetCloexecFlag(int fd)
+{
+ int flags;
+
+ if (fd == -1) {
+ /* fd is not present, no need to manipulate */
+ return TRUE;
+ }
+
+ flags = fcntl(fd, F_GETFD, 0);
+ if (flags < 0) {
+ g_printerr("Couldn't get the flags set for fd %d, error %u.", fd, errno);
+ return FALSE;
+ }
+ flags |= FD_CLOEXEC;
+ if (fcntl(fd, F_SETFD, flags) < 0) {
+ g_printerr("Couldn't set close-on-exec for fd %d, error %u.", fd, errno);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+
+/**
+ * Tools function to close the fds.
+ */
+
+static void
+ToolsCloseFds(void)
+{
+ if (gState.ctx.blockFD != -1) {
+ close(gState.ctx.blockFD);
+ }
+
+ /*
+ * uinputFD will be available only for wayland.
+ */
+ if (gState.ctx.uinputFD != -1) {
+ close(gState.ctx.uinputFD);
+ }
+}
+
+
/**
* Tools daemon entry function.
*
@@ -210,6 +265,27 @@ main(int argc,
g_free(argvCopy);
argvCopy = NULL;
+ /*
+ * Drops privilege to the real uid and gid of the process
+ * for the "vmusr" service.
+ */
+ if (TOOLS_IS_USER_SERVICE(&gState)) {
+ uid_t uid = getuid();
+ gid_t gid = getgid();
+
+ if ((Id_SetREUid(uid, uid) != 0) ||
+ (Id_SetREGid(gid, gid) != 0)) {
+ g_printerr("could not drop privileges: %s", strerror(errno));
+ ToolsCloseFds();
+ goto exit;
+ }
+ if (!ToolsSetCloexecFlag(gState.ctx.blockFD) ||
+ !ToolsSetCloexecFlag(gState.ctx.uinputFD)) {
+ ToolsCloseFds();
+ goto exit;
+ }
+ }
+
if (gState.pidFile != NULL) {
/*
* If argv[0] is not an absolute path, make it so; all other path
diff --git a/open-vm-tools/vmware-user-suid-wrapper/main.c b/open-vm-tools/vmware-user-suid-wrapper/main.c
index e9d7e5084..73ae9b9bb 100644
--- a/open-vm-tools/vmware-user-suid-wrapper/main.c
+++ b/open-vm-tools/vmware-user-suid-wrapper/main.c
@@ -1,5 +1,5 @@
/*********************************************************
- * Copyright (C) 2007-2018 VMware, Inc. All rights reserved.
+ * Copyright (C) 2007-2018,2023 VMware, Inc. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as published
@@ -156,8 +156,7 @@ MaskSignals(void)
*
* Obtains the library directory from the Tools locations database, then
* opens a file descriptor (while still root) to add and remove blocks,
- * drops privilege to the real uid of this process, and finally starts
- * vmware-user.
+ * and finally starts vmware-user.
*
* Results:
* Parent: TRUE on success, FALSE on failure.
@@ -173,8 +172,6 @@ static Bool
StartVMwareUser(char *const envp[])
{
pid_t pid;
- uid_t uid;
- gid_t gid;
int blockFd = -1;
char blockFdStr[8];
int uinputFd = -1;
@@ -191,8 +188,8 @@ StartVMwareUser(char *const envp[])
}
/*
- * Now create a child process, obtain a file descriptor as root, downgrade
- * privilege, and run vmware-user.
+ * Now create a child process, obtain a file descriptor as root and
+ * run vmware-user.
*/
pid = fork();
if (pid == -1) {
@@ -229,23 +226,6 @@ StartVMwareUser(char *const envp[])
}
}
- uid = getuid();
- gid = getgid();
-
- if ((setreuid(uid, uid) != 0) ||
- (setregid(gid, gid) != 0)) {
- Error("could not drop privileges: %s\n", strerror(errno));
- if (blockFd != -1) {
- close(blockFd);
- }
- if (useWayland) {
- if (uinputFd != -1) {
- close(uinputFd);
- }
- }
- return FALSE;
- }
-
/*
* Since vmware-user provides features that don't depend on vmblock, we
* invoke vmware-user even if we couldn't obtain a file descriptor or we

12
SOURCES/ovt-VGAuth-Allow-only-X509-certs-to-verify-the-SAML-toke.patch
View File

@ -1,19 +1,11 @@
<<<<<<< HEAD
From 2dc6f33e455c7d0dceb2d444632b35806613c510 Mon Sep 17 00:00:00 2001
=======
From a839cb975d58968237bd871b1fb4cbe191af085b Mon Sep 17 00:00:00 2001
>>>>>>> c8
From: Miroslav Rezanina <mrezanin@redhat.com>
Date: Thu, 7 Sep 2023 02:27:50 -0400
Subject: [PATCH] VGAuth: Allow only X509 certs to verify the SAML token
signature.
RH-Author: Miroslav Rezanina <mrezanin@redhat.com>
<<<<<<< HEAD
RH-Bugzilla: 2236544
=======
RH-Bugzilla: 2236543
>>>>>>> c8
RH-CVE: CVE-2023-20900
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
@ -43,7 +35,3 @@ index f5541a9a..0b2a945b 100644
--
2.39.3
<<<<<<< HEAD
=======
>>>>>>> c8

13
SPECS/open-vm-tools.spec
View File

@ -32,7 +32,7 @@
Name: open-vm-tools
Version: %{toolsversion}
Release: 3%{?dist}
Release: 3%{?dist}.1.alma.1
Summary: Open Virtual Machine Tools for virtual machines hosted on VMware
License: GPLv2
URL: https://github.com/vmware/%{name}
@ -57,6 +57,12 @@ Patch1: ovt-VGAuth-Allow-only-X509-certs-to-verify-the-SAML-toke.patch
# For RHEL-7012 - [RHEL8.10][ESXi]Latest version of open-vm-tools breaks VM backups
Patch2: ovt-Provide-alternate-method-to-allow-expected-pre-froze.patch
# Patches were taken from:
# https://github.com/vmware/open-vm-tools/commit/1bfe23d728b74e08f4f65cd9b0093ca73937003a
Patch3: Dont-accept-tokens-with-unrelated-certs.patch
# https://github.com/vmware/open-vm-tools/commit/63f7c79c4aecb14d37cc4ce9da509419e31d394f
Patch4: File-descriptor-vulnerability-in-the-open-vm-tools.patch
BuildRequires: autoconf
BuildRequires: automake
BuildRequires: libtool
@ -414,6 +420,11 @@ fi
%{_bindir}/vmware-vgauth-smoketest
%changelog
* Thu Nov 16 2023 Eduard Abdullin <eabdullin@almalinux.org> - 12.2.5-3.1.alma.1
- File descriptor vulnerability in the open-vm-tools
vmware-user-suid-wrapperx on Linux
- Don't accept tokens with unrelated certs
* Wed Sep 27 2023 Jon Maloy <jmaloy@redhat.com> - 12.2.5-3
- ovt-Provide-alternate-method-to-allow-expected-pre-froze.patch [RHEL-7012]
- Resolves: RHEL-7012