Skip to content

Commit c2337a9

Browse files
aditjha-msftaditjha-msft
authored
open-vm-tools: patch to address CVE-2023-34058 & CVE-2023-34059 (#8616)
1 parent f4d9443 commit c2337a9

3 files changed

Lines changed: 424 additions & 2 deletions

File tree

‎SPECS/open-vm-tools/CVE-2023-34058.patch‎

Lines changed: 236 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
From 6822b5a84f8cfa60d46479d6b8f1c63eb85eac87 Mon Sep 17 00:00:00 2001
2+
From: John Wolfe <jwolfe@vmware.com>
3+
Date: Wed, 18 Oct 2023 09:04:07 -0700
4+
Subject: [PATCH] Address CVE-2023-34058
5+
6+
VGAuth: don't accept tokens with unrelated certs.
7+
8+
---
9+
vgauth/common/certverify.c | 145 ++++++++++++++++++++++++
10+
vgauth/common/certverify.h | 4 +
11+
vgauth/common/prefs.h | 2 +
12+
vgauth/serviceImpl/saml-xmlsec1.c | 14 +++
13+
4 files changed, 165 insertions(+)
14+
15+
diff --git a/vgauth/common/certverify.c b/vgauth/common/certverify.c
16+
index 0ed78ed..e1d7cc6 100644
17+
--- a/vgauth/common/certverify.c
18+
+++ b/vgauth/common/certverify.c
19+
@@ -914,3 +914,148 @@ done:
20+
21+
return err;
22+
}
23+
+
24+
+
25+
+/*
26+
+ * Finds a cert with a subject (if checkSubj is set) or issuer (if
27+
+ * checkSUbj is unset), matching 'val' in the list
28+
+ * of certs. Returns a match or NULL.
29+
+ */
30+
+
31+
+static X509 *
32+
+FindCert(GList *cList,
33+
+ X509_NAME *val,
34+
+ int checkSubj)
35+
+{
36+
+ GList *l;
37+
+ X509 *c;
38+
+ X509_NAME *v;
39+
+
40+
+ l = cList;
41+
+ while (l != NULL) {
42+
+ c = (X509 *) l->data;
43+
+ if (checkSubj) {
44+
+ v = X509_get_subject_name(c);
45+
+ } else {
46+
+ v = X509_get_issuer_name(c);
47+
+ }
48+
+ if (X509_NAME_cmp(val, v) == 0) {
49+
+ return c;
50+
+ }
51+
+ l = l->next;
52+
+ }
53+
+ return NULL;
54+
+}
55+
+
56+
+
57+
+/*
58+
+ ******************************************************************************
59+
+ * CertVerify_CheckForUnrelatedCerts -- */ /**
60+
+ *
61+
+ * Looks over a list of certs. If it finds that they are not all
62+
+ * part of the same chain, returns failure.
63+
+ *
64+
+ * @param[in] numCerts The number of certs in the chain.
65+
+ * @param[in] pemCerts The chain of certificates to verify.
66+
+ *
67+
+ * @return VGAUTH_E_OK on success, VGAUTH_E_FAIL if unrelated certs are found.
68+
+ *
69+
+ ******************************************************************************
70+
+ */
71+
+
72+
+VGAuthError
73+
+CertVerify_CheckForUnrelatedCerts(int numCerts,
74+
+ const char **pemCerts)
75+
+{
76+
+ VGAuthError err = VGAUTH_E_FAIL;
77+
+ int chainLen = 0;
78+
+ int i;
79+
+ X509 **certs = NULL;
80+
+ GList *rawList = NULL;
81+
+ X509 *baseCert;
82+
+ X509 *curCert;
83+
+ X509_NAME *subject;
84+
+ X509_NAME *issuer;
85+
+
86+
+ /* common single cert case; nothing to do */
87+
+ if (numCerts == 1) {
88+
+ return VGAUTH_E_OK;
89+
+ }
90+
+
91+
+ /* convert all PEM to X509 objects */
92+
+ certs = g_malloc0(numCerts * sizeof(X509 *));
93+
+ for (i = 0; i < numCerts; i++) {
94+
+ certs[i] = CertStringToX509(pemCerts[i]);
95+
+ if (NULL == certs[i]) {
96+
+ g_warning("%s: failed to convert cert to X509\n", __FUNCTION__);
97+
+ goto done;
98+
+ }
99+
+ }
100+
+
101+
+ /* choose the cert to start the chain. shouldn't matter which */
102+
+ baseCert = certs[0];
103+
+
104+
+ /* put the rest into a list */
105+
+ for (i = 1; i < numCerts; i++) {
106+
+ rawList = g_list_append(rawList, certs[i]);
107+
+ }
108+
+
109+
+ /* now chase down to a leaf, looking for certs the baseCert issued */
110+
+ subject = X509_get_subject_name(baseCert);
111+
+ while ((curCert = FindCert(rawList, subject, 0)) != NULL) {
112+
+ /* pull it from the list */
113+
+ rawList = g_list_remove(rawList, curCert);
114+
+ /* set up the next find */
115+
+ subject = X509_get_subject_name(curCert);
116+
+ }
117+
+
118+
+ /*
119+
+ * walk up to the root cert, by finding a cert where the
120+
+ * issuer equals the subject of the current
121+
+ */
122+
+ issuer = X509_get_issuer_name(baseCert);
123+
+ while ((curCert = FindCert(rawList, issuer, 1)) != NULL) {
124+
+ /* pull it from the list */
125+
+ rawList = g_list_remove(rawList, curCert);
126+
+ /* set up the next find */
127+
+ issuer = X509_get_issuer_name(curCert);
128+
+ }
129+
+
130+
+ /*
131+
+ * At this point, anything on the list should be certs that are not part
132+
+ * of the chain that includes the original 'baseCert'.
133+
+ *
134+
+ * For a valid token, the list should be empty.
135+
+ */
136+
+ chainLen = g_list_length(rawList);
137+
+ if (chainLen != 0 ) {
138+
+ GList *l;
139+
+
140+
+ g_warning("%s: %d unrelated certs found in list\n",
141+
+ __FUNCTION__, chainLen);
142+
+
143+
+ /* debug helper */
144+
+ l = rawList;
145+
+ while (l != NULL) {
146+
+ X509* c = (X509 *) l->data;
147+
+ char *s = X509_NAME_oneline(X509_get_subject_name(c), NULL, 0);
148+
+
149+
+ g_debug("%s: unrelated cert subject: %s\n", __FUNCTION__, s);
150+
+ free(s);
151+
+ l = l->next;
152+
+ }
153+
+
154+
+ goto done;
155+
+ }
156+
+
157+
+ g_debug("%s: Success! no unrelated certs found\n", __FUNCTION__);
158+
+ err = VGAUTH_E_OK;
159+
+
160+
+done:
161+
+ g_list_free(rawList);
162+
+ for (i = 0; i < numCerts; i++) {
163+
+ X509_free(certs[i]);
164+
+ }
165+
+ g_free(certs);
166+
+ return err;
167+
+}
168+
diff --git a/vgauth/common/certverify.h b/vgauth/common/certverify.h
169+
index d7c6410..f582bb8 100644
170+
--- a/vgauth/common/certverify.h
171+
+++ b/vgauth/common/certverify.h
172+
@@ -67,6 +67,10 @@ VGAuthError CertVerify_CheckSignatureUsingCert(VGAuthHashAlg hash,
173+
size_t signatureLen,
174+
const unsigned char *signature);
175+
176+
+
177+
+VGAuthError CertVerify_CheckForUnrelatedCerts(int numCerts,
178+
+ const char **pemCerts);
179+
+
180+
gchar * CertVerify_StripPEMCert(const gchar *pemCert);
181+
182+
gchar * CertVerify_CertToX509String(const gchar *pemCert);
183+
diff --git a/vgauth/common/prefs.h b/vgauth/common/prefs.h
184+
index ff11692..87ccc9b 100644
185+
--- a/vgauth/common/prefs.h
186+
+++ b/vgauth/common/prefs.h
187+
@@ -136,6 +136,8 @@ msgCatalog = /etc/vmware-tools/vgauth/messages
188+
#define VGAUTH_PREF_ALIASSTORE_DIR "aliasStoreDir"
189+
/** The number of seconds slack allowed in either direction in SAML token date checks. */
190+
#define VGAUTH_PREF_CLOCK_SKEW_SECS "clockSkewAdjustment"
191+
+/** If unrelated certificates are allowed in a SAML token */
192+
+#define VGAUTH_PREF_ALLOW_UNRELATED_CERTS "allowUnrelatedCerts"
193+
194+
/** Ticket group name. */
195+
#define VGAUTH_PREF_GROUP_NAME_TICKET "ticket"
196+
diff --git a/vgauth/serviceImpl/saml-xmlsec1.c b/vgauth/serviceImpl/saml-xmlsec1.c
197+
index 14cba1b..57e9316 100644
198+
--- a/vgauth/serviceImpl/saml-xmlsec1.c
199+
+++ b/vgauth/serviceImpl/saml-xmlsec1.c
200+
@@ -49,6 +49,7 @@
201+
#include "vmxlog.h"
202+
203+
static int gClockSkewAdjustment = VGAUTH_PREF_DEFAULT_CLOCK_SKEW_SECS;
204+
+static gboolean gAllowUnrelatedCerts = FALSE;
205+
static xmlSchemaPtr gParsedSchemas = NULL;
206+
static xmlSchemaValidCtxtPtr gSchemaValidateCtx = NULL;
207+
208+
@@ -369,6 +370,10 @@ LoadPrefs(void)
209+
VGAUTH_PREF_DEFAULT_CLOCK_SKEW_SECS);
210+
Log("%s: Allowing %d of clock skew for SAML date validation\n",
211+
__FUNCTION__, gClockSkewAdjustment);
212+
+ gAllowUnrelatedCerts = Pref_GetBool(gPrefs,
213+
+ VGAUTH_PREF_ALLOW_UNRELATED_CERTS,
214+
+ VGAUTH_PREF_GROUP_NAME_SERVICE,
215+
+ FALSE);
216+
}
217+
218+
219+
@@ -1697,6 +1702,15 @@ SAML_VerifyBearerTokenAndChain(const char *xmlText,
220+
return VGAUTH_E_AUTHENTICATION_DENIED;
221+
}
222+
223+
+ if (!gAllowUnrelatedCerts) {
224+
+ err = CertVerify_CheckForUnrelatedCerts(num, (const char **) certChain);
225+
+ if (err != VGAUTH_E_OK) {
226+
+ VMXLog_Log(VMXLOG_LEVEL_WARNING,
227+
+ "Unrelated certs found in SAML token, failing\n");
228+
+ return VGAUTH_E_AUTHENTICATION_DENIED;
229+
+ }
230+
+ }
231+
+
232+
subj.type = SUBJECT_TYPE_NAMED;
233+
subj.name = *subjNameOut;
234+
err = ServiceVerifyAndCheckTrustCertChainForSubject(num,
235+
--
236+
2.6.2

0 commit comments

Comments
 (0)