From 6551a86d4d82153bb4831c2f790a0c5c828f88e0 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 11:40:30 +0100 Subject: [PATCH 01/12] LdapRepository: Drop method isAmbiguous() and introduce isRelatedDn() refs #10567 --- library/Icinga/Repository/LdapRepository.php | 23 ++++++++------------ 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/library/Icinga/Repository/LdapRepository.php b/library/Icinga/Repository/LdapRepository.php index 5e79c41bb..0ebde56dd 100644 --- a/library/Icinga/Repository/LdapRepository.php +++ b/library/Icinga/Repository/LdapRepository.php @@ -42,15 +42,6 @@ abstract class LdapRepository extends Repository 'groupofuniquenames' => 'groupOfUniqueNames' ); - /** - * Object attributes whose value is not distinguished name - * - * @var array - */ - protected $ambiguousAttributes = array( - 'posixGroup' => 'memberUid' - ); - /** * Create a new LDAP repository object * @@ -79,15 +70,19 @@ abstract class LdapRepository extends Repository } /** - * Return whether the given object attribute's value is not a distinguished name + * Return whether the given object DN is related to the given base DN * - * @param string $objectClass - * @param string $attributeName + * Will use the current connection's root DN if $baseDn is not given. + * + * @param string $dn The object DN to check + * @param string $baseDn The base DN to compare the object DN with * * @return bool */ - protected function isAmbiguous($objectClass, $attributeName) + protected function isRelatedDn($dn, $baseDn = null) { - return isset($this->ambiguousAttributes[$objectClass][$attributeName]); + $normalizedDn = strtolower(join(',', array_map('trim', explode(',', $dn)))); + $normalizedBaseDn = strtolower(join(',', array_map('trim', explode(',', $baseDn ?: $this->ds->getDn())))); + return strpos($normalizedDn, $normalizedBaseDn) !== false; } } From cfb26e22b35c8e19767233a4db3a22864453a483 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Mon, 9 Nov 2015 11:41:11 +0100 Subject: [PATCH 02/12] LdapUserGroupBackend: Dynamically verify member attribute ambiguity refs #10567 --- .../UserGroup/LdapUserGroupBackend.php | 54 ++++++++++++++----- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 9a4ab53c0..8c3fc3c3d 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -72,6 +72,13 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt */ protected $groupMemberAttribute; + /** + * Whether the attribute name where to find a group's member holds ambiguous values + * + * @var bool + */ + protected $ambiguousMemberAttribute; + /** * The custom LDAP filter to apply on a user query * @@ -357,6 +364,39 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt return $this->groupFilter; } + /** + * Return whether the attribute name where to find a group's member holds ambiguous values + * + * @return bool + * + * @throws ProgrammingError In case either $this->groupClass or $this->groupMemberAttribute + * has not been set yet + */ + protected function isMemberAttributeAmbiguous() + { + if ($this->ambiguousMemberAttribute === null) { + if ($this->groupClass === null) { + throw new ProgrammingError( + 'It is required to set the objectClass where to look for groups first' + ); + } elseif ($this->groupMemberAttribute === null) { + throw new ProgrammingError( + 'It is required to set a attribute name where to find a group\'s members first' + ); + } + + $sampleValue = $this->ds + ->select() + ->from($this->groupClass, array($this->groupMemberAttribute)) + ->setUnfoldAttribute($this->groupMemberAttribute) + ->setBase($this->groupBaseDn) + ->fetchOne(); + $this->ambiguousMemberAttribute = !$this->isRelatedDn($sampleValue); + } + + return $this->ambiguousMemberAttribute; + } + /** * Return a new query for the given columns * @@ -431,19 +471,9 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt * Initialize this repository's conversion rules * * @return array - * - * @throws ProgrammingError In case either $this->groupClass or $this->groupMemberAttribute - * has not been set yet */ protected function initializeConversionRules() { - if ($this->groupClass === null) { - throw new ProgrammingError('It is required to set the objectClass where to look for groups first'); - } - if ($this->groupMemberAttribute === null) { - throw new ProgrammingError('It is required to set a attribute name where to find a group\'s members first'); - } - $rules = array( 'group' => array( 'created_at' => 'generalized_time', @@ -454,7 +484,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt 'last_modified' => 'generalized_time' ) ); - if (! $this->isAmbiguous($this->groupClass, $this->groupMemberAttribute)) { + if (! $this->isMemberAttributeAmbiguous()) { $rules['group_membership']['user_name'] = 'user_name'; $rules['group_membership']['user'] = 'user_name'; $rules['group']['user_name'] = 'user_name'; @@ -566,7 +596,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt */ public function getMemberships(User $user) { - if ($this->isAmbiguous($this->groupClass, $this->groupMemberAttribute)) { + if ($this->isMemberAttributeAmbiguous()) { $queryValue = $user->getUsername(); } elseif (($queryValue = $user->getAdditional('ldap_dn')) === null) { $userQuery = $this->ds From a662fc9af0ba3a40c27fc38b6cd9d310d5bfd87e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Tue, 10 Nov 2015 16:08:02 +0100 Subject: [PATCH 03/12] Controller: Re-add "view" as preserved column We're still utilizing this in the dashboard.. --- library/Icinga/Web/Controller.php | 1 + 1 file changed, 1 insertion(+) diff --git a/library/Icinga/Web/Controller.php b/library/Icinga/Web/Controller.php index 891b70329..ca6cb7e23 100644 --- a/library/Icinga/Web/Controller.php +++ b/library/Icinga/Web/Controller.php @@ -203,6 +203,7 @@ class Controller extends ModuleActionController 'sort', // setupSortControl() 'dir', // setupSortControl() 'backend', // Framework + 'view', // Framework '_dev' // Framework ); From 60a951a97ddb6f6b9e6aff01caf4ce3eee832f2e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 09:59:28 +0100 Subject: [PATCH 04/12] Logger: Add method getLevel() refs #10567 --- library/Icinga/Application/Logger.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/library/Icinga/Application/Logger.php b/library/Icinga/Application/Logger.php index 9203a2f75..08568431f 100644 --- a/library/Icinga/Application/Logger.php +++ b/library/Icinga/Application/Logger.php @@ -136,6 +136,16 @@ class Logger return $this; } + /** + * Return the logging level being used + * + * @return int + */ + public function getLevel() + { + return $this->level; + } + /** * Register the given message as config error * From c85bce72115209c739844545d703a33c05228c11 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 10:01:00 +0100 Subject: [PATCH 05/12] LdapConnection: Add method ldapSearch() This will now emit a debug message for each issued search operation. refs #10567 --- .../Icinga/Protocol/Ldap/LdapConnection.php | 105 ++++++++++++++---- 1 file changed, 82 insertions(+), 23 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index ceefc81b8..8ca034653 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -377,14 +377,7 @@ class LdapConnection implements Selectable, Inspectable } $ds = $this->getConnection(); - $results = @ldap_search( - $ds, - $query->getBase() ?: $this->getDn(), - (string) $query, - array('dn'), - 0, - 0 - ); + $results = $this->ldapSearch($query, array('dn')); if ($results === false) { if (ldap_errno($ds) !== self::LDAP_NO_SUCH_OBJECT) { @@ -701,12 +694,10 @@ class LdapConnection implements Selectable, Inspectable } } - $results = @ldap_search( - $ds, - $query->getBase() ?: $this->rootDn, - (string) $query, + $results = $this->ldapSearch( + $query, array_values($fields), - 0, // Attributes and values + 0, $serverSorting && $limit ? $offset + $limit : 0 ); if ($results === false) { @@ -799,8 +790,6 @@ class LdapConnection implements Selectable, Inspectable $limit = $query->getLimit(); $offset = $query->hasOffset() ? $query->getOffset() : 0; - $queryString = (string) $query; - $base = $query->getBase() ?: $this->rootDn; if ($fields === null) { $fields = $query->getColumns(); @@ -835,12 +824,10 @@ class LdapConnection implements Selectable, Inspectable )); } - $results = @ldap_search( - $ds, - $base, - $queryString, + $results = $this->ldapSearch( + $query, array_values($fields), - 0, // Attributes and values + 0, $serverSorting && $limit ? $offset + $limit : 0 ); if ($results === false) { @@ -850,8 +837,8 @@ class LdapConnection implements Selectable, Inspectable throw new LdapException( 'LDAP query "%s" (base %s) failed. Error: %s', - $queryString, - $base, + (string) $query, + $query->getBase() ?: $this->getDn(), ldap_error($ds) ); } elseif (ldap_count_entries($ds, $results) === 0) { @@ -932,7 +919,8 @@ class LdapConnection implements Selectable, Inspectable // pagedResultsControl with the size set to zero (0) and the cookie set to the last cookie returned by // the server: https://www.ietf.org/rfc/rfc2696.txt ldap_control_paged_result($ds, 0, false, $cookie); - ldap_search($ds, $base, $queryString); // Returns no entries, due to the page size + // Returns no entries, due to the page size + ldap_search($ds, $query->getBase() ?: $this->getDn(), (string) $query); } if (! $serverSorting && $query->hasOrder()) { @@ -1119,6 +1107,77 @@ class LdapConnection implements Selectable, Inspectable return $ds; } + /** + * Perform a LDAP search and return the result + * + * @param LdapQuery $query + * @param array $attributes An array of the required attributes + * @param int $attrsonly Should be set to 1 if only attribute types are wanted + * @param int $sizelimit Enables you to limit the count of entries fetched + * @param int $timelimit Sets the number of seconds how long is spend on the search + * @param int $deref + * + * @return resource|bool A search result identifier or false on error + */ + public function ldapSearch( + LdapQuery $query, + array $attributes = null, + $attrsonly = 0, + $sizelimit = 0, + $timelimit = 0, + $deref = LDAP_DEREF_NEVER + ) { + $queryString = (string) $query; + $baseDn = $query->getBase() ?: $this->getDn(); + + if (Logger::getInstance()->getLevel() === Logger::DEBUG) { + // We're checking the level by ourself to avoid rendering the ldapsearch commandline for nothing + $starttlsParam = $this->encryption === static::STARTTLS ? ' -ZZ' : ''; + $ldapUrl = ($this->encryption === static::LDAPS ? 'ldaps://' : 'ldap://') + . $this->hostname + . ($this->port ? ':' . $this->port : ''); + + if ($this->bound) { + $bindParams = ' -D "' . $this->bindDn . '"' . ($this->bindPw ? ' -w "' . $this->bindPw . '"' : ''); + } + + if ($deref === LDAP_DEREF_NEVER) { + $derefName = 'never'; + } elseif ($deref === LDAP_DEREF_ALWAYS) { + $derefName = 'always'; + } elseif ($deref === LDAP_DEREF_SEARCHING) { + $derefName = 'search'; + } else { // $deref === LDAP_DEREF_FINDING + $derefName = 'find'; + } + + Logger::debug("Issueing LDAP search. Use '%s' to reproduce.", sprintf( + 'ldapsearch -P 3%s -H "%s"%s -b "%s" -s "sub" -z %u -l %u -a "%s"%s%s%s', + $starttlsParam, + $ldapUrl, + $bindParams, + $baseDn, + $sizelimit, + $timelimit, + $derefName, + $attrsonly ? ' -A' : '', + $queryString ? ' "' . $queryString . '"' : '', + $attributes ? ' "' . join('" "', $attributes) . '"' : '' + )); + } + + return @ldap_search( + $this->getConnection(), + $baseDn, + $queryString, + $attributes, + $attrsonly, + $sizelimit, + $timelimit, + $deref + ); + } + /** * Create an LDAP entry * From 453aa864cc236b43bbacf10c050aa4caae844cbf Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 11:38:32 +0100 Subject: [PATCH 06/12] LdapUserGroupBackend: Set the appropriate base dn when resolving dns refs #10567 --- .../Icinga/Authentication/UserGroup/LdapUserGroupBackend.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 8c3fc3c3d..bac2f4525 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -507,6 +507,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt ->select() ->from($this->userClass, array()) ->where($this->userNameAttribute, $name) + ->setBase($this->userBaseDn) ->setUsePagedResults(false) ->fetchDn(); if ($userDn) { @@ -517,6 +518,7 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt ->select() ->from($this->groupClass, array()) ->where($this->groupNameAttribute, $name) + ->setBase($this->groupBaseDn) ->setUsePagedResults(false) ->fetchDn(); if ($groupDn) { From 31b584b3388a596fed007dbeda9849fa550d472e Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 12:44:08 +0100 Subject: [PATCH 07/12] LdapConnection: Fix method fetchOne() The method suffered from multiple issues: * Actual NULL values were interpreted as if the row does not have any cols * Which attribute's value got returned was dependent on the result set instead of the desired columns refs #10567 --- .../Icinga/Protocol/Ldap/LdapConnection.php | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/library/Icinga/Protocol/Ldap/LdapConnection.php b/library/Icinga/Protocol/Ldap/LdapConnection.php index 77872781c..1cad06680 100644 --- a/library/Icinga/Protocol/Ldap/LdapConnection.php +++ b/library/Icinga/Protocol/Ldap/LdapConnection.php @@ -476,8 +476,28 @@ class LdapConnection implements Selectable, Inspectable */ public function fetchOne(LdapQuery $query, array $fields = null) { - $row = (array) $this->fetchRow($query, $fields); - return array_shift($row) ?: false; + $row = $this->fetchRow($query, $fields); + if ($row === false) { + return false; + } + + $values = get_object_vars($row); + if (empty($values)) { + return false; + } + + if ($fields === null) { + // Fetch the desired columns from the query if not explicitly overriden in the method's parameter + $fields = $query->getColumns(); + } + + if (empty($fields)) { + // The desired columns may be empty independently whether provided by the query or the method's parameter + return array_shift($values); + } + + $alias = key($fields); + return $values[is_string($alias) ? $alias : $fields[$alias]]; } /** From cf193f2c1b06c21c56b52e9573847c33b68779e2 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 12:48:01 +0100 Subject: [PATCH 08/12] RepositoryQuery: Initialize property $query before requiring a new table Since $this gets passed to Repository::requireTable() it may be possible that some repository tries to access the underlying native query so we need to ensure that we're able to actually provide it. refs #10567 --- library/Icinga/Repository/RepositoryQuery.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/library/Icinga/Repository/RepositoryQuery.php b/library/Icinga/Repository/RepositoryQuery.php index 532cae7c6..d227d9fde 100644 --- a/library/Icinga/Repository/RepositoryQuery.php +++ b/library/Icinga/Repository/RepositoryQuery.php @@ -86,9 +86,8 @@ class RepositoryQuery implements QueryInterface, SortRules, FilterColumns, Itera */ public function from($target, array $columns = null) { - $this->query = $this->repository->getDataSource()->select()->from( - $this->repository->requireTable($target, $this) - ); + $this->query = $this->repository->getDataSource()->select(); + $this->query->from($this->repository->requireTable($target, $this)); $this->query->columns($this->prepareQueryColumns($target, $columns)); $this->target = $target; return $this; From 8bf4e8d2171a1bae07a902847cd8dbff0d8ba75a Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 12:54:49 +0100 Subject: [PATCH 09/12] LdapUserGroupBackend: Set a query's base DN when a table gets required This ensures that the query receives the correct base DN even if the table gets adjusted by calling from() subsequently. refs #10567 --- .../UserGroup/LdapUserGroupBackend.php | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php index 2c366338a..122453990 100644 --- a/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php +++ b/library/Icinga/Authentication/UserGroup/LdapUserGroupBackend.php @@ -397,20 +397,6 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt return $this->ambiguousMemberAttribute; } - /** - * Return a new query for the given columns - * - * @param array $columns The desired columns, if null all columns will be queried - * - * @return RepositoryQuery - */ - public function select(array $columns = null) - { - $query = parent::select($columns); - $query->getQuery()->setBase($this->groupBaseDn); - return $query; - } - /** * Initialize this repository's virtual tables * @@ -575,8 +561,11 @@ class LdapUserGroupBackend extends LdapRepository implements UserGroupBackendInt */ public function requireTable($table, RepositoryQuery $query = null) { - if ($query !== null && $table === 'group' && $this->groupFilter) { - $query->getQuery()->setNativeFilter($this->groupFilter); + if ($query !== null) { + $query->getQuery()->setBase($this->groupBaseDn); + if ($table === 'group' && $this->groupFilter) { + $query->getQuery()->setNativeFilter($this->groupFilter); + } } return parent::requireTable($table, $query); From d2cc854a617a97be7b6168510092badd6febd8fe Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 12:55:17 +0100 Subject: [PATCH 10/12] LdapUserBackend: Set a query's base DN when a table gets required This ensures that the query receives the correct base DN even if the table gets adjusted by calling from() subsequently. refs #10567 --- .../Authentication/User/LdapUserBackend.php | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/library/Icinga/Authentication/User/LdapUserBackend.php b/library/Icinga/Authentication/User/LdapUserBackend.php index 930ac4c99..11fc0093e 100644 --- a/library/Icinga/Authentication/User/LdapUserBackend.php +++ b/library/Icinga/Authentication/User/LdapUserBackend.php @@ -190,24 +190,6 @@ class LdapUserBackend extends LdapRepository implements UserBackendInterface, In ->setFilter($config->filter); } - /** - * Return a new query for the given columns - * - * @param array $columns The desired columns, if null all columns will be queried - * - * @return RepositoryQuery - */ - public function select(array $columns = null) - { - $query = parent::select($columns); - $query->getQuery()->setBase($this->baseDn); - if ($this->filter) { - $query->getQuery()->setNativeFilter($this->filter); - } - - return $query; - } - /** * Initialize this repository's virtual tables * @@ -335,6 +317,28 @@ class LdapUserBackend extends LdapRepository implements UserBackendInterface, In return ((int) $value) >= $bigBang->diff($now)->days; } + /** + * Validate that the requested table exists + * + * @param string $table The table to validate + * @param RepositoryQuery $query An optional query to pass as context + * + * @return string + * + * @throws ProgrammingError In case the given table does not exist + */ + public function requireTable($table, RepositoryQuery $query = null) + { + if ($query !== null) { + $query->getQuery()->setBase($this->baseDn); + if ($this->filter) { + $query->getQuery()->setNativeFilter($this->filter); + } + } + + return parent::requireTable($table, $query); + } + /** * Validate that the given column is a valid query target and return it or the actual name if it's an alias * From cf7c99ff02f5f9a55c82fdd5fb6fe9c004d94874 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 13:03:55 +0100 Subject: [PATCH 11/12] puppet: Correct DN used to add jdoe as member to the group Users --- .puppet/profiles/icingaweb2_dev/files/openldap/users.ldif | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif b/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif index f5ad802a3..1526344c9 100644 --- a/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif +++ b/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif @@ -37,7 +37,7 @@ userpassword: password dn: cn=Users,ou=groups,dc=icinga,dc=org objectClass: groupOfUniqueNames cn: Users -uniqueMember: cn=Jon Doe,ou=people,dc=icinga,dc=org +uniqueMember: cn=John Doe,ou=people,dc=icinga,dc=org uniqueMember: cn=Jane Smith,ou=people,dc=icinga,dc=org uniqueMember: cn=John Q. Public,ou=people,dc=icinga,dc=org uniqueMember: cn=Richard Roe,ou=people,dc=icinga,dc=org From d896972f5fcb66c7d0042f75fced4dbd452d3aff Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 11 Nov 2015 13:04:26 +0100 Subject: [PATCH 12/12] puppet: Add new openldap group based on the nis.schema --- .../profiles/icingaweb2_dev/files/openldap/users.ldif | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif b/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif index 1526344c9..d0760dafc 100644 --- a/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif +++ b/.puppet/profiles/icingaweb2_dev/files/openldap/users.ldif @@ -41,3 +41,12 @@ uniqueMember: cn=John Doe,ou=people,dc=icinga,dc=org uniqueMember: cn=Jane Smith,ou=people,dc=icinga,dc=org uniqueMember: cn=John Q. Public,ou=people,dc=icinga,dc=org uniqueMember: cn=Richard Roe,ou=people,dc=icinga,dc=org + +dn: cn=PosixUsers,ou=groups,dc=icinga,dc=org +objectClass: posixGroup +cn: PosixUsers +gidNumber: 2001 +memberUid: jdoe +memberUid: jsmith +memberUid: jqpublic +memberUid: rroe