Re: pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX

From: Asif Naeem <anaeem(dot)it(at)gmail(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andreas Karlsson <andreas(at)proxel(dot)se>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pgcrypto compilation error due to stack-allocated EVP_CIPHER_CTX
Date: 2016-12-06 12:31:35
Message-ID: CAEB4t-M568v2TOv2YwoSLR1yuVBXpRdAcN+WiPY+VWiQMw=LzQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Michael,

Thank you for v2 patch, I would like to comment on it. It seems that you
have used function EVP_CIPHER_CTX_reset in the patch that was introduced in
OpenSSL 1.1.0, older library version might not work now, is it intentional
change ?.

Regards,
Muhammad Asif Naeem

On Tue, Dec 6, 2016 at 12:15 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Dec 5, 2016 at 6:09 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > On Mon, Dec 5, 2016 at 5:11 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
> wrote:
> >> I'm afraid if we just start using EVP_CIPHER_CTX_new(), we'll leak the
> >> context on any error. We had exactly the same problem with
> EVP_MD_CTX_init
> >> being removed, in the patch that added OpenSSL 1.1.0 support. We'll
> have to
> >> use a resource owner to track it, just like we did with EVP_MD_CTX in
> commit
> >> 593d4e47. Want to do that, or should I?
> >
> > I'll send a patch within 24 hours.
>
> And within the delay, attached is the promised patch.
>
> While testing with a linked list, I have found out that the linked
> list could leak with cases like that, where decryption and encryption
> are done in a single transaction;
> select pgp_sym_decrypt(pgp_sym_encrypt(repeat('x',65530),'1'),'1') =
> repeat('x',65530);
>
> What has taken me a bit of time was to figure out that this bit is
> needed to free correctly elements in the open list:
> @@ -219,6 +220,8 @@ encrypt_free(void *priv)
> {
> struct EncStat *st = priv;
>
> + if (st->ciph)
> + pgp_cfb_free(st->ciph);
> px_memset(st, 0, sizeof(*st));
> px_free(st);
> }
> This does not matter on back-branches as things get cleaned up once
> the transaction memory context gets freed.
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2016-12-06 12:50:30 Re: PSQL commands: \quit_if, \quit_unless
Previous Message Amit Kapila 2016-12-06 12:00:27 Re: Hash Indexes