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 ?.
On Mon, Dec 5, 2016 at 6:09 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > On Mon, Dec 5, 2016 at 5:11 PM, Heikki Linnakangas <hlinnaka@iki.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