Re: pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

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.

     /* 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;
> }


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


On Wed, Dec 7, 2016 at 2:23 AM, Michael Paquier <michael.paquier@gmail.com>
wrote:

> On Tue, Dec 6, 2016 at 11:42 PM, Asif Naeem <anaeem.it@gmail.com> wrote:
> > Thanks for updated patch. Although EVP_CIPHER_CTX_cleanup() seems
> deprecated
> > in OpenSSL >= 1.1.0 i.e.
> >
> >> # if OPENSSL_API_COMPAT < 0x10100000L
> >> #  define EVP_CIPHER_CTX_init(c)      EVP_CIPHER_CTX_reset(c)
> >> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
> >> # endif
> >
> >
> > I guess use of deprecated function is fine, until OpenSSL library support
> > it.
>
> We could use some ifdef block with the OpenSSL version number, but I
> am not sure if that's worth complicating the code at this stage.
> --
> Michael
>