search

Thalhammer / jwt-cpp

A header only library for creating and validating json web tokens in c++
https://thalhammer.github.io/jwt-cpp/
MIT License
917 stars 243 forks source link

Fix build and tests for wolfSSL #352

Closed julek-wolfssl closed 4 months ago

julek-wolfssl commented 5 months ago

Tested with wolfSSL master

cmake -B build -DJWT_SSL_LIBRARY:STRING=wolfSSL -DJWT_BUILD_TESTS=ON  .
make -j -C build
./build/tests/jwt-cpp-test
Thalhammer commented 5 months ago

Great to see someone from inside wolfSSL taking the time to improve support in opensource libraries.

I noticed the symbol names for wolfSSL are different from openssl (hence the new SYMBOL_NAME macro), which I assume are then converted to the OpenSSL ones using a define. The OpenSSL Error test works by providing a definition for the weakly defined symbols in openssl (which is also why it doesn't work with static linking). Given wolfssl has different symbol names, shouldn't the naming of the methods change as well, not just the methods they resolve to ? Keep in mind I have never worked with wolfSSL before, so I might be talking nonsense.

julek-wolfssl commented 5 months ago

Given wolfssl has different symbol names, shouldn't the naming of the methods change as well, not just the methods they resolve to ?

Our compatibility layer headers are a set of macros that change the OpenSSL names to wolfSSL compatibility equivalents. Like SSL_new -> wolfSSL_new (#define SSL_new wolfSSL_new inside wolfssl/openssl/ssl.h.

julek-wolfssl commented 5 months ago 
/home/runner/work/jwt-cpp/jwt-cpp/tests/OpenSSLErrorTest.cpp:340:1: error: ‘RSA’ does not name a type
  340 | RSA* EVP_PKEY_get1_RSA(EVP_PKEY* pkey) {
      | ^~~

This is failing in the OpenSSL without deprecated functions action. I think the solution would be for EVP_PKEY_get1_RSA to not be used when building against OpenSSL with deprecated functions removed. But I also don't think that this is the PR to do this in.

julek-wolfssl commented 5 months ago

I appreciate the review. I will try to get back to this PR next week.

prince-chrismc commented 4 months ago

🤞 this should be ready. Thanks so much for contributing to both side to make the integration better 🚀

I wont be updating the docs with the new tested version because I have a PR to update the the SSL versions which would conflict.

julek-wolfssl commented 4 months ago

Thanks for helping out here. I haven't had time to bring it across the finish line in the past month.