Browse Source

HPCC-19956 LDAPSecMgr uses insecure LDAP even when LDAPS specified

Even though the admin configured LDAPS on secure port 636, when the ldap
security manager starts up it performs an anonymous bind to LDAP/389 in
order to query some DC feature support. This PR changes the code to first
try to anonymously bind to the configured port, if that fails tries to bind
to that same port with configured credentials, and if those fail tries an
anonymous bind to 389

Signed-off-by: Russ Whitehead <william.whitehead@lexisnexis.com>
Russ Whitehead 7 years ago
parent
commit
34e01e95ac

+ 25 - 1
system/security/LdapSecurity/ldapconnection.cpp

@@ -340,9 +340,33 @@ public:
         for (int numHosts=0; numHosts < getHostCount(); numHosts++)
         {
             getLdapHost(hostbuf);
+            unsigned port = strieq("ldaps",m_protocol) ? m_ldap_secure_port : m_ldapport;
+            StringBuffer sysUserDN, decPwd;
+
+            {
+                StringBuffer pwd;
+                cfg->getProp(".//@systemPassword", pwd);
+                if (pwd.isEmpty())
+                    throw MakeStringException(-1, "systemPassword is empty");
+                decrypt(decPwd, pwd.str());
+
+                StringBuffer sysUserCN;
+                cfg->getProp(".//@systemCommonName", sysUserCN);
+                if (sysUserCN.isEmpty())
+                    throw MakeStringException(-1, "systemCommonName is empty");
+
+                StringBuffer sysBasedn;
+                cfg->getProp(".//@systemBasedn", sysBasedn);
+                if (sysBasedn.isEmpty())
+                    throw MakeStringException(-1, "systemBasedn is empty");
+
+                //Guesstimate system user baseDN based on config settings. It will be used if anonymous bind fails
+                sysUserDN.append("cn=").append(sysUserCN.str()).append(",").append(sysBasedn.str());
+            }
+
             for(int retries = 0; retries <= LDAPSEC_MAX_RETRIES; retries++)
             {
-                rc = LdapUtils::getServerInfo(hostbuf.str(), m_ldapport, dcbuf, m_serverType, ldapDomain, m_timeout);
+                rc = LdapUtils::getServerInfo(hostbuf.str(), sysUserDN.str(), decPwd.str(), m_protocol, port, dcbuf, m_serverType, ldapDomain, m_timeout);
                 if(!LdapServerDown(rc) || retries >= LDAPSEC_MAX_RETRIES)
                     break;
                 sleep(LDAPSEC_RETRY_WAIT);

+ 46 - 12
system/security/LdapSecurity/ldaputils.cpp

@@ -30,7 +30,7 @@
 //------------------------------------
 // LdapUtils implementation
 //------------------------------------
-LDAP* LdapUtils::LdapInit(const char* protocol, const char* host, int port, int secure_port)
+LDAP* LdapUtils::LdapInit(const char* protocol, const char* host, int port, int secure_port, bool throwOnError)
 {
     LDAP* ld = NULL;
     if(stricmp(protocol, "ldaps") == 0)
@@ -74,7 +74,10 @@ LDAP* LdapUtils::LdapInit(const char* protocol, const char* host, int port, int
         int rc = LDAP_INIT(&ld, uri.str());
         if(rc != LDAP_SUCCESS)
         {
-            throw MakeStringException(-1, "ldap_initialize error %s", ldap_err2string(rc));
+            if (throwOnError)
+                throw MakeStringException(-1, "ldap_initialize error %s", ldap_err2string(rc));
+            DBGLOG("ldap_initialize error %s", ldap_err2string(rc));
+            return nullptr;
         }
         int reqcert = LDAP_OPT_X_TLS_NEVER;
         ldap_set_option(NULL, LDAP_OPT_X_TLS_REQUIRE_CERT, &reqcert);
@@ -83,7 +86,6 @@ LDAP* LdapUtils::LdapInit(const char* protocol, const char* host, int port, int
     else
     {
         // Initialize an LDAP session
-        DBGLOG("connecting to ldap://%s:%d", host, port);
 #ifdef _WIN32
         ld = LDAP_INIT(host, port);
         if(NULL == ld)
@@ -93,10 +95,14 @@ LDAP* LdapUtils::LdapInit(const char* protocol, const char* host, int port, int
 #else
         StringBuffer uri("ldap://");
         uri.appendf("%s:%d", host, port);
+        DBGLOG("connecting to %s", uri.str());
         int rc = LDAP_INIT(&ld, uri.str());
         if(rc != LDAP_SUCCESS)
         {
-            throw MakeStringException(-1, "ldap_initialize(%s,%d) error %s", host, port, ldap_err2string(rc));
+            if (throwOnError)
+                throw MakeStringException(-1, "ldap_initialize(%s,%d) error %s", host, port, ldap_err2string(rc));
+            DBGLOG("ldap_initialize error %s", ldap_err2string(rc));
+            return nullptr;
         }
 #endif
     }
@@ -208,20 +214,48 @@ int LdapUtils::LdapBind(LDAP* ld, int ldapTimeout, const char* domain, const cha
     return rc;
 }
 
-int LdapUtils::getServerInfo(const char* ldapserver, int ldapport, StringBuffer& domainDN, LdapServerType& stype, const char* domainname, int timeout)
+LDAP* LdapUtils::ldapInitAndSimpleBind(const char* ldapserver, const char* userDN, const char* pwd, const char* ldapprotocol, int ldapport, int timeout, int * err)
+{
+    LDAP* ld = LdapInit(ldapprotocol, ldapserver, ldapport, ldapport, false);
+    if (ld == nullptr)
+    {
+        VStringBuffer uri("%s://%s:%d", ldapprotocol, ldapserver, ldapport);
+        ERRLOG("ldap init error(%s)",uri.str());
+        *err = -1;
+        return nullptr;
+    }
+    *err = LdapSimpleBind(ld, timeout, (char*)userDN, (char*)pwd);
+    if (*err != LDAP_SUCCESS)
+    {
+        DBGLOG("LdapSimpleBind error (%d) - %s for admin user %s", *err, ldap_err2string(*err), isEmptyString(userDN) ? "NULL" : userDN);
+        if (!isEmptyString(userDN))
+            DBGLOG("Please make sure your LDAP configuration 'systemBasedn' contains the complete path, including the complete 'dc=domainComponent'");
+        return nullptr;
+    }
+    return ld;
+}
+
+int LdapUtils::getServerInfo(const char* ldapserver, const char* userDN, const char* pwd, const char* ldapprotocol, int ldapport, StringBuffer& domainDN, LdapServerType& stype, const char* domainname, int timeout)
 {
     LdapServerType deducedSType = LDAPSERVER_UNKNOWN;
-    LDAP* ld = LdapInit("ldap", ldapserver, ldapport, 636);
-    if(ld == NULL)
+
+    //First try anonymous bind using selected protocol/port
+    int err = -1;
+    LDAP* ld = ldapInitAndSimpleBind(ldapserver, nullptr, nullptr, ldapprotocol, ldapport, timeout, &err);
+
+    //if that failed, try bind with credentials
+    if (nullptr == ld)
     {
-        ERRLOG("ldap init error");
-        return false;
+        ld = ldapInitAndSimpleBind(ldapserver, userDN, pwd, ldapprotocol, ldapport, timeout, &err);
+
+        //if that failed, and was for ldaps, see if we can do anonymous bind using ldap/389
+        if (nullptr == ld  && strieq(ldapprotocol,"ldaps"))
+            ld = ldapInitAndSimpleBind(ldapserver, nullptr, nullptr, "ldap", 389, timeout, &err);
     }
 
-    int err = LdapSimpleBind(ld, timeout,NULL, NULL);
-    if(err != LDAP_SUCCESS)
+    if(nullptr == ld)
     {
-        DBGLOG("ldap anonymous bind error (%d) - %s", err, ldap_err2string(err));
+        DBGLOG("ldap bind error (%d) - %s", err, ldap_err2string(err));
 
         // for new versions of openldap, version 2.2.*
         if(err == LDAP_PROTOCOL_ERROR)

+ 3 - 2
system/security/LdapSecurity/ldaputils.hpp

@@ -37,12 +37,13 @@
 class LdapUtils
 {
 public:
-    static LDAP* LdapInit(const char* protocol, const char* host, int port, int secure_port);
+    static LDAP* LdapInit(const char* protocol, const char* host, int port, int secure_port, bool throwOnError = true);
     static int LdapSimpleBind(LDAP* ld, int ldapTimeout, char* userdn, char* password);
     // userdn is required for ldap_simple_bind_s, not really necessary for ldap_bind_s.
     static int LdapBind(LDAP* ld, int ldapTimeout, const char* domain, const char* username, const char* password, const char* userdn, LdapServerType server_type, const char* method="kerberos");
     static void bin2str(MemoryBuffer& from, StringBuffer& to);
-    static int getServerInfo(const char* ldapserver, int ldapport, StringBuffer& domainDN, LdapServerType& stype, const char* domainname, int timeout);
+    static LDAP* ldapInitAndSimpleBind(const char* ldapserver, const char* userDN, const char* pwd, const char* ldapprotocol, int ldapport, int timeout, int * err);
+    static int getServerInfo(const char* ldapserver, const char * user, const char *pwd, const char* ldapprotocol, int ldapport, StringBuffer& domainDN, LdapServerType& stype, const char* domainname, int timeout);
     static void normalizeDn(const char* dn, const char* basedn, StringBuffer& dnbuf);
     static bool containsBasedn(const char* str);
     static void cleanupDn(const char* dn, StringBuffer& dnbuf);