Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116848
MIME-Version: 1.0
References: <77c88aeb-5792-b3f1-56bc-ffad95cf42ff@woltlab.com>
In-Reply-To: <77c88aeb-5792-b3f1-56bc-ffad95cf42ff@woltlab.com>
Date: Sat, 8 Jan 2022 00:03:32 +0530
Message-ID: <CANLbj-oEe_wXSrAUvYVv56F7asRSfNdeRBXO6PtJ-y1idQ-YRA@mail.gmail.com>
To: =?UTF-8?Q?Tim_D=C3=BCsterhus=2C_WoltLab_GmbH?= <duesterhus@woltlab.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Pre-RFC: Redacting parameters in back traces
 (SensitiveParameter Attribute)
From: ayesh@php.watch (Ayesh Karunaratne)

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=C3=BCsterhus, 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=3Dloca...', =
'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 eithe=
r:
> - 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-paramet=
er
>
> 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=C3=BCsterhus
> 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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>