Re: [openssl.org #1795] [ bug report + tentative patch ] SSL_CTX
On Fri, Nov 28, 2008 at 7:10 PM, Peter Edwards [EMAIL PROTECTED] wrote:
[...]
a) either forego on the side effect and resort to a linear (slow)
[...]
This isn't ideal, obviously
Yup. I know.
b) provide a threadsafe environment by surrounding a least each sort
[...]
You're walking a well worn path WRT race conditions here: if you allow
arbitrary calls to the STACK interface to add and remove items, then all
A.. the 'arbitrary' in there... n-o-o-o-o-ooo, didn't say
/that/! (more precisely: didn't mention it at all; another of those
rounds where I forget to list /all/ caveats.
So: 'yup!' again. I was well aware of that, but I didn't list that
particular caveat of STACK: using insert/delete on a STACK-based
object is definitely thread UNsafe. And verily. It was before and it
will be after the suggested fix to find(), no matter what one does.
Should've mentioned it, mea culpa, but my take here is that this
caveat has nothing to do with the STACK implementation per se; the
sort of screw-up you describe happens to anyone 'intelligent' enough
(koff koff) to mutate collections in a multithreading scenario where
such collections present pointers/references to the collected
instances to the outside world.
So, in short, I wasn't planning on fixing /that/, because nobody can. Or should.
What I /did/ plan though, is fix a bug particular to the current STACK
implementation and the _fixable_ thread-unsafety threat (with 't') it
contains.
Take this scenario, which _should_ be safe when regarded from a black
box perspective:
[single thread running:]
//
// INIT phase of code
//
new STACK();
STACK.push(obj1);
STACK.push(obj2);
// insert/delete/push/pop are okay here!
kickstart_multiple_threads();
[-- multiple threads running in parallel, all doing this:]
//--
// OPERATIONAL phase of the code
//--
...
const type *objref = STACK.find(key);
// insert/delete/push/pop are prohibited here!
//
// yes, that means STACK, and the objects it points at, are
// immutable once those multiple threads started.
// That's implicit to the reference-instead-of-copy STACK
// storage approach.
...
-
Look at the above: this can work, no locking required /anywhere/.
UNLESS you start to play smart and modify find() in such a way that,
upon hitting an UNsorted collection, it first sorts the collection,
then performs a binary search. (As OpenSSL sk_*() does). The sort() in
the find() violates the 'immutable' requirement, so you end up with a
headache.
:-)) You got me thinking again this morning with your remark (much
appreciated!) but after reconsidering my suggestion from yesterday
night, it still stands. Here's why:
1) I did /not/ try to fix 'arbitrary' so I'll consider that part
settled. See above.
2) Why would a mutating sort() work - including scenarios such as the
one you show?
Because
(a) the sort is now made to happen only once. Never again. No matter
how many times you call find() once you've entered the 'STACK is now
assumed immutable as we're in multithreaded operational run mode'.
(b) thanks to the (now atomic) {is_sorted() + sort()} collective,
EVERY thread trying to find() will run into the lock surrounding the
sort() and will *wait* there until one of them has _completed_ the
sort.
Which means that your scenario becomes (augmented a bit to show worst
case parallel calls and the free() in there is a mutating operation
which is prohibited anyway, so it can't be in there):
Note: thread1 is the thread that just happens (by random chance!) to
be the first ever to call find():
Thread 1 Thread 2
item = find() item= find()
{
lock() -- winner!
lock()-- WAIT
if (is_sorted()) -- FALSE
sort()
unlock()
@ lock granted
if (is_sorted()) -- TRUE
unlock()
{-- from here on out, the STACK is immutable,
so parallel internal_find() is okay from now --}
item = internal_find()
item = internal_find()
/* free(item) */
^^^ that's mutating STACK -- prohibited!
use(const item)
use(const item)
which is fine.
The whole point is that before my suggested fix, setting up a stack in
the init code phase will give you a short window for a fatal race
condition later on (!) when a programmer/user of sk_*() [implicitly]
calls sk_*_find() in (multithreaded) run mode.
Compare the above with the EXISTING state of the union, erm, stack.
(this is exactly the same scenario, where, just like in the above,
thread1, by random chance, is to be the first to call find() on the
(by now regarded as immutable) shared stack instance):
Thread 1 Thread 2
item = find() item= find()
{
// no lock
// no lock, so go on down
if
[openssl.org #1792] 0.9.8h failing to build with VS2006
[EMAIL PROTECTED] - Tue Nov 25 17:11:56 2008]: hi, i do these command using vs6++ and get an error. F:\Program Files\Microsoft Visual Studio\VC98\Bin\vcvars32.bat cd C:\devdiv\openssl-0.9.8i ms\do_ms nmake -f ms\ntdll.mak Did you do: perl Configure VC-WIN32 first? __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Draft FIPS Module v1.2 User Guide
Well, it's still not as finished as I'd like but since I'll be out of town and offline until next week I'm releasing the OpenSSL FIPS Object Module v1.2 User Guide document: http://www.openssl.org/docs/fips/UserGuide-1.2-RC1.pdf. It's still labeled as a draft as I anticipate revisions over the next few weeks. Feedback on errors/omissions/improvements will be greatly appreciated. Section 4.2.1 is incorrect, I believe. The security policy states to use ./config fipscanisterbuild but the user guide 1.2 rc states the old method of ./config fips ... Section 4.2.3 should probably also mention --with-fipslibdir so you can specify the location of the fipscanister ... Section 5.3.1, I'd probably mention that you can pass 'fipsld' as the CC env for configure scripts as well, since many projects use autoconf/automake. -Brad __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Draft FIPS Module v1.2 User Guide
I'm not sure that CC is the appropriate place for fipsld. Maybe LD, but CC has other uses. -Kyle H On Sat, Nov 29, 2008 at 5:41 PM, Brad House [EMAIL PROTECTED] wrote: Well, it's still not as finished as I'd like but since I'll be out of town and offline until next week I'm releasing the OpenSSL FIPS Object Module v1.2 User Guide document: http://www.openssl.org/docs/fips/UserGuide-1.2-RC1.pdf. It's still labeled as a draft as I anticipate revisions over the next few weeks. Feedback on errors/omissions/improvements will be greatly appreciated. Section 4.2.1 is incorrect, I believe. The security policy states to use ./config fipscanisterbuild but the user guide 1.2 rc states the old method of ./config fips ... Section 4.2.3 should probably also mention --with-fipslibdir so you can specify the location of the fipscanister ... Section 5.3.1, I'd probably mention that you can pass 'fipsld' as the CC env for configure scripts as well, since many projects use autoconf/automake. -Brad __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED] __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Draft FIPS Module v1.2 User Guide
Section 5.3.1, I'd probably mention that you can pass 'fipsld' as the CC env for configure scripts as well, since many projects use autoconf/automake. I'm not sure that CC is the appropriate place for fipsld. Maybe LD, but CC has other uses. Well, that's an arguable point (not that I'm looking for an argument/ flame war) considering most linking processes are processed through the compiler, which then calls ld, rather than calling ld directly. Also, the 'fipsld' script does infact compile fips_premain.c, so it's not simply linking, if you want to get technical about it. The 'fipsld' script is designed to pass-thru non-linking operations, and will only 'touch' a linking request if it is referencing libcrypto. It appears to have been designed as a CC wrapper, and the current UserGuide does explain what it does fairly well. I see no need in rewriting the section of the doc, or altering the way fipsld works, I'm only suggesting adding a small reference in the doc for projects which use autoconf/automake. -Brad __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: Draft FIPS Module v1.2 User Guide
Steve Marquess wrote: Well, it's still not as finished as I'd like but since I'll be out of town and offline until next week I'm releasing the OpenSSL FIPS Object Module v1.2 User Guide document: http://www.openssl.org/docs/fips/UserGuide-1.2-RC1.pdf. It's still labeled as a draft as I anticipate revisions over the next few weeks. Feedback on errors/omissions/improvements will be greatly appreciated. -Steve M. The Windows section looks like it needs a close review. The list of changes for this revision states that nasm is no longer supported and yet the instructions still refer to configuring and building with nasm. Jeffrey Altman smime.p7s Description: S/MIME Cryptographic Signature
Re: Draft FIPS Module v1.2 User Guide
Jeffrey Altman wrote: Steve Marquess wrote: Well, it's still not as finished as I'd like but since I'll be out of town and offline until next week I'm releasing the OpenSSL FIPS Object Module v1.2 User Guide document: http://www.openssl.org/docs/fips/UserGuide-1.2-RC1.pdf. It's still labeled as a draft as I anticipate revisions over the next few weeks. Feedback on errors/omissions/improvements will be greatly appreciated. -Steve M. The Windows section looks like it needs a close review. The list of changes for this revision states that nasm is no longer supported and yet the instructions still refer to configuring and building with nasm. Actually, I missed that one, yeah, MASM and NASM need to be reversed in section 2.3, since it is NASM that is used, and MASM that is not... Good catch Jeffrey. -Brad __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
