On 24 January 2016 at 20:34, Jakub Zelenka <bu...@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/
