Ping?

You seem to have fixed it for nodejs, can we ask you to create a PR for OpenSSL proper as well?

I assume, btw, that this is an issue both in master and in the 1.1.0 branch, correct?

You seem to have fixed it for nodejs, can we ask you to create a PR for OpenSSL proper as well?

Emmmm yes, sir. But I don't understand, this is a PR, isn't it? Please correct me if there is anything wrong.

I assume, btw, that this is an issue both in master and in the 1.1.0 branch, correct?

Correct. So far there is nothing that checks OPENSSL_NO_STATIC_ENGINE in 1.1.0 stable branch.

D'oh, you're right, this is indeed a PR. I have no idea how I missed that earlier today. My apologies...

Uhmmmm, I gotta ask, what is it that would cause dladdr to call dlopen? After all, DSO_pathbyaddr ends up calling dladdr, at least on dlfcn systems.

what is it that would cause dladdr to call dlopen? After all, DSO_pathbyaddr ends up calling dladdr, at least on dlfcn systems.

Sorry, could you explain more about your question?

I'd like to make a short summary here, for your information:
A statically linked OpenSSL libcrypto:

1. OPENSSL_init_crypto called RUN_ONCE(&load_crypto_nodelete, ossl_init_load_crypto_nodelete),

2. ossl_init_load_crypto_nodelete called DSO_free(DSO_dsobyaddr(&base_inited, DSO_FLAG_NO_UNLOAD_ON_FREE));.
   where static int base_inited = 0; in "crypto/init.c". This address is linked into the body of the main program, when no-shared.

3. DSO_dsobyaddr called DSO_pathbyaddr(addr, ..., ...) to get the path of OpenSSL crypto library itself, using dladdr(). But when no-shared, it will get the path of the main program.

4. DSO_dsobyaddr later, called DSO_load(NULL, filename, NULL, flags), using dlopen() to load the main program.

Sorry, I mixed up DSO_dsobyaddr and DSO_pathbyaddr...

It seems a bit uncertain how this gets addressed... after all, we create a static library even when building the shared library (except on Windows 'cause reasons), and that might end up in the application as well and still cause your issue. On the other hand, it may also be linked into another shared library.

But awright, this is a specific use case and we can treat it as such.

Note that I'm approving with a bit of trepidation...

I'm unconvinced but not to the point of rejecting this. Not having static engines doesn't mean OpenSSL is statically linked, it just means that OpenSSL doesn't have any built in engines. It can be either statically or dynamically linked in such a case.

I'm having difficulties trying to work out how a library could detect if it is statically linked or dynamically loaded. The best that comes to mind is a comparison of returns from DSO_dsobyaddr(&main) and DSO_dsobyaddr(&OPENSSL_init_crypto) but this won't be universal.

How about this?

Let the build script

1. compiles and links .so without OPENSSL_STATIC_LIBCRYPTO, then,
2. compiles and links .a with OPENSSL_STATIC_LIBCRYPTO

trying to work out how a library could detect if it is statically linked or dynamically loaded.

Sure, detecting at runtime will be better.

How about this?

Let the build script

1. compiles and links .so without OPENSSL_STATIC_LIBCRYPTO, then,
2. compiles and links .a with OPENSSL_STATIC_LIBCRYPTO

Note that this won't cover when a static library is linked into another shared library. I assume that's not a common thing to do, but it's still perfectly possible, given a build with -fPIC or similar...

Anyway, this will require creating separate object files for shared library and for static library, which our build system currently doesn't support (it's in the planning for the next major release, but nothing really stops us from introducing it earlier... however, considering where we are in the release cycle for the moment, it's probably much too late to get this in for 1.1.1).

trying to work out how a library could detect if it is statically linked or dynamically loaded.

Sure, detecting at runtime will be better.

Can this be done? How?

The best that comes to mind is a comparison of returns from DSO_dsobyaddr(&main) and DSO_dsobyaddr(&OPENSSL_init_crypto) but this won't be universal.

I would suggest using DSO_pathbyaddr instead, since DSO_dsobyaddr might do a load, which isn't what we want in this case.

Sure, detecting at runtime will be better.
Can this be done? How?

No idea.

Note that this won't cover when a static library is linked into another shared library.

That's quite interesting. In fact, I'm wondering why libcrypto.so tries to prevent itself from unloading. I assume that is for improving performance.

Normally, a program "linked a shared library" loads the library by ld.so automatically and will never "dlclose" it. The underlying reference count should be always greater than one, so leaking another reference to prevent from unloading by "dlclose" is a no-op in fact.

[main] == DT_NEEDED => [libcrypto.so]
[main] == DT_NEEDED => [libfoo.so] == DT_NEEDED => [libcrypto.so]

A program "linked a static library" will never call the unload handler until the program itself is going to exit, so adding reference count is also a no-op, or in fact not permitted by system or letting PIE program to be reloaded.

[main & libcrypto.a]
[main & (libfoo.a & libcrypto.a)]

This is a chain, all libraries actually (recursively) "linked" with the main program always stay with the living process, regardless of "static" or "shared".

Programs or libraries which NOT link libcrypto.so but "dlopen" it during runtime, may "dlclose" it and "dlopen" it again soon. If we leak a reference, libcrypto.so will stay there. I think this might be (one of) the case(s) that it should leak a reference. Am I correct?

[any] -- dlopen() --> [libcrypto.so]  # init
      -- dlclose() -> [libcrypto.so]  # unload
      -- dlopen() --> [libcrypto.so]  # init
      -- dlclose() -> [libcrypto.so]  # unload

Now, consider a library was statically linked into another library, say libpassword-manager.so. Now the same pattern here: a program "dlopen" libpassword-manager.so and "dlclose" it soon. Shall we force libpassword-manager.so to leak one reference? What if the parts outside of libcrypto.a want to be unloaded (e.g. to synchronize files, to clean buffers, to disconnect sockets) as the "dlclose" of the main program implies?

[any] -- dlopen() --> [libpm.so & libcrypto.a]  # init libpm, init libcrypto
      -- dlclose() -> [libpm.so & libcrypto.a]  # unload libpm, unload libcrypto
      -- dlopen() --> [libpm.so & libcrypto.a]  # init libpm, init libcrypto
      -- dlclose() -> [libpm.so & libcrypto.a]  # unload libpm, unload libcrypto

So my conclusion is .a shouldn't leak any reference of itself, otherwise other parts than OpenSSL could be lingering in the memory and unload handlers will not be called as intended.

Let the build script
compiles and links .so without
OPENSSL_STATIC_LIBCRYPTO, then,
compiles and links .a with
OPENSSL_STATIC_LIBCRYPTO

Note that this won't cover when a static library is linked into another shared library.

If you agree with the comment above, that a static library shouldn't do the "nodelete" no matter it was linked into a program or another library, preventing the static library from doing this does simply cover all the situations in my opinion.

The no-delete thing was added to cover for another issue, our use of atexit, which executes at process takedown, not when libcrypto is unloaded. It's quite possible that there's a better mechanism that will actually run on libcrypto unload, but if there is such a thing, we need it to be portable.

On many Unix systems, there is a concept of a dtor (destructor) list which corresponds to the ctor (constructor) one but runs on library/program unload. I'd be surprised in Windows didn't have something similar.

On many Unix systems, there is a concept of a dtor (destructor) list which corresponds to the ctor (constructor) one but runs on library/program unload.

Sure... and they don't necessarily look the same. This is completely undefined by POSIX. So there is that portability bit...

I'd be surprised in Windows didn't have something similar.

One word: DllMain

It's quite possible that there's a better mechanism that will actually run on libcrypto unload, but if there is such a thing, we need it to be portable.

"The most portable" mechanism is DT_INIT and DT_FINI in standard ELF format. They are corresponding to _init(void) and _fini(void) in UNIX. They are exclusive thus there can only be one handler for init, one handler for fini.

Most of UNIX-like system also implemented DT_INIT_ARRAY and DT_FINI_ARRAY. GNU C library places a function in the array so it can provide atexit(void) and on_exit(arg). Some compilers have __attribute__((constructor)) and __attribute__((destructor)) and these function attributes add functions to that array.

An incomplete list of compilers support __attribute__(({con,de}structor)):

• GNU C compiler: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
• Clang compiler: llvm-mirror/clang@3068ae0f (snice 2008)
• Arm Compiler: http://www.keil.com/support/man/docs/armcc/armcc_chr1359124975087.htm
• Solaris C compiler: https://docs.oracle.com/cd/E24457_01/html/E21990/gjzke.html
• Intel C++ Compiler: https://software.intel.com/en-us/c-compilers claims the compilers have source compatibility with the GNU Compiler Collection (GCC) on Linux*, macOS*, and Android*.

IBM XL C/C++ for AIX does not support __attribute__(({con,de}structor)) but _init(void)/_fini(void).

As I described above, atexit(void) in GNU C library is placed at DT_FINI_ARRAY so it will also be called when the shared library is unloaded. But the original POSIX standard said it will only be call "at normal process termination". Such uncertainty could be hard to deal with.

If we don't use "no-delete" things, I think the most general method will be __attribute__(({con,de}structor)). I can also found some other uses in the source code of OpenSSL https://github.com/openssl/openssl/search?q=__attribute__+constructor

I know that ELF is quite popular these days, and that certainly simplifies things... but OpenSSL doesn't count on being built on ELF only.

I'll throw in a curve ball: ftp://ftp.mrynet.com/os/HP-MPE/docs.hp.com/en/11782/LinkerOnliinehelp0709/libraryroutines.htm

Albeit using a different method.

Exactly, hence portability problems that will need to be resolved should we decide to adopt this route. Personally, I would have much preferred to use the initializers and finalizers, but the choice that was made does reflect that we couldn't be sure we covered all the bases.

Emm... Using one method to try to workaround all the platforms (except Windows) is neat and does keep the code small.

However, will preprocessor branches work better for the portability issue? Branches for __attribute__((destructor)), for DllMain(), for #progma fini, and for other situations…?

Yeah, I can see us doing that. It will require some careful coding and should therefore not be rushed. I don't think it's likely to go into 1.1.1, but probably in a subsequent update, unless this is seen as a showstopper. @mattcaswell, what are your thoughts on the matter?

Yeah, I can see us doing that. It will require some careful coding and should therefore not be rushed. I don't think it's likely to go into 1.1.1, but probably in a subsequent update, unless this is seen as a showstopper. @mattcaswell, what are your thoughts on the matter?

Absolutely not to be rushed and not a showstopper for 1.1.1 IMO.

Could this be looked at again?

Could this be looked at again?

The OPENSSL_NO_PINSHARED was added, which does achieves that fix, but the build system does not seem to use that. I don't know if the user is expected to specify the define themselves or if it just was forgotten. However I guess it renders this PR outdated.

The OPENSSL_NO_PINSHARED was added, which does achieves that fix, but the build system does not seem to use that. I don't know if the user is expected to specify the define themselves or if it just was forgotten. However I guess it renders this PR outdated.

Specifying the "no-pinshared" option to Configure will cause that define to be specified.

Ok, I overlooked that. I looked for a more "automatic" way, like when building only static. But it is probably hard to detect for many cases, by "automatic" means.

This issue is in a state where it requires action by @openssl/committers but the last update was 363 days ago

Closing as this is already handled by the no-pinshared option.