Re: pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

On Fri, Dec 9, 2016 at 1:11 AM, Asif Naeem <anaeem.it@gmail.com> wrote:
> It make sense. I would like to share more comments as following i.e.
>
>> static int
>> bf_check_supported_key_len(void)
>> {
>> ...
>>      /* encrypt with 448bits key and verify output */
>>      evp_ctx = EVP_CIPHER_CTX_new();
>>      if (!evp_ctx)
>>           return 1;
>>      if (!EVP_EncryptInit_ex(evp_ctx, EVP_bf_ecb(), NULL, NULL, NULL))
>>           goto leave;
>>      if (!EVP_CIPHER_CTX_set_key_length(evp_ctx, 56))
>>           goto leave;
>>      if (!EVP_EncryptInit_ex(evp_ctx, NULL, NULL, key, NULL))
>>           goto leave;
>>      if (!EVP_EncryptUpdate(evp_ctx, out, &outlen, data, 8))
>>           goto leave;
>>      if (memcmp(out, res, 8) != 0)
>>           goto leave;                    /* Output does not match ->
>> strong cipher is
>>                                          * not supported */
>>      status = 1;
>> leave:
>>      EVP_CIPHER_CTX_free(evp_ctx);
>>      return status;
>> }
>
>
> It seems that it need to return 0 instead of 1 in case of failure i.e.

Yep that's wrong. Thanks for pointing that out.

>>      /* encrypt with 448bits key and verify output */
>>      evp_ctx = EVP_CIPHER_CTX_new();
>>      if (!evp_ctx)
>>           return 0;
>
> We can avoid multiple if conditions and goto statement something like i.e.
>
>>      if (EVP_EncryptInit_ex(evp_ctx, EVP_bf_ecb(), NULL, NULL, NULL) &&
>>          EVP_CIPHER_CTX_set_key_length(evp_ctx, 56) &&
>>          EVP_EncryptInit_ex(evp_ctx, NULL, NULL, key, NULL) &&
>>          EVP_EncryptUpdate(evp_ctx, out, &outlen, data, 8) &&
>>          memcmp(out, res, 8) == 0 )) /* Output does not match -> strong
>> cipher is not supported */
>>      status = 1;
>>      EVP_CIPHER_CTX_free(evp_ctx);
>>      return status;
>> }

I thought about doing that as well to be honest :) One way or the
other is fine, still I recall seeing more the style of the current
patch in PG code though, and that sticks better with current HEAD. But
my impressions may be wrong.

> What is your opinion ?. I am hopeful I will be able to share all my findings
> tomorrow. Thanks.

Thanks for looking at the patch. Looking forward to hearing more!
-- 
Michael