Merge pull request #3757 from Icinga/fix/ldap-connection-fail-with-multiple-servers-3639

Fix ldap connection fail with multiple servers

application/forms/Config/Resource/LdapResourceForm.php

@@ -48,38 +48,7 @@ class LdapResourceForm extends Form
                     'The hostname or address of the LDAP server to use for authentication.'
                     . ' You can also provide multiple hosts separated by a space'
                 ),
-                'value'         => 'localhost',
+                'value'         => 'localhost'
-                'validators'    => array(
-                    array(
-                        'Callback',
-                        false,
-                        array(
-                            'callback'  => function ($v) {
-                                $withoutScheme = $withScheme = false;
-                                foreach (explode(' ', $v) as $uri) {
-                                    if (preg_match('~^(?<!://)[^:]+:\d+$~', $uri)) {
-                                        return false;
-                                    }
-
-                                    $url = Url::fromPath($uri);
-                                    if ($url->getScheme()) {
-                                        $withScheme = true;
-                                    } else {
-                                        $withoutScheme = true;
-                                    }
-                                }
-
-                                return $withScheme ^ $withoutScheme;
-                            },
-                            'messages'  => array(
-                                'callbackValue' => $this->translate(
-                                    'A protocol scheme such as ldap:// or ldaps:// is mandatory for URIs with a given'
-                                    . ' port and for all other URIs as well once a scheme is given for a single one.'
-                                )
-                            )
-                        )
-                    )
-                )
             )
         );
         $this->addElement(

library/Icinga/Protocol/Ldap/LdapConnection.php

@@ -344,11 +344,10 @@ class LdapConnection implements Selectable, Inspectable
         $success = @ldap_bind($ds, $this->bindDn, $this->bindPw);
         if (! $success) {
             throw new LdapException(
-                'LDAP bind (%s / %s) to %s with default port %s failed: %s',
+                'LDAP bind (%s / %s) to %s failed: %s',
                 $this->bindDn,
                 '***' /* $this->bindPw */,
-                $this->hostname,
+                $this->normalizeHostname($this->hostname),
-                $this->port,
                 ldap_error($ds)
             );
         }
@@ -1179,25 +1178,7 @@ class LdapConnection implements Selectable, Inspectable
             $info = new Inspection('');
         }
 
-        $hostname = $this->hostname;
+        $hostname = $this->normalizeHostname($this->hostname);
-        if ($this->encryption === static::LDAPS) {
-            $info->write('Connect using LDAPS');
-            $ldapUrls = explode(' ', $hostname);
-            if (count($ldapUrls) > 1) {
-                foreach ($ldapUrls as & $uri) {
-                    if (preg_match('/:\d+$/', $uri) === 0) {
-                        $uri = $uri . ':' . $this->port;
-                    }
-                    if (strpos($uri, '://') === false) {
-                        $uri = 'ldaps://' . $uri;
-                    }
-                }
-
-                $hostname = implode(' ', $ldapUrls);
-            } else {
-                $hostname = 'ldaps://' . $hostname . ':' . $this->port;
-            }
-        }
 
         $ds = ldap_connect($hostname, $this->port);
 
@@ -1212,7 +1193,9 @@ class LdapConnection implements Selectable, Inspectable
         // Not setting this results in "Operations error" on AD when using the whole domain as search base
         ldap_set_option($ds, LDAP_OPT_REFERRALS, 0);
 
-        if ($this->encryption === static::STARTTLS) {
+        if ($this->encryption === static::LDAPS) {
+            $info->write('Connect using LDAPS');
+        } elseif ($this->encryption === static::STARTTLS) {
             $this->encrypted = true;
             $info->write('Connect using STARTTLS');
             if (! ldap_start_tls($ds)) {
@@ -1256,23 +1239,6 @@ class LdapConnection implements Selectable, Inspectable
             // We're checking the level by ourselves to avoid rendering the ldapsearch commandline for nothing
             $starttlsParam = $this->encryption === static::STARTTLS ? ' -ZZ' : '';
 
-            $ldapUrls = array();
-            $defaultScheme = $this->encryption === static::LDAPS ? 'ldaps://' : 'ldap://';
-            foreach (explode(' ', $this->hostname) as $uri) {
-                $url = Url::fromPath($uri);
-                if (! $url->getScheme()) {
-                    $uri = $defaultScheme . $uri . ($this->port ? ':' . $this->port : '');
-                } else {
-                    if ($url->getPort() === null) {
-                        $url->setPort($this->port);
-                    }
-
-                    $uri = $url->getAbsoluteUrl();
-                }
-
-                $ldapUrls[] = $uri;
-            }
-
             $bindParams = '';
             if ($this->bound) {
                 $bindParams = ' -D "' . $this->bindDn . '"' . ($this->bindPw ? ' -W' : '');
@@ -1291,7 +1257,7 @@ class LdapConnection implements Selectable, Inspectable
             Logger::debug("Issueing LDAP search. Use '%s' to reproduce.", sprintf(
                 'ldapsearch -P 3%s -H "%s"%s -b "%s" -s "%s" -z %u -l %u -a "%s"%s%s%s',
                 $starttlsParam,
-                implode(' ', $ldapUrls),
+                $this->normalizeHostname($this->hostname),
                 $bindParams,
                 $baseDn,
                 $scope,
@@ -1511,11 +1477,10 @@ class LdapConnection implements Selectable, Inspectable
         // Try a bind-command with the given user credentials, this must not fail
         $success = @ldap_bind($ds, $this->bindDn, $this->bindPw);
         $msg = sprintf(
-            'LDAP bind (%s / %s) to %s with default port %s',
+            'LDAP bind (%s / %s) to %s',
             $this->bindDn,
             '***' /* $this->bindPw */,
-            $this->hostname,
+            $this->normalizeHostname($this->hostname)
-            $this->port
         );
         if (! $success) {
             // ldap_error does not return any proper error messages in case of certificate errors. Connecting
@@ -1546,4 +1511,33 @@ class LdapConnection implements Selectable, Inspectable
         }
         return $insp;
     }
+
+    protected function normalizeHostname($hostname)
+    {
+        $scheme = $this->encryption === static::LDAPS ? 'ldaps://' : 'ldap://';
+        $normalizeHostname = function ($hostname) use ($scheme) {
+            if (strpos($hostname, $scheme) === false) {
+                $hostname = $scheme . $hostname;
+            }
+
+            if (! preg_match('/:\d+$/', $hostname)) {
+                $hostname .= ':' . $this->port;
+            }
+
+            return $hostname;
+        };
+
+        $ldapUrls = explode(' ', $hostname);
+        if (count($ldapUrls) > 1) {
+            foreach ($ldapUrls as & $uri) {
+                $uri = $normalizeHostname($uri);
+            }
+
+            $hostname = implode(' ', $ldapUrls);
+        } else {
+            $hostname = $normalizeHostname($hostname);
+        }
+
+        return $hostname;
+    }
 }