Skip to content

Commit 0cbffc7

Browse files
come-ncbackportbot[bot]
authored andcommitted
fix(user_ldap): Harmonize parameter obfuscation and serialization accross logging methods
Debug log, profiler and ldap debug log had a different logic for sanitizing of parameters, aligning them. Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com> [skip ci]
1 parent 66bc0f6 commit 0cbffc7

File tree

1 file changed

+27
-18
lines changed

1 file changed

+27
-18
lines changed

apps/user_ldap/lib/LDAP.php

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@
1111
use OCA\User_LDAP\DataCollector\LdapDataCollector;
1212
use OCA\User_LDAP\Exceptions\ConstraintViolationException;
1313
use OCP\IConfig;
14+
use OCP\ILogger;
1415
use OCP\Profiler\IProfiler;
1516
use Psr\Log\LoggerInterface;
1617

1718
class LDAP implements ILDAPWrapper {
1819
protected string $logFile = '';
1920
protected array $curArgs = [];
2021
protected LoggerInterface $logger;
22+
protected IConfig $config;
2123

2224
private ?LdapDataCollector $dataCollector = null;
2325

@@ -291,6 +293,21 @@ protected function invokeLDAPMethod(string $func, ...$arguments) {
291293
return null;
292294
}
293295

296+
/**
297+
* Turn resources into string, and removes potentially problematic cookie string to avoid breaking logfiles
298+
*/
299+
private function sanitizeFunctionParameters(array $args): array {
300+
return array_map(function ($item) {
301+
if ($this->isResource($item)) {
302+
return '(resource)';
303+
}
304+
if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') {
305+
$item[0]['value']['cookie'] = '*opaque cookie*';
306+
}
307+
return $item;
308+
}, $args);
309+
}
310+
294311
private function preFunctionCall(string $functionName, array $args): void {
295312
$this->curArgs = $args;
296313
if (strcasecmp($functionName, 'ldap_bind') === 0 || strcasecmp($functionName, 'ldap_exop_passwd') === 0) {
@@ -301,32 +318,24 @@ private function preFunctionCall(string $functionName, array $args): void {
301318
$args[2] = IConfig::SENSITIVE_VALUE;
302319
}
303320

304-
$this->logger->debug('Calling LDAP function {func} with parameters {args}', [
305-
'app' => 'user_ldap',
306-
'func' => $functionName,
307-
'args' => json_encode($args),
308-
]);
321+
if ($this->config->getSystemValue('loglevel') === ILogger::DEBUG) {
322+
/* Only running this if debug loglevel is on, to avoid processing parameters on production */
323+
$this->logger->debug('Calling LDAP function {func} with parameters {args}', [
324+
'app' => 'user_ldap',
325+
'func' => $functionName,
326+
'args' => $this->sanitizeFunctionParameters($args),
327+
]);
328+
}
309329

310330
if ($this->dataCollector !== null) {
311-
$args = array_map(function ($item) {
312-
if ($this->isResource($item)) {
313-
return '(resource)';
314-
}
315-
if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') {
316-
$item[0]['value']['cookie'] = '*opaque cookie*';
317-
}
318-
return $item;
319-
}, $this->curArgs);
320-
321331
$backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS);
322-
$this->dataCollector->startLdapRequest($functionName, $args, $backtrace);
332+
$this->dataCollector->startLdapRequest($functionName, $this->sanitizeFunctionParameters($args), $backtrace);
323333
}
324334

325335
if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) {
326-
$args = array_map(fn ($item) => (!$this->isResource($item) ? $item : '(resource)'), $this->curArgs);
327336
file_put_contents(
328337
$this->logFile,
329-
$functionName . '::' . json_encode($args) . "\n",
338+
$functionName . '::' . json_encode($this->sanitizeFunctionParameters($args)) . "\n",
330339
FILE_APPEND
331340
);
332341
}

0 commit comments

Comments
 (0)