Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:90888
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.215.54 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAEKnhAEhsCB26uW3Lg_-ckgMNTM5rZkxuHiuTepvCbv29UOtxw@mail.gmail.com>
References: <CAEKnhAHJW_v-yHx8ZJwu8O63Yfwc+9+bJnRoJwH7qXdzWE8T=w@mail.gmail.com>
 <CADyq6sKcmXSm9ouGS-K_+0hNm=vnUq4Fa_BpW=OHYZ1+b4V4uA@mail.gmail.com>
 <CAEKnhAFeMj0RgpjjZgOxzBwFRj0z6NbYPJL6WfR0EuT0daXCwA@mail.gmail.com>
 <CADyq6s+dL5R4K60yQwG_f_AsSUZ85nUGdeYfVgcMdZE=NW-qRA@mail.gmail.com> <CAEKnhAEhsCB26uW3Lg_-ckgMNTM5rZkxuHiuTepvCbv29UOtxw@mail.gmail.com>
Date: Sun, 24 Jan 2016 20:43:53 +0100
Message-ID: <CADyq6s+ozMYiniHq82D-ZAEZLAWmugwikipy0uQ7vzaEbHH4dA@mail.gmail.com>
To: Jakub Zelenka <bukka@php.net>
Cc: PHP internals list <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11412ca444341b052a19ada0
Subject: Re: [PHP-DEV] [RFC][VOTE] OpenSSL AEAD support
From: ocramius@gmail.com (Marco Pivetta)

--001a11412ca444341b052a19ada0
Content-Type: text/plain; charset=UTF-8

On 24 January 2016 at 20:34, Jakub Zelenka <bukka@php.net> wrote:

> On 24 Jan 2016 14:49, "Marco Pivetta"
> >
> > The BC breaks may happen in future, given the number of added
> parameters. You even designed your additional parameters to avoid BC
> breaks: what will happen next time someone needs additional functionality?
>
> I'm not aware about any new functionality that could be added here in the
> near future and even if there was any, new parameters can still be added.
> I'm pretty sure that once this happens we will have already named params
> support which should address all your concerns. I actualy think that with
> the named params this will be much better API change than all other
> propasals.
>

Exactly: like anyone else, you are not yet aware about it. What happens in
5 years?

> >>
> >> >  - more security issues if the defaults are unsafe (or become unsafe,
> for whatever reason)
> >>
> >> the current value (16) is effective maximum for the currently supported
> AEAD cipher algs and it cannot change.
> >
> > "current" was used two times in this sentence: that's exactly my fear
>
> As said later in my previous email, the dafault can be different in
> dependency on cipher alg. It means that you will have to have default for
> AES GCM always 16 (it cannot be higher due to block size) and for some
> FUTURE XYZ it would be 32. There will never be need to change it and no bc
> break can happen.
>
> >> and the same defaults. It would just be a new function that would work
> only for selected cipher algs. It wouldn't address any of your concerns and
> would just duplicate things.
> >
> > It would reduce the extent of BC breaks, as you won't need to notify
> everybody using `openssl_encrypt`, but just those that use
> `openssl_encrypt_aead`. Also, you'd be able to make the parameters
> mandatory, which makes the proposed signature addition [, string &$tag =
> NULL [, string $aad = "" [, int $tag_length = 16 ]]]] much less confusing.
> You may even just create a `new AEADFlags()` struct to make the hinting
> more clear and flexible, so that optional params aren't a problem anymore,
> but that would simply anger who still thinks procedural programming is a
> maintainable solution.
>
> I'm sorry but I don't think we should add any objects here. OpenSSL ext is
> purely procedural and objects wouldn't fit. It would also require some code
> reordering and clean up.
>
Doesn't really matter if you design it with an object or with optional
parameters, or even with an array of `$options`, as long as you can enforce
correct types passed in, and the order of the default parameter values
doesn't become a mess (as it currently is becoming).

> >>
> >> If you mean some fancy context related set of functions, then it would
> require some special handling and much more work. It would add to the
> maintenance effort as well. As I said before I have a crypto pecl extension
> that does exactly that. Personaly I think that such decision would probably
> mean that we won't have AEAD support in openssl ext for quite a long time
> as it won't be done anytime soon.
> >
> > No, I just really mean following:
> >
> > `openssl_encrypt_aead(string $data , string $method , string $password,
> AEADFlags $flags, [, int $options = 0 [, string $iv = "" ]] )`
> >
> > Much simpler:
> >
> >  * the AEAD flags are required, less error prone (you can't pass in
> wrong flags unless you subclassed)
> >  * the AEAD flags encapsulate the default values, which means that the
> BC breaks are easier to document, and easier to "fix" in userland (simply
> provide a custom AEADFlags instance, with the old params)
> >  * the API wasn't butchered just to keep BC with `openssl_encrypt`
>
> As I said above, this wouldn't be procedural. It would also require
> significantly more time and doesn't bring anything extra useful. Everithing
> that you really need in most cases is just a tag anyway. Another thing is
> also a maintenance point of view. If you look to the commits and handling
> PR's for this ext, then it's probably just me who actively maintains it
> which is quite sad because I have just a little bit of time (usually after
> work when I work... :) ) and I also have other exts to care about.
>
Again, the above was just a suggestion: feel free to do whatever you
prefer, but in a new endpoint.

> So the thing is that I don't really have time for such changes even if I
> thought that it is useful which I don't. And considering that no one else
> had time or knowledge to implement AEAD support to openssl ext in the last
> 10 years or so (since GCM is supported by OpenSSL), it might mean that it
> takes another 5 years waiting if this this proposal doesn't get in which
> wouldn't be great for users...
>
> >>
> >> That being said I know that the current proposal is not the nicest but
> I think it fits to the current openssl ext API which is not the nicest
> either.
> >
> > This is indeed an adapter: if OpenSSL sucks, then don't jump off the
> bridge as well ;-)
>
> I have never said that OpenSSL sucks. :) It's a great library written by
> one of the smartest people in the world. I meant that our openssl extension
> doesn't have the nicest API. However the API is more or less done in the
> same style which we shouldn't change IMHO.
>
I write loads of stuff that may or may not suck from a consumer
perspective. I am blaming the API, not the developer, let's not bring it to
a personal level.

We are designing (changing, eek!) an API here: the fact that it may or not
reflect how OpenSSL itself exposes it is totally irrelevant.

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

--001a11412ca444341b052a19ada0--