Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable23] user_ldap fix ldap connection resets #31421 #31514

Merged
merged 4 commits into from
Mar 31, 2022
Merged

[stable23] user_ldap fix ldap connection resets #31421 #31514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
414d4bf
Fix executeRead when connection is reset because of a timeout
come-nc Mar 3, 2022
92415ea
Fix unit tests
come-nc Mar 3, 2022
0b88d72
Inherit doc from interface in LDAP class
come-nc Mar 29, 2022
ab6439a
Fix $baseDN parameter type in ILDAPWrapper::read
come-nc Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 27 additions & 41 deletions apps/user_ldap/lib/Access.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
$values = [];
$isRangeRequest = false;
do {
$result = $this->executeRead($cr, $dn, $attrToRead, $filter, $maxResults);
$result = $this->executeRead($dn, $attrToRead, $filter, $maxResults);
if (is_bool($result)) {
// when an exists request was run and it was successful, an empty
// array must be returned
Expand Down Expand Up @@ -260,17 +260,12 @@ public function readAttribute($dn, $attr, $filter = 'objectClass=*') {
/**
* Runs an read operation against LDAP
*
* @param resource $cr the LDAP connection
* @param string $dn
* @param string $attribute
* @param string $filter
* @param int $maxResults
* @return array|bool false if there was any error, true if an exists check
* was performed and the requested DN found, array with the
* returned data on a successful usual operation
* @throws ServerNotAvailableException
*/
public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
public function executeRead(string $dn, string $attribute, string $filter, int $maxResults) {
try {
$this->initPagedSearch($filter, $dn, [$attribute], $maxResults, 0);
} catch (NoMoreResults $e) {
Expand All @@ -280,7 +275,7 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
return false;
}
$dn = $this->helper->DNasBaseParameter($dn);
$rr = @$this->invokeLDAPMethod('read', $cr, $dn, $filter, [$attribute]);
$rr = @$this->invokeLDAPMethod('read', $dn, $filter, [$attribute]);
if (!$this->ldap->isResource($rr)) {
if ($attribute !== '') {
//do not throw this message on userExists check, irritates
Expand All @@ -289,18 +284,18 @@ public function executeRead($cr, $dn, $attribute, $filter, $maxResults) {
//in case an error occurs , e.g. object does not exist
return false;
}
if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $cr, $rr) === 1)) {
if ($attribute === '' && ($filter === 'objectclass=*' || $this->invokeLDAPMethod('countEntries', $rr) === 1)) {
$this->logger->debug('readAttribute: ' . $dn . ' found', ['app' => 'user_ldap']);
return true;
}
$er = $this->invokeLDAPMethod('firstEntry', $cr, $rr);
$er = $this->invokeLDAPMethod('firstEntry', $rr);
if (!$this->ldap->isResource($er)) {
//did not match the filter, return false
return false;
}
//LDAP attributes are not case sensitive
$result = \OCP\Util::mb_array_change_key_case(
$this->invokeLDAPMethod('getAttributes', $cr, $er), MB_CASE_LOWER, 'UTF-8');
$this->invokeLDAPMethod('getAttributes', $er), MB_CASE_LOWER, 'UTF-8');

return $result;
}
Expand Down Expand Up @@ -381,10 +376,10 @@ public function setPassword($userDN, $password) {
}
try {
// try PASSWD extended operation first
return @$this->invokeLDAPMethod('exopPasswd', $cr, $userDN, '', $password) ||
@$this->invokeLDAPMethod('modReplace', $cr, $userDN, $password);
return @$this->invokeLDAPMethod('exopPasswd', $userDN, '', $password) ||
@$this->invokeLDAPMethod('modReplace', $userDN, $password);
} catch (ConstraintViolationException $e) {
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), $e->getCode());
throw new HintException('Password change rejected.', \OC::$server->getL10N('user_ldap')->t('Password change rejected. Hint: ') . $e->getMessage(), (int)$e->getCode());
}
}

Expand Down Expand Up @@ -1092,26 +1087,23 @@ public function countObjects($limit = null, $offset = null) {
*/

/**
* @param mixed[] $arguments
* @return mixed
* @throws \OC\ServerNotAvailableException
*/
private function invokeLDAPMethod() {
$arguments = func_get_args();
$command = array_shift($arguments);
$cr = array_shift($arguments);
private function invokeLDAPMethod(string $command, ...$arguments) {
if ($command == 'controlPagedResultResponse') {
// php no longer supports call-time pass-by-reference
// thus cannot support controlPagedResultResponse as the third argument
// is a reference
throw new \InvalidArgumentException('Invoker does not support controlPagedResultResponse, call LDAP Wrapper directly instead.');
}
if (!method_exists($this->ldap, $command)) {
return null;
}
array_unshift($arguments, $cr);
// php no longer supports call-time pass-by-reference
// thus cannot support controlPagedResultResponse as the third argument
// is a reference
array_unshift($arguments, $this->connection->getConnectionResource());
$doMethod = function () use ($command, &$arguments) {
if ($command == 'controlPagedResultResponse') {
throw new \InvalidArgumentException('Invoker does not support controlPagedResultResponse, call LDAP Wrapper directly instead.');
} else {
return call_user_func_array([$this->ldap, $command], $arguments);
}
return call_user_func_array([$this->ldap, $command], $arguments);
};
try {
$ret = $doMethod();
Expand Down Expand Up @@ -1172,8 +1164,7 @@ private function executeSearch(
return false;
}

$sr = $this->invokeLDAPMethod('search', $cr, $base, $filter, $attr);
// cannot use $cr anymore, might have changed in the previous call!
$sr = $this->invokeLDAPMethod('search', $base, $filter, $attr);
$error = $this->ldap->errno($this->connection->getConnectionResource());
if (!$this->ldap->isResource($sr) || $error !== 0) {
$this->logger->error('Attempt for Paging? ' . print_r($pagedSearchOK, true), ['app' => 'user_ldap']);
Expand Down Expand Up @@ -1308,7 +1299,7 @@ private function count(
* @throws ServerNotAvailableException
*/
private function countEntriesInSearchResults($sr): int {
return (int)$this->invokeLDAPMethod('countEntries', $this->connection->getConnectionResource(), $sr);
return (int)$this->invokeLDAPMethod('countEntries', $sr);
}

/**
Expand Down Expand Up @@ -1349,7 +1340,6 @@ public function search(
return [];
}
[$sr, $pagedSearchOK] = $search;
$cr = $this->connection->getConnectionResource();

if ($skipHandling) {
//i.e. result do not need to be fetched, we just need the cookie
Expand All @@ -1359,7 +1349,7 @@ public function search(
return [];
}

$findings = array_merge($findings, $this->invokeLDAPMethod('getEntries', $cr, $sr));
$findings = array_merge($findings, $this->invokeLDAPMethod('getEntries', $sr));
$iFoundItems = max($iFoundItems, $findings['count']);
unset($findings['count']);

Expand Down Expand Up @@ -1536,7 +1526,7 @@ private function combineFilter($filters, $operator) {
* @param string $search the search term
* @return string the final filter part to use in LDAP searches
*/
public function getFilterPartForUserSearch($search) {
public function getFilterPartForUserSearch($search): string {
return $this->getFilterPartForSearch($search,
$this->connection->ldapAttributesForUserSearch,
$this->connection->ldapUserDisplayName);
Expand All @@ -1548,7 +1538,7 @@ public function getFilterPartForUserSearch($search) {
* @param string $search the search term
* @return string the final filter part to use in LDAP searches
*/
public function getFilterPartForGroupSearch($search) {
public function getFilterPartForGroupSearch($search): string {
return $this->getFilterPartForSearch($search,
$this->connection->ldapAttributesForGroupSearch,
$this->connection->ldapGroupDisplayName);
Expand Down Expand Up @@ -1642,10 +1632,8 @@ private function prepareSearchTerm($term) {

/**
* returns the filter used for counting users
*
* @return string
*/
public function getFilterForUserCount() {
public function getFilterForUserCount(): string {
$filter = $this->combineFilterWithAnd([
$this->connection->ldapUserFilter,
$this->connection->ldapUserDisplayName . '=*'
Expand Down Expand Up @@ -1994,8 +1982,7 @@ private function abandonPagedSearch() {
if ($this->lastCookie === '') {
return;
}
$cr = $this->connection->getConnectionResource();
$this->invokeLDAPMethod('controlPagedResult', $cr, 0, false);
$this->invokeLDAPMethod('controlPagedResult', 0, false);
$this->getPagedSearchResultState();
$this->lastCookie = '';
}
Expand Down Expand Up @@ -2082,7 +2069,7 @@ private function initPagedSearch(
$this->abandonPagedSearch();
}
$pagedSearchOK = true === $this->invokeLDAPMethod(
'controlPagedResult', $this->connection->getConnectionResource(), $limit, false
'controlPagedResult', $limit, false
);
if ($pagedSearchOK) {
$this->logger->debug('Ready for a paged search', ['app' => 'user_ldap']);
Expand All @@ -2102,7 +2089,6 @@ private function initPagedSearch(
// be returned.
$pageSize = (int)$this->connection->ldapPagingSize > 0 ? (int)$this->connection->ldapPagingSize : 500;
$pagedSearchOK = $this->invokeLDAPMethod('controlPagedResult',
$this->connection->getConnectionResource(),
$pageSize, false);
}

Expand Down
38 changes: 31 additions & 7 deletions apps/user_ldap/lib/Connection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,25 @@
*/
class Connection extends LDAPUtility {
private $ldapConnectionRes = null;

/**
* @var string
*/
private $configPrefix;

/**
* @var ?string
*/
private $configID;

/**
* @var bool
*/
private $configured = false;
//whether connection should be kept on __destruct

/**
* @var bool whether connection should be kept on __destruct
*/
private $dontDestruct = false;

/**
Expand All @@ -91,33 +106,42 @@ class Connection extends LDAPUtility {
*/
public $hasGidNumber = true;

//cache handler
protected $cache;
/**
* @var \OCP\ICache|null
*/
protected $cache = null;

/** @var Configuration settings handler **/
protected $configuration;

/**
* @var bool
*/
protected $doNotValidate = false;

/**
* @var bool
*/
protected $ignoreValidation = false;

/**
* @var array{dn?: mixed, hash?: string, result?: bool}
*/
protected $bindResult = [];

/** @var LoggerInterface */
protected $logger;

/**
* Constructor
* @param ILDAPWrapper $ldap
* @param string $configPrefix a string with the prefix for the configkey column (appconfig table)
* @param string|null $configID a string with the value for the appid column (appconfig table) or null for on-the-fly connections
*/
public function __construct(ILDAPWrapper $ldap, $configPrefix = '', $configID = 'user_ldap') {
public function __construct(ILDAPWrapper $ldap, string $configPrefix = '', ?string $configID = 'user_ldap') {
parent::__construct($ldap);
$this->configPrefix = $configPrefix;
$this->configID = $configID;
$this->configuration = new Configuration($configPrefix,
!is_null($configID));
$this->configuration = new Configuration($configPrefix, !is_null($configID));
$memcache = \OC::$server->getMemCacheFactory();
if ($memcache->isAvailable()) {
$this->cache = $memcache->createDistributed();
Expand Down
2 changes: 1 addition & 1 deletion apps/user_ldap/lib/Group_LDAP.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,7 @@ public function getDisplayName(string $gid): string {
$this->access->groupname2dn($gid),
$this->access->connection->ldapGroupDisplayName);

if ($displayName && (count($displayName) > 0)) {
if (($displayName !== false) && (count($displayName) > 0)) {
$displayName = $displayName[0];
$this->access->connection->writeToCache($cacheKey, $displayName);
return $displayName;
Expand Down
6 changes: 3 additions & 3 deletions apps/user_ldap/lib/ILDAPWrapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public function nextEntry($link, $result);
/**
* Read an entry
* @param resource $link LDAP link resource
* @param array $baseDN The DN of the entry to read from
* @param string $baseDN The DN of the entry to read from
* @param string $filter An LDAP filter
* @param array $attr array of the attributes to read
* @return resource an LDAP search result resource
Expand Down Expand Up @@ -178,8 +178,8 @@ public function modReplace($link, $userDN, $password);
/**
* Sets the value of the specified option to be $value
* @param resource $link LDAP link resource
* @param string $option a defined LDAP Server option
* @param int $value the new value for the option
* @param int $option a defined LDAP Server option
* @param mixed $value the new value for the option
* @return bool true on success, false otherwise
*/
public function setOption($link, $option, $value);
Expand Down
Loading