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

Unsupported OpenSSL library imported dynamically #5000

Closed
FedericoCeratto opened this issue Nov 6, 2016 · 12 comments
Closed

Unsupported OpenSSL library imported dynamically #5000

FedericoCeratto opened this issue Nov 6, 2016 · 12 comments
Labels

Comments

@FedericoCeratto
Copy link
Member

FedericoCeratto commented Nov 6, 2016

When an SSL-enable application is run on Linux it will fail to start with "could not import: SSL_library_init" if /usr/lib/x86_64-linux-gnu/libcrypto.so is present and links to /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1

If the symlink is removed the application will correctly pick up /usr/lib/x86_64-linux-gnu/libcrypto.so.1.0.2

The breaking change has been introduced in #3495 - the import order starts with the shared object file without version suffix instead of trying the versioned files before. The same patters is in iup.nim and sqlite3.nim

Related to nim-lang/nimble#257

@Araq
Copy link
Member

Araq commented Nov 7, 2016

Shouldn't the version without suffix link to some lib.so that actually works out of the box?

@cheatfate
Copy link
Member

@Araq,

OpenSSL currently has 5 branches:

  • Version 1.1.0 will be supported until 2018-08-31.
  • Version 1.0.2 will be supported until 2019-12-31 (LTS).
  • Support for version 1.0.1 will cease on 2016-12-31. No further releases of 1.0.1 will be made after that date. Security fixes only will be applied to 1.0.1 until then.
  • Version 1.0.0 is no longer supported.
  • Version 0.9.8 is no longer supported.

So its possible for Linux to have one or many of them. All releases has some differences in defaults and in supported options/algorithms.

@FedericoCeratto
Copy link
Member Author

FedericoCeratto commented Jan 9, 2017

This is due to the versions = "(|.10|.1.0.2|.1.0.1|.1.0.0|.0.9.9|.0.9.8)" line in openssl.nim
The leftmost item seem to pick up a soname without version numbers at first, ignoring more specific filenames.

Nimble from Nim 0.16.0 is currently picking up libcrypto.so and libssl.so and running OK even if they are linking to OpenSSL 1.1 but 1.1 is not listed in the supported versions.

(Related to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=846475)

@FedericoCeratto
Copy link
Member Author

FedericoCeratto commented Nov 25, 2017

Related to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=882549 as well

This impacts #782 #784 #6664

An example of how different versions are being handled in Python: https://github.com/python/cpython/blob/master/Modules/_ssl.c

@userifydev
Copy link

userifydev commented Dec 2, 2017

@FedericoCeratto

When an SSL-enable application is run on Linux it will fail to start with "could not import: SSL_library_init" if /usr/lib/x86_64-linux-gnu/libcrypto.so is present and links to /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1
If the symlink is removed the application will correctly pick up /usr/lib/x86_64-linux-gnu/libcrypto.so.1.0.2

Why does libcrypto.so.1.1, if the symlink exists and is pointing to the correct 1.1 file, not work?

Is there an incompatibility with 1.1, or are you're saying that if there's a broken/dangling symlink then it won't compile? Because if it's the latter, this seems to be a distribution packaging issue, not a nim problem.

@FedericoCeratto
Copy link
Member Author

@cthurbson the current import mechanism matches the .so file first and then a bunch of supported versions. Even if the import order is reversed it's still possible that an unsupported version is picked up if none of the versioned sonames is matched. Different versions of openssl have different behaviors even for the same functions. (and this could be true for any other library now or in future)
Perhaps the import system should expose the library version[s] at runtime to allow different code paths.

@FedericoCeratto
Copy link
Member Author

@jamiesonbecker Python is using a mixture of defines at build time: https://github.com/python/cpython/blob/master/Modules/_ssl.c#L1699

@userifydev
Copy link

Perhaps the import system should expose the library version[s] at runtime to allow different code paths.

@FedericoCeratto this seems like a better way (run-time version detection/adjustment) than the python method, since then it'll allow dynamic linking correctly across multiple platforms.

@barcharcraz
Copy link
Contributor

Windows libraries don't carry versions in most cases afaik. for cases where the library has a super unstable ABI a "when" based on some version function or symbol in the imported dso is probably the most stable way to go. ofc this would mean loading the dso at compile time, which is problematic in a whole lot of ways.

@userifydev
Copy link

userifydev commented Dec 6, 2017

Agreed; to your point, feature tests at runtime might be even better than version checks.

@FedericoCeratto
Copy link
Member Author

My proposal is to add an "OpenSSL older than 1.1.0" define and use SSL methods accordingly.
At runtime, detect exported symbols and refuse to run if the an old OpenSSL is found but the directive was not set.

@dom96
Copy link
Contributor

dom96 commented Dec 12, 2017

So if I understand correctly, you aim to support multiple versions. But only enable support for the older (presumably deprecated or soon-to-be deprecated) versions if the define is specified. Sounds good.

FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 13, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 17, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 17, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 25, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 26, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 26, 2017
Add a simple online test
FedericoCeratto pushed a commit to FedericoCeratto/Nim that referenced this issue Dec 28, 2017
Add a simple online test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants