Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The new findMbedTLS from NNG 1.7.2 fails #1771

Closed
shikokuchuo opened this issue Feb 13, 2024 · 24 comments
Closed

The new findMbedTLS from NNG 1.7.2 fails #1771

shikokuchuo opened this issue Feb 13, 2024 · 24 comments
Labels

Comments

@shikokuchuo
Copy link
Contributor

shikokuchuo commented Feb 13, 2024

Describe the bug
The improvements to cmake in NNG 1.7.2 prevent MbedTLS being found.

Actual Behavior
The final lines when running cmake do not appear for me on Ubuntu 22.04:

-- Looking for mbedtls_ssl_init
-- Looking for mbedtls_ssl_init - found
Mbed TLS version: 3.5.2
-- Found MbedTLS: MbedTLS::mbedtls  
-- Configuring done

Tested with:
(i) apt installed MbedTLS 2.28 in /usr and
(ii) MbedTLS 3.5.2 installed in /usr/local
Both are not picked up.

This causes the library to be built without TLS support.

To Reproduce
However it works again if I simplify this part in src/supplemental/tls/mbedtls/CMakeLists.txt:

    # If Mbed TLS was added by a consuming project, then we should use that
    # instance of it, instead of configuring our own.
    if (TARGET mbedtls)
        nng_link_libraries(mbedtls)
    else()
        # We want to prefer config mode over our local find package.
        # mbedTLS v3 has a config file, which should work better than
        # what we do here.  We do restore the setting though because
        # user applications might not expect this.
        if (NOT CMAKE_FIND_PAKCAGE_PREFER_CONFIG)
            set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE)
            find_package(MbedTLS REQUIRED)
            set(CMAKE_FIND_PACKAGE_PREFER_CONFIG FALSE)
        else()
            find_package(MbedTLS REQUIRED)
        endif()
        nng_link_libraries_public(MbedTLS::mbedtls MbedTLS::mbedcrypto MbedTLS::mbedx509)
    endif()

to this:

    # If Mbed TLS was added by a consuming project, then we should use that
    # instance of it, instead of configuring our own.
    if (TARGET mbedtls)
        nng_link_libraries(mbedtls)
    else()
        find_package(MbedTLS REQUIRED)
        nng_link_libraries_public(MbedTLS::mbedtls MbedTLS::mbedcrypto MbedTLS::mbedx509)
    endif()

I have no idea what CMAKE_FIND_PACKAGE_PREFER_CONFIG is meant for, but perhaps it can be reverted if not important.

Note that there is actually a typo in one of the places where it appears above, but that is not causing this bug.

** Environment Details **

  • NNG version 1.7.2
  • Ubuntu 22.04.3
  • gcc 11.4.0
  • Static lib
  • CMake 3.22.1
@shikokuchuo
Copy link
Contributor Author

My above workaround doesn't appear to fix it entirely.

I build static libs on a range of Linux, Mac OS and Windows platforms. I still get a failure on UCRT Windows (oddly not on older setups) where it starts:

-- The C compiler identification is GNU 12.3.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: D:/rtools43/x86_64-w64-mingw32.static.posix/bin/gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring for NNG version 1.7.2

but ends with:

-- Looking for mbedtls_ssl_init
-- Looking for mbedtls_ssl_init - not found

For my purposes I've reverted to the previous find mbedtls CMake scripts which still work fine.

@gdamore
Copy link
Contributor

gdamore commented Feb 18, 2024

I'm not sure about the static libraries. The situation here with CMake is actually completely awful, and probably you need to "help" it find your installation of Mbed TLS by supplying extra arguments to the CMake process.

This one can help:

MBEDTLS_ROOT_DIR

Also, which version of CMake are you using?

@gdamore
Copy link
Contributor

gdamore commented Feb 18, 2024

Ah, I see, CMake 3.22.

@gdamore
Copy link
Contributor

gdamore commented Feb 18, 2024

I just did a fresh install on a brand spanking new Ubuntu 22 container (ARM64 because that's what is easy on my mac):

  • apt install of git, gcc, g++, cmake, ninja-build, and libmbedtls-dev (all stock from Ubuntu 22)
  • git clone nng
  • mkdir nng/build; cd nng/build
  • cmake -G Ninja -DNNG_ENABLE_TLS=ON -DNNG_STATIC_LIB=ON ..

(I also tried dynamic lib without -DNNG_STATIC_LIB=ON, and that worked too.)

This all worked flawlessly.

How did you install Mbed TLS 3.5.2?

@gdamore
Copy link
Contributor

gdamore commented Feb 18, 2024

It would not surprise me if there is some confusion between your mbedTLS 3.5.2 installation and the mbed TLS 2.x default installation. These have different symbols (slightly) and if your headers don't match the libraries, that would cause problems.

@gdamore
Copy link
Contributor

gdamore commented Feb 18, 2024

The explanation of CMAKE_FIND_PACKAGE_PREFER_CONFIG is that we prefer to use the configuration mode rather than module mode.

That said, there is a typo here which means that we won't allow the user override here that we should have... but I don't think that's what is happening in your case.

@shikokuchuo
Copy link
Contributor Author

Thanks for the explanation. I need to find some time to pinpoint what's going wrong in my case.

@shikokuchuo
Copy link
Contributor Author

I've narrowed it down somewhat. After removing all traces of my mbedtls 3.5.2 install in /usr/local, NNG now finds the system mbedtls in /usr.

-- Looking for socketpair
-- Looking for socketpair - found
CMake Warning at src/supplemental/tls/mbedtls/CMakeLists.txt:11 (message):
  

          ************************************************************
          Linking against Mbed TLS may change license terms.
          Consult a lawyer and the license files for details.
          ************************************************************


-- Looking for mbedtls_ssl_init
-- Looking for mbedtls_ssl_init - found
Mbed TLS version: 2.28.0
-- Found MbedTLS: MbedTLS::mbedtls  
-- Found UnixCommands: /usr/bin/bash  
-- Looking for nl_langinfo
-- Looking for nl_langinfo - found
-- Test zt disabled (unconfigured)
-- The CXX compiler identification is GNU 11.4.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done

If I then make a clean install of mbedtls in /usr/local (using cmake, pretty much the same as NNG), NNG is no longer able to find it correctly. I get the following output and mbedtls is NOT linked:

-- Looking for socketpair
-- Looking for socketpair - found
CMake Warning at src/supplemental/tls/mbedtls/CMakeLists.txt:11 (message):
  

          ************************************************************
          Linking against Mbed TLS may change license terms.
          Consult a lawyer and the license files for details.
          ************************************************************


-- Found UnixCommands: /usr/bin/bash  
-- Looking for nl_langinfo
-- Looking for nl_langinfo - found
-- Test zt disabled (unconfigured)
-- The CXX compiler identification is GNU 11.4.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done

Using the old CMake script, this is not a problem.

I know there is MBEDTLS_ROOT_DIR and I actually set this to CMAKE_INSTALL_PREFIX when I build the static libs sequentially. But I do detect if existing libs are present, in which case the previous script worked previously and this one no longer seems to.

I haven't made more progress yet, hope to do so when I get the chance.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

I expect that the version of the library in /usr is being found before the one in /usr/local.

Are you building purely static?

@shikokuchuo
Copy link
Contributor Author

shikokuchuo commented Feb 25, 2024

Yes always static. I don't think it's dynamic linking taking precedence.

I tried building shared libs as well for mbedtls 3.5.2 in /usr/local, didn't make a difference.

I also removed the system libmbedtls in /usr and that didn't help either.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

Whoa. Removing the system libmbedtls didn't solve this?!

This is very surprising.

Maybe we need to change the environment we give it for finding libmbedtls. Let me double check.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

From that document, it looks like MBEDTLS_ROOT might be the variable you want to set.

@shikokuchuo
Copy link
Contributor Author

Ok, thanks. I'll take a look. Sorry I've not been approaching this very methodically thus far.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

This PR may help: #1785

But you'll want to use MBED_ROOT -- this just makes that work with either the new or old configuration. (And provides a fallback for the old.)

@shikokuchuo
Copy link
Contributor Author

Sorry didn't do anything for me. The docs mention that it should check the CMAKE_PREFIX_PATH automatically? That explicitly referred to common installation locations including /usr/local on Linux.

The strange thing is that the old Findmbedtls.cmake continues to work well for me. It finds MbedTLS 2.28, 3.52 etc. when it is already present in the standard filesystem locations, and also when I build as static libs sequentially in a temp install dir just for linking. I'm sure the new one solves something, but for me it is the opposite.

One other thing that came to me is that the following lines do not even appear:

-- Looking for mbedtls_ssl_init
-- Looking for mbedtls_ssl_init - found

I have seen a case on a Windows platform (I mentioned above) where it says 'not found', so does that give you a clue? I'm just not quite sure what's going on with cmake.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

Ugh. I really really hate CMake. Its better than autotools, but that's about it.

The old Find package works ok, but I've been told by others that it didn't work for them with Mbed TLS 3.x.

I actually tried to build the Mbed TLS 3.x packages from source, so I could reproduce your behavior, but didn't have luck yet. I may try again in a bit. Also, it should be MBEDTLS_ROOT .. please make sure you try that. (I got it wrong earlier.)

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

You can also look the CMake log files to see what it is testing and how - that might provide more clue.

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

So I just built it all on Ubuntu 20... with Mbed TLS 3.5.2.

Here's my shell history (in a brand spanking new docker container running ubuntu:jammy):

    1  which git
    2  apt install git
    3  apt update -y
    4  apt install -y git gcc g++ binutils
    5  apt install ninja-build cmake
    6  git clone https://github.com/Mbed-TLS/mbedtls
    7  git tag -l
    8  git tag
    9  cd mbedtls/
   10  git tag -l
   11  git checkout mbedtls-3.5.2
   12  mkdir build
   13  cd build
   14  cmake -G Ninja ..
   15  apt install python3
   16  cmake -G Ninja ..
   17  ccmake .
   18  apt install cmake-curses-gui
   19  ccmake .
   20  ninja
   21  ninja install
   22  cd .../
   23  cd ../..
   24  git clone https://github.com/nanomsg/nng
   25  cd nng/
   26  mkdir build
   27  cd build
   28  cmake -G Ninja -DNNG_ENABLE_TLS=ON ..
   29  ninja

Note that steps 17, 18, and 19 were me checking to see what the install prefix for Mbed TLS was -- it was already /usr/local, so I didn't change anything else. Obviously there were some mistakes there -- like the bad directory in step 22, and the attempts to run git tag (to check the version of mbed TLS to check out) in steps 7 & 8.

This built perfectly. It looks like it built statically too.

root@636ce4ea47ef:/nng/build# ldd ./tests/tls
	linux-vdso.so.1 (0x0000ffffa71f5000)
	libc.so.6 => /lib/aarch64-linux-gnu/libc.so.6 (0x0000ffffa6ea0000)
	/lib/ld-linux-aarch64.so.1 (0x0000ffffa71bc000)

And that TLS test runs fine.

At this point I think you should investigate your environment.

In your build directory, CMakeFiles/CMakeOutput.log may have useful content. Also CMakeFiles/CMakeError.log.

As I cannot seem to reproduce your test failure, I don't have any further guidance.

Here's a cleaned up version of the shell history refactored as a script, with unnecessary or erroneous commands removed - you should be able to run this in a fresh docker ubuntu:jammy container:

apt update -y
apt install -y git gcc g++ binutils cmake ninja-build python3
git clone https://github.com/Mbed-TLS/mbedtls
cd mbedtls/
git checkout mbedtls-3.5.2
mkdir build
cd build
cmake -G Ninja ..
ninja
ninja install
cd ../..
git clone https://github.com/nanomsg/nng
cd nng/
mkdir build
cd build
cmake -G Ninja -DNNG_ENABLE_TLS=ON ..
ninja

@gdamore
Copy link
Contributor

gdamore commented Feb 25, 2024

(Oh make sure you did actually install the Mbed TLS libraries and headers, not just build them..)

@shikokuchuo
Copy link
Contributor Author

shikokuchuo commented Feb 26, 2024

I finally understand what this Config mode for CMake find_package() is all about now.

It turns out for me Config mode just fails, when Mbed TLS 3.5.2 is installed in /usr/local (with the CMake files in /usr/local/lib/cmake). I don't know why - the only real difference I can see is that you use Ninja whilst I just use CMake. It could be something peculiar to my environment - I will let you know if I find it.

To be clear - it somehow fails in Config mode, doesn't fall back to Module mode and doesn't generate anything in the log file.

If I don't force set(CMAKE_FIND_PACKAGE_PREFER_CONFIG TRUE) in src/supplemental/tls/mbedtls/CMakeLists.txt then it works via Module mode i.e. the FindMbedTLS.cmake file provided by NNG works fine.

The src/supplemental/tls/mbedtls/CMakeLists.txt file, however currently doesn't respect CMAKE_FIND_PACKAGE_PREFER_CONFIG if I want to set this to FALSE myself. I think the logic should be that the default is set to TRUE as an option, so that I can opt to override this.

@gdamore
Copy link
Contributor

gdamore commented Feb 29, 2024

CMake doesn't replace Ninja or vice versa. We always use CMake -- the question is what generator you use. There are numerous options, but Ninja is the only sane one IMO. The others (msbuild or visual studio solution files for Windows, Makefiles for UNIX/Linux) are all much slower and inferior.

As far as my choices here; the intent was to respect the installed Mbed TLS configuration. Unfortunately, the default for FindPackage is the opposite of the default I want.

The code is not well orchestratred to allow for overriding it to false. Let me think on that.

@gdamore
Copy link
Contributor

gdamore commented Feb 29, 2024

Fix to allow you to override it however you like was easy. I've pushed it.

@gdamore
Copy link
Contributor

gdamore commented Feb 29, 2024

(You can now use -DCMAKE_FIND_PACKAGE_PREFER_CONFIG=ON (or OFF) as you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants