From c1be13f4eb5b3d24a273b6c8c9ca5421dcfac87b Mon Sep 17 00:00:00 2001 From: Katy Feng Date: Tue, 17 Oct 2023 15:24:48 -0700 Subject: [PATCH 1/2] Don't accept tokens with unrelated certs RH-Author: Ani Sinha RH-MergeRequest: 41: Don't accept tokens with unrelated certs RH-Jira: RHEL-14642 RH-Acked-by: Vitaly Kuznetsov 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