rfc:redact_parameters_in_back_traces

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision		Previous revision
rfc:redact_parameters_in_back_traces [2022/01/17 14:40] – Expand introduction, version 1.2. timwollarfc: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.1+  * 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: Under Discussion+  * Status: Implemented 
 +  * Target Version: PHP 8.2 
 +  * Implementation: https://github.com/php/php-src/commit/90851977348cbb8c65fce19a1670868422414fae (Creation of the attribute) / https://github.com/php/php-src/pull/8352 (Applying the attribute)
   * First Published at: http://wiki.php.net/rfc/redact_parameters_in_back_traces   * First Published at: http://wiki.php.net/rfc/redact_parameters_in_back_traces
  
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's error log. These error logs are then commonly shipped to a log analysis or error tracking services and in many cases these services are provided by a third party as a SaaS offering. Sending plaintext passwords, credit card numbers or other personally identifying information to such a log analysis service might put the operator in a violation of their respective privacy laws. 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's error log. These error logs are then commonly shipped to a log analysis or error tracking services and in many cases these services are provided by a third party as a SaaS offering. Sending plaintext passwords, credit card numbers or other personally identifying information to such a log analysis service might put the operator in a violation of their respective privacy laws.
 +
 +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 <php>\SensitiveParameter</php> attribute that can be applied to a function's parameter to indicate that the parameter contains sensitive information that must not appear in back traces.+To prevent these sensitive parameters from appearing within a stack trace this RFC proposes a new standardized <php>\SensitiveParameter</php> attribute that can be applied to a function's parameter to indicate that the parameter contains sensitive information that must not appear in back traces. 
 + 
 +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 <php>\SensitiveParameter</php> attribute will not have its value stored in the backtrace, but instead will be replaced with a <php>\SensitiveParameterValue</php> object.
  
-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 <php>\SensitiveParameter</php> attribute will not have its value stored in the backtrace, but instead will be replaced with a <php>\SensitiveParameter</php> object. 
  
 ==== Choice of replacement value ==== ==== Choice of replacement value ====
  
-This RFC proposes a <php>\SensitiveParameter</php> object as the replacement value, instead of something simpler such as a <php>'[redacted]'</php> string.+This RFC proposes a <php>\SensitiveParameterValue</php> object as the replacement value, instead of something simpler such as a <php>'[redacted]'</php> string.
  
 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 <php>Keypair</php> object) within an exception handler. For parameters that are type-hinted to take a specific argument it is generally not possible to generically construct a replacement value that does not violate the type-hint. 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 <php>Keypair</php> object) within an exception handler. For parameters that are type-hinted to take a specific argument it is generally not possible to generically construct a replacement value that does not violate the type-hint.
  
-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 <php>\SensitiveParameter</php> object will almost certainly violate a type hint, but it allows userland code to reliably detect the difference between a real value and a parameter that was redacted by using an <php>$foo instanceof \SensitiveParameter</php> check.+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 <php>\SensitiveParameterValue</php> object will almost certainly violate a type hint, but it allows userland code to reliably detect the difference between a real value and a parameter that was redacted by using an <php>$foo instanceof \SensitiveParameterValue</php> check. 
 + 
 +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>\SensitiveParameterValue</php> class is: 
 + 
 +<PHP> 
 +<?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(): array { return []; } 
 + 
 +    /* Hide the value from serialization. */ 
 +    public function __serialize(): array { return []; } 
 + 
 +    /* Prevent unserialization, as the stored value cannot round-trip. */ 
 +    public function __unserialize(array $data): void { 
 +        throw new \Exception('...'); 
 +    } 
 +} 
 +</PHP>
  
 ==== 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): test('foo', Object(SensitiveParameter), 'baz')+#0 test.php(11): test('foo', Object(SensitiveParameterValue), 'baz')
 #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): test(NULL, Object(SensitiveParameter), 'baz')+#0 test.php(13): test(NULL, Object(SensitiveParameterValue), 'baz')
 #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): test(NULL, Object(SensitiveParameter), 'baz')+#0 test.php(11): test(NULL, Object(SensitiveParameterValue), 'baz')
 #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): test('foo', Object(SensitiveParameter), Object(SensitiveParameter), Object(SensitiveParameter))+#0 test.php(10): test('foo', Object(SensitiveParameterValue), Object(SensitiveParameterValue), Object(SensitiveParameterValue))
 #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): test('foo', Object(SensitiveParameter), 'baz') +#0 test.php(16): test('foo', Object(SensitiveParameterValue), 'baz') 
-#1 test.php(19): test2(Object(SensitiveParameter), 'bar', 'baz')+#1 test.php(19): test2(Object(SensitiveParameterValue), 'bar', 'baz')
 #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): {closure}('foo', Object(SensitiveParameter), 'baz')+#0 test.php(11): {closure}('foo', Object(SensitiveParameterValue), 'baz')
 #1 {main} #1 {main}
   thrown in test.php on line 8   thrown in test.php on line 8
Line 216: Line 247:
     \assert($testFrame['function'] === 'test');     \assert($testFrame['function'] === 'test');
     \assert($testFrame['args'][0] === 'foo');     \assert($testFrame['args'][0] === 'foo');
-    \assert($testFrame['args'][1] instanceof \SensitiveParameter);+    \assert($testFrame['args'][1] instanceof \SensitiveParameterValue); 
 +    // Explicitly retrieve the original value. 
 +    \assert($testFrame['args'][1]->getValue() === 'bar');
     \assert($testFrame['args'][2] === 'baz');     \assert($testFrame['args'][2] === 'baz');
 } }
Line 279: Line 312:
                         }                         }
                     i:1;                     i:1;
-                        O:18:"SensitiveParameter":0:{}+                        O:18:"SensitiveParameterValue":0:{}
                 }                 }
         }         }
Line 303: Line 336:
  
 /* /*
-#0 test.php(12): test('foo', Object(SensitiveParameter), 'baz')+#0 test.php(12): test('foo', Object(SensitiveParameterValue), 'baz')
 array(1) { array(1) {
   [0]=>   [0]=>
Line 318: Line 351:
       string(3) "foo"       string(3) "foo"
       [1]=>       [1]=>
-      object(SensitiveParameter)#1 (0) {+      object(SensitiveParameterValue)#1 (0) {
       }       }
       [2]=>       [2]=>
Line 341: Line 374:
  
   * 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.   * 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.
 +
 +=== Creating a wrapper class for sensitive strings ===
 +
 +The [[https://github.com/paragonie/hidden-string|paragonie/hidden-string]] library attempts to solve the same problem of sensitive parameters appearing in stack traces. While this attempt works with functions that specifically expect such a <php>HiddenString</php> to be passed, it will not work with parameters that are type-hinted to take a <php>string</php>. Either the underlying string needs to be explicitly extracted, or <php>__toString()</php> support needs to be enabled within the library. In both cases the scalar string will appear in the stack trace.
 +
 +Furthermore such a wrapper class is limited to string values and cannot easily protect other types of sensitive values.
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
  
-1. The <php>\SensitiveParameter</php> class name will no longer be available to userland code.+1. The <php>\SensitiveParameter</php> and <php>\SensitiveParameterValue</php> class name will no longer be available to userland code.
  
-This is very unlikely to break existing code. The class name is fairly specific and GitHub's search for <php>\SensitiveParameter</php> in PHP code only returns 6 results, all of them strings:+This is very unlikely to break existing code. The class name is fairly specific and GitHub's search for <php>SensitiveParameter</php> in PHP code only returns 6 results, all of them strings:
  
 https://github.com/search?l=PHP&q=SensitiveParameter&type=Code https://github.com/search?l=PHP&q=SensitiveParameter&type=Code
  
-2. Custom exception handlers might see objects of class <php>\SensitiveParameter</php>, despite the parameter having a different type within the method's signature.+2. Custom exception handlers might see objects of class <php>\SensitiveParameterValue</php>, despite the parameter having a different type within the method's signature.
  
-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 <php>$foo instanceof \SensitiveParameter</php> check is considered trivial and will not break compatibility with older PHP versions.+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 <php>$foo instanceof \SensitiveParameterValue</php> check is considered trivial and will not break compatibility with older PHP versions.
  
 ===== Proposed PHP Version(s) ===== ===== Proposed PHP Version(s) =====
Line 367: Line 406:
  
 Extensions should verify any existing parameters and add the <php>\SensitiveParameter</php> attribute for parameters deemed sensitive. Extensions should verify any existing parameters and add the <php>\SensitiveParameter</php> attribute for parameters deemed sensitive.
 +
 +Debuggers might be affected. Changes might be required to expose the original values during a debugging session, e.g. when stepping through the code.
  
 ==== To Opcache ==== ==== To Opcache ====
  
-Probably none? [to be discussed]+None.
  
 ==== New Constants ==== ==== New Constants ====
Line 382: Line 423:
 ===== Open Issues ===== ===== Open Issues =====
  
-  - The prototype patch does not yet add the <php>\SensitiveParameter</php> attribute to any internal functions.+None.
  
 ===== Unaffected PHP Functionality ===== ===== Unaffected PHP Functionality =====
  
-This RFC only affects the collected arguments within a back trace. Unless the back trace is processed programmatically, the only change is that a developer will notice is that some error messages show <html>Object(SensitiveParameter)</html> in place of a real parameter.+This RFC only affects the collected arguments within a back trace. Unless the back trace is processed programmatically, the only change is that a developer will notice is that some error messages show <html>Object(SensitiveParameterValue)</html> in place of a real parameter.
  
 ===== Future Scope ===== ===== Future Scope =====
Line 394: Line 435:
 ===== Proposed Voting Choices ===== ===== Proposed Voting Choices =====
  
-Add the <php>\SensitiveParameter</php> attribute and redact values in back traces for parameters having this attribute?+Add the <php>\SensitiveParameter</php> attribute and replace parameters having this attribute in back traces by <php>\SensitiveParameterValue</php>? 
 + 
 +Voting started on 2022-02-09. Voting runs until 2022-02-23 at 13:30 UTC. 
 + 
 +<doodle title="Redacting parameters in back traces" auth="timwolla" voteType="single" closed="true"> 
 +   * Yes 
 +   * No 
 +</doodle>
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
Line 400: Line 448:
 Prototype patch: https://github.com/php/php-src/pull/7921 Prototype patch: https://github.com/php/php-src/pull/7921
  
-We would need assistance by a more experienced developer in cleaning up the implementation and adding this attribute to existing functions.+===== Errata ===== 
 + 
 +During code review it was noticed that the proposed serialization behavior of <php>\SensitiveParameterValue</php> was not useful: 
 + 
 +  * https://github.com/php/php-src/pull/7921#discussion_r813743903 
 +  * https://externals.io/message/117136 
 + 
 +Compared to the proposal a userland implementation of <php>\SensitiveParameterValue</php> class would look like the following: 
 + 
 +<PHP> 
 +<?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(): array { return []; } 
 + 
 +    /* Prevent serialization. */ 
 +    public function __serialize(): array { 
 +        throw new \Exception('...'); 
 +    } 
 + 
 +    /* Prevent unserialization. */ 
 +    public function __unserialize(array $data): void { 
 +        throw new \Exception('...'); 
 +    } 
 +} 
 +</PHP> 
 + 
 +Note that the <php>__serialize()</php> and <php>__unserialize()</php> methods are not actually implemented. Serialization is prevented using a flag on the internal class implementation.
  
 ===== Implementation ===== ===== Implementation =====
  
-n/a+This was merged into PHP 8.2 in https://github.com/php/php-src/commit/90851977348cbb8c65fce19a1670868422414fae, based on the PR https://github.com/php/php-src/pull/7921. 
 + 
 +The attribute was applied to existing functions in https://github.com/php/php-src/pull/8352. 
  
 ===== References ===== ===== References =====
Line 411: Line 495:
   * Existing user-land implementation: https://github.com/WoltLab/WCF/blob/c4d4b97448213a1f806e3b7b7af8d50d68034b02/wcfsetup/install/files/lib/core.functions.php#L720-L741   * Existing user-land implementation: https://github.com/WoltLab/WCF/blob/c4d4b97448213a1f806e3b7b7af8d50d68034b02/wcfsetup/install/files/lib/core.functions.php#L720-L741
   * Discussion thread: https://externals.io/message/116853   * Discussion thread: https://externals.io/message/116853
 +  * PHP Internals News Podcast: https://phpinternals.news/97
 +  * Library attempting to solve the same problem by wrapping strings in objects: https://github.com/paragonie/hidden-string
  
 ===== Rejected Features ===== ===== Rejected Features =====
Line 418: Line 504:
 ===== Changelog ===== ===== Changelog =====
  
 +  * 1.5: Store the original value.
 +  * 1.4: Use SensitiveParameterValue as the replacement value.
 +  * 1.3: "Creating a wrapper class" section, resolved open issues/questions, future scope.
   * 1.2: Expanded Introduction: Clarify that secrets are not limited to database passwords. Clarify that they might result in error tracking services.   * 1.2: Expanded Introduction: Clarify that secrets are not limited to database passwords. Clarify that they might result in error tracking services.
   * 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.txt · Last modified: 2022/06/13 09:15 by timwolla

Page Tools