The simplest fix involves setting the default only once, as wih the
callbacks, but here I feel that's a shaky idea, that I should allow RAND
method changes at any time, in a thread-safe manner -- more work for
me, but less surprising.
There is no 'safe' way to do this other than hardwired. Admitted, we have a
fairly ugly stack on which to find that out, multiple independently
developed lumps of code jammed into the same process, quite a few using
dlopen()/dlclose() on other libraries - multiples of them calling the
crypto. code.
All those lumps of code think they 'own' the crypto. stack - worst case
scenario was a dlopen()'d library setting the callbacks, then being
unloaded while other parts of the stack were still using crypto.
Surprisingly - that still worked on some OS's - but some (like AIX/HPUX)
ummap program text immediately on dlclose().
Personally, I'd suggest making it build option to turn off default locking
and use whatever the OS provides by default. That'll allow the few corner
cases to continue doing whatever wierd things they were doing before, but
remove the big risk factor for the vast majority of users. And it is
becoming a big risk factor now.
It certainly shouldn't be an issue for the OS installed OpenSSL which
probably covers most of your users, the only sane choice there is the OS
default locking scheme anyway.
Peter
From: Ben Laurie <b...@links.org>
To: openssl-dev@openssl.org,
Date: 23/10/2013 00:33
Subject: Re: Self-initialization of locking/threadid callbacks and
auto-detection of features
Sent by: owner-openssl-...@openssl.org
On 22 October 2013 06:47, Nico Williams <n...@cryptonector.com> wrote:
On Monday, October 21, 2013, Salz, Rich wrote:
I like your proposal, but I'd prefer to see an "already initialized"
error code returned. Or a flag to the (new?) init api that says "ignore
if already set"
Thanks for your reply!
I can add an error, but note that the caller can set then get the
callbacks and compare to check whether the caller's callbacks were taken.
I could also add a new set of callback setters with ignore-if-set flags.
As long as the existing ones behave reliably in the already-set case.
In the already-set case I think it may well be best to ignore without
failing on the theory that the caller that first set the callbacks must
have set sufficiently useful ones anyways... and that where the OS has a
good enough default threading library, that's the one that will be used
by all DSOs calling OpenSSL in the same process, as otherwise all hell
would already be breaking loose anyways! (I can imagine twisted cases
where this would not be true, but they seem exceedingly unlikely.)
If you want to see the half-baked bits I have (which build on Linux, but
which aren't tested) to see what I'm up to, see
https://github.com/nicowilliams/openssl, specifically the thread_safety
branch. See the XXX comments in rand_lib.c in particular. The outline:
add a thread-safe one-time initialization function, built on whatever the
OS provides, then use that to make callback init thread-safe.
What I need to know:
- should i add new targets to ./Configure? for now I modified the
linux-elf target, but this feels wrong to me.
- what about Windows? I either need to have different targets for
pre-vista/2008 or. i have to write a once initialization function for
older Windows (which I can and know how to do, it's just more work that,
and in particular i couldn't test it, so I'm not inclined to do it).
- if so, should ./config automatically pick the new targets where there
is appropriate threading support?
I've been musing about a more autoconf-like approach for some time now
(but, for the love of all that is fluffy, not using autoconf itself, which
sucks) - it seems this is a good reason to go down that path.
Interesting question is: what to do if no appropriate locking mechanism is
discovered?
- how to allocate error codes for "already initialized" errors that you
suggest?
- should I work to make sure that it's possible to change the default
RAND method after it's been set once?
The code in rand_lib.c is currently fundamentally thread-unsafe,
though it could be accidentally thread-safe if, e.g., ENGINE_finish()
doesn't actually tear down state at all. The simplest fix involves
setting the default only once, as wih the callbacks, but here I feel
that's a shaky idea, that I should allow RAND method changes at any time,
in a thread-safe manner -- more work for me, but less surprising.
Nico
--
(sent from a mobile device with lousy typing options, and no plain text
button)
(my patches need rebasing to squash and split up, need tests, need
finishing, but if you have comments I would love them sooner than
later! :)
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
