Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116847
To: PHP internals <internals@lists.php.net>
Message-ID: <77c88aeb-5792-b3f1-56bc-ffad95cf42ff@woltlab.com>
Date: Fri, 7 Jan 2022 19:10:36 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Pre-RFC: Redacting parameters in back traces (SensitiveParameter
 Attribute)
From: duesterhus@woltlab.com (=?UTF-8?Q?Tim_D=c3=bcsterhus=2c_WoltLab_GmbH?=)

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