Support build against LibreSSL

[marcfir]

Some projects use LibreSSL instead of OpenSSL.
In umati/Dashboard-OPCUA-Client we use a patch for this, but direct support would simplify our build https://github.com/umati/Dashboard-OPCUA-Client#633.

This also solves #1426

[GoetzGoerisch]

@icraggs anything we can help to get this merged?

[icraggs]

My main question here is how does this get tested? What OSes would be supported? At the moment the CI tests are run for MacOS, Linux and Windows. Installation of LibreSSL in a Linux environment, at least on my usual Ubuntu OS, seems like it's not a readily installable package and it's not available by default in the CI environments I've looked at so far. I could merge it as unsupported but that would not be ideal.

[GoetzGoerisch]

Thanks for the explanation.
So adding a workflow which builds and tests the compile option would be sufficient?

[icraggs]

That would be a great help, yes. I also have to check the CMake changes against PR #1427 which I haven't merged yet, but intend to do so.

[marcfir]

@icraggs I added tests for Linux and MacOS. The tests on Windows go into a timeout CI LibreSSL. But tests with OpenSSL do the same CI OpenSSL.

[GoetzGoerisch]

@icraggs anything missing to get this merged?

[icraggs]

@GoetzGoerisch I've been in North America for a week for the eclipse. I'm back now, and just been merging Frank's PR for CMake into the 1.4 branch. This PR is next on my list.

[icraggs]

3 questions as a result of merging

[icraggs]

In merging this PR, I don't understand this line. Are we zeroing out the LIBRESSL root_dir, in which case the first parameter should be LIBRESSL_ROOT_DIR?

[icraggs]

Same question as above but in reverse

[marcfir]

Yes, these lines are mixed up

[icraggs]

This doesn't seem right for OPENSSL, is it?

[marcfir]

Yes. I add a fix for that.

[icraggs]

Ok, I merged the PR yesterday, with a couple of changes of my own to answer my questions, into the develop and 1.4 branches. You can check my changes there.

If you propose any further changes they should be targeted at the develop branch, not master, as the CMake build files have changed due to another PR I've merged.

[icraggs]

I'm going to close this PR now, as it's in the develop/1.4 branches. If you have any further comments you can make them here or in another PR/issue. Thanks for your contributions.

[fpagliughi]

Apologies, I hadn't seen this PR before it was merged. I just wanted to ask a few questions before this is released. @marcfir

I started writing this message with a number of questions about the use of cache variables where regular variables would have been adequate (preferable?) and why the xxxSSL_ROOT_DIR variables were being cleared each run, as if it were expected that the find_package() call would be setting this...

But then I realized this behavior was already in the CMake files from before the PR!

IF (PAHO_WITH_SSL)
    SET(OPENSSL_ROOT_DIR "" CACHE PATH "Directory containing OpenSSL libraries and includes")
    find_package(OpenSSL REQUIRED)

So I guess the style was just copy-pasted for this PR?

This (the original usage) doesn't seem correct to me. I don't think the CMake file should be randomly clearing a cache variable. The expectation (the purpose of the cache) is to retain variables between runs. Also, the proper usage for OPENSSL/LIBRESSL_ROOT_DIR is for the user to pass in a hint (as an environment variable) about where the library can be found.

Normally this isn't done and the variable is blank, which also makes the current configuration output look broken:

-- Found OpenSSL: /usr/lib/x86_64-linux-gnu/libcrypto.so (found version "3.0.2")  
Use OpenSSL at                                       <--- *** At nowhere? Looks like a bug ***
SSL_INCLUDE_DIR: /usr/include  

Then, when dumping the library information, it would be good to show the include directory and the main SSL library file (with path). That would have helped me recently on Windows where it was finding the headers from and older version (1.1) and then linking to a DLL with a different version (3.x), resulting in missing symbols in the link.

I suggest changing the src/CMakeLists.txt to this:

if(PAHO_WITH_SSL OR PAHO_WITH_LIBRESSL)
  if(PAHO_WITH_LIBRESSL)
    find_package(LibreSSL REQUIRED)
    set(SSL_LIBRARY_NAME LibreSSL)
    message(STATUS "Using LibreSSL with headers at ${LIBRESSL_INCLUDE_DIR}, lib ${LIBRESSL_SSL_LIBRARY}")
  else()
    find_package(OpenSSL REQUIRED)
    set(SSL_LIBRARY_NAME OpenSSL)
    message(STATUS "Using OpenSSL with headers at ${OPENSSL_INCLUDE_DIR}, lib ${OPENSSL_SSL_LIBRARY}")
  endif()
  ...

I can send a PR.

[marcfir]

Hi @fpagliughi,
Yes, I just copied the style of including OpenSSL as is. Your suggestion and PR looks very good.