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(dot)paquier(at)gmail(dot)com>
wrote:

> On Tue, Dec 6, 2016 at 11:42 PM, Asif Naeem <anaeem(dot)it(at)gmail(dot)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
>