Pre-RFC: Redacting parameters in back traces (SensitiveParameter Attribute)

Hi Tim,
Thank you for opening the discussion. I personally find this feature
useful, and glancing at the diff, it looks like a rather
straightforward and minimal change.

FWIW, it is possible to selectively expose class properties in a class
object being var_dump-ed with the __debugInfo magic method, but it
is not used for stack traces.

For the RFC, someone with the permission to grant RFC karma should
grant it for you to start the RFC and open it for voting. You emailed
the right mailing list, I think someone will see this soon.
You can open a PR and start code reviews in the meantime.

Cheers,
Ayesh.

On Fri, Jan 7, 2022 at 11:40 PM Tim Düsterhus, WoltLab GmbH
duesterhus@woltlab.com wrote:

Hi Internals!

PHP's stack traces in exceptions are very useful for debugging, because
they include the original parameters for each stack frame, allowing the
developer to see exactly what data is passed to a function call.

Unfortunately sometimes this verbosity is a drawback. Specifically when
sensitive values, such as credentials, are passed to a function and some
other function deep down the call stack throws an Exception.

One common "offender" is PDO which takes the database password as a
constructor parameter and immediately attempts to connect to the
database within the constructor, instead of having a pure constructor
and a separate ->connect() method. Thus when the database connection
fails the stack trace will include the database password:

PDOException: SQLSTATE[HY000] [2002] No such file or directory in /var/www/html/test.php:3
Stack trace:
#0 /var/www/html/test.php(3): PDO->__construct('mysql:host=loca...', 'root', 'password')
#1 {main}
We're developing an application that is commonly self-hosted by
laypersons at a shared hosting provider. This application includes an
error log, logging any uncaught exception including its stack trace.
Depending on the configuration by the customer it might also publicly
print error details, because this eases debugging by the layperson
administrator.

To keep (database) credentials out of the log files - and more
importantly hidden from any public visitors - our exception handler
specifically scans the full stack trace for the PDO constructor to
replace any parameters. More recently the exception handler was extended
to also check each parameter via Reflection and redact any values for
parameters called $password, $secret, and similar names. It also redacts
any parameter having a SensitiveArgument attribute defined in our
software.

While this works reasonably well for any code we write, it does not work
for non-stock exception handlers, e.g. loggers such as Sentry. It also
cannot redact sensitive arguments in third party libraries that do not
match one of the special-cased parameter names, because they won't have
the SensitiveArgument attribute.

For this reason we'd like to propose a standardized attribute to mark
parameters as sensitive. This attribute can then be used by libraries to
indicate their sensitive parameters.

With regards to loggers our proposal would be that PHP itself performs
the redaction when collecting the parameters for all stack frames.
Specifically we propose that PHP will place an object of the
SensitiveParameter class instead of the actual parameter value. This way
an exception handler can easily detect which parameters have been
redacted by performing an instanceof SensitiveParameter check.

The zend.exception_ignore_args option is not an alternative in our
opinion:

• On shared webhosters, the customer might not be able to configure it.
• The stack trace parameters are just too useful for debugging to
  completely strip them.

The zend.exception_string_param_max_len option is not alternative either:

• Many sensitive values might already be fully exposed before they are
  truncated. This specifically includes end-user credentials which tend to
  be low-entropy and shortish.

We have created a proof of concept implementation of our proposal at:

https://github.com/php/php-src/compare/master...WoltLab:sensitive-parameter

For the RFC and the final implementation we would need assistance by
someone who is more experienced in PHP internals and the RFC process.

What our implementation already handles:

• Redaction of ordered arguments.
• Redaction of named arguments.
• Redaction of variadic arguments.

What's missing:

• Adding arguments to native functions / methods / classes (e.g. PDO's
  constructor)

Thank you all for reading through our proposal. We're happy to hear your
feedback!

I already registered a Wiki account (timwolla, duesterhus@woltlab.com)
and would require RFC karma to proceed if you feel like the proposal is
worthwhile writing up as an RFC.

PS: It appears that the time on the ezmlm host is misconfigured. The
email to confirm my subscription was dated 09:42:51 -0000, when it
actually was 11:50:20 +0100 (i.e. off by 1 hour, 8 minutes).

Best regards
Tim Düsterhus
Developer WoltLab GmbH

--

WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam

Tel.: +49 331 96784338

duesterhus@woltlab.com
www.woltlab.com

Managing director:
Marcel Werk

AG Potsdam HRB 26795 P

--

To unsubscribe, visit: https://www.php.net/unsub.php