On Wed, Jan 6, 2016 at 9:32 PM, Bishop Bettini <bis...@php.net> wrote:
> On Wed, Jan 6, 2016 at 2:59 PM, Jakub Zelenka <bu...@php.net> wrote: > >> On Wed, Jan 6, 2016 at 7:28 PM, Bishop Bettini <bis...@php.net> wrote: >> >>> On Wed, Jan 6, 2016 at 11:09 AM, Jakub Zelenka <bu...@php.net> wrote: >>> >>>> Hi, >>>> >>>> On Wed, Jan 6, 2016 at 3:35 PM, Bishop Bettini <bis...@php.net> wrote: >>>> >>>>> >>>>> I think the API might need to be more generic so that any future >>>>> cipher modes with different parameters could also be passed in. >>>>> >>>>> Please see note in >>>> https://wiki.php.net/rfc/openssl_aead#rejected_features . Any context >>>> related features will add a lot to the size of the implementation. In this >>>> case it would also mean introducing an object with dimension handler to the >>>> openssl ext which doesn't really match with the rest of the extension API. >>>> The proposed API is more conformant to the rest and the code addition is >>>> also limited which is very important from the maintenance point of view. >>>> >>> >>> Ok, a context resource may not be pragmatic. Perhaps a compromise in the >>> form of a thin wrapper: >>> >>> string openssl_encrypt_aead(string $data , string $method , string >>> $password [, int $options = 0 [, string $iv = "" [, string &$tag = "" [, >>> string $aad = "" [, int $tag_length = 16 ]]]]) >>> >>> string openssl_decrypt_aead(string $data , string $method , string >>> $password [, int $options = 0 [, string $iv = "" [, string $tag = "" [, >>> string $aad = "" ]]]] ) >>> >>> This actually feels more right anyway: openssl_encrypt only does >>> encryption, whereas openssl_encrypt_aead does encryption *and* >>> integrity. I would hate for users to pass a method of aes128 and think >>> they can forgo an HMAC because they thought PHP would give them back a >>> valid tag. >>> >> >> This is a good point. I would probably go with a bit different and maybe >> simpler solution. How about emitting notice when $tag param is supplied for >> non aead mode? >> > > I think the need for a message highlights an API design problem, and I'd > rather eliminate the need with a purpose-built API. > > I have been thinking about it and I don't think we need to add special functions that would work just for cipher algs ($method) with AEAD modes. The logic and other parameters are the same. The only difference is supplying the tag. We could compare it to the ciphers modes with IV that requires an $iv parameter. Then a notice is triggered if IV is not supplied as well as a notice is triggered if you supply IV for ECB mode. I know that this is not the same but the notices are already used in this way so I think that this is a more conforming way. This should be of course documented. There should be definitely a note that a tag will be provided only for GCM and CCM modes. It means only for following methods: aes-128-gcm aes-192-gcm aes-256-gcm aes-128-ccm aes-192-ccm aes-256-ccm and their upper case aliases of course... I will update the patch and RFC accordingly. Thanks for the feedback! Jakub
