Re: [Openvpn-devel] [PATCH] mbedtls: fix segfault by calling mbedtls_cipher_free() in cipher_ctx_free()

2019-08-17 Thread Arne Schwabe
Am 17.08.2019 um 08:48 schrieb Gert Doering:
> Hi,
>
> On Fri, Aug 16, 2019 at 10:49:45PM +0200, Antonio Quartulli wrote:
>> Commit ("openssl: Fix compilation without deprecated OpenSSL 1.1 APIs")
>> has removed the cipher_ctx_cleanup() API, as it is not anymore required
>> to be a distinct call. However, while doing so it also touched the
>> mbedtls backend in a wrong way causing a systematic segfault upon connection.
>>
>> Basically mbedtls_cipher_free(ctx) was moved from the defunct 
>> cipher_ctx_cleanup()
>> to md_ctx_free(), while it was supposed to go into cipher_ctx_free().
>> This was clearly wrong as also the type of the ctx variable was not
>> correct anymore.
>>
>> Fix this mistake by actually moving mbedtls_cipher_free(ctx) to
>> cipher_ctx_free().
> This looks reasonable, the explanation makes sense and it pacifies
> buildbot (gave this a test run on all the bots, and we're back to
> "t_client test 1 fails on ubuntu+fedora29", and no more crashes).
>
> So I'm good with it, though I'd welcome a confirmation from Arne
> or Steffan.

Yeah, sorry that looks like a very stupid mistake from my side :(

Acked-By: Arne Schwabe 




___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH] mbedtls: fix segfault by calling mbedtls_cipher_free() in cipher_ctx_free()

2019-08-17 Thread Gert Doering
Hi,

On Fri, Aug 16, 2019 at 10:49:45PM +0200, Antonio Quartulli wrote:
> Commit ("openssl: Fix compilation without deprecated OpenSSL 1.1 APIs")
> has removed the cipher_ctx_cleanup() API, as it is not anymore required
> to be a distinct call. However, while doing so it also touched the
> mbedtls backend in a wrong way causing a systematic segfault upon connection.
> 
> Basically mbedtls_cipher_free(ctx) was moved from the defunct 
> cipher_ctx_cleanup()
> to md_ctx_free(), while it was supposed to go into cipher_ctx_free().
> This was clearly wrong as also the type of the ctx variable was not
> correct anymore.
> 
> Fix this mistake by actually moving mbedtls_cipher_free(ctx) to
> cipher_ctx_free().

This looks reasonable, the explanation makes sense and it pacifies
buildbot (gave this a test run on all the bots, and we're back to
"t_client test 1 fails on ubuntu+fedora29", and no more crashes).

So I'm good with it, though I'd welcome a confirmation from Arne
or Steffan.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel