diff --git a/CHANGES b/CHANGES index d19870387685a9757a4838b4c0581fbef4f4e7ea..5c31f057701600a9a600d25ad5a832e47a0b840b 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,23 @@ Changes between 0.9.6 and 0.9.7 [xx XXX 2000] + *) Add functionality to apps/openssl.c for detecting locking + problems: As the program is single-threaded, all we have + to do is register a locking callback using an array for + storing which locks are currently held by the program. + + Fix a deadlock in CRYPTO_mem_leaks() that was detected in + apps/openssl.c. + [Bodo Moeller] + + *) Use a lock around the call to CRYPTO_get_ex_new_index() in + SSL_get_ex_data_X509_STORE_idx(), which is used in + ssl_verify_cert_chain() and thus can be called at any time + during TLS/SSL handshakes so that thread-safety is essential. + Unfortunately, the ex_data design is not at all suited + for multi-threaded use, so it probably should be abolished. + [Bodo Moeller] + *) Added Broadcom "ubsec" ENGINE to OpenSSL. [Broadcom, tweaked and integrated by Geoff Thorpe] diff --git a/apps/openssl.c b/apps/openssl.c index 4ec9606a060d351acf1aeb1c760434b7f7e788dc..6c69a29bd66cb28b505853ebda7055d7eac79a0b 100644 --- a/apps/openssl.c +++ b/apps/openssl.c @@ -55,6 +55,60 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ +/* ==================================================================== + * Copyright (c) 1998-2000 The OpenSSL Project. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * 3. All advertising materials mentioning features or use of this + * software must display the following acknowledgment: + * "This product includes software developed by the OpenSSL Project + * for use in the OpenSSL Toolkit. (http://www.openssl.org/)" + * + * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to + * endorse or promote products derived from this software without + * prior written permission. For written permission, please contact + * openssl-core@openssl.org. + * + * 5. Products derived from this software may not be called "OpenSSL" + * nor may "OpenSSL" appear in their names without prior written + * permission of the OpenSSL Project. + * + * 6. Redistributions of any form whatsoever must retain the following + * acknowledgment: + * "This product includes software developed by the OpenSSL Project + * for use in the OpenSSL Toolkit (http://www.openssl.org/)" + * + * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY + * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR + * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + * ==================================================================== + * + * This product includes cryptographic software written by Eric Young + * (eay@cryptsoft.com). This product includes software written by Tim + * Hudson (tjh@cryptsoft.com). + * + */ + #include #include @@ -92,6 +146,71 @@ char *default_config_file=NULL; BIO *bio_err=NULL; #endif + +static void lock_dbg_cb(int mode, int type, const char *file, int line) + { + static int modes[CRYPTO_NUM_LOCKS]; /* = {0, 0, ... } */ + const char *errstr = NULL; + int rw; + + rw = mode & (CRYPTO_READ|CRYPTO_WRITE); + if (!((rw == CRYPTO_READ) || (rw == CRYPTO_WRITE))) + { + errstr = "invalid mode"; + goto err; + } + + if (type < 0 || type > CRYPTO_NUM_LOCKS) + { + errstr = "type out of bounds"; + goto err; + } + + if (mode & CRYPTO_LOCK) + { + if (modes[type]) + { + errstr = "already locked"; + /* must not happen in a single-threaded program + * (would deadlock) */ + goto err; + } + + modes[type] = rw; + } + else if (mode & CRYPTO_UNLOCK) + { + if (!modes[type]) + { + errstr = "not locked"; + goto err; + } + + if (modes[type] != rw) + { + errstr = (rw == CRYPTO_READ) ? + "CRYPTO_r_unlock on write lock" : + "CRYPTO_w_unlock on read lock"; + } + + modes[type] = 0; + } + else + { + errstr = "invalid mode"; + goto err; + } + + err: + if (errstr) + { + /* we cannot use bio_err here */ + fprintf(stderr, "openssl (lock_dbg_cb): %s (mode=%d, type=%d) at %s:%d\n", + errstr, mode, type, file, line); + } + } + + int main(int Argc, char *Argv[]) { ARGS arg; @@ -112,6 +231,13 @@ int main(int Argc, char *Argv[]) CRYPTO_malloc_debug_init(); CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON); +#if 0 + if (getenv("OPENSSL_DEBUG_LOCKING") != NULL) +#endif + { + CRYPTO_set_locking_callback(lock_dbg_cb); + } + apps_startup(); if (bio_err == NULL) diff --git a/crypto/cryptlib.c b/crypto/cryptlib.c index 9de60fd52816baf297cd9b3e73ccb400fbe95071..8634c078d8f376a55b7b19c5ea1482bb5acb77f6 100644 --- a/crypto/cryptlib.c +++ b/crypto/cryptlib.c @@ -133,11 +133,11 @@ int CRYPTO_get_new_lockid(char *name) char *str; int i; +#if defined(WIN32) || defined(WIN16) /* A hack to make Visual C++ 5.0 work correctly when linking as * a DLL using /MT. Without this, the application cannot use * and floating point printf's. * It also seems to be needed for Visual C 1.5 (win16) */ -#if defined(WIN32) || defined(WIN16) SSLeay_MSVC5_hack=(double)name[0]*(double)name[1]; #endif diff --git a/crypto/ex_data.c b/crypto/ex_data.c index 35ea2c298271bb6a4066ec5864428becfc3db8fe..3898c33f86731dd07304f2de2b1b37a58cbd1f0c 100644 --- a/crypto/ex_data.c +++ b/crypto/ex_data.c @@ -1,4 +1,17 @@ /* crypto/ex_data.c */ + +/* + * This is not thread-safe, nor can it be changed to become thread-safe + * without changing various function prototypes and using a lot of locking. + * Luckily, it's not really used anywhere except in ssl_verify_cert_chain + * via SSL_get_ex_data_X509_STORE_CTX_idx (ssl/ssl_cert.c), where + * new_func, dup_func, and free_func all are 0. + * + * Any multi-threaded application crazy enough to use ex_data for its own + * purposes had better make sure that SSL_get_ex_data_X509_STORE_CTX_idx + * is called once before multiple threads are created. + */ + /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * diff --git a/crypto/mem_dbg.c b/crypto/mem_dbg.c index 9ed8184e45f9ec0064713c39b660b4bb6e53f645..a6c70e431a9f67ed5781b384a5b9e281da48e456 100644 --- a/crypto/mem_dbg.c +++ b/crypto/mem_dbg.c @@ -678,7 +678,15 @@ void CRYPTO_mem_leaks(BIO *b) * void_fn_to_char kludge in CRYPTO_mem_leaks_cb. * Otherwise the code police will come and get us.) */ + int old_mh_mode; + CRYPTO_w_lock(CRYPTO_LOCK_MALLOC); + + /* avoid deadlock when lh_free() uses CRYPTO_dbg_free(), + * which uses CRYPTO_is_mem_check_on */ + old_mh_mode = mh_mode; + mh_mode = CRYPTO_MEM_CHECK_OFF; + if (mh != NULL) { lh_free(mh); @@ -692,6 +700,8 @@ void CRYPTO_mem_leaks(BIO *b) amih = NULL; } } + + mh_mode = old_mh_mode; CRYPTO_w_unlock(CRYPTO_LOCK_MALLOC); } MemCheck_on(); /* releases CRYPTO_LOCK_MALLOC2 */ diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 0f4110cc64b23ae888d6cb8f09b2ac8134914633..32515cbcc40608a8142052fdab000bde763ad183 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -80,10 +80,7 @@ const char *X509_version="X.509" OPENSSL_VERSION_PTEXT; static STACK_OF(CRYPTO_EX_DATA_FUNCS) *x509_store_ctx_method=NULL; static int x509_store_ctx_num=0; -#if 0 -static int x509_store_num=1; -static STACK *x509_store_method=NULL; -#endif + static int null_callback(int ok, X509_STORE_CTX *e) { @@ -702,12 +699,17 @@ int X509_get_pubkey_parameters(EVP_PKEY *pkey, STACK_OF(X509) *chain) int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func, CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) - { - x509_store_ctx_num++; - return CRYPTO_get_ex_new_index(x509_store_ctx_num-1, + { + /* This function is (usually) called only once, by + * SSL_get_ex_data_X509_STORE_CTX_idx (ssl/ssl_cert.c). + * That function uses locking, so we don't (usually) + * have to worry about locking here. For the whole cruel + * truth, see crypto/ex_data.c */ + x509_store_ctx_num++; + return CRYPTO_get_ex_new_index(x509_store_ctx_num-1, &x509_store_ctx_method, - argl,argp,new_func,dup_func,free_func); - } + argl,argp,new_func,dup_func,free_func); + } int X509_STORE_CTX_set_ex_data(X509_STORE_CTX *ctx, int idx, void *data) { diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c index 130bb79068319ffcf3d5d4ecd1fb478a9fc9ea02..85d58c86688a454dd09da025b5dfdb0da8fae2c2 100644 --- a/ssl/ssl_cert.c +++ b/ssl/ssl_cert.c @@ -129,15 +129,23 @@ int SSL_get_ex_data_X509_STORE_CTX_idx(void) { - static int ssl_x509_store_ctx_idx= -1; + static volatile int ssl_x509_store_ctx_idx= -1; - /* FIXME: should do locking */ if (ssl_x509_store_ctx_idx < 0) { - ssl_x509_store_ctx_idx=X509_STORE_CTX_get_ex_new_index( - 0,"SSL for verify callback",NULL,NULL,NULL); + /* any write lock will do; usually this branch + * will only be taken once anyway */ + CRYPTO_w_lock(CRYPTO_LOCK_SSL_CTX); + + if (ssl_x509_store_ctx_idx < 0) + { + ssl_x509_store_ctx_idx=X509_STORE_CTX_get_ex_new_index( + 0,"SSL for verify callback",NULL,NULL,NULL); + } + + CRYPTO_w_unlock(CRYPTO_LOCK_SSL_CTX); } - return(ssl_x509_store_ctx_idx); + return ssl_x509_store_ctx_idx; } CERT *ssl_cert_new(void) @@ -452,13 +460,15 @@ int ssl_verify_cert_chain(SSL *s,STACK_OF(X509) *sk) if (SSL_get_verify_depth(s) >= 0) X509_STORE_CTX_set_depth(&ctx, SSL_get_verify_depth(s)); X509_STORE_CTX_set_ex_data(&ctx,SSL_get_ex_data_X509_STORE_CTX_idx(),s); + /* We need to set the verify purpose. The purpose can be determined by * the context: if its a server it will verify SSL client certificates * or vice versa. - */ - - if(s->server) i = X509_PURPOSE_SSL_CLIENT; - else i = X509_PURPOSE_SSL_SERVER; + */ + if (s->server) + i = X509_PURPOSE_SSL_CLIENT; + else + i = X509_PURPOSE_SSL_SERVER; X509_STORE_CTX_purpose_inherit(&ctx, i, s->purpose, s->trust); diff --git a/ssl/ssltest.c b/ssl/ssltest.c index 77ac362c814bd0b60129252bdfdd7202aca2b92a..9d513baf8089f81f07a5c30800eda54c648d017f 100644 --- a/ssl/ssltest.c +++ b/ssl/ssltest.c @@ -55,6 +55,59 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ +/* ==================================================================== + * Copyright (c) 1998-2000 The OpenSSL Project. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in + * the documentation and/or other materials provided with the + * distribution. + * + * 3. All advertising materials mentioning features or use of this + * software must display the following acknowledgment: + * "This product includes software developed by the OpenSSL Project + * for use in the OpenSSL Toolkit. (http://www.openssl.org/)" + * + * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to + * endorse or promote products derived from this software without + * prior written permission. For written permission, please contact + * openssl-core@openssl.org. + * + * 5. Products derived from this software may not be called "OpenSSL" + * nor may "OpenSSL" appear in their names without prior written + * permission of the OpenSSL Project. + * + * 6. Redistributions of any form whatsoever must retain the following + * acknowledgment: + * "This product includes software developed by the OpenSSL Project + * for use in the OpenSSL Toolkit (http://www.openssl.org/)" + * + * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY + * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR + * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED + * OF THE POSSIBILITY OF SUCH DAMAGE. + * ==================================================================== + * + * This product includes cryptographic software written by Eric Young + * (eay@cryptsoft.com). This product includes software written by Tim + * Hudson (tjh@cryptsoft.com). + * + */ #include #include @@ -202,6 +255,69 @@ static void print_details(SSL *c_ssl, const char *prefix) BIO_printf(bio_stdout,"\n"); } +static void lock_dbg_cb(int mode, int type, const char *file, int line) + { + static int modes[CRYPTO_NUM_LOCKS]; /* = {0, 0, ... } */ + const char *errstr = NULL; + int rw; + + rw = mode & (CRYPTO_READ|CRYPTO_WRITE); + if (!((rw == CRYPTO_READ) || (rw == CRYPTO_WRITE))) + { + errstr = "invalid mode"; + goto err; + } + + if (type < 0 || type > CRYPTO_NUM_LOCKS) + { + errstr = "type out of bounds"; + goto err; + } + + if (mode & CRYPTO_LOCK) + { + if (modes[type]) + { + errstr = "already locked"; + /* must not happen in a single-threaded program + * (would deadlock) */ + goto err; + } + + modes[type] = rw; + } + else if (mode & CRYPTO_UNLOCK) + { + if (!modes[type]) + { + errstr = "not locked"; + goto err; + } + + if (modes[type] != rw) + { + errstr = (rw == CRYPTO_READ) ? + "CRYPTO_r_unlock on write lock" : + "CRYPTO_w_unlock on read lock"; + } + + modes[type] = 0; + } + else + { + errstr = "invalid mode"; + goto err; + } + + err: + if (errstr) + { + /* we cannot use bio_err here */ + fprintf(stderr, "openssl (lock_dbg_cb): %s (mode=%d, type=%d) at %s:%d\n", + errstr, mode, type, file, line); + } + } + int main(int argc, char *argv[]) { char *CApath=NULL,*CAfile=NULL; @@ -235,6 +351,8 @@ int main(int argc, char *argv[]) debug = 0; cipher = 0; + CRYPTO_set_locking_callback(lock_dbg_cb); + CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ON); RAND_seed(rnd_seed, sizeof rnd_seed);