rfc:redact_parameters_in_back_traces
Differences
This shows you the differences between two versions of the page.
Both sides previous revisionPrevious revisionNext revision | Previous revision | ||
rfc:redact_parameters_in_back_traces [2022/01/27 13:39] – Fix formatting. timwolla | rfc:redact_parameters_in_back_traces [2022/06/13 09:15] (current) – Add the PR that applied the attribute to the existing functions. timwolla | ||
---|---|---|---|
Line 1: | Line 1: | ||
====== PHP RFC: Redacting parameters in back traces ====== | ====== PHP RFC: Redacting parameters in back traces ====== | ||
- | * Version: 1.2 | + | * Version: 1.5 |
* Date: 2022-01-10 | * Date: 2022-01-10 | ||
* Author: Tim Düsterhus, duesterhus@woltlab.com | * Author: Tim Düsterhus, duesterhus@woltlab.com | ||
- | * Status: | + | * Status: |
+ | * Target Version: PHP 8.2 | ||
+ | * Implementation: | ||
* First Published at: http:// | * First Published at: http:// | ||
Line 24: | Line 26: | ||
Even if the application follows best practices by not exposing raw error messages to the visitor, the error messages, including their stack trace, will usually end up within the application' | Even if the application follows best practices by not exposing raw error messages to the visitor, the error messages, including their stack trace, will usually end up within the application' | ||
+ | |||
+ | Not having a standardized solution to parameter redaction makes it hard for userland exception handlers to reliably scrub stack traces, because the handler will need to guess which parameters might or might not hold sensitive values. Furthermore it requires reimplementing the scrubbing logic within every library, framework or application. | ||
===== Proposal ===== | ===== Proposal ===== | ||
- | To prevent these sensitive parameters from appearing within a stack trace this RFC proposes a new < | + | To prevent these sensitive parameters from appearing within a stack trace this RFC proposes a new standardized |
+ | |||
+ | 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. Specifically when the backtrace is generated, any parameter that has a < | ||
- | 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. Specifically when the backtrace is generated, any parameter that has a < | ||
==== Choice of replacement value ==== | ==== Choice of replacement value ==== | ||
- | This RFC proposes a < | + | This RFC proposes a < |
While strings are likely the most commonly encountered type of sensitive parameter, some sensitive values might also be passed as an object that might get serialized and then shipped to a logging service (e.g. a < | While strings are likely the most commonly encountered type of sensitive parameter, some sensitive values might also be passed as an object that might get serialized and then shipped to a logging service (e.g. a < | ||
- | For this reason, the replacement value will need to violate the type-hint for at least some of the parameters the attribute is applied to. Using a < | + | For this reason, the replacement value will need to violate the type-hint for at least some of the parameters the attribute is applied to. Using a < |
+ | |||
+ | Furthermore the replacement object will store the original value, allowing it to retrieve it on explicit request, while making it hard to accidentally expose it. | ||
+ | |||
+ | The userland equivalent of the < | ||
+ | |||
+ | < | ||
+ | <?php | ||
+ | |||
+ | final class SensitiveParameterValue | ||
+ | { | ||
+ | public function __construct(private readonly mixed $value) {} | ||
+ | |||
+ | public function getValue(): mixed { return $value; } | ||
+ | |||
+ | /* Hide the value from var_dump(). */ | ||
+ | public function __debugInfo(): | ||
+ | |||
+ | /* Hide the value from serialization. */ | ||
+ | public function __serialize(): | ||
+ | |||
+ | /* Prevent unserialization, | ||
+ | public function __unserialize(array $data): void { | ||
+ | throw new \Exception(' | ||
+ | } | ||
+ | } | ||
+ | </ | ||
==== Examples ==== | ==== Examples ==== | ||
Line 59: | Line 90: | ||
Fatal error: Uncaught Exception: Error in test.php:8 | Fatal error: Uncaught Exception: Error in test.php:8 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(11): | + | #0 test.php(11): |
#1 {main} | #1 {main} | ||
thrown in test.php on line 8 | thrown in test.php on line 8 | ||
Line 86: | Line 117: | ||
Fatal error: Uncaught Exception: Error in test.php:8 | Fatal error: Uncaught Exception: Error in test.php:8 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(13): | + | #0 test.php(13): |
#1 {main} | #1 {main} | ||
thrown in test.php on line 8 | thrown in test.php on line 8 | ||
Line 109: | Line 140: | ||
Fatal error: Uncaught Exception: Error in test.php:8 | Fatal error: Uncaught Exception: Error in test.php:8 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(11): | + | #0 test.php(11): |
#1 {main} | #1 {main} | ||
thrown in test.php on line 8 | thrown in test.php on line 8 | ||
Line 132: | Line 163: | ||
Fatal error: Uncaught Exception: Error in test.php:7 | Fatal error: Uncaught Exception: Error in test.php:7 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(10): | + | #0 test.php(10): |
#1 {main} | #1 {main} | ||
thrown in test.php on line 7 | thrown in test.php on line 7 | ||
Line 164: | Line 195: | ||
Fatal error: Uncaught Exception: Error in test.php:8 | Fatal error: Uncaught Exception: Error in test.php:8 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(16): | + | #0 test.php(16): |
- | #1 test.php(19): | + | #1 test.php(19): |
#2 {main} | #2 {main} | ||
thrown in test.php on line 8 | thrown in test.php on line 8 | ||
Line 189: | Line 220: | ||
Fatal error: Uncaught Exception: Error in test.php:8 | Fatal error: Uncaught Exception: Error in test.php:8 | ||
Stack trace: | Stack trace: | ||
- | #0 test.php(11): | + | #0 test.php(11): |
#1 {main} | #1 {main} | ||
thrown in test.php on line 8 | thrown in test.php on line 8 | ||
Line 216: | Line 247: | ||
\assert($testFrame[' | \assert($testFrame[' | ||
\assert($testFrame[' | \assert($testFrame[' | ||
- | \assert($testFrame[' | + | \assert($testFrame[' |
+ | // Explicitly retrieve the original value. | ||
+ | \assert($testFrame[' | ||
\assert($testFrame[' | \assert($testFrame[' | ||
} | } | ||
Line 279: | Line 312: | ||
} | } | ||
i:1; | i:1; | ||
- | O:18:"SensitiveParameter":0:{} | + | O:18:"SensitiveParameterValue":0:{} |
} | } | ||
} | } | ||
Line 303: | Line 336: | ||
/* | /* | ||
- | #0 test.php(12): | + | #0 test.php(12): |
array(1) { | array(1) { | ||
[0]=> | [0]=> | ||
Line 318: | Line 351: | ||
string(3) " | string(3) " | ||
[1]=> | [1]=> | ||
- | object(SensitiveParameter)#1 (0) { | + | object(SensitiveParameterValue)#1 (0) { |
} | } | ||
[2]=> | [2]=> | ||
Line 350: | Line 383: | ||
===== Backward Incompatible Changes ===== | ===== Backward Incompatible Changes ===== | ||
- | 1. The < | + | 1. The < |
- | This is very unlikely to break existing code. The class name is fairly specific and GitHub' | + | This is very unlikely to break existing code. The class name is fairly specific and GitHub' |
https:// | https:// | ||
- | 2. Custom exception handlers might see objects of class < | + | 2. Custom exception handlers might see objects of class < |
- | Clearly indicating any redacted parameters is considered to outweight this minor BC break. It is unlikely that an exception handler would use reflection to learn about the parameter type and then validate the passed value. In any case updating the exception handler to include an < | + | Clearly indicating any redacted parameters is considered to outweight this minor BC break. It is unlikely that an exception handler would use reflection to learn about the parameter type and then validate the passed value. In any case updating the exception handler to include an < |
===== Proposed PHP Version(s) ===== | ===== Proposed PHP Version(s) ===== | ||
Line 394: | Line 427: | ||
===== Unaffected PHP Functionality ===== | ===== Unaffected PHP Functionality ===== | ||
- | This RFC only affects the collected arguments within a back trace. Unless the back trace is processed programmatically, | + | This RFC only affects the collected arguments within a back trace. Unless the back trace is processed programmatically, |
===== Future Scope ===== | ===== Future Scope ===== | ||
- | * Storing the original value within the replacement value. Care needs to be taken that this does not easily expose the original value, e.g. when serializing. | + | None. |
===== Proposed Voting Choices ===== | ===== Proposed Voting Choices ===== | ||
- | Add the < | + | Add the < |
+ | |||
+ | Voting started on 2022-02-09. Voting runs until 2022-02-23 at 13:30 UTC. | ||
+ | |||
+ | <doodle title=" | ||
+ | * Yes | ||
+ | * No | ||
+ | </ | ||
===== Patches and Tests ===== | ===== Patches and Tests ===== | ||
Line 408: | Line 448: | ||
Prototype patch: https:// | Prototype patch: https:// | ||
- | We would need assistance by a more experienced developer in cleaning up the implementation | + | ===== Errata ===== |
+ | |||
+ | During code review it was noticed that the proposed serialization behavior of < | ||
+ | |||
+ | * https:// | ||
+ | * https:// | ||
+ | |||
+ | Compared to the proposal a userland implementation of < | ||
+ | |||
+ | < | ||
+ | <?php | ||
+ | |||
+ | final class SensitiveParameterValue | ||
+ | { | ||
+ | public function __construct(private readonly mixed $value) {} | ||
+ | |||
+ | public function getValue(): mixed { return $value; } | ||
+ | |||
+ | /* Hide the value from var_dump(). */ | ||
+ | public function __debugInfo(): | ||
+ | |||
+ | /* Prevent serialization. */ | ||
+ | public function __serialize(): | ||
+ | throw new \Exception(' | ||
+ | } | ||
+ | |||
+ | /* Prevent unserialization. */ | ||
+ | public function __unserialize(array $data): void { | ||
+ | throw new \Exception(' | ||
+ | } | ||
+ | } | ||
+ | </ | ||
+ | |||
+ | Note that the < | ||
===== Implementation ===== | ===== Implementation ===== | ||
- | n/a | + | This was merged into PHP 8.2 in https:// |
+ | |||
+ | The attribute was applied to existing functions in https:// | ||
===== References ===== | ===== References ===== | ||
Line 419: | Line 495: | ||
* Existing user-land implementation: | * Existing user-land implementation: | ||
* Discussion thread: https:// | * Discussion thread: https:// | ||
+ | * PHP Internals News Podcast: https:// | ||
* Library attempting to solve the same problem by wrapping strings in objects: https:// | * Library attempting to solve the same problem by wrapping strings in objects: https:// | ||
Line 427: | Line 504: | ||
===== Changelog ===== | ===== Changelog ===== | ||
+ | * 1.5: Store the original value. | ||
+ | * 1.4: Use SensitiveParameterValue as the replacement value. | ||
+ | * 1.3: " | ||
* 1.2: Expanded Introduction: | * 1.2: Expanded Introduction: | ||
* 1.1: Clarified language, justifying the choice of replacement value, Closure example, Keypair example, debug_backtrace example. | * 1.1: Clarified language, justifying the choice of replacement value, Closure example, Keypair example, debug_backtrace example. | ||
rfc/redact_parameters_in_back_traces.1643290793.txt.gz · Last modified: 2022/01/27 13:39 by timwolla