On Tue, Mar 5, 2019 at 12:51 PM Matt Caswell <matt@xxxxxxxxxxx> wrote:
>
> On 04/03/2019 23:37, Yann Ylavic wrote:
> > So my question is, why isn't no-pinshared the default?
> > ISTM that pinshared is enabled on linux only,
>
> That isn't correct. pinshared is the default everywhere. The way it is achieved
> is different for different platforms (so on Linux we use -znodelete).
Yes sorry, I meant -znodelete which, IMHO, is the worst option to keep
openssl loaded, at least double dlopen() can be worked around with
double dlclose(), while -znodelete is cast in stone at (distro-)build
time.
Also the double dlopen() could be opt-in/out at init time (some new
OPENSSL_INIT_[NO_]UNLOAD option), which looks far better for
usability, and with the right compatibility default could possibly
make it to 1.1 :p
>
> > and linux has
> > __cxa_atexit semantics for atexit() already, so dlclose() should
> > already call OPENSSL_cleanup() when needed.
> > Thus with OPENSSL_INIT_NO_ATEXIT now available the user could choose
> > at runtime whether (s)he wants to call OPENSSL_cleanup() explicitely
> > or let openssl clean up by itself.
>
> Actually if all platforms behaved like Linux then there would be no need for
> pinshared at all. Unfortunately they don't and on some platforms atexit handlers
> can get called even after they have been unloaded - which obviously leads to
> crashes.
Sure I understand that, but I think capable platforms shouldn't pay
the penalty, losing unload-ability looks like a serious 1.1 regression
to me.
This also begs the question of atexit() choice for cleanup, and why
there is a cleanup in the first place since it's called only when the
program exits (freeing memory is quite useless there, while
clean(s)ing it or the filesystem or whatever is definitely something
to be done on unload if that needs to happen).
Each system/compiler has probably its own way to run a callback on
dynamic unloading (__attribute__ destructor, #pragma fini, ...), so I
think it could/should be on a case by case basis. But since
OPENSSL_init_{crypto,ssl}/cleanup() is a nice step and now simple
solution for the user to control init/deinit, the poor man's choice of
"do it yourself" would still be better than "you can't do it", IMHO.
>
> Feasibly we could make no-pinshared the default on platforms where it isn't
> really needed (such as Linux). However:
>
> 1) This introduces a change of OpenSSL behaviour based on platform - which isn't
> ideal for application developers targeting multiple platforms. Not sure how big
> a deal this is.
Not sure either, though the current state is not ideal for developers
targeting other (not that insane) usages, which used to work
previously.
As an example, a server couldn't on restart, depending on its
configuration, switch to/from
INIT_{LOAD_CONFIG,ENGINE_*,ASYNC,ZLIB,...}.
>
> 2) The no-pinshared option does not appear in 1.1.1 or 1.1.1a. It first appears
> in 1.1.1b. Backporting the option was considered ok. But changing the default
> mid-series is probably not a good idea.
>
> Changing the default could be considered for 3.0.
Yes please, as it stands the 1.1 series is unloadable on the most used
openssl libraries, distros'. I find this a bit unfortunate, and more
#ifdef-ery to come (though I'd like the OPENSSL_INIT_[NO_]UNLOAD one
:) ).
Regards,
Yann.
