From f399e1591f27495e5faf6507a1489c2265ef35fe Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Fri, 13 Apr 2018 16:39:27 +0200 Subject: [PATCH] Log classnames of arguments in exception trace Signed-off-by: Robin Appelman --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Log.php | 98 +------------- lib/private/Log/ExceptionSerializer.php | 140 ++++++++++++++++++++ 4 files changed, 145 insertions(+), 95 deletions(-) create mode 100644 lib/private/Log/ExceptionSerializer.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 58f4a15dbd..99023096bc 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -740,6 +740,7 @@ return array( 'OC\\Log' => $baseDir . '/lib/private/Log.php', 'OC\\Log\\ErrorHandler' => $baseDir . '/lib/private/Log/ErrorHandler.php', 'OC\\Log\\Errorlog' => $baseDir . '/lib/private/Log/Errorlog.php', + 'OC\\Log\\ExceptionSerializer' => $baseDir . '/lib/private/Log/ExceptionSerializer.php', 'OC\\Log\\File' => $baseDir . '/lib/private/Log/File.php', 'OC\\Log\\Rotate' => $baseDir . '/lib/private/Log/Rotate.php', 'OC\\Log\\Syslog' => $baseDir . '/lib/private/Log/Syslog.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index b0c04eea22..33533569e9 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -770,6 +770,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Log' => __DIR__ . '/../../..' . '/lib/private/Log.php', 'OC\\Log\\ErrorHandler' => __DIR__ . '/../../..' . '/lib/private/Log/ErrorHandler.php', 'OC\\Log\\Errorlog' => __DIR__ . '/../../..' . '/lib/private/Log/Errorlog.php', + 'OC\\Log\\ExceptionSerializer' => __DIR__ . '/../../..' . '/lib/private/Log/ExceptionSerializer.php', 'OC\\Log\\File' => __DIR__ . '/../../..' . '/lib/private/Log/File.php', 'OC\\Log\\Rotate' => __DIR__ . '/../../..' . '/lib/private/Log/Rotate.php', 'OC\\Log\\Syslog' => __DIR__ . '/../../..' . '/lib/private/Log/Syslog.php', diff --git a/lib/private/Log.php b/lib/private/Log.php index 85e439359f..e1f9eb213b 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -37,6 +37,7 @@ namespace OC; use InterfaSys\LogNormalizer\Normalizer; +use OC\Log\ExceptionSerializer; use OC\Log\File; use OCP\ILogger; use OCP\Support\CrashReport\IRegistry; @@ -68,50 +69,6 @@ class Log implements ILogger { /** @var IRegistry */ private $crashReporters; - protected $methodsWithSensitiveParameters = [ - // Session/User - 'completeLogin', - 'login', - 'checkPassword', - 'checkPasswordNoLogging', - 'loginWithPassword', - 'updatePrivateKeyPassword', - 'validateUserPass', - 'loginWithToken', - '{closure}', - - // TokenProvider - 'getToken', - 'isTokenPassword', - 'getPassword', - 'decryptPassword', - 'logClientIn', - 'generateToken', - 'validateToken', - - // TwoFactorAuth - 'solveChallenge', - 'verifyChallenge', - - // ICrypto - 'calculateHMAC', - 'encrypt', - 'decrypt', - - // LoginController - 'tryLogin', - 'confirmPassword', - - // LDAP - 'bind', - 'areCredentialsValid', - 'invokeLDAPMethod', - - // Encryption - 'storeKeyPair', - 'setupUser', - ]; - /** * @param string $logger The logger that should be used * @param SystemConfig $config the system config object @@ -324,56 +281,6 @@ class Log implements ILogger { return min($this->config->getValue('loglevel', Util::WARN), Util::FATAL); } - private function filterTrace(array $trace) { - $sensitiveValues = []; - $trace = array_map(function (array $traceLine) use (&$sensitiveValues) { - foreach ($this->methodsWithSensitiveParameters as $sensitiveMethod) { - if (strpos($traceLine['function'], $sensitiveMethod) !== false) { - $sensitiveValues = array_merge($sensitiveValues, $traceLine['args']); - $traceLine['args'] = ['*** sensitive parameters replaced ***']; - return $traceLine; - } - } - return $traceLine; - }, $trace); - return array_map(function (array $traceLine) use ($sensitiveValues) { - $traceLine['args'] = $this->removeValuesFromArgs($traceLine['args'], $sensitiveValues); - return $traceLine; - }, $trace); - } - - private function removeValuesFromArgs($args, $values) { - foreach($args as &$arg) { - if (in_array($arg, $values, true)) { - $arg = '*** sensitive parameter replaced ***'; - } else if (is_array($arg)) { - $arg = $this->removeValuesFromArgs($arg, $values); - } - } - return $args; - } - - private function serializeException(\Throwable $exception) { - $data = [ - 'Exception' => get_class($exception), - 'Message' => $exception->getMessage(), - 'Code' => $exception->getCode(), - 'Trace' => $this->filterTrace($exception->getTrace()), - 'File' => $exception->getFile(), - 'Line' => $exception->getLine(), - ]; - - if ($exception instanceof HintException) { - $data['Hint'] = $exception->getHint(); - } - - if ($exception->getPrevious()) { - $data['Previous'] = $this->serializeException($exception->getPrevious()); - } - - return $data; - } - /** * Logs an exception very detailed * @@ -386,7 +293,8 @@ class Log implements ILogger { $app = $context['app'] ?? 'no app in context'; $level = $context['level'] ?? Util::ERROR; - $data = $this->serializeException($exception); + $serializer = new ExceptionSerializer(); + $data = $serializer->serializeException($exception); $data['CustomMessage'] = $context['message'] ?? '--'; $minLevel = $this->getLogLevel($context); diff --git a/lib/private/Log/ExceptionSerializer.php b/lib/private/Log/ExceptionSerializer.php new file mode 100644 index 0000000000..be025cb9e5 --- /dev/null +++ b/lib/private/Log/ExceptionSerializer.php @@ -0,0 +1,140 @@ + + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OC\Log; + +use OC\HintException; + +class ExceptionSerializer { + const methodsWithSensitiveParameters = [ + // Session/User + 'completeLogin', + 'login', + 'checkPassword', + 'checkPasswordNoLogging', + 'loginWithPassword', + 'updatePrivateKeyPassword', + 'validateUserPass', + 'loginWithToken', + '{closure}', + + // TokenProvider + 'getToken', + 'isTokenPassword', + 'getPassword', + 'decryptPassword', + 'logClientIn', + 'generateToken', + 'validateToken', + + // TwoFactorAuth + 'solveChallenge', + 'verifyChallenge', + + // ICrypto + 'calculateHMAC', + 'encrypt', + 'decrypt', + + // LoginController + 'tryLogin', + 'confirmPassword', + + // LDAP + 'bind', + 'areCredentialsValid', + 'invokeLDAPMethod', + + // Encryption + 'storeKeyPair', + 'setupUser', + ]; + + private function filterTrace(array $trace) { + $sensitiveValues = []; + $trace = array_map(function (array $traceLine) use (&$sensitiveValues) { + foreach (self::methodsWithSensitiveParameters as $sensitiveMethod) { + if (strpos($traceLine['function'], $sensitiveMethod) !== false) { + $sensitiveValues = array_merge($sensitiveValues, $traceLine['args']); + $traceLine['args'] = ['*** sensitive parameters replaced ***']; + return $traceLine; + } + } + return $traceLine; + }, $trace); + return array_map(function (array $traceLine) use ($sensitiveValues) { + $traceLine['args'] = $this->removeValuesFromArgs($traceLine['args'], $sensitiveValues); + return $traceLine; + }, $trace); + } + + private function removeValuesFromArgs($args, $values) { + foreach ($args as &$arg) { + if (in_array($arg, $values, true)) { + $arg = '*** sensitive parameter replaced ***'; + } else if (is_array($arg)) { + $arg = $this->removeValuesFromArgs($arg, $values); + } + } + return $args; + } + + private function encodeTrace($trace) { + $filteredTrace = $this->filterTrace($trace); + return array_map(function (array $line) { + $line['args'] = array_map([$this, 'encodeArg'], $line['args']); + return $line; + }, $filteredTrace); + } + + private function encodeArg($arg) { + if (is_object($arg)) { + $data = get_object_vars($arg); + $data['__class__'] = get_class($arg); + return array_map([$this, 'encodeArg'], $data); + } else if (is_array($arg)) { + return array_map([$this, 'encodeArg'], $arg); + } else { + return $arg; + } + } + + public function serializeException(\Throwable $exception) { + $data = [ + 'Exception' => get_class($exception), + 'Message' => $exception->getMessage(), + 'Code' => $exception->getCode(), + 'Trace' => $this->encodeTrace($exception->getTrace()), + 'File' => $exception->getFile(), + 'Line' => $exception->getLine(), + ]; + + if ($exception instanceof HintException) { + $data['Hint'] = $exception->getHint(); + } + + if ($exception->getPrevious()) { + $data['Previous'] = $this->serializeException($exception->getPrevious()); + } + + return $data; + } +} \ No newline at end of file