diff mbox series

[Openvpn-devel] configure: disable engines if OPENSSL_NO_ENGINE is defined

Message ID 20230903142947.20906-1-orbea@riseup.net
State Changes Requested
Headers
Series [Openvpn-devel] configure: disable engines if OPENSSL_NO_ENGINE is defined |

Commit Message

orbea Sept. 3, 2023, 2:29 p.m. UTC 
  From: orbea <orbea@riseup.net>

Starting with LibreSSL 3.8.1 the engines have been removed which causes
the OpenVPN build to fail. This can be solved during configure by
checking if OPENSSL_NO_ENGINE is defined in opensslconf.h.
---
 configure.ac | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Antonio Quartulli Sept. 3, 2023, 2:47 p.m. UTC | #1 
Hi,

On 03/09/2023 16:29, orbea@riseup.net wrote:
> From: orbea <orbea@riseup.net>
> 
> Starting with LibreSSL 3.8.1 the engines have been removed which causes
> the OpenVPN build to fail. This can be solved during configure by
> checking if OPENSSL_NO_ENGINE is defined in opensslconf.h.
> ---
>   configure.ac | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 2f65cbd5..b5a835dc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -926,11 +926,12 @@ if test "${with_crypto_library}" = "openssl"; then
>   	    AC_COMPILE_IFELSE(
>   				    [AC_LANG_PROGRAM(
>   					    [[
> +	    #include <openssl/opensslconf.h>
>   	    #include <openssl/opensslv.h>
>   					    ]],
>   					    [[
>   	    /*	     Version encoding: MNNFFPPS - see opensslv.h for details */
> -	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
> +	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L || defined(OPENSSL_NO_ENGINE)
>   	    #error Engine supported disabled by default in OpenSSL 3.0+

Maybe the message should be changed now? Or we could have an entirely 
different message for this case?

Cheers,

>   	    #endif
>   					    ]]
orbea Sept. 3, 2023, 4:17 p.m. UTC | #2 
On Sun, 3 Sep 2023 16:47:31 +0200
Antonio Quartulli <a@unstable.cc> wrote:

> Hi,
> 
> On 03/09/2023 16:29, orbea@riseup.net wrote:
> > From: orbea <orbea@riseup.net>
> > 
> > Starting with LibreSSL 3.8.1 the engines have been removed which
> > causes the OpenVPN build to fail. This can be solved during
> > configure by checking if OPENSSL_NO_ENGINE is defined in
> > opensslconf.h. ---
> >   configure.ac | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 2f65cbd5..b5a835dc 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -926,11 +926,12 @@ if test "${with_crypto_library}" = "openssl";
> > then AC_COMPILE_IFELSE(
> >   				    [AC_LANG_PROGRAM(
> >   					    [[
> > +	    #include <openssl/opensslconf.h>
> >   	    #include <openssl/opensslv.h>
> >   					    ]],
> >   					    [[
> >   	    /*	     Version encoding: MNNFFPPS - see
> > opensslv.h for details */
> > -	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
> > +	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L ||
> > defined(OPENSSL_NO_ENGINE) #error Engine supported disabled by
> > default in OpenSSL 3.0+  
> 
> Maybe the message should be changed now? Or we could have an entirely 
> different message for this case?
> 
> Cheers,
> 
> >   	    #endif
> >   					    ]]  
> 

Do you think it might be preferable to only check OPENSSL_NO_ENGINE? I
see other code bases such as Tor only checking that define.
orbea Sept. 3, 2023, 4:55 p.m. UTC | #3 
On Sun, 3 Sep 2023 09:17:21 -0700
orbea <orbea@riseup.net> wrote:

> On Sun, 3 Sep 2023 16:47:31 +0200
> Antonio Quartulli <a@unstable.cc> wrote:
> 
> > Hi,
> > 
> > On 03/09/2023 16:29, orbea@riseup.net wrote:  
> > > From: orbea <orbea@riseup.net>
> > > 
> > > Starting with LibreSSL 3.8.1 the engines have been removed which
> > > causes the OpenVPN build to fail. This can be solved during
> > > configure by checking if OPENSSL_NO_ENGINE is defined in
> > > opensslconf.h. ---
> > >   configure.ac | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/configure.ac b/configure.ac
> > > index 2f65cbd5..b5a835dc 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -926,11 +926,12 @@ if test "${with_crypto_library}" =
> > > "openssl"; then AC_COMPILE_IFELSE(
> > >   				    [AC_LANG_PROGRAM(
> > >   					    [[
> > > +	    #include <openssl/opensslconf.h>
> > >   	    #include <openssl/opensslv.h>
> > >   					    ]],
> > >   					    [[
> > >   	    /*	     Version encoding: MNNFFPPS - see
> > > opensslv.h for details */
> > > -	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
> > > +	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L ||
> > > defined(OPENSSL_NO_ENGINE) #error Engine supported disabled by
> > > default in OpenSSL 3.0+    
> > 
> > Maybe the message should be changed now? Or we could have an
> > entirely different message for this case?
> > 
> > Cheers,
> >   
> > >   	    #endif
> > >   					    ]]    
> >   
> 
> Do you think it might be preferable to only check OPENSSL_NO_ENGINE? I
> see other code bases such as Tor only checking that define.
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Here is a patch that preserves the version check and adds a second
check for OPENSSL_NO_ENGINE which seems to also be useful for BoringSSL.

From d6700ec0f5af2522bb4eb136d3760f5b1445c9d1 Mon Sep 17 00:00:00 2001
From: orbea <orbea@riseup.net>
Date: Sat, 2 Sep 2023 23:06:22 -0700
Subject: [PATCH] configure: disable engines if OPENSSL_NO_ENGINE is defined

Starting with LibreSSL 3.8.1 the engines have been removed which causes
the OpenVPN build to fail. This can be solved during configure by
checking if OPENSSL_NO_ENGINE is defined in opensslconf.h.
---
 configure.ac | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 2f65cbd5..1adfb9d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -927,11 +927,17 @@ if test "${with_crypto_library}" = "openssl"; then
 				    [AC_LANG_PROGRAM(
 					    [[
 	    #include <openssl/opensslv.h>
+	    #include <openssl/opensslconf.h>
 					    ]],
 					    [[
 	    /*	     Version encoding: MNNFFPPS - see opensslv.h for details */
 	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
-	    #error Engine supported disabled by default in OpenSSL 3.0+
+	    #error Engine support disabled by default in OpenSSL 3.0+
+	    #endif
+
+	    /*	     BoringSSL and LibreSSL >= 3.8.1 removed engine support */
+	    #ifdef OPENSSL_NO_ENGINE
+	    #error Engine support disabled by default in openssl/opensslconf.h
 	    #endif
 					    ]]
 				    )],
Antonio Quartulli Sept. 3, 2023, 6:15 p.m. UTC | #4 
Hi,

On 03/09/2023 18:55, orbea wrote:
> Here is a patch that preserves the version check and adds a second
> check for OPENSSL_NO_ENGINE which seems to also be useful for BoringSSL.
> 

I prefer this version, but I'll let other chime in.
On top of that I think we may need a proper v2 PATCH having no extra 
body, as I am not sure your message can be properly parsed by git-am.

Cheers,

>  From d6700ec0f5af2522bb4eb136d3760f5b1445c9d1 Mon Sep 17 00:00:00 2001
> From: orbea <orbea@riseup.net>
> Date: Sat, 2 Sep 2023 23:06:22 -0700
> Subject: [PATCH] configure: disable engines if OPENSSL_NO_ENGINE is defined
> 
> Starting with LibreSSL 3.8.1 the engines have been removed which causes
> the OpenVPN build to fail. This can be solved during configure by
> checking if OPENSSL_NO_ENGINE is defined in opensslconf.h.
> ---
>   configure.ac | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 2f65cbd5..1adfb9d4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -927,11 +927,17 @@ if test "${with_crypto_library}" = "openssl"; then
>   				    [AC_LANG_PROGRAM(
>   					    [[
>   	    #include <openssl/opensslv.h>
> +	    #include <openssl/opensslconf.h>
>   					    ]],
>   					    [[
>   	    /*	     Version encoding: MNNFFPPS - see opensslv.h for details */
>   	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
> -	    #error Engine supported disabled by default in OpenSSL 3.0+
> +	    #error Engine support disabled by default in OpenSSL 3.0+
> +	    #endif
> +
> +	    /*	     BoringSSL and LibreSSL >= 3.8.1 removed engine support */
> +	    #ifdef OPENSSL_NO_ENGINE
> +	    #error Engine support disabled by default in openssl/opensslconf.h
>   	    #endif
>   					    ]]
>   				    )],
Gert Doering Sept. 3, 2023, 7:22 p.m. UTC | #5 
Hi,

On Sun, Sep 03, 2023 at 08:15:05PM +0200, Antonio Quartulli wrote:
> On 03/09/2023 18:55, orbea wrote:
> > Here is a patch that preserves the version check and adds a second
> > check for OPENSSL_NO_ENGINE which seems to also be useful for BoringSSL.
> 
> I prefer this version, but I'll let other chime in.
> On top of that I think we may need a proper v2 PATCH having no extra body,
> as I am not sure your message can be properly parsed by git-am.

It should be fine, and if not, I'll massage it to make it go in.

gert
orbea Sept. 3, 2023, 8:44 p.m. UTC | #6 
On Sun, 3 Sep 2023 21:22:15 +0200
Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
> 
> On Sun, Sep 03, 2023 at 08:15:05PM +0200, Antonio Quartulli wrote:
> > On 03/09/2023 18:55, orbea wrote:  
> > > Here is a patch that preserves the version check and adds a second
> > > check for OPENSSL_NO_ENGINE which seems to also be useful for
> > > BoringSSL.  
> > 
> > I prefer this version, but I'll let other chime in.
> > On top of that I think we may need a proper v2 PATCH having no
> > extra body, as I am not sure your message can be properly parsed by
> > git-am.  
> 
> It should be fine, and if not, I'll massage it to make it go in.
> 
> gert

Thank you, the patch was generated with 'git format-patch' so I would
think 'git am' can consume it, but please let me know if there is
anything else I can change.
Gert Doering Sept. 3, 2023, 8:46 p.m. UTC | #7 
Hi,

On Sun, Sep 03, 2023 at 01:44:13PM -0700, orbea wrote:
> Thank you, the patch was generated with 'git format-patch' so I would
> think 'git am' can consume it, but please let me know if there is
> anything else I can change.

For the sake of the archives (so, nothing to do for you now) - what
we usually do is, for a v2 of the patch, to tag it as such, and use
git-send-email to send it

  $ git-send-email -v2 --in-reply-to=$original-message-id -1

this will make it show up with a nice "[PATCH v2]" on the list, threaded
to the original v1.

gert
Gert Doering Sept. 8, 2023, 4:10 p.m. UTC | #8 
Hi,

so the v2 patch itself is good, but...

On Sun, Sep 03, 2023 at 09:55:45AM -0700, orbea wrote:
> From d6700ec0f5af2522bb4eb136d3760f5b1445c9d1 Mon Sep 17 00:00:00 2001
> From: orbea <orbea@riseup.net>

... it would be really preferred to have a real author name here, and
a "Signed-off-by:" line (git commit -s), and...

> Date: Sat, 2 Sep 2023 23:06:22 -0700
> Subject: [PATCH] configure: disable engines if OPENSSL_NO_ENGINE is defined
> 
> Starting with LibreSSL 3.8.1 the engines have been removed which causes
> the OpenVPN build to fail. This can be solved during configure by
> checking if OPENSSL_NO_ENGINE is defined in opensslconf.h.
> ---
[..]
> +	    /*	     BoringSSL and LibreSSL >= 3.8.1 removed engine support */
> +	    #ifdef OPENSSL_NO_ENGINE
> +	    #error Engine support disabled by default in openssl/opensslconf.h
>  	    #endif

... I think the "by default" needs to go from this #error - it's disabled,
period.  "disabled by default" is the OpenSSL 3.0 thing...

(The code change itself looks reasonable, and passes our GHA build
farm with various OSes and mbedTLS / OpenSSL 1.1 / OpenSSL 3.0)

gert
diff mbox series

Patch

diff --git a/configure.ac b/configure.ac
index 2f65cbd5..b5a835dc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -926,11 +926,12 @@  if test "${with_crypto_library}" = "openssl"; then
 	    AC_COMPILE_IFELSE(
 				    [AC_LANG_PROGRAM(
 					    [[
+	    #include <openssl/opensslconf.h>
 	    #include <openssl/opensslv.h>
 					    ]],
 					    [[
 	    /*	     Version encoding: MNNFFPPS - see opensslv.h for details */
-	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L
+	    #if OPENSSL_VERSION_NUMBER >= 0x30000000L || defined(OPENSSL_NO_ENGINE)
 	    #error Engine supported disabled by default in OpenSSL 3.0+
 	    #endif
 					    ]]