Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:103372 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 64005 invoked from network); 22 Oct 2018 23:11:31 -0000 Received: from unknown (HELO smtp115.ord1d.emailsrvr.com) (184.106.54.115) by pb1.pair.com with SMTP; 22 Oct 2018 23:11:31 -0000 Received: from smtp7.relay.ord1d.emailsrvr.com (localhost [127.0.0.1]) by smtp7.relay.ord1d.emailsrvr.com (SMTP Server) with ESMTP id B13E2C030E; Mon, 22 Oct 2018 15:26:13 -0400 (EDT) X-Auth-ID: fsb@thefsb.org Received: by smtp7.relay.ord1d.emailsrvr.com (Authenticated sender: fsb-AT-thefsb.org) with ESMTPSA id 628DAC033D; Mon, 22 Oct 2018 15:26:13 -0400 (EDT) X-Sender-Id: fsb@thefsb.org Received: from [10.2.0.1] (c-76-119-4-100.hsd1.ma.comcast.net [76.119.4.100]) (using TLSv1.2 with cipher AES256-GCM-SHA384) by 0.0.0.0:465 (trex/5.7.12); Mon, 22 Oct 2018 15:26:13 -0400 To: "Sammy Kaye Powers" Cc: "PHP Internals" Date: Mon, 22 Oct 2018 15:26:01 -0400 X-Mailer: MailMate (1.12r5523) Message-ID: In-Reply-To: References: <1D2D9809-6A2D-49E5-9F86-FBB52AF538B5@thefsb.org> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="=_MailMate_FC61BAE5-650E-4DE8-B548-E80523C106B6_=" Content-Transfer-Encoding: 8bit Subject: Re: [RFC] Improve openssl_random_pseudo_bytes() From: fsb@thefsb.org ("Tom Worster") --=_MailMate_FC61BAE5-650E-4DE8-B548-E80523C106B6_= Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Hi Sammy, On 22 Oct 2018, at 9:46, Sammy Kaye Powers wrote: > What makes the function obsolete? The addition of the `random_bytes()` Yes. > What makes the function obsolete? The addition of the `random_bytes()` > CSPRNG (which uses the kernel's CSPRNG) doesn't invalidate OpenSSL's > CSPRNG. According to one argument that has a lot of currency, it does. From the point of view of a consumer of unpredictable randoms comparing two APIs: a CSPRNG implemented in user-space memory vs. the system call to the kernel's non-blocking CSPRNG, the user-space CSPRNG 1) cannot do anything you need that the system call cannot, and 2) relies on the kernel for its entropy and periodic reseeding. Hence the user-space CSPRNG adds potential failure modes and adds to the attack surface and is therefore less trustworthy. The same thing stated from a different point of view: Nobody knows how to verify CSPRNG code so staking CSPRNGs is a bad idea. This argument has perhaps most famously been made by Thomas Ptacek https://sockpuppet.org/blog/2014/02/25/safely-generate-random-numbers/ The argument seems to win a lot including on this list where I've never seen it fail. Personally, I find the argument generally convincing and I wouldn't dare argue with Thomas Ptacek. While it's a broad argument with sweeping consequences, I think it is sound general advice for most consumers and probably all PHP coders. >> and A Bad Choice™ in all versions of PHP except when OS==Windows >> AND 5.4.0 <= PHP < 7.0. > > What makes it a Bad Choice™? :) Historically, it certainly has had > it's fair share of implementation disasters: > > 1. It implemented `RAND_pseudo_bytes()` (PRNG) instead of > `RAND_bytes()` (CSPRNG) all the way up until PHP 5.6.11: > https://github.com/php/php-src/blob/PHP-5.6.11/ext/openssl/openssl.c#L5408 > but that got fixed: https://bugs.php.net/bug.php?id=70014 > 2. `RAND_bytes()` is not fork safe by default causing major issues for > packages like ramsey/uuid: > https://benramsey.com/blog/2016/04/ramsey-uuid/#when-uuids-collide but > that got fixed in PHP 5.6.24: https://bugs.php.net/bug.php?id=71915 > 3. It fails open (this patch will fix that) > 4. The API is confusing with the second `$crypto_strong` parameter > which doesn't do anything (the other thing that this patch will fix) Such a list supports concern over potential failure modes and attack surface. It's good to fix bugs but it's even better to avoid them. The only thing you can loose by avoiding openssl_random_pseudo_bytes is hazard. Personal story: sometime early in the 7.0 epoc, I tried to argue that random_compat should not use openssl_random_pseudo_bytes except on Windows. I failed and, iirc, item 2 in your list was a consequence. >> The only reason to keep this function is BC > > I'm not sure of the reasons for why this would be the case. :) It's because anyone authoring new code should use the safer option. > `RAND_bytes()` is a proper CSPRNG and isn't set up to be used as a > deterministic PRNG. Yes. (Btw, "a proper CSPRNG" might be misinterpreted as a **bold** claim. Some CSPRNG implementations have a relatively good record (so far) but that's about as much as you can confidently say. I saw some interesting research on formal verification at 33c3 but I believe it's still true that code review is the state of the art. https://fahrplan.events.ccc.de/congress/2016/Fahrplan/events/8099.html https://formal.iti.kit.edu/klebanov/software/entroposcope/ from which I also learned to my shame that statistical tests are useless as they only test the output bit mixing function.) > My original ping to internals was to alias > `openssl_random_pseudo_bytes()` to `random_bytes()`, but as others > have pointed out, having more than one CSPRNG isn't necessarily a bad > thing to have. :) I missed those remarks. I think differently that it is a good thing if PHP offers only one CSRNG. > Thanks again for the feedback! Maybe the vote should be split up into > two parts: 1) Fail closed & 2) deprecate the second parameter. :) Idk. It's your RFC and I kinda hijacked the thread. Tom --=_MailMate_FC61BAE5-650E-4DE8-B548-E80523C106B6_=--