rfc:improve-openssl-random-pseudo-bytes

Differences

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

Link to this comparison view

Next revision
Previous revision
Last revisionBoth sides next revision
rfc:improve-openssl-random-pseudo-bytes [2018/10/19 17:55] – created sammykrfc:improve-openssl-random-pseudo-bytes [2018/11/16 18:24] – Mark as accepted sammyk
Line 3: Line 3:
   * Date: 2018-10-19   * Date: 2018-10-19
   * Author: Sammy Kaye Powers <sammyk@php.net>   * Author: Sammy Kaye Powers <sammyk@php.net>
-  * Status: Draft+  * Implementation: https://github.com/php/php-src/pull/3649 
 +  * Status: Accepted
  
 ===== Introduction ===== ===== Introduction =====
-The ''openssl_random_pseudo_bytes()'' function is a wrapper for OpenSSL's [[https://www.openssl.org/docs/man1.0.2/crypto/RAND_bytes.html|''RAND_bytes'' CSPRNG]]. CSPRNG implementations should always fail closed, but ''openssl_random_pseudo_bytes()'' fails open pushing critical fail checks into userland. It also has an unnecessary second parameter that confuses the usage of the API.+The ''openssl_random_pseudo_bytes()'' function is a wrapper for OpenSSL's [[https://www.openssl.org/docs/man1.0.2/crypto/RAND_bytes.html|RAND_bytes CSPRNG]]. CSPRNG implementations should always fail closed, but ''openssl_random_pseudo_bytes()'' fails open pushing critical fail checks into userland. It also has an unnecessary second parameter that confuses the usage of the API.
  
 ===== The Fail-Open Problem ===== ===== The Fail-Open Problem =====
Line 59: Line 60:
  
 ===== Proposal ===== ===== Proposal =====
-To fix the fail-open problem, we simply throw an ''\Error'' exception (just like [[https://github.com/php/php-src/blob/8d3f8ca12a0b00f2a74a27424790222536235502/ext/standard/random.c#L179|''random_bytes()'' does]]). This is the Correct Behavior™️ for any CSPRNG implementation.+To fix the fail-open problem, we simply throw an ''\Exception'' (just like [[https://github.com/php/php-src/blob/8d3f8ca12a0b00f2a74a27424790222536235502/ext/standard/random.c#L179|''random_bytes()'' does]]). This is the Correct Behavior™️ for any CSPRNG implementation.
  
 To fix the the confusing-api problem, we should deprecate the usage of the second ''$crypto_strong'' parameter and just make it always set the value to true. In PHP 8.0 we'd completely remove the second parameter and upgrade the function's ZPP to ''ZEND_PARSE_PARAMS_THROW'' causing the following fatal error when attempting to send in the second argument. To fix the the confusing-api problem, we should deprecate the usage of the second ''$crypto_strong'' parameter and just make it always set the value to true. In PHP 8.0 we'd completely remove the second parameter and upgrade the function's ZPP to ''ZEND_PARSE_PARAMS_THROW'' causing the following fatal error when attempting to send in the second argument.
Line 78: Line 79:
 Requires a 2/3 majority Requires a 2/3 majority
  
-===== Patches and Tests ===== +Voting started **2018-11-02 @ 19:30 UTC** and will close sometime around **2018-11-16 @ 19:30 UTC**
-Links to any external patches and tests go here.+
  
-If there is no patch, make it clear who will create a patch, or whether a volunteer to help with implementation is needed.+==== Vote #1: Make openssl_random_pseudo_bytes() fail closed ====
  
-Make it clear if the patch is intended to be the final patch, or is just a prototype.+<doodle title="Make openssl_random_pseudo_bytes() fail closed" auth="sammyk" voteType="single" closed="true"> 
 +   * Yes 
 +   * No 
 +</doodle>
  
-For changes affecting the core language, you should also provide a patch for the language specification.+==== Vote #2: Deprecate the usage of the $crypto_strong parameter ==== 
 + 
 +<doodle title="Deprecate the usage of the $crypto_strong parameter" auth="sammyk" voteType="single" closed="true"> 
 +   * Yes 
 +   * No 
 +</doodle> 
 + 
 +===== Patches and Tests ===== 
 +  - [[https://github.com/php/php-src/compare/master...SammyK:rfc-improve-openssl-random-pseudo-bytes|The patch & tests]]
  
 ===== Implementation ===== ===== Implementation =====
Line 96: Line 107:
 ===== References ===== ===== References =====
   - [[https://externals.io/message/103331|Initial discussion]]   - [[https://externals.io/message/103331|Initial discussion]]
 +  - [[https://externals.io/message/103345|Under-discussion announcement]]
  
 ===== Rejected Features ===== ===== Rejected Features =====
-The original ping to @internals suggested aliasing ''openssl_random_pseudo_bytes()'' to ''random_bytes()'', but this was not received well so that idea got put in the bin.+  - The original ping to @internals suggested aliasing ''openssl_random_pseudo_bytes()'' to ''random_bytes()'', but this was not received well so that idea got put in the bin.
rfc/improve-openssl-random-pseudo-bytes.txt · Last modified: 2019/01/11 10:18 by nikic