diff options
author | Arthur Schiwon <blizzz@arthur-schiwon.de> | 2022-07-29 00:30:17 +0300 |
---|---|---|
committer | Arthur Schiwon <blizzz@arthur-schiwon.de> | 2022-07-29 00:30:17 +0300 |
commit | 2a6f46e6891ee82b613be6151d2f51583c45c2bf (patch) | |
tree | 9c2419d41c3fac9674b5b6323e79d4e66b018c64 /lib/private | |
parent | bbe15b4b43f95e1600f5122a7bf72a64ee404b36 (diff) |
allow apps to specify methods carrying sensitive parameters
… in order to remove them from logging.
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Diffstat (limited to 'lib/private')
-rw-r--r-- | lib/private/AppFramework/Bootstrap/RegistrationContext.php | 23 | ||||
-rw-r--r-- | lib/private/Log.php | 39 | ||||
-rw-r--r-- | lib/private/Log/ExceptionSerializer.php | 13 |
3 files changed, 65 insertions, 10 deletions
diff --git a/lib/private/AppFramework/Bootstrap/RegistrationContext.php b/lib/private/AppFramework/Bootstrap/RegistrationContext.php index 8f6aff228e1..c98f968c999 100644 --- a/lib/private/AppFramework/Bootstrap/RegistrationContext.php +++ b/lib/private/AppFramework/Bootstrap/RegistrationContext.php @@ -121,6 +121,9 @@ class RegistrationContext { /** @var ServiceRegistration<ICalendarProvider>[] */ private $calendarProviders = []; + /** @var ParameterRegistration[] */ + private $sensitiveMethods = []; + /** @var LoggerInterface */ private $logger; @@ -304,6 +307,14 @@ class RegistrationContext { $migratorClass ); } + + public function registerSensitiveMethods(string $class, array $methods): void { + $this->context->registerSensitiveMethods( + $this->appId, + $class, + $methods + ); + } }; } @@ -430,6 +441,11 @@ class RegistrationContext { $this->userMigrators[] = new ServiceRegistration($appId, $migratorClass); } + public function registerSensitiveMethods(string $appId, string $class, array $methods): void { + $methods = array_filter($methods, 'is_string'); + $this->sensitiveMethods[] = new ParameterRegistration($appId, $class, $methods); + } + /** * @param App[] $apps */ @@ -712,4 +728,11 @@ class RegistrationContext { public function getUserMigrators(): array { return $this->userMigrators; } + + /** + * @return ParameterRegistration[] + */ + public function getSensitiveMethods(): array { + return $this->sensitiveMethods; + } } diff --git a/lib/private/Log.php b/lib/private/Log.php index 95e0a833b66..4ab647bc6c1 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -36,8 +36,11 @@ declare(strict_types=1); */ namespace OC; +use Exception; use Nextcloud\LogNormalizer\Normalizer; +use OC\AppFramework\Bootstrap\Coordinator; use OCP\Log\IDataLogger; +use Throwable; use function array_merge; use OC\Log\ExceptionSerializer; use OCP\ILogger; @@ -228,7 +231,7 @@ class Log implements ILogger, IDataLogger { $this->crashReporters->delegateBreadcrumb($entry['message'], 'log', $context); } } - } catch (\Throwable $e) { + } catch (Throwable $e) { // make sure we dont hard crash if logging fails } } @@ -300,19 +303,19 @@ class Log implements ILogger, IDataLogger { /** * Logs an exception very detailed * - * @param \Exception|\Throwable $exception + * @param Exception|Throwable $exception * @param array $context * @return void * @since 8.2.0 */ - public function logException(\Throwable $exception, array $context = []) { + public function logException(Throwable $exception, array $context = []) { $app = $context['app'] ?? 'no app in context'; $level = $context['level'] ?? ILogger::ERROR; // if an error is raised before the autoloader is properly setup, we can't serialize exceptions try { - $serializer = new ExceptionSerializer($this->config); - } catch (\Throwable $e) { + $serializer = $this->getSerializer(); + } catch (Throwable $e) { $this->error("Failed to load ExceptionSerializer serializer while trying to log " . $exception->getMessage()); return; } @@ -338,7 +341,7 @@ class Log implements ILogger, IDataLogger { if (!is_null($this->crashReporters)) { $this->crashReporters->delegateReport($exception, $context); } - } catch (\Throwable $e) { + } catch (Throwable $e) { // make sure we dont hard crash if logging fails } } @@ -361,7 +364,7 @@ class Log implements ILogger, IDataLogger { } $context['level'] = $level; - } catch (\Throwable $e) { + } catch (Throwable $e) { // make sure we dont hard crash if logging fails } } @@ -401,4 +404,26 @@ class Log implements ILogger, IDataLogger { } return array_merge(array_diff_key($context, $usedContextKeys), [$messageKey => strtr($message, $replace)]); } + + /** + * @throws Throwable + */ + protected function getSerializer(): ExceptionSerializer { + $serializer = new ExceptionSerializer($this->config); + try { + /** @var Coordinator $coordinator */ + $coordinator = \OCP\Server::get(Coordinator::class); + foreach ($coordinator->getRegistrationContext()->getSensitiveMethods() as $registration) { + $serializer->enlistSensitiveMethods($registration->getName(), $registration->getValue()); + } + // For not every app might be initialized at this time, we cannot assume that the return value + // of getSensitiveMethods() is complete. Running delegates in Coordinator::registerApps() is + // not possible due to dependencies on the one hand. On the other it would work only with + // adding public methods to the PsrLoggerAdapter and this class. + // Thus, serializer cannot be a property. + } catch (Throwable $t) { + // ignore app-defined sensitive methods in this case - they weren't loaded anyway + } + return $serializer; + } } diff --git a/lib/private/Log/ExceptionSerializer.php b/lib/private/Log/ExceptionSerializer.php index 3c3ff95f8e1..aaf6a39235e 100644 --- a/lib/private/Log/ExceptionSerializer.php +++ b/lib/private/Log/ExceptionSerializer.php @@ -109,7 +109,7 @@ class ExceptionSerializer { $this->systemConfig = $systemConfig; } - public const methodsWithSensitiveParametersByClass = [ + protected array $methodsWithSensitiveParametersByClass = [ SetupController::class => [ 'run', 'display', @@ -190,8 +190,8 @@ class ExceptionSerializer { $sensitiveValues = []; $trace = array_map(function (array $traceLine) use (&$sensitiveValues) { $className = $traceLine['class'] ?? ''; - if ($className && isset(self::methodsWithSensitiveParametersByClass[$className]) - && in_array($traceLine['function'], self::methodsWithSensitiveParametersByClass[$className], true)) { + if ($className && isset($this->methodsWithSensitiveParametersByClass[$className]) + && in_array($traceLine['function'], $this->methodsWithSensitiveParametersByClass[$className], true)) { return $this->editTrace($sensitiveValues, $traceLine); } foreach (self::methodsWithSensitiveParameters as $sensitiveMethod) { @@ -289,4 +289,11 @@ class ExceptionSerializer { return $data; } + + public function enlistSensitiveMethods(string $class, array $methods): void { + if (!isset($this->methodsWithSensitiveParametersByClass[$class])) { + $this->methodsWithSensitiveParametersByClass[$class] = []; + } + $this->methodsWithSensitiveParametersByClass[$class] = array_merge($this->methodsWithSensitiveParametersByClass[$class], $methods); + } } |