Re: [Openvpn-devel] [PATCHv2] openssl: Fix compilation without deprecated OpenSSL 1.1 APIs
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
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
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
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
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
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
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
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