Fix build against LibreSSL by ismaell · Pull Request #434 · linux-nvme/libnvme

ismaell · 2022-07-18T16:29:49Z

Check the 3.x API for completeness, in case LibreSSL > 3.5.3 implements
the missing bits.

Signed-off-by: Ismael Luceno ismael@iodev.co.uk

igaw · 2022-07-19T08:46:18Z

Please don't add XXX to the comment? No idea what it suppose to mean.

So this is a bit backwards in my opinion. LibreSSL is trying to be like OpenSSL, hence I prefer your idea with

openssl_is_libressl = cc.has_header_symbol('openssl/opensslv.h', 'LIBRESSL_VERSION_NUMBER')

This explicitly tells us what is happening. We could rename CONFIG_OPENSSL_1 to something like CONFIG_OPENSSL_API_V1 to make it even more explicit.

igaw · 2022-07-19T09:19:01Z

I've played with this here but unfortunaly, this doesn't work as expected the 'has_header' seems to find my local installed OpenSSL header file /usr/include/openssl and hence the test will pass.

diff --git a/meson.build b/meson.build
index 472172d4de3f..5e8c7509a825 100644
--- a/meson.build
+++ b/meson.build
@@ -69,13 +69,25 @@ openssl_dep = dependency('openssl',
                          required: get_option('openssl'),
                          fallback : ['openssl', 'libssl_dep'])
 if openssl_dep.found()
+  api_v1 = openssl_dep.version().version_compare('<2.0.0') and
+           openssl_dep.version().version_compare('>=1.1.0')
+  api_v3 = openssl_dep.version().version_compare('>=3.0.0')
+
+  openssl_is_libressl = cc.has_header_symbol('openssl/opensslv.h',
+                                             'LIBRESSL_VERSION_NUMBER',
+                                             dependencies: openssl_dep)
+  if openssl_is_libressl and api_v3
+    if not cc.has_header('openssl/core_names.h', dependencies: openssl_dep)
+      # LibreSSL v3.x without the OpenSSL v3 APIs
+      api_v1 = true
+      api_v3 = false
+    endif
+  endif
+
   conf.set('CONFIG_OPENSSL', true, description: 'Is OpenSSL available?')
-  conf.set('CONFIG_OPENSSL_1',
-           openssl_dep.version().version_compare('<2.0.0') and
-           openssl_dep.version().version_compare('>=1.1.0'),
+  conf.set('CONFIG_OPENSSL_1', api_v1,
            description: 'OpenSSL version 1.x')
-  conf.set('CONFIG_OPENSSL_3',
-           openssl_dep.version().version_compare('>=3.0.0'),
+  conf.set('CONFIG_OPENSSL_3', api_v3,
            description: 'OpenSSL version 3.x')
 endif

Header <openssl/opensslv.h> has symbol "LIBRESSL_VERSION_NUMBER" with dependency openssl: YES 
Has header "openssl/core_names.h" with dependency openssl: YES

Command line:  cc -I/tmp/libressl/include /home/wagi/work/libnvme/.build/meson-private/tmp1e_w2i2l/testfile.c -E -P -D_FILE_OFFSET_BITS=64 -P -O0 -std=gnu99 

Code:
 
        #ifdef __has_include
         #if !__has_include("openssl/core_names.h")
          #error "Header 'openssl/core_names.h' could not be found"
         #endif
        #else
         #include <openssl/core_names.h>
        #endif
Compiler stdout:
 
Compiler stderr:
 
Has header "openssl/core_names.h" with dependency openssl: YES

igaw · 2022-07-19T09:25:30Z

Ah, with '-nostdinc' the test works:

  openssl_is_libressl = cc.has_header_symbol('openssl/opensslv.h',
                                             'LIBRESSL_VERSION_NUMBER',
                                             dependencies: openssl_dep,
                                             args: '-nostdinc')
  if openssl_is_libressl and api_v3
    if not cc.has_header('openssl/core_names.h',
                          dependencies: openssl_dep,
                          args: '-nostdinc')

igaw · 2022-07-19T09:29:52Z

for completes, this works:

diff --git a/meson.build b/meson.build
index 472172d4de3f..96c50379170f 100644
--- a/meson.build
+++ b/meson.build
@@ -69,13 +69,28 @@ openssl_dep = dependency('openssl',
                          required: get_option('openssl'),
                          fallback : ['openssl', 'libssl_dep'])
 if openssl_dep.found()
+  api_v1 = openssl_dep.version().version_compare('<2.0.0') and
+           openssl_dep.version().version_compare('>=1.1.0')
+  api_v3 = openssl_dep.version().version_compare('>=3.0.0')
+
+  openssl_is_libressl = cc.has_header_symbol('openssl/opensslv.h',
+                                             'LIBRESSL_VERSION_NUMBER',
+                                             dependencies: openssl_dep,
+                                             args: '-nostdinc')
+  if openssl_is_libressl and api_v3
+    if not cc.has_header('openssl/core_names.h',
+                          dependencies: openssl_dep,
+                          args: '-nostdinc')
+      # LibreSSL v3.x without the OpenSSL v3 APIs
+      api_v1 = true
+      api_v3 = false
+    endif
+  endif
+
   conf.set('CONFIG_OPENSSL', true, description: 'Is OpenSSL available?')
-  conf.set('CONFIG_OPENSSL_1',
-           openssl_dep.version().version_compare('<2.0.0') and
-           openssl_dep.version().version_compare('>=1.1.0'),
+  conf.set('CONFIG_OPENSSL_1', api_v1,
            description: 'OpenSSL version 1.x')
-  conf.set('CONFIG_OPENSSL_3',
-           openssl_dep.version().version_compare('>=3.0.0'),
+  conf.set('CONFIG_OPENSSL_3', api_v3,
            description: 'OpenSSL version 3.x')
 endif

ismaell · 2022-07-19T11:29:01Z

On 19/Jul/2022 01:47, Daniel Wagner wrote: <...>

Please don't add `XXX` to the comment? No idea what it suppose to mean.

Convention of BSD origin; I suspect the idea of using XXX (and certainly why I use it) is that it's likely to show only in comments, much like TODO and FIXME.

`openssl_is_libressl = cc.has_header_symbol('openssl/opensslv.h', 'LIBRESSL_VERSION_NUMBER')` This explicitly tells us what is happening. We could rename CONFIG_OPENSSL_1 to something like `CONFIG_OPENSSL_API_V1` to make it even more explicit.

The problem with this is that we can't predict which version of LibreSSL will add the missing API, by testing explicitly at least it becomes obvious when this changes...

ismaell · 2022-07-19T11:37:09Z

On 19/Jul/2022 02:30, Daniel Wagner wrote: for completes, this works:

<...> +1 but pkg-config seems to remove -I/usr/include, so -nostdinc alone doesn't work :(. I'll try to handle this tonight.

igaw · 2022-07-19T13:24:19Z

BTW, we could do something like this:

conf.set('CONFIG_OPENSSL_API_V@0@'.format(api_version), ....

(re XXX, sure, it's a todo but what was the todo here?)

eli-schwartz · 2022-07-19T20:26:28Z

What's the advantage of supporting libressl at all? The general trend in Linux distros has been to drop support for libressl as the system SSL library in any distro that used to use it.

e.g. https://lwn.net/Articles/841664/

I know of 3 distros that used to support it: Gentoo, Alpine, and Void. All switched back to openssl.

Python also dropped support for "an SSL module with some missing features and broken tests": https://peps.python.org/pep-0644/

igaw · 2022-07-20T12:19:54Z

Yes, I am fully aware of this argument. That's why I would like to see LibreSSL workarounds really clearly separated from OpenSSL code.

Anyway, what is the general mood on this topic?

I am sitting on the fence. If the support for LibreSSL is really just a couple meson.build changes I don't mind. Obviously, if we have to start changing source code I would object.

ismaell · 2022-07-20T15:46:59Z

On 19/Jul/2022 13:26, Eli Schwartz wrote: What's the advantage of supporting libressl at all? The general trend in Linux distros has been to drop support for libressl as the system SSL library in any distro that used to use it. e.g. https://lwn.net/Articles/841664/ I know of 3 distros that used to support it: Gentoo, Alpine, and Void. All switched back to openssl. Python also dropped support for "an SSL module with some missing features and broken tests": https://peps.python.org/pep-0644/

Source Mage GNU/Linux still supports LibreSSL, and will continue to support it in the foreseeable future; at this point mainly because of OpenSSL's CLA. Alpine has an open discussion on supporting LibreSSL again: https://gitlab.alpinelinux.org/alpine/tsc/-/issues/28

igaw · 2022-07-22T10:22:06Z

This drops the condition:

openssl_dep.version().version_compare('<2.0.0') and openssl_dep.version().version_compare('>=1.1.0'),

Especially the lower bound should stay.

Check the 3.x API for completeness, in case LibreSSL > 3.5.3 implements the missing bits. Closes: linux-nvme#430 Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

igaw · 2022-07-22T11:48:53Z

I am not really a big fan of your solution because it is difficult to follow. I am going to merge my version. Hope this okay.

ismaell · 2022-07-22T12:47:12Z

I am not really a big fan of your solution because it is difficult to follow. I am going to merge my version. Hope this okay.

See the comment on your PR; your solution is broken on libressl-native systems.

ismaell · 2022-07-22T12:48:00Z

Also, you removed the lower version bound... wasn't that the one you wanted to keep?

igaw · 2022-07-22T12:52:09Z

Also, you removed the lower version bound... wasn't that the one you wanted to keep?

I realized that we already have the lower bound tests a few lines above.

ismaell force-pushed the libressl branch from 42405e1 to 8e31cf3 Compare July 18, 2022 16:30

igaw requested changes Jul 19, 2022

View reviewed changes

ismaell force-pushed the libressl branch from 8e31cf3 to ca550da Compare July 21, 2022 19:31

igaw reviewed Jul 22, 2022

View reviewed changes

Fix build against LibreSSL

3759621

Check the 3.x API for completeness, in case LibreSSL > 3.5.3 implements the missing bits. Closes: linux-nvme#430 Signed-off-by: Ismael Luceno <ismael@iodev.co.uk>

ismaell force-pushed the libressl branch from ca550da to 3759621 Compare July 22, 2022 11:22

igaw mentioned this pull request Jul 22, 2022

build: Add support to build against LibreSSL #439

Merged

igaw closed this in #439 Jul 22, 2022

Conversation

ismaell commented Jul 18, 2022

Uh oh!

igaw Jul 19, 2022

Choose a reason for hiding this comment

Uh oh!

igaw commented Jul 19, 2022

Uh oh!

igaw commented Jul 19, 2022

Uh oh!

igaw commented Jul 19, 2022

Uh oh!

ismaell commented Jul 19, 2022 via email

Uh oh!

ismaell commented Jul 19, 2022 via email

Uh oh!

igaw commented Jul 19, 2022

Uh oh!

eli-schwartz commented Jul 19, 2022

Uh oh!

igaw commented Jul 20, 2022

Uh oh!

ismaell commented Jul 20, 2022 via email

Uh oh!

igaw Jul 22, 2022

Choose a reason for hiding this comment

Uh oh!

igaw commented Jul 22, 2022

Uh oh!

ismaell commented Jul 22, 2022

Uh oh!

ismaell commented Jul 22, 2022

Uh oh!

igaw commented Jul 22, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants