Skip to content

Compilation fails with OpenSSL 3.0 and OPENSSL_NO_DEPRECATED #223

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

Closed
1 task done
janblome opened this issue Apr 6, 2022 · 9 comments · Fixed by #228
Closed
1 task done

Compilation fails with OpenSSL 3.0 and OPENSSL_NO_DEPRECATED #223

janblome opened this issue Apr 6, 2022 · 9 comments · Fixed by #228
Assignees
Milestone

Comments

@janblome
Copy link
Contributor

janblome commented Apr 6, 2022

What happened?

Using OpenSSL 3.0.1 with OPENSSL_NO_DEPRECATED defined (as document here) results in compilation errors in jwt.h.

How To Reproduce?

#define OPENSSL_NO_DEPRECATED
#include "jwt-cpp/jwt.h"

int main() { return 0; }

Version

0.6.0

What OS are you seeing the problem on?

Windows

What compiler are you seeing the problem on?

MSVC

Relevant log output

jwt.h(1410): error C2065: 'RSA_PKCS1_PSS_PADDING': undeclared identifier
jwt.h(1410): error C3861: 'EVP_PKEY_CTX_set_rsa_padding': identifier not found
jwt.h(1417): error C3861: 'EVP_PKEY_CTX_set_rsa_pss_saltlen': identifier not found
jwt.h(1464): error C2065: 'RSA_PKCS1_PSS_PADDING': undeclared identifier
jwt.h(1464): error C3861: 'EVP_PKEY_CTX_set_rsa_padding': identifier not found
jwt.h(1471): error C3861: 'EVP_PKEY_CTX_set_rsa_pss_saltlen': identifier not found

Code of Conduct

  • I agree to follow this project's Code of Conduct
@janblome janblome added the bug label Apr 6, 2022
@janblome
Copy link
Contributor Author

janblome commented Apr 6, 2022

I think this is caused by a missing include. The man page for EVP_PKEY_CTX_set_rsa_padding places the function in <openssl/rsa.h>, which does not seem to be included in jwt.h. Explicitly including it before jwt.h fixes the issue for me:

#define OPENSSL_NO_DEPRECATED
#include <openssl/rsa.h>
#include "jwt-cpp/jwt.h"

int main() { return 0; }

@prince-chrismc
Copy link
Collaborator

prince-chrismc commented Apr 6, 2022

Looking at OpenSSL, at least one is not deprecated https://github.com/openssl/openssl/blob/1bf649b5998ac98511203f54ce954eccfaf75467/include/openssl/rsa.h#L119 so I am curious is the problem is platform specific....

Can you please share how you are building, installing and linking against OpenSSL. There's no standard way on windows AFAIK.

We test this configuration on Linux and it's passing https://github.com/Thalhammer/jwt-cpp/runs/5853386571?check_suite_focus=true

I am curious what the windows vs Linux is 🤔


run: cmake . -DJWT_BUILD_TESTS=ON -DOPENSSL_ROOT_DIR=/tmp -DCMAKE_CXX_FLAGS="-DOPENSSL_NO_DEPRECATED_3_0=1" -DCMAKE_C_FLAGS="-DOPENSSL_NO_DEPRECATED_3_0=1"

@janblome
Copy link
Contributor Author

janblome commented Apr 6, 2022

Thanks for the quick response! I'm using Conan to pull both OpenSSL and jwt-cpp itself from ConanCenter. My CMakeLists.txt looks like this:

cmake_minimum_required(VERSION 3.16)
project(jwt-cpp-demo)

include("conan.cmake")

conan_cmake_configure(
    REQUIRES jwt-cpp/0.6.0
    REQUIRES openssl/3.0.1
    GENERATORS cmake
)
conan_cmake_autodetect(
    CONAN_SETTINGS
)
conan_cmake_install(
    PATH_OR_REFERENCE .
    REMOTE conancenter
    SETTINGS ${CONAN_SETTINGS}
)

include("${CMAKE_BINARY_DIR}/conanbuildinfo.cmake")
conan_basic_setup(TARGETS)

add_executable(
    demo
    "source/main.cpp"
)
target_link_libraries(
    demo
    CONAN_PKG::jwt-cpp
    CONAN_PKG::openssl
)
target_compile_definitions(
    demo
    PRIVATE JWT_DISABLE_PICOJSON
    #PRIVATE OPENSSL_NO_DEPRECATED
)

The main.cpp reference there just contains the code I put in the "How To Reproduce" section.

Since this way of pulling dependencies is platform independent this should also work on Linux. I just tried it in a Ubuntu Docker container and I do get similar errors there (using gcc):

jwt.h: In member function 'std::string jwt::algorithm::pss::sign(const string&, std::error_code&) const':
jwt.h:1410:43: error: 'RSA_PKCS1_PSS_PADDING' was not declared in this scope
 1410 |     if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PSS_PADDING) <= 0) {
      |                                           ^~~~~~~~~~~~~~~~~~~~~
jwt.h:1410:9: error: 'EVP_PKEY_CTX_set_rsa_padding' was not declared in this scope; did you mean 'EVP_PKEY_CTX_set_app_data'?
 1410 |     if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PSS_PADDING) <= 0) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         EVP_PKEY_CTX_set_app_data
jwt.h:1417:9: error: 'EVP_PKEY_CTX_set_rsa_pss_saltlen' was not declared in this scope; did you mean 'EVP_PKEY_CTX_set_app_data'?
 1417 |     if (EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -1) <= 0) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         EVP_PKEY_CTX_set_app_data
jwt.h: In member function 'void jwt::algorithm::pss::verify(const string&, const string&, std::error_code&) const':
jwt.h:1464:43: error: 'RSA_PKCS1_PSS_PADDING' was not declared in this scope
 1464 |     if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PSS_PADDING) <= 0) {
      |                                           ^~~~~~~~~~~~~~~~~~~~~
jwt.h:1464:9: error: 'EVP_PKEY_CTX_set_rsa_padding' was not declared in this scope; did you mean 'EVP_PKEY_CTX_set_app_data'?
 1464 |     if (EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_PSS_PADDING) <= 0) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         EVP_PKEY_CTX_set_app_data
jwt.h:1471:9: error: 'EVP_PKEY_CTX_set_rsa_pss_saltlen' was not declared in this scope; did you mean 'EVP_PKEY_CTX_set_app_data'?
 1471 |     if (EVP_PKEY_CTX_set_rsa_pss_saltlen(ctx, -1) <= 0) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         EVP_PKEY_CTX_set_app_data

I didn't realize this project had a specific test for this, that's very cool! From the snippet you posted it looks like the test is setting OPENSSL_NO_DEPRECATED_3_0 instead of OPENSSL_NO_DEPRECATED. And indeed, when I set only OPENSSL_NO_DEPRECATED_3_0 then the compiler errors are gone. But from a quick search I can't find any documentation on this define, so it is not clear to me what the difference between the two is.

@prince-chrismc prince-chrismc self-assigned this Apr 6, 2022
@prince-chrismc prince-chrismc added this to the 0.6.1 milestone Apr 6, 2022
@prince-chrismc
Copy link
Collaborator

prince-chrismc commented Apr 6, 2022

Perfect amazing. I should be able to repro the problem.

Thanks for all the details ❤️

@Thalhammer
Copy link
Owner

Might be that we rely on transient includes from openssl which are no longer included if OPENSSL_NO_DEPRECATED is defined.
Seems like other projects had similar issues: chriskohlhoff/asio#126
According to https://github.com/openssl/openssl/blob/44fde441937fc8db8ea6a7ac2e7c683ad9d5f8e0/include/openssl/macros.h OPENSSL_NO_DEPRECATED causes all of the following to be defined:

OPENSSL_NO_DEPRECATED_3_0
OPENSSL_NO_DEPRECATED_1_1_1
OPENSSL_NO_DEPRECATED_1_1_0
OPENSSL_NO_DEPRECATED_1_0_2
OPENSSL_NO_DEPRECATED_1_0_1
OPENSSL_NO_DEPRECATED_1_0_0
OPENSSL_NO_DEPRECATED_0_9_8

which might help to narrow down the issue.

@janblome
Copy link
Contributor Author

I looked a bit through the OpenSSL headers and might have an idea of what is happening here. The missing functions the compiler complains about are defined in openssl/rsa.h, which is indirectly included by jwt.h like this:

  • jwt.h includes openssl/ssl.h
  • openssl/ssl.h includes openssl/x509.h, unless OPENSSL_NO_DEPRECATED_1_1_0 is defined
  • openssl/x509.h includes openssl/rsa.h, again unless OPENSSL_NO_DEPRECATED_1_1_0 is defined

As @Thalhammer pointed out setting OPENSSL_NO_DEPRECATED also sets OPENSSL_NO_DEPRECATED_1_1_0, so that would cause openssl/rsa.h to not be included. Which also explains why only setting OPENSSL_NO_DEPRECATED_3_0 doesn't cause the error I was seeing.

So my proposed fix for this would be to simply add an explicit include for openssl/rsa.h in jwt.h. I also checked OpenSSL 1.0.2 and the header already exists there, so don't think this would break compatibility with any supported versions. Optionally I would also suggest to change the test to use OPENSSL_NO_DEPRECATED, since that seems to be the more strict setting.

If you agree with this approach I could prepare a pull request with these changes 🙂.

@prince-chrismc
Copy link
Collaborator

Please feel free to try opening a PR! ❤️

I gave it a quick test but it seems there are more deprecated API calls https://github.com/prince-chrismc/jwt-cpp/runs/6382096564?check_suite_focus=true

https://www.openssl.org/docs/man3.0/man3/EC_KEY_new.html#HISTORY

EVP_EC_gen() was added in OpenSSL 3.0. All other functions described here were deprecated in OpenSSL 3.0. For replacement see EVP_PKEY-EC(7).

@janblome
Copy link
Contributor Author

janblome commented May 11, 2022

Thanks a lot! I took a look at the file with the deprecated function calls. If I understand that file correctly it re-defines some OpenSSL functions to be able to change their behavior for testing. But as far as I can tell the re-definitions of the deprecated functions aren't actually needed since the library itself doesn't use them anymore 😄. I also checked the variables used to change their behavior and they seem to also not be used anywhere. So I think the functions can simply be removed.

I tried that in a fork, and it looks like all the checks succeeded there: janblome#1. So I also create a PR here: #228.

@prince-chrismc
Copy link
Collaborator

But as far as I can tell the re-definitions of the deprecated functions aren't actually needed since the library itself doesn't use them anymore 😄

Thank you so much for looking deeper into that and getting a PR submitted!!!

I triggered the CI and it should be merged soon 🤞

@prince-chrismc prince-chrismc modified the milestones: 0.6.1, 0.7.0 Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants