| Message ID | 20200528225920.6983-3-James.Bottomley@HansenPartnership.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | add support for engine keys | expand |
Am 29.05.20 um 00:59 schrieb James Bottomley: > The test programme for the new openssl engine code requires overriding ^^^ Unrelated to this commit but I wondered about this spelling and it seems that the British programme spelling usually is a TV programme while program is used in computer context. AE just uses program for both. > the system default configuration file to point to the location of the > test engine. Add an initialization stanza that makes this behaviour > universal, so now anyone running openvpn configured with openssl can > specify their own configuration file with the OPENSSL_CONF environment > variable. Acked-By: Arne Schwabe <arne@rfc2549.org> I also tested that with a configuration file with an invalid statement this does not abort OpenVPN (the bug we had a few weeks ago) Arne
Your patch has been applied to the master branch.
commit a4071b20115c7d4e808df81169a986e65cec4efa
Author: James Bottomley
Date: Thu May 28 15:59:19 2020 -0700
crypto_openssl: add initialization to pick up local configuration
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20200528225920.6983-3-James.Bottomley@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19936.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
Hi, > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index 4ac77fde..fd57edd2 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -149,6 +149,11 @@ crypto_init_lib_engine(const char *engine_name) > void > crypto_init_lib(void) > { > +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) > + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); > +#else > + OPENSSL_config(NULL); > +#endif This is failing on some of the Travis builds, with the error message 1191 crypto_openssl.c: In function `crypto_init_lib': 1192 crypto_openssl.c:155:5: error: implicit declaration of function `OPENSSL_config'; did you mean `OPENSSL_init'? [-Werror=implicit-function-declaration] 1193 OPENSSL_config(NULL); 1194 ^~~~~~~~~~~~~~ https://travis-ci.org/github/OpenVPN/openvpn/builds/695633823 From what I can see, this is for the builds with "openssl 1.0.1u" and "openssl 1.0.2u" on Bionic. I'm not sure if we intend to support these versions, but this needs to be fixed - either the code needs to be adjusted or the travis build matrix (for master). gert
On Sun, 2020-06-07 at 13:11 +0200, Gert Doering wrote: > Hi, > > > diff --git a/src/openvpn/crypto_openssl.c > > b/src/openvpn/crypto_openssl.c > > index 4ac77fde..fd57edd2 100644 > > --- a/src/openvpn/crypto_openssl.c > > +++ b/src/openvpn/crypto_openssl.c > > @@ -149,6 +149,11 @@ crypto_init_lib_engine(const char > > *engine_name) > > void > > crypto_init_lib(void) > > { > > +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) > > + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); > > +#else > > + OPENSSL_config(NULL); > > +#endif > > This is failing on some of the Travis builds, with the error message > > 1191 crypto_openssl.c: In function `crypto_init_lib': > 1192 crypto_openssl.c:155:5: error: implicit declaration of function > `OPENSSL_config'; did you mean `OPENSSL_init'? [-Werror=implicit- > function-declaration] > 1193 OPENSSL_config(NULL); > 1194 ^~~~~~~~~~~~~~ > > https://travis-ci.org/github/OpenVPN/openvpn/builds/695633823 > > From what I can see, this is for the builds with "openssl 1.0.1u" and > "openssl 1.0.2u" on Bionic. > > > I'm not sure if we intend to support these versions, but this needs > to be fixed - either the code needs to be adjusted or the travis > build matrix (for master). Sorry about that. Best guess is it's missing an include for openssl/conf.h. You don't need that today because pretty much every other openssl header includes it, but that may not always have been so. Does the below patch fix it? If it does, it should probably be folded into the other patch. It should be safe because openssl/conf.h has existed for every version of openssl you support. James ---8>8>8><8<8<8--- From: James Bottomley <James.Bottomley@HansenPartnership.com> Subject: [PATCH] crypto_openssl: add include for openssl/conf.h Fix build failure on older versions of openssl. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- src/openvpn/crypto_openssl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index fd57edd2..94b6d85b 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -43,6 +43,7 @@ #include "crypto_backend.h" #include "openssl_compat.h" +#include <openssl/conf.h> #include <openssl/des.h> #include <openssl/err.h> #include <openssl/evp.h>
> > Sorry about that. Best guess is it's missing an include for > openssl/conf.h. You don't need that today because pretty much every > other openssl header includes it, but that may not always have been so. > > Does the below patch fix it? If it does, it should probably be folded > into the other patch. It should be safe because openssl/conf.h has > existed for every version of openssl you support. I am also puzzeld by this. On my local machine the OpenSSL 1.0.2 buildis fine without this extra include. But I am ACKing this alone on the basis that including the conf.h header is the corect thing to do (the man page says it should be included) It fixes the travis build error (https://travis-ci.org/github/schwabe/openvpn/builds/695947378) but I do not really understand why OpenSSL behaves different there. So Acked-By: Arne Schwabe <arne@rfc2549.org> > > James > > ---8>8>8><8<8<8--- > From: James Bottomley <James.Bottomley@HansenPartnership.com> > Subject: [PATCH] crypto_openssl: add include for openssl/conf.h > > Fix build failure on older versions of openssl. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > src/openvpn/crypto_openssl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > index fd57edd2..94b6d85b 100644 > --- a/src/openvpn/crypto_openssl.c > +++ b/src/openvpn/crypto_openssl.c > @@ -43,6 +43,7 @@ > #include "crypto_backend.h" > #include "openssl_compat.h" > > +#include <openssl/conf.h> > #include <openssl/des.h> > #include <openssl/err.h> > #include <openssl/evp.h> >
пн, 8 июн. 2020 г. в 15:06, Arne Schwabe <arne@rfc2549.org>: > > > > > Sorry about that. Best guess is it's missing an include for > > openssl/conf.h. You don't need that today because pretty much every > > other openssl header includes it, but that may not always have been so. > > > > Does the below patch fix it? If it does, it should probably be folded > > into the other patch. It should be safe because openssl/conf.h has > > existed for every version of openssl you support. > > > I am also puzzeld by this. On my local machine the OpenSSL 1.0.2 buildis > fine without this extra include. > > But I am ACKing this alone on the basis that including the conf.h header > is the corect thing to do (the man page says it should be included) > > It fixes the travis build error > (https://travis-ci.org/github/schwabe/openvpn/builds/695947378) but I do > not really understand why OpenSSL behaves different there. > it might happen if includes are mixed from OS openssl and custom openssl. I'll have a look (what is include path in travis). > > So > > Acked-By: Arne Schwabe <arne@rfc2549.org> > > > > > James > > > > ---8>8>8><8<8<8--- > > From: James Bottomley <James.Bottomley@HansenPartnership.com> > > Subject: [PATCH] crypto_openssl: add include for openssl/conf.h > > > > Fix build failure on older versions of openssl. > > > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > > --- > > src/openvpn/crypto_openssl.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c > > index fd57edd2..94b6d85b 100644 > > --- a/src/openvpn/crypto_openssl.c > > +++ b/src/openvpn/crypto_openssl.c > > @@ -43,6 +43,7 @@ > > #include "crypto_backend.h" > > #include "openssl_compat.h" > > > > +#include <openssl/conf.h> > > #include <openssl/des.h> > > #include <openssl/err.h> > > #include <openssl/evp.h> > > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">пн, 8 июн. 2020 г. в 15:06, Arne Schwabe <<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br> > <br> > Sorry about that. Best guess is it's missing an include for<br> > openssl/conf.h. You don't need that today because pretty much every<br> > other openssl header includes it, but that may not always have been so.<br> > <br> > Does the below patch fix it? If it does, it should probably be folded<br> > into the other patch. It should be safe because openssl/conf.h has<br> > existed for every version of openssl you support.<br> <br> <br> I am also puzzeld by this. On my local machine the OpenSSL 1.0.2 buildis<br> fine without this extra include.<br> <br> But I am ACKing this alone on the basis that including the conf.h header<br> is the corect thing to do (the man page says it should be included)<br> <br> It fixes the travis build error<br> (<a href="https://travis-ci.org/github/schwabe/openvpn/builds/695947378" rel="noreferrer" target="_blank">https://travis-ci.org/github/schwabe/openvpn/builds/695947378</a>) but I do<br> not really understand why OpenSSL behaves different there.<br></blockquote><div><br></div><div>it might happen if includes are mixed from OS openssl and custom openssl.</div><div>I'll have a look (what is include path in travis).<br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> So<br> <br> Acked-By: Arne Schwabe <<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>><br> <br> > <br> > James<br> > <br> > ---8>8>8><8<8<8---<br> > From: James Bottomley <James.Bottomley@HansenPartnership.com><br> > Subject: [PATCH] crypto_openssl: add include for openssl/conf.h<br> > <br> > Fix build failure on older versions of openssl.<br> > <br> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com><br> > ---<br> > src/openvpn/crypto_openssl.c | 1 +<br> > 1 file changed, 1 insertion(+)<br> > <br> > diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c<br> > index fd57edd2..94b6d85b 100644<br> > --- a/src/openvpn/crypto_openssl.c<br> > +++ b/src/openvpn/crypto_openssl.c<br> > @@ -43,6 +43,7 @@<br> > #include "crypto_backend.h"<br> > #include "openssl_compat.h"<br> > <br> > +#include <openssl/conf.h><br> > #include <openssl/des.h><br> > #include <openssl/err.h><br> > #include <openssl/evp.h><br> > <br> <br> <br> _______________________________________________<br> Openvpn-devel mailing list<br> <a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br> <a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br> </blockquote></div></div>
Your patch has been applied to the master branch.
commit 25266ebba97d7f4169f902b8b0d3c38eaa4c43a4
Author: James Bottomley
Date: Sun Jun 7 15:10:58 2020 -0700
crypto_openssl: add include for openssl/conf.h
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1591567858.4011.15.camel@HansenPartnership.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19996.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 4ac77fde..fd57edd2 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -149,6 +149,11 @@ crypto_init_lib_engine(const char *engine_name) void crypto_init_lib(void) { +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L) + OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CONFIG, NULL); +#else + OPENSSL_config(NULL); +#endif /* * If you build the OpenSSL library and OpenVPN with * CRYPTO_MDEBUG, you will get a listing of OpenSSL
The test programme for the new openssl engine code requires overriding the system default configuration file to point to the location of the test engine. Add an initialization stanza that makes this behaviour universal, so now anyone running openvpn configured with openssl can specify their own configuration file with the OPENSSL_CONF environment variable. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- src/openvpn/crypto_openssl.c | 5 +++++ 1 file changed, 5 insertions(+)