Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-24 Thread Arne Schwabe
Am 24.07.19 um 12:28 schrieb Steffan Karger:
> Hi all,
> 
> On 14-06-19 12:38, Arne Schwabe wrote:
>>> +#if !defined(HAVE_EVP_CIPHER_CTX_INIT)
>>> +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset
>>> +#endif
>>> +
>>> +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP)
>>> +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset
>>> +#endif
>>
>> These two keep the older API instead of switching to the new one, from
>> OpenSSL.
>>
>> # if OPENSSL_API_COMPAT < 0x1010L
>> #  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
>> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
>> # endif
>>
>> Since just using only the new API in this case does not really work I
>> think in case it would be better to rather always use
>> EVP_CIPHER_CTX_reset isntead of init and  have ifdefs in the 2-3 places
>> where we actually use EVP_CIPHER_CTX_cleanup so we can remove the old
>> API when we bump our minimum OpenSSL version (and find this thing easy
>> since it is an ifdef depending on the openssl version).

Okay, I double checked this.


The new API drops EVP_CIPHER_CTX_cleanup, so we have do either ifdef the
EVP_CIPHER_CTX_cleanup calls or remove them completely.

Currently we only call the EVP_CIPHER_CTX_cleanup directly before
EVP_CIPHER_CTX_free.

Looking at the 1.0.2 openssl code:


void EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx)
{
   if (ctx) {
   EVP_CIPHER_CTX_cleanup(ctx);
  OPENSSL_free(ctx);
   }
}

it looks like the EVP_CIPHER_CTX_cleanup is implicit already, so we can
remove those call and drop the cipher_ctx_cleanup function and combine
cipher_ctx_cleanup and _free into one for mbed TLS.

Does that sound reasonble?

Arne


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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-24 Thread Steffan Karger
Hi all,

On 14-06-19 12:38, Arne Schwabe wrote:
>> +#if !defined(HAVE_EVP_CIPHER_CTX_INIT)
>> +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset
>> +#endif
>> +
>> +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP)
>> +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset
>> +#endif
> 
> These two keep the older API instead of switching to the new one, from
> OpenSSL.
> 
> # if OPENSSL_API_COMPAT < 0x1010L
> #  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
> # endif
> 
> Since just using only the new API in this case does not really work I
> think in case it would be better to rather always use
> EVP_CIPHER_CTX_reset isntead of init and  have ifdefs in the 2-3 places
> where we actually use EVP_CIPHER_CTX_cleanup so we can remove the old
> API when we bump our minimum OpenSSL version (and find this thing easy
> since it is an ifdef depending on the openssl version).

Why wouldn't using the new API work? _reset() is basically the new name
for _cleanup(), which some some actual cleanup + init.

As far as I can see it would work perfectly fine to use _reset() instead
of the _init/_cleanup calls everywhere. We never call _init on
uninitialized memory (which is the only case where _init() would work
while _cleanup() would fail).

All we'd have to do than is add something like

#ifndef HAVE_EVP_CIPHER_CTX_RESET
#define EVP_CIPHER_CTX_reset EVP_CIPHER_CTX_cleanup
#endif

to openssl_compat.h.

That way we would just use the new API everywhere, and can get rid of
the lines in the compat file once we drop support for OpenSSL 1.0.

Or am I missing something here?

-Steffan



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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-12 Thread Rosen Penev
On Fri, Jun 14, 2019 at 3:38 AM Arne Schwabe  wrote:
>
> Am 04.04.19 um 00:56 schrieb Rosen Penev:
> > EVP_CIPHER_CTX_init and _cleanup were deprecated in 1.1 and both were
> > replaced with _reset.
> >
> > Also removed initialization with OpenSSL 1.1 as it is no longer needed and
> > causes compilation errors when disabling deprecated APIs.
> >
> > Same with SSL_CTX_set_ecdh_auto as it got removed.
> >
>
> This gets kind of an ACK but needs some additional changes to be really
> good.
>
>
> >
> > +#if !defined(HAVE_EVP_CIPHER_CTX_INIT)
> > +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset
> > +#endif
> > +
> > +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP)
> > +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset
> > +#endif
>
> These two keep the older API instead of switching to the new one, from
> OpenSSL.
Yes I know. I feel that it's a cleaner solution as _init and _cleanup
both get defined to _reset.
>
> # if OPENSSL_API_COMPAT < 0x1010L
> #  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
> #  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
> # endif
>
> Since just using only the new API in this case does not really work I
> think in case it would be better to rather always use
> EVP_CIPHER_CTX_reset isntead of init and  have ifdefs in the 2-3 places
> where we actually use EVP_CIPHER_CTX_cleanup so we can remove the old
> API when we bump our minimum OpenSSL version (and find this thing easy
> since it is an ifdef depending on the openssl version).
OK. Will change.
>
> > +
> > +#if !defined(HAVE_X509_GET0_NOTBEFORE)
> > +#define X509_get0_notBefore X509_get_notBefore
> > +#endif
> > +
> > +#if !defined(HAVE_X509_GET0_NOTAFTER)
> > +#define X509_get0_notAfter X509_get_notAfter
> > +#endif
> > +
> >  #if !defined(HAVE_HMAC_CTX_RESET)
> >  /**
> >   * Reset a HMAC context
> > diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> > index 8bcebac4..e41cafa5 100644
> > --- a/src/openvpn/ssl_openssl.c
> > +++ b/src/openvpn/ssl_openssl.c
> > @@ -76,12 +76,13 @@ int mydata_index; /* GLOBAL */
> >  void
> >  tls_init_lib(void)
> >  {
> > +#if (OPENSSL_VERSION_NUMBER < 0x1010L && 
> > !defined(LIBRESSL_VERSION_NUMBER))
> >  SSL_library_init();
> > -#ifndef ENABLE_SMALL
> > +# ifndef ENABLE_SMALL
>
> The space between # and ifndef looks wrong.
Will eliminate. Not sure exactly why I added the space ATM.
>
>
> Arne
>


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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-12 Thread Gert Doering
Hi,

On Fri, Jul 12, 2019 at 06:44:06PM +0200, Arne Schwabe wrote:
> Looks "wrong" in the sense that it does not match the style we want to
> have in openvpn.

Indeed.  I have seen and used "#   cpp-thing" in other projects, but
OpenVPN seems to have decided to not use space-indenting for nested
#if-things.

So, we'll keep this (and try to get rid of #ifdefs anyway :-) ).

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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-12 Thread Arne Schwabe
Am 12.07.19 um 18:37 schrieb Matthias Andree:
> Am 14.06.19 um 12:38 schrieb Arne Schwabe:
>>
>>> -#ifndef ENABLE_SMALL
>>> +# ifndef ENABLE_SMALL
>> The space between # and ifndef looks wrong.
> 
> It's standard C. (Chapter 3.8 in the 1989/1990 standard, chapter 6.10 in
> recent editions, I checked 1999 and 2017/2018, although worded in a
> quite convoluted way) and I have yet to see a relevant C compiler that
> would reject this.

I know :).

Looks "wrong" in the sense that it does not match the style we want to
have in openvpn.


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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-07-12 Thread Matthias Andree
Am 14.06.19 um 12:38 schrieb Arne Schwabe:
>
>> -#ifndef ENABLE_SMALL
>> +# ifndef ENABLE_SMALL
> The space between # and ifndef looks wrong.

It's standard C. (Chapter 3.8 in the 1989/1990 standard, chapter 6.10 in
recent editions, I checked 1999 and 2017/2018, although worded in a
quite convoluted way) and I have yet to see a relevant C compiler that
would reject this.

The # is a token by itself can can be separated from the other
preprocessing tokens on the same line by space and horizontal-tab.




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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-06-14 Thread Arne Schwabe
Am 04.04.19 um 00:56 schrieb Rosen Penev:
> EVP_CIPHER_CTX_init and _cleanup were deprecated in 1.1 and both were
> replaced with _reset.
> 
> Also removed initialization with OpenSSL 1.1 as it is no longer needed and
> causes compilation errors when disabling deprecated APIs.
> 
> Same with SSL_CTX_set_ecdh_auto as it got removed.
> 

This gets kind of an ACK but needs some additional changes to be really
good.


>  
> +#if !defined(HAVE_EVP_CIPHER_CTX_INIT)
> +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset
> +#endif
> +
> +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP)
> +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset
> +#endif

These two keep the older API instead of switching to the new one, from
OpenSSL.

# if OPENSSL_API_COMPAT < 0x1010L
#  define EVP_CIPHER_CTX_init(c)  EVP_CIPHER_CTX_reset(c)
#  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
# endif

Since just using only the new API in this case does not really work I
think in case it would be better to rather always use
EVP_CIPHER_CTX_reset isntead of init and  have ifdefs in the 2-3 places
where we actually use EVP_CIPHER_CTX_cleanup so we can remove the old
API when we bump our minimum OpenSSL version (and find this thing easy
since it is an ifdef depending on the openssl version).

> +
> +#if !defined(HAVE_X509_GET0_NOTBEFORE)
> +#define X509_get0_notBefore X509_get_notBefore
> +#endif
> +
> +#if !defined(HAVE_X509_GET0_NOTAFTER)
> +#define X509_get0_notAfter X509_get_notAfter
> +#endif
> +
>  #if !defined(HAVE_HMAC_CTX_RESET)
>  /**
>   * Reset a HMAC context
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 8bcebac4..e41cafa5 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -76,12 +76,13 @@ int mydata_index; /* GLOBAL */
>  void
>  tls_init_lib(void)
>  {
> +#if (OPENSSL_VERSION_NUMBER < 0x1010L && 
> !defined(LIBRESSL_VERSION_NUMBER))
>  SSL_library_init();
> -#ifndef ENABLE_SMALL
> +# ifndef ENABLE_SMALL

The space between # and ifndef looks wrong.


Arne



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


Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs

2019-05-08 Thread Arne Schwabe
Am 04.04.19 um 00:56 schrieb Rosen Penev:
> EVP_CIPHER_CTX_init and _cleanup were deprecated in 1.1 and both were
> replaced with _reset.
> 
> Also removed initialization with OpenSSL 1.1 as it is no longer needed and
> causes compilation errors when disabling deprecated APIs.
> 
> Same with SSL_CTX_set_ecdh_auto as it got removed.

> +#if !defined(HAVE_EVP_CIPHER_CTX_INIT)
> +#define EVP_CIPHER_CTX_init EVP_CIPHER_CTX_reset
> +#endif
> +
> +#if !defined(HAVE_EVP_CIPHER_CTX_CLEANUP)
> +#define EVP_CIPHER_CTX_cleanup EVP_CIPHER_CTX_reset
> +#endif
> +

Generally we tried to avoid things like implementing the old API with
the new API but instead try to only use the current OpenSSL API and
implement it using the older API on older OpenSSL version. Is there way
for these functions to do that as well?

Arne



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