Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Dan On 2/7/22 17:36, Dan Ackroyd wrote: So basically all the other languages I researched do not provide arguments within back traces. Uh, that kind of suggests that providing arguments at all is a mistake, and that removing could be the way to go. I mean other than everyone complaining about the BC break. Personally I wouldn't complain about the BC break per se, but about that making debugging harder :-) An awkward question; why is this hard-coding of the behaviour dependent on a new attribute the correct thing to do? Reading through the RFC I concede that this is not (yet) explicitly spelled out. However the following sentence appears within the "Proposal" section: To reliably apply this protection for all types of back traces and all types of exception and error handlers, the redaction should happen when collecting the parameter values during back trace generation. "reliably apply [...] for all [...] error handlers". I've explained that in more detail in the podcast with Derick (https://phpinternals.news/97). e.g. at the end of 1:05: But in any case, this exception handler will miss sensitive information because it needs to basically guess what parameters are sensitive values and which don't. - PHP allows people to set_error_handler() to process errors how they like. Conceiveably, allowing users to set a custom function to redact data from stack traces would allow users to inspect and redact the parameters however they like. Why is adding a special attribute that is recognised by the PHP engine itself the right thing to do? As explained above: Without having a standardized solution, redacting those arguments results in guesswork for the exception handler. A userland solution (e.g. standardized in PHP FIG) does not cut it, because then native functions will not be protected (e.g. ldap_bind). I have made the following change to the RFC to hopefully make this clearer within the introduction: https://wiki.php.net/rfc/redact_parameters_in_back_traces?do=diff%5B0%5D=1643972253%5B1%5D=1644252789=sidebyside Please let me know if this is sufficiently clear, or if you'd like to see this expanded further. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Tue, 11 Jan 2022 at 09:11, Tim Düsterhus, WoltLab GmbH wrote: > > So basically all the other languages I researched do not provide > arguments within back traces. Uh, that kind of suggests that providing arguments at all is a mistake, and that removing could be the way to go. I mean other than everyone complaining about the BC break. An awkward question; why is this hard-coding of the behaviour dependent on a new attribute the correct thing to do? PHP allows people to set_error_handler() to process errors how they like. Conceiveably, allowing users to set a custom function to redact data from stack traces would allow users to inspect and redact the parameters however they like. Why is adding a special attribute that is recognised by the PHP engine itself the right thing to do? To be clear, I can think of at least two reasons. But also, I think every attribute that is proposed to be added to PHP core, needs to have the reasons why it's the right thing to do listed. If nothing else, it will help to reject attributes in the future if they don't have the same strong justifications. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Internals! On 1/31/22 10:54, Tim Düsterhus, WoltLab GmbH wrote: I plan to open voting on Wednesday, February, 2nd. Voting will run 2 weeks, 2/3 majority with the concept being voted on as explained in the "Proposed Voting Choice" section: https://wiki.php.net/rfc/redact_parameters_in_back_traces#proposed_voting_choices This did not happen because of Alex' remark that storing the original value should be part of this vote. I've made the necessary changes and did not receive any further remarks, thus: I plan to open voting on Wednesday, February, 9th. Voting will run 2 weeks. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Alex On 2/1/22 07:38, Alexandru Pătrănescu wrote: I think storing the original value within the replacement value should be considered and voted in this RFC as well, even if implemented in a separate PR. I did write some code where I process programmatically the backtraces and while I might not have used it with sensitive parameters, it would be good to have the code generic, if this passes. I'm guessing that mostly means accepting the value as a constructor parameter exposing a getValue() method And, of course, making sure var_dump/print_r/string-casting does not print it. I mean, it looks like the implementation is doable. I've now proceeded with this: https://wiki.php.net/rfc/redact_parameters_in_back_traces?do=diff%5B0%5D=1643710897%5B1%5D=1643972253=sidebyside I've also updated the example implementation: https://github.com/php/php-src/pull/7921 Please take a look, looking forward to your feedback! Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Alex
On 2/1/22 07:38, Alexandru Pătrănescu wrote:
I think storing the original value within the replacement value should be
considered and voted in this RFC as well, even if implemented in a separate
PR.
I did write some code where I process programmatically the backtraces and
while I might not have used it with sensitive parameters, it would be good
to have the code generic, if this passes.
That's fair. I guess you are thinking of including this in the primary
vote, instead of a secondary vote, right? It doesn't make sense to leave
this out if you already have a use case that would break otherwise.
I'm guessing that mostly means accepting the value as a constructor
parameter exposing a getValue() method
And, of course, making sure var_dump/print_r/string-casting does not print
it. I mean, it looks like the implementation is doable.
I believe the following (userland) implementation should do the right thing:
final class SensitiveParameterValue
{
public function __construct(private readonly mixed $value) {}
public function getValue(): mixed { return $value; }
public function __debugInfo(): array { return []; }
public function __serialize(): array { return []; }
}
It allows you to explicitly retrieve the original value, but makes it
hard to accidentally expose it, by hiding it from 'var_dump()' and
'serialize()'.
Thinking about this will bring a small issue into plain sight, the
attribute is the same class as the replacing placeholder,
\SensitiveParameter.
I believe they should be separate classes, \SensitiveParameter marked as an
Attribute that can be applied to parameters and something like
\SensitiveParameterValue that replaces the original value in stack traces.
You are right. If we also want to store the original value, we should
use a separate class. In any case re-using the attribute class will
limit future extensions.
I've already adjusted the RFC (and the PoC implementation) to update the
replacement value to SensitiveParameterValue:
https://wiki.php.net/rfc/redact_parameters_in_back_traces. Regarding
storing the original value I'll wait for your reply.
Best regards
Tim Düsterhus
Developer WoltLab GmbH
--
WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam
Tel.: +49 331 96784338
duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Mon, Jan 31, 2022 at 11:55 AM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Internals! > > On 1/10/22 15:05, Tim Düsterhus, WoltLab GmbH wrote: > > https://wiki.php.net/rfc/redact_parameters_in_back_traces > At the end of last week I've updated the RFC a little based on the > questions Derick Rethan asked me for episode #97 of PHP Internals News > podcast: > > https://phpinternals.news/97 > > https://github.com/php/php-src/pull/7921 > > now adds the \SensitiveParameter attribute to PDO::__construct()'s > $password parameter and to password_hash()'s $password parameter. > > > I believe I've answered all open questions and I also managed to resolve > the open issues I listed in my initial email. > > Hey Tim, I think storing the original value within the replacement value should be considered and voted in this RFC as well, even if implemented in a separate PR. I did write some code where I process programmatically the backtraces and while I might not have used it with sensitive parameters, it would be good to have the code generic, if this passes. I'm guessing that mostly means accepting the value as a constructor parameter exposing a getValue() method And, of course, making sure var_dump/print_r/string-casting does not print it. I mean, it looks like the implementation is doable. Thinking about this will bring a small issue into plain sight, the attribute is the same class as the replacing placeholder, \SensitiveParameter. I believe they should be separate classes, \SensitiveParameter marked as an Attribute that can be applied to parameters and something like \SensitiveParameterValue that replaces the original value in stack traces. Regards, Alex
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Internals! On 1/10/22 15:05, Tim Düsterhus, WoltLab GmbH wrote: https://wiki.php.net/rfc/redact_parameters_in_back_traces At the end of last week I've updated the RFC a little based on the questions Derick Rethan asked me for episode #97 of PHP Internals News podcast: https://phpinternals.news/97 - As indicated within the RFC and my previous email we still need a more experienced developer for the final implementation, as I have next to no experience with PHP's implementation. Specifically adding this attribute to existing functions is not clear to me. It is probably required to update the stub parser/generator to add support for attributes? If someone creates an example implementation for one function, I'll likely be able to apply this to other functions myself. I also managed to figure this out. The proof of concept implementation at https://github.com/php/php-src/pull/7921 now adds the \SensitiveParameter attribute to PDO::__construct()'s $password parameter and to password_hash()'s $password parameter. - The RFC Impact to Opcache is not clear to me. I don't believe there is any, but I am not sure. So if someone knows, I'm happy to update that section. For this I got a confirmation in private that Opcache will not be affected. - I believe I've answered all open questions and I also managed to resolve the open issues I listed in my initial email. As more than two weeks have passed since I started this discussion thread and as the discussion appears to have died down by now I'd like to start the vote on this RFC. I plan to open voting on Wednesday, February, 2nd. Voting will run 2 weeks, 2/3 majority with the concept being voted on as explained in the "Proposed Voting Choice" section: https://wiki.php.net/rfc/redact_parameters_in_back_traces#proposed_voting_choices This being my first RFC, please let me know if I missed something with regard to the procedure! Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Benjamin On 1/15/22 7:07 PM, Benjamin Eberlei wrote: I believe it wouldn't hurt the RFC to add more words around the fact that stacktraces are often sent to third party services (Exception Tracking software) and as such a redaction of the parameters would be powerful for additional redaction of credit cards, email addresses and other personal data. The example with PDO::__construct is an obvious choice to redact passwords, but application level data is a second source of input that is critical to redact. Thank you for the feedback. I've expanded (and hopefully clarified) the "Introduction" section in version 1.2: https://wiki.php.net/rfc/redact_parameters_in_back_traces?rev=1642064843=diff Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Tim, On Mon, Jan 10, 2022 at 3:06 PM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Internals! > > this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th. > > Christoph Becker granted me RFC editing permissions and I've now written > up our proposal as a proper RFC: > > https://wiki.php.net/rfc/redact_parameters_in_back_traces This is a very good addition in my opinion. And as one of the attributes RFC authors I am thrilled about their use here ;-) I believe it wouldn't hurt the RFC to add more words around the fact that stacktraces are often sent to third party services (Exception Tracking software) and as such a redaction of the parameters would be powerful for additional redaction of credit cards, email addresses and other personal data. The example with PDO::__construct is an obvious choice to redact passwords, but application level data is a second source of input that is critical to redact. > > > I recommend also taking a look at my previous email: > > https://externals.io/message/116847 > > It contains some additional context that did not really fit within the > language of a "neutral" RFC that will remain as the permanent record. > > - As indicated within the RFC and my previous email we still need a more > experienced developer for the final implementation, as I have next to no > experience with PHP's implementation. > > Specifically adding this attribute to existing functions is not clear to > me. It is probably required to update the stub parser/generator to add > support for attributes? If someone creates an example implementation for > one function, I'll likely be able to apply this to other functions myself. > - The RFC Impact to Opcache is not clear to me. I don't believe there is > any, but I am not sure. So if someone knows, I'm happy to update that > section. > > Best regards > Tim Düsterhus > Developer WoltLab GmbH > > -- > > WoltLab GmbH > Nedlitzer Str. 27B > 14469 Potsdam > > Tel.: +49 331 96784338 > > duester...@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 > >
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Thu, Jan 13, 2022 at 10:04 AM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Lynn > > On 1/12/22 9:30 AM, Lynn wrote: > > I was thinking more of a "keep track of the values replaced, and in the > > end purge all those values from the end-result" kinda thing. > > > > Thank you for the clarification. This still is not in scope, because I > believe that to be harmful, as the parameter redaction will be > completely unpredictable. > > Consider a sensitive parameter that is of type '?string', i.e. nullable. > Now with your proposal, whenever 'null' is passed to this parameter, all > 'null's within the stack trace would be hidden, even if they are > completely unrelated. > > Yeah I'm with you 100%, this has more edge cases than I originally thought of. The RFC as it is already improves a lot for me so I'll be glad to see it regardless!
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Lynn On 1/12/22 9:30 AM, Lynn wrote: I was thinking more of a "keep track of the values replaced, and in the end purge all those values from the end-result" kinda thing. Thank you for the clarification. This still is not in scope, because I believe that to be harmful, as the parameter redaction will be completely unpredictable. Consider a sensitive parameter that is of type '?string', i.e. nullable. Now with your proposal, whenever 'null' is passed to this parameter, all 'null's within the stack trace would be hidden, even if they are completely unrelated. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Wed, Jan 12, 2022 at 9:17 AM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Lynn > > On 1/11/22 11:23 AM, Lynn wrote: > > One possible addition; would it be possible to analyze the masked values > > and mask any 100% matches elsewhere? > > No, this is not in scope for this RFC, as it would require accurate > tracking of variable contents across reassignments and possibly function > calls. > > My understanding is that this basically would require support for > tainted variables, i.e. this very old RFC: https://wiki.php.net/rfc/taint I was thinking more of a "keep track of the values replaced, and in the end purge all those values from the end-result" kinda thing.
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Lynn On 1/11/22 11:23 AM, Lynn wrote: One possible addition; would it be possible to analyze the masked values and mask any 100% matches elsewhere? No, this is not in scope for this RFC, as it would require accurate tracking of variable contents across reassignments and possibly function calls. My understanding is that this basically would require support for tainted variables, i.e. this very old RFC: https://wiki.php.net/rfc/taint Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Tim, On Tue, Jan 11, 2022 at 4:40 PM Tim Düsterhus, WoltLab GmbH wrote: > > Hi Pierre > > On 1/11/22 4:48 AM, Pierre Joye wrote: > > Also sensitive data goes way beyond arguments, GDPR brings a lot of > > issues here too. Userland packages like monolog provide filters or > > custom output, I think that is where it should be handled. > > I believe that the author of a function is in the best position to > decide whether a specific argument generally holds sensitive data or > not. This avoids every exception handler / logger / … having to check > what function parameters hold sensitive data and scrubbing them, > possibly missing some. You are fully correct here. My thoughts differ as I think there are better tools to handle this than PHP itself, as well as all other cases where sensitive data may be exposed (a lot more than backtrace, which is handled already using the ini setting). Things like query errors, etc. There is no way we can handle them all and this is really the developer responsibility to handle this data in a safe manner. We do our best at the engine level for critical data from an engine point of view. Application level critical data are the developers responsibility (config, code, etc.). Best, -- Pierre @pierrejoye | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Mon, Jan 10, 2022 at 3:05 PM Tim Düsterhus, WoltLab GmbH <
duester...@woltlab.com> wrote:
> Hi Internals!
>
> this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th.
>
> Christoph Becker granted me RFC editing permissions and I've now written
> up our proposal as a proper RFC:
>
> https://wiki.php.net/rfc/redact_parameters_in_back_traces
>
> I recommend also taking a look at my previous email:
>
> https://externals.io/message/116847
Heya, thanks for this RFC!
The product I work on provides integration with dozens, if not hundreds of
remote services. This can be anything from mailboxes, (S)FTP(S), HTTP, and
we have a lot of databases for our tenants. Being the legacy code it is,
it's a never ending battle to not expose critical information to the end
user. Even though I've been working here for several years now, I still
keep finding these things occasionally. As we also use external logging
tools (such as the ELK stack), we want as little critical information sent
anywhere and this RFC seems to really help reduce the leaking of this
information here. In the ideal scenario the connection information minus
the password is logged explicitly, so that I have the information available
whenever one of the many tenants systems failed to connect to one of the
many dynamically configured APIs, but doing this retroactively through
millions of lines of code that's over 10 years old is a lot of work.
One possible addition; would it be possible to analyze the masked values
and mask any 100% matches elsewhere?
https://3v4l.org/G0RaQ
```
function one(string $param) {
throw new Exception($param);
}
function two(#[SensitiveParameter] string $param) {
one($param);
}
two('the secret');
Fatal error: Uncaught Exception: the secret in /in/G0RaQ:4
Stack trace:
#0 /in/G0RaQ(8): one('the secret')
#1 /in/G0RaQ(11): two('the secret')
#2 {main}
thrown in /in/G0RaQ on line 4
```
This would help in the scenario where the function `one` comes from an
external library, but it also means that I don't have to go through every
single layer and add the attributes. In the above example I want any
mention of `the secret` to be hidden. Let's say the sensitive data is a
password, then I don't want that password shown at all in the stacktrace.
The original RFC is already very valuable to me without this, but would
still expose a lot of sensitive data I fear.
The result I want from this:
```
Fatal error: Uncaught Exception: Object(SensitiveParameter) in /in/G0RaQ:4
Stack trace:
#0 /in/G0RaQ(8): one(Object(SensitiveParameter))
#1 /in/G0RaQ(11): two(Object(SensitiveParameter))
#2 {main}
thrown in /in/G0RaQ on line 4
```
This would also cover cases where for some reason the sensitive data is
added to the exception. Yes, this big facepalm is something I encounter
sadly too often in legacy code.
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Pierre On 1/11/22 4:48 AM, Pierre Joye wrote: Also sensitive data goes way beyond arguments, GDPR brings a lot of issues here too. Userland packages like monolog provide filters or custom output, I think that is where it should be handled. I believe that the author of a function is in the best position to decide whether a specific argument generally holds sensitive data or not. This avoids every exception handler / logger / … having to check what function parameters hold sensitive data and scrubbing them, possibly missing some. Of course these exception handlers / loggers will still need to take care of any other data they are getting from the request context. But in that case the affected values (e.g. the user object) often need to be explicitly passed into the handler, because they are application specific. As a side note, the RFC mentions that zend.exception_ignore_args may not be configurable on some shared hosters, it is INI_ALL, so even in the code could change it, any time, back and forth: I've seen all kinds of broken configurations / broken builds at shared web hosting over time, where things that generally work, do not for some reason. But good point indeed, I've removed that list item and only left the other one. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Alex On 1/11/22 4:10 AM, Alexandru Pătrănescu wrote: As the trace in the exception is in the same format as the one generated by debug_backtrace(), do you intend to have the changes affecting debug_backtrace() and debug_print_backtrace()? My proof of concept patch adjusts the internal 'debug_backtrace_get_args' function which the function that collects the function parameters whenever a back trace is generated (Exceptions, debug_backtrace, debug_print_backtrace). I've added a new example showing debug_backtrace to the RFC. Also, globally configuring generated exception traces to not include parameters would solve the issue? I mean, similarly with how DEBUG_BACKTRACE_IGNORE_ARGS works? This is already configurable with 'zend.exception_ignore_args'. There's a section in the RFC why we believe this to not be a solution: https://wiki.php.net/rfc/redact_parameters_in_back_traces#why_existing_features_are_insufficient Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Dan On 1/10/22 6:01 PM, Dan Ackroyd wrote: How do other languages handle this problem? Or how do they avoid it in the first place? Ryan already replied here, but I've also researched this: - Java is unable to provide parameters in stack traces. - In C you generally have a core dump which contains everything. Parameters might or might not be available depending on optimization level. - In Python parameters are not provided by default, but it appears to be available via https://docs.python.org/3/library/inspect.html#inspect.getargvalues. - In JavaScript the '.stack' property is non-standard and the behavior depends on the engine. Generally no parameters are provided. - In Ruby parameters are not provided. So basically all the other languages I researched do not provide arguments within back traces. I might have missed something, as I am not experienced in all of them. From the RFC: Specifically the back trace collection should be updated to use an object of class \SensitiveParameter as the value for all parameters that are marked with the \SensitiveParameter attribute. To methese words are not clear. Does the following sentence say the same thing? "When the backtrace is generated, any parameter that has a 'SensitiveParameter' attribute will not have it's value stored in the backtrace, but instead will be replaced with an SensitiveParameter object. If so, the RFC could be updated to be clearer.if not, then the RFC should be updated to be clearer. Yes, your phrasing is correct. It's much better than mine, so I've taken it. Also, having parameters replaced with another type doesn't seem obviously correct. There should probably be some words justifying why that is the correct thing to do, rather than just replacing any values with "REDACTED***" or other simple behaviour. I updated the RFC with a new "Choice of replacement value" section: https://wiki.php.net/rfc/redact_parameters_in_back_traces#choice_of_replacement_value On shared web hosting, the customer might not be able to configure it. My personal opinion is that shared web hosting shouldn't be a thing that exists in 2022. And definitely shouldn't be used for anything where secrets need to be maintained. Yeah shared hosts might have a DB they can connect to, but those credentials should only be usuable from the shared host to the DB. While I agree with regard to shared web hosting generally, I'd rather see a customer using a shared web hosting than attempting to self-host and running outdated vulnerable PHP versions. Also while PDO is the prominent example at the beginning of the RFC, other functions are also affected. As an example 'ldap_bind()' might include a user's password when the directory is unavailable, possibly logging the user's password within Sentry or whatever error collector is in use. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Mon, Jan 10, 2022 at 9:37 PM Michael Morris wrote: > > If someone can inject a debug_backtrace into your code and get it executed > you have bigger problems than a parameter being exposed. And if you > configure your prod servers to be all chatty Kathy to the world on error, > you need to learn how to do better. A change to the language is not in > order here. > These things can also be logged as well. This isn't a security concern only in the sense of the backtrace being displayed on a webpage output or something. There are legal requirements in many jurisdictions about how data can be retained and where. It is entirely possible that something could be accidentally logged that would inadvertently violate a local regulation for handling of customer data. Jordan
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Mon, Jan 10, 2022 at 8:05 AM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Internals! > > this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th. > > Christoph Becker granted me RFC editing permissions and I've now written > up our proposal as a proper RFC: > > https://wiki.php.net/rfc/redact_parameters_in_back_traces > > I recommend also taking a look at my previous email: > > https://externals.io/message/116847 > > It contains some additional context that did not really fit within the > language of a "neutral" RFC that will remain as the permanent record. > > - As indicated within the RFC and my previous email we still need a more > experienced developer for the final implementation, as I have next to no > experience with PHP's implementation. > > Specifically adding this attribute to existing functions is not clear to > me. It is probably required to update the stub parser/generator to add > support for attributes? If someone creates an example implementation for > one function, I'll likely be able to apply this to other functions myself. > - The RFC Impact to Opcache is not clear to me. I don't believe there is > any, but I am not sure. So if someone knows, I'm happy to update that > section. > > > If someone can inject a debug_backtrace into your code and get it executed you have bigger problems than a parameter being exposed. And if you configure your prod servers to be all chatty Kathy to the world on error, you need to learn how to do better. A change to the language is not in order here.
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Good morning Tim, On Mon, Jan 10, 2022 at 9:06 PM Tim Düsterhus, WoltLab GmbH wrote: I am not sure it makes sense to make the code so verbose to prevent users from showing sensitive data as it never stops (next print_r/var_dump and userland version of them?). Also sensitive data goes way beyond arguments, GDPR brings a lot of issues here too. Userland packages like monolog provide filters or custom output, I think that is where it should be handled. As a side note, the RFC mentions that zend.exception_ignore_args may not be configurable on some shared hosters, it is INI_ALL, so even in the code could change it, any time, back and forth: http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
On Mon, Jan 10, 2022 at 4:05 PM Tim Düsterhus, WoltLab GmbH < duester...@woltlab.com> wrote: > Hi Internals! > > this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th. > > Christoph Becker granted me RFC editing permissions and I've now written > up our proposal as a proper RFC: > > https://wiki.php.net/rfc/redact_parameters_in_back_traces Hey Tim, As the trace in the exception is in the same format as the one generated by debug_backtrace(), do you intend to have the changes affecting debug_backtrace() and debug_print_backtrace()? Also, globally configuring generated exception traces to not include parameters would solve the issue? I mean, similarly with how DEBUG_BACKTRACE_IGNORE_ARGS works? Regards, Alex
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Answering the question: How do other languages handle this problem? Or how do they avoid it in the first place? Python basically doesn't handle the problem at all and offers this advice: Be sure to delete all debugging related code before code delivery! See section [9.2.1 production code cannot contain any debug entry points] https://www.fatalerrors.org/a/python-general-programming-specification-09-security-programming-specification.html On Mon, Jan 10, 2022 at 10:01 AM Dan Ackroyd wrote: > Hi Tim, > > On Mon, 10 Jan 2022 at 14:05, Tim Düsterhus, WoltLab GmbH > wrote: > > > > this is a follow-up for my "Pre-RFC" email from last Friday, January, > 7th. > > > > https://wiki.php.net/rfc/redact_parameters_in_back_traces > > > > How do other languages handle this problem? Or how do they avoid it in > the first place? > > From the RFC: > > Specifically the back trace collection should be updated to use an > object of class > > \SensitiveParameter as the value for all parameters that are marked with > the > > \SensitiveParameter attribute. > > To methese words are not clear. Does the following sentence say > the same thing? > > "When the backtrace is generated, any parameter that has a > 'SensitiveParameter' attribute will not have it's value stored in the > backtrace, but instead will be replaced with an SensitiveParameter > object. > > If so, the RFC could be updated to be clearer.if not, then the RFC > should be updated to be clearer. > > Also, having parameters replaced with another type doesn't seem > obviously correct. There should probably be some words justifying why > that is the correct thing to do, rather than just replacing any values > with "REDACTED***" or other simple behaviour. > > > On shared web hosting, the customer might not be able to configure it. > > My personal opinion is that shared web hosting shouldn't be a thing > that exists in 2022. And definitely shouldn't be used for anything > where secrets need to be maintained. Yeah shared hosts might have a DB > they can connect to, but those credentials should only be usuable from > the shared host to the DB. > > cheers > Dan > Ack > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: https://www.php.net/unsub.php > >
Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Tim, On Mon, 10 Jan 2022 at 14:05, Tim Düsterhus, WoltLab GmbH wrote: > > this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th. > > https://wiki.php.net/rfc/redact_parameters_in_back_traces > How do other languages handle this problem? Or how do they avoid it in the first place? >From the RFC: > Specifically the back trace collection should be updated to use an object of > class > \SensitiveParameter as the value for all parameters that are marked with the > \SensitiveParameter attribute. To methese words are not clear. Does the following sentence say the same thing? "When the backtrace is generated, any parameter that has a 'SensitiveParameter' attribute will not have it's value stored in the backtrace, but instead will be replaced with an SensitiveParameter object. If so, the RFC could be updated to be clearer.if not, then the RFC should be updated to be clearer. Also, having parameters replaced with another type doesn't seem obviously correct. There should probably be some words justifying why that is the correct thing to do, rather than just replacing any values with "REDACTED***" or other simple behaviour. > On shared web hosting, the customer might not be able to configure it. My personal opinion is that shared web hosting shouldn't be a thing that exists in 2022. And definitely shouldn't be used for anything where secrets need to be maintained. Yeah shared hosts might have a DB they can connect to, but those credentials should only be usuable from the shared host to the DB. cheers Dan Ack -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
Hi Internals! this is a follow-up for my "Pre-RFC" email from last Friday, January, 7th. Christoph Becker granted me RFC editing permissions and I've now written up our proposal as a proper RFC: https://wiki.php.net/rfc/redact_parameters_in_back_traces I recommend also taking a look at my previous email: https://externals.io/message/116847 It contains some additional context that did not really fit within the language of a "neutral" RFC that will remain as the permanent record. - As indicated within the RFC and my previous email we still need a more experienced developer for the final implementation, as I have next to no experience with PHP's implementation. Specifically adding this attribute to existing functions is not clear to me. It is probably required to update the stub parser/generator to add support for attributes? If someone creates an example implementation for one function, I'll likely be able to apply this to other functions myself. - The RFC Impact to Opcache is not clear to me. I don't believe there is any, but I am not sure. So if someone knows, I'm happy to update that section. Best regards Tim Düsterhus Developer WoltLab GmbH -- WoltLab GmbH Nedlitzer Str. 27B 14469 Potsdam Tel.: +49 331 96784338 duester...@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
