nextcloud / nextcloud/server

Allow custom sensitive values to be filtered out by `OC\Log\ExceptionSerializer`

Open
#36,611 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage enhancement feature: logging security
Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

How to use GitHub
  • Please use the 👍 reaction to show that you are interested into the same feature.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Is your feature request related to a problem? Please describe.
The OC\Log\ExceptionSerializer class provides a mechanism to redact sensitive values from the log with the SENSITIVE_VALUE_PLACEHOLDER constant. However, some apps and integrations use secret values that should also not be written to the log. For example, the JWT of the OnlyOffice app and the certificates of the SSO & SAML Authentication (user_saml) app should never be written to the log and should be redacted.

Describe the solution you'd like
There should be a way to add custom values to the ExceptionSerializer or OC\Log classes in order to have them filtered out of the stack trace and error log.

Describe alternatives you've considered
I managed to write an app that registers a custom error logger class to the server which overrides and replaces the default logger. However, if errors occur before all apps are loaded, my custom logger will not be available.

Additional context
There are a number of considerations to be made to provide this functionality:

  • It cannot be provided using an app. As previously mentioned, any errors that occur before all apps have loaded will not be filtered if the solution is app-based.
  • App developers could submit PRs to the ExceptionSerializer class to support any sensitive values used by their apps. However, hard-coding these values isn't a great solution. It would be better to somehow allow adding custom values.
  • Many sensitive values are dynamic and only accessible via a function. For example, if an authentication app is using OCP\Authentication\IProvideUserSecretBackend, the value is different for every user, and getCurrentUserSecret() must be used to get the value. Registering these types of values with the ExceptionSerializer would be difficult.

I wouldn't mind tackling this project, but given these sorts of challenges, I wanted to have a discussion to see if anyone had suggestions on how best to proceed.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading OC\Log\ExceptionSerializer and the OC\Log classes to understand how SENSITIVE_VALUE_PLACEHOLDER redacts values and when logging is initialized. Investigate the early-startup constraint and dynamic values such as IProvideUserSecretBackend; done means a documented design and implementation path for filtering app-specific secrets before all apps load.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
backend, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.