Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93803
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain paragonie.com designates 209.85.218.42 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAEZPtU4a8EpVhOmy5SSoP1mb_4PYjYuX=7G1d+12EC_YWxmqxA@mail.gmail.com>
References: <CAKws9z114YcHkDaXO3o-E0JuHR10WsPzyedeF_8Q=shHDziwug@mail.gmail.com>
	<CAEZPtU4a8EpVhOmy5SSoP1mb_4PYjYuX=7G1d+12EC_YWxmqxA@mail.gmail.com>
Date: Sun, 5 Jun 2016 04:31:10 -0400
Message-ID: <CAKws9z1J-BN4gzVizV8s2XqehPoAe6ZjTb8PE5e_-t1WSwS_tw@mail.gmail.com>
To: Pierre Joye <pierre.php@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Libsodium - Discussion
From: scott@paragonie.com (Scott Arciszewski)

Hi Pierre,

On Thu, Jun 2, 2016 at 10:30 AM, Pierre Joye <pierre.php@gmail.com> wrote:
> hi Scott,
>
> On Wed, Jun 1, 2016 at 2:49 PM, Scott Arciszewski <scott@paragonie.com> wrote:
>> Hi PHP Internals Team,
>>
>> Let's begin discussing the prospect of adding libsodium as a core extension
>> in PHP 7.1. I've updated the RFC to explain why this would be a good idea
>> and the benefits it offers.
>>
>> https://wiki.php.net/rfc/libsodium
>>
>> If the subsequent discussion goes smoothly, I would like to open voting on
>> June 15.
>>
>> Together, let's make PHP cryptography so safe that it becomes boring.
>
> Good work and very good choice for the backend library.
>
> I am overall in favor of having this extension in the core. However a
> couple of things are sub optimal or not ideal (in no special order):
>
>
> - \Sodium\library_version_major() \Sodium\library_version_minor() and
> \Sodium\version_string() should be constants

I agree.

> For  \Sodium\version_string(), the name is not consistent as it refers
> to the library version not the extension version ("Returns a string
> identifier of the current version of the sodium library installed.")
> (edit: used would better represent what is actually happening)
>
>
> - memzero, memcmp, hex2bin
>
> I am not totally convinced that memzero and maybe memcmp names are
> good nor they should be there. Both would be very useful as operator
> on variables. Given the simplicity of the implementations, it could be
> very useful in many other areas in case this ext is not installed

IMO: memzero is fine; memcmp isn't that great.

For what it's worth, we have hash_equals() already.

> For hex2bin, the optional parameter could be added to the existing
> functions. As this function does not require crypto safe
> implementation (and does not need from an implementation), we should
> have them as part of the engine instead.

We should seriously just make bin2hex, hex2bin, base64_encode, and
base64_decode constant-time now so we don't have to worry later when
some clever CS post-doc find a way to exfiltrate crypto keys through
cache misses. That calls for a separate RFC, but there's no salient
argument against this change.

> - buf and other abbreviations should be better. I think we had a
> discussion some time ago about how to provide interfaces for non C
> developers.

No argument there.

> - compare should be string_compare, or it could be confusing about
> what it can compare, especially in code review while checking crypt
> code, where many other types come into the game

This is an excellent point.

>
> Cheers,
> --
> Pierre
>
> @pierrejoye | http://www.libgd.org

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com>