From: Oliver Kurth Date: Fri, 2 Nov 2018 22:28:18 +0000 (-0700) Subject: More logging improvements X-Git-Tag: stable-11.0.0~344 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=387121656a29e9643cc2624133669a72c77deda5;p=thirdparty%2Fopen-vm-tools.git More logging improvements vSECR doesn't want usernames going to the VMX, so remove them. Dump cert details when xmlsec fails to add the cert to its keystore. This can occur when the cert chain in the token has a bad cert, or one that isn't signed by the root cert in the token's chain. This can occur if a user has mis-configured an SSO server. --- diff --git a/open-vm-tools/vgauth/serviceImpl/alias.c b/open-vm-tools/vgauth/serviceImpl/alias.c index 66abf0859..8919a1459 100644 --- a/open-vm-tools/vgauth/serviceImpl/alias.c +++ b/open-vm-tools/vgauth/serviceImpl/alias.c @@ -2680,10 +2680,10 @@ check_map: SU_(alias.addid, "Alias added to Alias store owned by '%s' by user '%s'"), userName, reqUserName); + // security -- don't expose username VMXLog_Log(VMXLOG_LEVEL_WARNING, - "%s: alias added for user '%s' with Subject '%s'", + "%s: alias added with Subject '%s'", __FUNCTION__, - (userName != NULL) ? userName : "", (ai->type == SUBJECT_TYPE_ANY) ? "" : ai->name); } @@ -2954,15 +2954,15 @@ update: "Alias removed from Alias store owned by '%s' by user '%s'"), userName, reqUserName); if (removeAll) { + // security -- don't expose username VMXLog_Log(VMXLOG_LEVEL_WARNING, - "%s: all aliases removed for user '%s'", - __FUNCTION__, - (userName != NULL) ? userName : ""); + "%s: all aliases removed for requested username", + __FUNCTION__); } else { + // security -- don't expose username VMXLog_Log(VMXLOG_LEVEL_WARNING, - "%s: alias removed for user '%s' with Subject '%s'", + "%s: alias removed with Subject '%s'", __FUNCTION__, - (userName != NULL) ? userName : "", (subj->type == SUBJECT_TYPE_ANY) ? "" : subj->name); } } diff --git a/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c b/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c index bd1698fba..787a5faa8 100644 --- a/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c +++ b/open-vm-tools/vgauth/serviceImpl/saml-xmlsec1.c @@ -94,6 +94,7 @@ XmlErrorHandler(void *ctx, * Treat all as warning. */ g_warning("XML Error: %s", msgStr); + VMXLog_Log(VMXLOG_LEVEL_WARNING, "XML Error: %s", msgStr); } @@ -124,14 +125,20 @@ XmlSecErrorHandler(const char *file, const char *msg) { /* - * Treat all as warning. - */ + * Treat all as warning. */ g_warning("XMLSec Error: %s:%s(line %d) object %s" " subject %s reason: %d, msg: %s", file, func, line, errorObject ? errorObject : "", errorSubject ? errorSubject : "", reason, msg); + VMXLog_Log(VMXLOG_LEVEL_WARNING, + "XMLSec Error: %s:%s(line %d) object %s" + " subject %s reason: %d, msg: %s", + file, func, line, + errorObject ? errorObject : "", + errorSubject ? errorSubject : "", + reason, msg); } @@ -385,6 +392,11 @@ SAML_Init(void) "Make sure that you have xmlsec1-openssl installed and\n" "check shared libraries path\n" "(LD_LIBRARY_PATH) environment variable.\n"); + VMXLog_Log(VMXLOG_LEVEL_WARNING, + "Error: unable to load openssl xmlsec-crypto library.\n " + "Make sure that you have xmlsec1-openssl installed and\n" + "check shared libraries path\n" + "(LD_LIBRARY_PATH) environment variable.\n"); return VGAUTH_E_FAIL; } #endif /* XMLSEC_CRYPTO_DYNAMIC_LOADING */ @@ -415,6 +427,10 @@ SAML_Init(void) Log("%s: Using xmlsec1 %d.%d.%d for XML signature support\n", __FUNCTION__, XMLSEC_VERSION_MAJOR, XMLSEC_VERSION_MINOR, XMLSEC_VERSION_SUBMINOR); + VMXLog_Log(VMXLOG_LEVEL_WARNING, + "%s: Using xmlsec1 %d.%d.%d for XML signature support\n", + __FUNCTION__, XMLSEC_VERSION_MAJOR, XMLSEC_VERSION_MINOR, + XMLSEC_VERSION_SUBMINOR); return VGAUTH_E_OK; } @@ -1108,7 +1124,11 @@ BuildCertChain(xmlNodePtr x509Node, xmlSecKeyDataFormatPem, xmlSecKeyDataTypeTrusted); if (ret < 0) { - g_warning("Failed to add cert to key manager\n"); + g_warning("%s: Failed to add cert to key manager\n", __FUNCTION__); + g_warning("PEM cert: %s\n", pemCert); + VMXLog_Log(VMXLOG_LEVEL_WARNING, + "%s: Failed to add cert to key manager\n", __FUNCTION__); + VMXLog_Log(VMXLOG_LEVEL_WARNING, "PEM cert: %s\n", pemCert); goto done; } @@ -1370,6 +1390,7 @@ VerifySAMLToken(const gchar *token, bRet = VerifySignature(doc, numCerts, certChain); if (FALSE == bRet) { g_warning("Failed to verify Signature\n"); + // XXX Can we log the token at this point without risking security? goto done; } diff --git a/open-vm-tools/vgauth/serviceImpl/verify.c b/open-vm-tools/vgauth/serviceImpl/verify.c index c527809b8..f69edc0ab 100644 --- a/open-vm-tools/vgauth/serviceImpl/verify.c +++ b/open-vm-tools/vgauth/serviceImpl/verify.c @@ -147,9 +147,11 @@ ServiceVerifyAndCheckTrustCertChainForSubject(int numCerts, /* * No username, no mapped certs, no chance. */ - Warning("%s: no mapping entries or userName\n", __FUNCTION__); + Warning("%s: no mapping entries or specified userName\n", + __FUNCTION__); VMXLog_Log(VMXLOG_LEVEL_WARNING, - "%s: no mapping entries or userName\n", __FUNCTION__); + "%s: no mapping entries or specified userName\n", + __FUNCTION__); err = VGAUTH_E_AUTHENTICATION_DENIED; goto done; } @@ -215,7 +217,7 @@ ServiceVerifyAndCheckTrustCertChainForSubject(int numCerts, if (!UsercheckUserExists(queryUserName)) { Warning("%s: User '%s' doesn't exist\n", __FUNCTION__, queryUserName); VMXLog_Log(VMXLOG_LEVEL_WARNING, - "%s: User '%s' doesn't exist\n", __FUNCTION__, queryUserName); + "%s: User doesn't exist\n", __FUNCTION__); err = VGAUTH_E_AUTHENTICATION_DENIED; goto done; }