From 078b7abe78ba1b7b62127345d86714369de1d700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Mon, 2 Jun 2025 11:36:02 +0200 Subject: [PATCH] fix(user_ldap): Harmonize parameter obfuscation and serialization accross logging methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Debug log, profiler and ldap debug log had a different logic for sanitizing of parameters, aligning them. Signed-off-by: Côme Chilliet --- apps/user_ldap/lib/LDAP.php | 49 +++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/apps/user_ldap/lib/LDAP.php b/apps/user_ldap/lib/LDAP.php index dcf2e6f62e12c..1d2b12f685aad 100644 --- a/apps/user_ldap/lib/LDAP.php +++ b/apps/user_ldap/lib/LDAP.php @@ -11,13 +11,16 @@ use OCA\User_LDAP\DataCollector\LdapDataCollector; use OCA\User_LDAP\Exceptions\ConstraintViolationException; use OCP\IConfig; +use OCP\ILogger; use OCP\Profiler\IProfiler; +use OCP\Server; use Psr\Log\LoggerInterface; class LDAP implements ILDAPWrapper { protected string $logFile = ''; protected array $curArgs = []; protected LoggerInterface $logger; + protected IConfig $config; private ?LdapDataCollector $dataCollector = null; @@ -31,7 +34,8 @@ public function __construct(string $logFile = '') { $profiler->add($this->dataCollector); } - $this->logger = \OCP\Server::get(LoggerInterface::class); + $this->logger = Server::get(LoggerInterface::class); + $this->config = Server::get(IConfig::class); } /** @@ -291,6 +295,21 @@ protected function invokeLDAPMethod(string $func, ...$arguments) { return null; } + /** + * Turn resources into string, and removes potentially problematic cookie string to avoid breaking logfiles + */ + private function sanitizeFunctionParameters(array $args): array { + return array_map(function ($item) { + if ($this->isResource($item)) { + return '(resource)'; + } + if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') { + $item[0]['value']['cookie'] = '*opaque cookie*'; + } + return $item; + }, $args); + } + private function preFunctionCall(string $functionName, array $args): void { $this->curArgs = $args; if (strcasecmp($functionName, 'ldap_bind') === 0 || strcasecmp($functionName, 'ldap_exop_passwd') === 0) { @@ -301,32 +320,24 @@ private function preFunctionCall(string $functionName, array $args): void { $args[2] = IConfig::SENSITIVE_VALUE; } - $this->logger->debug('Calling LDAP function {func} with parameters {args}', [ - 'app' => 'user_ldap', - 'func' => $functionName, - 'args' => json_encode($args), - ]); + if ($this->config->getSystemValue('loglevel') === ILogger::DEBUG) { + /* Only running this if debug loglevel is on, to avoid processing parameters on production */ + $this->logger->debug('Calling LDAP function {func} with parameters {args}', [ + 'app' => 'user_ldap', + 'func' => $functionName, + 'args' => $this->sanitizeFunctionParameters($args), + ]); + } if ($this->dataCollector !== null) { - $args = array_map(function ($item) { - if ($this->isResource($item)) { - return '(resource)'; - } - if (isset($item[0]['value']['cookie']) && $item[0]['value']['cookie'] !== '') { - $item[0]['value']['cookie'] = '*opaque cookie*'; - } - return $item; - }, $this->curArgs); - $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); - $this->dataCollector->startLdapRequest($functionName, $args, $backtrace); + $this->dataCollector->startLdapRequest($functionName, $this->sanitizeFunctionParameters($args), $backtrace); } if ($this->logFile !== '' && is_writable(dirname($this->logFile)) && (!file_exists($this->logFile) || is_writable($this->logFile))) { - $args = array_map(fn ($item) => (!$this->isResource($item) ? $item : '(resource)'), $this->curArgs); file_put_contents( $this->logFile, - $functionName . '::' . json_encode($args) . "\n", + $functionName . '::' . json_encode($this->sanitizeFunctionParameters($args)) . "\n", FILE_APPEND ); }