Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93719
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.218.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAKws9z114YcHkDaXO3o-E0JuHR10WsPzyedeF_8Q=shHDziwug@mail.gmail.com>
References: <CAKws9z114YcHkDaXO3o-E0JuHR10WsPzyedeF_8Q=shHDziwug@mail.gmail.com>
Date: Thu, 2 Jun 2016 21:30:11 +0700
Message-ID: <CAEZPtU4a8EpVhOmy5SSoP1mb_4PYjYuX=7G1d+12EC_YWxmqxA@mail.gmail.com>
To: Scott Arciszewski <scott@paragonie.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] [RFC] Libsodium - Discussion
From: pierre.php@gmail.com (Pierre Joye)

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

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

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.


- 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.

- 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



Cheers,
-- 
Pierre

@pierrejoye | http://www.libgd.org