238 lines
7.6 KiB
Diff
238 lines
7.6 KiB
Diff
From 6822b5a84f8cfa60d46479d6b8f1c63eb85eac87 Mon Sep 17 00:00:00 2001
|
|
From: John Wolfe <jwolfe@vmware.com>
|
|
Date: Wed, 18 Oct 2023 09:04:07 -0700
|
|
Subject: [PATCH] Address CVE-2023-34058
|
|
|
|
VGAuth: don't accept tokens with unrelated certs.
|
|
|
|
---
|
|
open-vm-tools/vgauth/common/certverify.c | 145 ++++++++++++++++++++++++
|
|
open-vm-tools/vgauth/common/certverify.h | 4 +
|
|
open-vm-tools/vgauth/common/prefs.h | 2 +
|
|
open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c | 14 +++
|
|
4 files changed, 165 insertions(+)
|
|
|
|
diff --git a/open-vm-tools/vgauth/common/certverify.c b/open-vm-tools/vgauth/common/certverify.c
|
|
index 0ed78ed..e1d7cc6 100644
|
|
--- a/open-vm-tools/vgauth/common/certverify.c
|
|
+++ b/open-vm-tools/vgauth/common/certverify.c
|
|
@@ -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 d7c6410..f582bb8 100644
|
|
--- a/open-vm-tools/vgauth/common/certverify.h
|
|
+++ b/open-vm-tools/vgauth/common/certverify.h
|
|
@@ -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 ff11692..87ccc9b 100644
|
|
--- a/open-vm-tools/vgauth/common/prefs.h
|
|
+++ b/open-vm-tools/vgauth/common/prefs.h
|
|
@@ -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 14cba1b..57e9316 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);
|
|
}
|
|
|
|
|
|
@@ -1697,6 +1702,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,
|
|
--
|
|
2.6.2
|
|
|