httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Darryl Miles <darryl-mailingli...@netbauds.net>
Subject Re: Creating a thread safe module and the problem of calling of 'CRYPTO_set_locking_callback' twice!
Date Thu, 07 Dec 2006 18:41:12 GMT
Frank wrote:
> Joe Orton wrote:
>> On Wed, Dec 06, 2006 at 06:20:55PM +0000, Darryl Miles wrote:
>> [...]
>>> Is there an API to get the current value ?
>>
>>
>> Yes, CRYPTO_get_locking_callback/CRYPTO_get_id_callback.
>> [...]
> 
> I already know that this functions exists. But what if my module gets 
> inited before mod_ssl, which doesn't use the get-functions to determine 
> that something is already there? I was in the hope to see a clean 
> general purpose solution. :-)

Your thinking is correct there is a problem.  Those OpenSSL functions 
are not documented in my man page but exist in the library.  Yes there 
is a read-test-write race window by using those APIs alone.


> After this long and informative discussion I really think there is need 
> for a ssl_thread_init_if_not_already_done inside Apache. (Besides the 
> correction/removal of OpenSSL's stupid global locking mechanism.)

I disagree I should not need to compile apache mod_core in a way that 
has to understand the quirks of OpenSSL.  So adding a function 
ssl_thread_init_if_not_already_done() to apache core is too specific to 
that one problem.

I'm saying its possible to solve a bunch of problems like this in a more 
generic way.  This helps you and the next man.



> Maybe there is some (small) re-design of the Apache code needed?

Agreed, something needs to be added.  I'm saying there is no need to 
make it specific to OpenSSL.  Serializing the initialization can be made 
generic such that these objectives are met:

* When building Apache mod_core with minimal functionality it is not 
necessary to build it with special API function to handle OpenSSL.  So 
we get the co-ordination mechanism by default.

* I can build my mod_ssl or mod_frank at any point later without having 
to rebuild apache mod_core as well.

* Can provide a generic mechanism which can be used to solve the same 
problem where a common subordinate library is linked with by a module 
but is not require by mod_core.

* The overhead of adding the handful of new functions to apache mod_core 
should low.  Subjective but...

* Because the solution is generic it can be reused for other purposes 
where one module has to co-ordinate with another module at runtime over 
the use of a shared resource, but due to the modular nature of apache 
its impossible to tell if/when/what other modules need to co-ordinate 
their behavior until runtime.

* Only those modules that use OpenSSL directly need to be linked against 
OpenSSL.

* Only those modules that link with OpenSSL need openssl around at build 
time.  So the helper functions below would be repeated within each 
mod_xxxx that uses OpenSSL.

Those a rough concerns to address.



All Apache should provide is an accessible keyed namespace to attach 
arbitrary pieces of memory where the attachment functions provides some 
thread-safe guarantees.  Maybe there is already a mechanism to achieve 
most of that within apache.

All these functions must be thread-safe with respect to themselves.  The 
key itself could be a string or an integer.  Or even both if you wanted 
to abstract the key, like X11 Xwindows "atoms" work, you lookup/create 
your key independently and get back a reference which is usually a 
cardinal integer starting at 1 and then use this api with that (not the 
key directly).

* void *get(key) this gets the (void *) key value.

* void *attach(key, newdata) this overrides the current key value with 
an optional exchange (by returning the old value).

* void *attach_smart_exchange(key, newdata) this does an exchange but 
will only exchange a NULL for non-NULL and non-NULL for NULL.  This is 
used to set without overwriting.  It always returns the pre-existing 
value by checking that value in the context of how you called it you can 
work out if you were successful or not.


Sample hypothetical usage in self-documenting non compilable code:

struct openssl_init_struct {
	long magic;	/* do versioning here */
	int boolean_flag;
	mutex_t mutex;
} *ois;

ois = calloc_from_global_pool(1, sizeof(openssl_init_struct));
ois->magic = OPENSSL_INIT_STRUCT_V1;	/* magic constant */
ois->boolean_flag = FALSE;
mutex_init(&ois->mutex);
/* as first use we lock before attachment */
mutex_lock(&ois->mutex);

const char *label_key = "mylabel_openssl_initialization_serializer";	/* 
could be integer based key system */

/* This ap_newapi_attach_smart_exchange function is a thread-safe atomic 
attachment of "mydata" to the global namespace, it will only attach if 
the current value for the key does not exist or is NULL already, which 
are essentially treated identically (does not exist vs NULL value) */

void *retval = 
apache_core_handle->ap_newapi_attach_smart_exchange(label_key, ois);

if(retval == NULL) {
	/* we won the attachment, we already locked it and we own that lock, 
fall thru to code below */
} else {
	free_from_global_pool(ois);	/* FIXME: destruct mutex etc.. properly */

	/* we lost, go get the winner, you can optimize this a bit by trying 
this first before callocing our copy */
	ois  = apache_code_handle->ap_newapi_get(label_key);
	if(ois == NULL) {
		/* HUH! Abort! Abort!  Or loop around again a few times and ultimately 
catch fire. */
		exit(1);
	}
	/* ok aquire lock and fall thru */
	mutex_lock(&ois->mutex);
}

/* FIXME: do versioning ois->magic check here to ensure the object we 
got was what we can handle */
if(ois->boolean_flag != TRUE) {
	CRYPTO_do_stuff_to_init();	/* Do stuff here */

	ois->boolean_flag = TRUE;
}

mutex_unlock(&ois->mutex);
/* we're done */




Then each OpenSSL module user is modified to make used of a common 
shared initialization helper mechanism outlined above.  The above code 
should not be part of core but of the module that uses openssl.

All Apache needs to provide is the 3 primitive functions to attach, get, 
get_atomic_exchange and maybe some additional 'deatach' to complete things.


Darryl


Mime
View raw message