From c1be13f4eb5b3d24a273b6c8c9ca5421dcfac87b Mon Sep 17 00:00:00 2001

From: Katy Feng <fkaty@vmware.com>

Date: Tue, 17 Oct 2023 15:24:48 -0700

Subject: [PATCH 1/2] Don't accept tokens with unrelated certs

RH-Author: Ani Sinha <None>

RH-MergeRequest: 41: Don't accept tokens with unrelated certs

RH-Jira: RHEL-14642

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Commit: [1/1] e1be40a04b53412393050e1e6106e8edad8b1716

If a SAML token has a cert that's not a part of a chain,

fail the token as invalid.

CVE-2023-34058

(cherry picked from commit 1bfe23d728b74e08f4f65cd9b0093ca73937003a)

---

 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 db280e91..b84ebc7d 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 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

@@ -893,3 +893,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 3fe1bcb4..228c143c 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 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

@@ -66,6 +66,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 ff116928..6c58f3f4 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 57db3b88..1c3e96e7 100644

--- a/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c

+++ b/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c

@@ -47,6 +47,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;

@@ -313,6 +314,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);

 }

@@ -1525,6 +1530,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.39.3