From e5c1d346e29e5b1227ed30ee4d725a09eca0e532 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Fri, 5 Oct 2012 11:52:52 +0200 Subject: crypto: Make unloading of crypto safer Facts: crypto nif-lib registers callback functions that openssl uses for memory management and thread synchronization. The callback functions can only be set once, openssl does not allow changing the callback functions. Problem: If openssl is dynamicly linked to crypto, you might get s scenario where the crypto lib is unloaded while leaving openssl loaded with its old pointers to the unloaded crypto code intact. If crypto is then reloaded (by init:restart() for example), the crypto nif-lib might get relocated at a different address. crypto calls openssl which in turn calls the old invalid callback functions...kaboom. Solution: Break apart the callback functions into a separate dynamic lib that crypto loads with dlopen. When crypto is unloaded the callback lib is left in place to be reused if/when crypto is loaded again. --- lib/crypto/c_src/crypto.c | 219 ++++++++++++++++++---------------------------- 1 file changed, 87 insertions(+), 132 deletions(-) (limited to 'lib/crypto/c_src/crypto.c') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index 91ab244620..8bd0690a17 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -53,6 +53,8 @@ #include #include +#include "crypto_callback.h" + #if OPENSSL_VERSION_NUMBER >= 0x00908000L && !defined(OPENSSL_NO_SHA224) && defined(NID_sha224)\ && !defined(OPENSSL_NO_SHA256) /* disabled like this in my sha.h (?) */ # define HAVE_SHA224 @@ -125,7 +127,6 @@ /* NIF interface declarations */ static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info); -static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info); static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, ERL_NIF_TERM load_info); static void unload(ErlNifEnv* env, void* priv_data); @@ -204,17 +205,6 @@ static ERL_NIF_TERM bf_ecb_crypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM ar static ERL_NIF_TERM blowfish_ofb64_encrypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]); -/* openssl callbacks */ -#ifdef OPENSSL_THREADS -static void locking_function(int mode, int n, const char *file, int line); -static unsigned long id_function(void); -static struct CRYPTO_dynlock_value* dyn_create_function(const char *file, - int line); -static void dyn_lock_function(int mode, struct CRYPTO_dynlock_value* ptr, - const char *file, int line); -static void dyn_destroy_function(struct CRYPTO_dynlock_value *ptr, - const char *file, int line); -#endif /* OPENSSL_THREADS */ /* helpers */ static void init_digest_types(ErlNifEnv* env); @@ -325,7 +315,7 @@ static ErlNifFunc nif_funcs[] = { {"blowfish_ofb64_encrypt", 3, blowfish_ofb64_encrypt} }; -ERL_NIF_INIT(crypto,nif_funcs,load,reload,upgrade,unload) +ERL_NIF_INIT(crypto,nif_funcs,load,NULL,upgrade,unload) #define MD5_CTX_LEN (sizeof(MD5_CTX)) @@ -347,7 +337,6 @@ ERL_NIF_INIT(crypto,nif_funcs,load,reload,upgrade,unload) #define HMAC_OPAD 0x5c -static ErlNifRWLock** lock_vec = NULL; /* Static locks used by openssl */ static ERL_NIF_TERM atom_true; static ERL_NIF_TERM atom_false; static ERL_NIF_TERM atom_sha; @@ -374,55 +363,97 @@ static ERL_NIF_TERM atom_none; static ERL_NIF_TERM atom_notsup; static ERL_NIF_TERM atom_digest; - -static int is_ok_load_info(ErlNifEnv* env, ERL_NIF_TERM load_info) +static int change_basename(char* buf, int bufsz, const char* newfile) { - int i; - return enif_get_int(env,load_info,&i) && i == 101; -} -static void* crypto_alloc(size_t size) -{ - return enif_alloc(size); + char* p = strrchr(buf, '/'); + p = (p == NULL) ? buf : p + 1; + + if ((p - buf) + strlen(newfile) >= bufsz) { + /*fprintf(stderr, "CRYPTO: lib name too long\r\n");*/ + return 0; + } + strcpy(p, newfile); + return 1; } -static void* crypto_realloc(void* ptr, size_t size) + +static void error_handler(void* null, const char* errstr) { - return enif_realloc(ptr, size); -} -static void crypto_free(void* ptr) -{ - enif_free(ptr); + /*fprintf(stderr, "CRYPTO LOADING ERROR: '%s'\r\n", errstr);*/ } -static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) +static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) { ErlNifSysInfo sys_info; - CRYPTO_set_mem_functions(crypto_alloc, crypto_realloc, crypto_free); - - if (!is_ok_load_info(env, load_info)) { - return -1; + const char callback_lib[] = "crypto_callback"; + void* handle; + get_crypto_callbacks_t* funcp; + struct crypto_callbacks* ccb; + int nlocks = 0; + int tpl_arity; + const ERL_NIF_TERM* tpl_array; + int vernum; + char lib_buf[1000]; + + /* load_info: {201, "/full/path/of/this/library"} */ + if (!enif_get_tuple(env, load_info, &tpl_arity, &tpl_array) + || tpl_arity != 2 + || !enif_get_int(env, tpl_array[0], &vernum) + || vernum != 201 + || enif_get_string(env, tpl_array[1], lib_buf, sizeof(lib_buf), ERL_NIF_LATIN1) <= 0) { + + /*enif_fprintf(stderr, "CRYPTO: Invalid load_info '%T'\n", load_info);*/ + return 0; + } + if (library_refc > 0) { + return 1; } + if (!change_basename(lib_buf, sizeof(lib_buf), callback_lib)) { + return 0; + } + + if (!(handle = enif_dlopen(lib_buf, &error_handler, NULL))) { + return 0; + } + if (!(funcp = (get_crypto_callbacks_t*) enif_dlsym(handle, "get_crypto_callbacks", + &error_handler, NULL))) { + return 0; + } + #ifdef OPENSSL_THREADS enif_system_info(&sys_info, sizeof(sys_info)); - if (sys_info.scheduler_threads > 1) { - int i; - lock_vec = enif_alloc(CRYPTO_num_locks()*sizeof(*lock_vec)); - if (lock_vec==NULL) return -1; - memset(lock_vec,0,CRYPTO_num_locks()*sizeof(*lock_vec)); - - for (i=CRYPTO_num_locks()-1; i>=0; --i) { - lock_vec[i] = enif_rwlock_create("crypto_stat"); - if (lock_vec[i]==NULL) return -1; - } - CRYPTO_set_locking_callback(locking_function); - CRYPTO_set_id_callback(id_function); - CRYPTO_set_dynlock_create_callback(dyn_create_function); - CRYPTO_set_dynlock_lock_callback(dyn_lock_function); - CRYPTO_set_dynlock_destroy_callback(dyn_destroy_function); + nlocks = CRYPTO_num_locks(); } /* else no need for locks */ +#endif + + ccb = (*funcp)(nlocks); + + if (!ccb || ccb->sizeof_me != sizeof(*ccb)) { + /*fprintf(stderr, "Invalid 'crypto_callbacks'\r\n");*/ + return 0; + } + + CRYPTO_set_mem_functions(ccb->crypto_alloc, ccb->crypto_realloc, ccb->crypto_free); + +#ifdef OPENSSL_THREADS + if (nlocks > 0) { + CRYPTO_set_locking_callback(ccb->locking_function); + CRYPTO_set_id_callback(ccb->id_function); + CRYPTO_set_dynlock_create_callback(ccb->dyn_create_function); + CRYPTO_set_dynlock_lock_callback(ccb->dyn_lock_function); + CRYPTO_set_dynlock_destroy_callback(ccb->dyn_destroy_function); + } #endif /* OPENSSL_THREADS */ + return 1; +} + +static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) +{ + if (!init(env, load_info)) { + return -1; + } atom_true = enif_make_atom(env,"true"); atom_false = enif_make_atom(env,"false"); @@ -456,8 +487,12 @@ static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) return 0; } -static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) -{ +static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, + ERL_NIF_TERM load_info) +{ + if (*old_priv_data != NULL) { + return -1; /* Don't know how to do that */ + } if (*priv_data != NULL) { return -1; /* Don't know how to do that */ } @@ -466,43 +501,16 @@ static int reload(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) when to (re)set the callbacks for allocation and locking. */ return -2; } - if (!is_ok_load_info(env, load_info)) { + if (!init(env, load_info)) { return -1; } - return 0; -} - -static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, - ERL_NIF_TERM load_info) -{ - int i; - if (*old_priv_data != NULL) { - return -1; /* Don't know how to do that */ - } - i = reload(env,priv_data,load_info); - if (i != 0) { - return i; - } library_refc++; return 0; } static void unload(ErlNifEnv* env, void* priv_data) { - if (--library_refc <= 0) { - CRYPTO_cleanup_all_ex_data(); - - if (lock_vec != NULL) { - int i; - for (i=CRYPTO_num_locks()-1; i>=0; --i) { - if (lock_vec[i] != NULL) { - enif_rwlock_destroy(lock_vec[i]); - } - } - enif_free(lock_vec); - } - } - /*else NIF library still used by other (new) module code */ + --library_refc; } static ERL_NIF_TERM info_lib(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) @@ -2338,59 +2346,6 @@ static ERL_NIF_TERM blowfish_ofb64_encrypt(ErlNifEnv* env, int argc, const ERL_N -#ifdef OPENSSL_THREADS /* vvvvvvvvvvvvvvv OPENSSL_THREADS vvvvvvvvvvvvvvvv */ - -static INLINE void locking(int mode, ErlNifRWLock* lock) -{ - switch (mode) { - case CRYPTO_LOCK|CRYPTO_READ: - enif_rwlock_rlock(lock); - break; - case CRYPTO_LOCK|CRYPTO_WRITE: - enif_rwlock_rwlock(lock); - break; - case CRYPTO_UNLOCK|CRYPTO_READ: - enif_rwlock_runlock(lock); - break; - case CRYPTO_UNLOCK|CRYPTO_WRITE: - enif_rwlock_rwunlock(lock); - break; - default: - ASSERT(!"Invalid lock mode"); - } -} - -/* Callback from openssl for static locking - */ -static void locking_function(int mode, int n, const char *file, int line) -{ - ASSERT(n>=0 && n Date: Tue, 16 Oct 2012 20:14:48 +0200 Subject: crypto: Enable runtime upgrade of crypto --- lib/crypto/c_src/crypto.c | 62 +++++++++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 32 deletions(-) (limited to 'lib/crypto/c_src/crypto.c') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index 8bd0690a17..c98dacc3eb 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -405,9 +405,39 @@ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) return 0; } if (library_refc > 0) { + /* Repeated loading of this library (module upgrade). + * Atoms and callbacks are already set, we are done. + */ return 1; } + atom_true = enif_make_atom(env,"true"); + atom_false = enif_make_atom(env,"false"); + atom_sha = enif_make_atom(env,"sha"); + atom_sha224 = enif_make_atom(env,"sha224"); + atom_sha256 = enif_make_atom(env,"sha256"); + atom_sha384 = enif_make_atom(env,"sha384"); + atom_sha512 = enif_make_atom(env,"sha512"); + atom_md5 = enif_make_atom(env,"md5"); + atom_ripemd160 = enif_make_atom(env,"ripemd160"); + atom_error = enif_make_atom(env,"error"); + atom_rsa_pkcs1_padding = enif_make_atom(env,"rsa_pkcs1_padding"); + atom_rsa_pkcs1_oaep_padding = enif_make_atom(env,"rsa_pkcs1_oaep_padding"); + atom_rsa_no_padding = enif_make_atom(env,"rsa_no_padding"); + atom_undefined = enif_make_atom(env,"undefined"); + atom_ok = enif_make_atom(env,"ok"); + atom_not_prime = enif_make_atom(env,"not_prime"); + atom_not_strong_prime = enif_make_atom(env,"not_strong_prime"); + atom_unable_to_check_generator = enif_make_atom(env,"unable_to_check_generator"); + atom_not_suitable_generator = enif_make_atom(env,"not_suitable_generator"); + atom_check_failed = enif_make_atom(env,"check_failed"); + atom_unknown = enif_make_atom(env,"unknown"); + atom_none = enif_make_atom(env,"none"); + atom_notsup = enif_make_atom(env,"notsup"); + atom_digest = enif_make_atom(env,"digest"); + + init_digest_types(env); + if (!change_basename(lib_buf, sizeof(lib_buf), callback_lib)) { return 0; } @@ -455,33 +485,6 @@ static int load(ErlNifEnv* env, void** priv_data, ERL_NIF_TERM load_info) return -1; } - atom_true = enif_make_atom(env,"true"); - atom_false = enif_make_atom(env,"false"); - atom_sha = enif_make_atom(env,"sha"); - atom_sha224 = enif_make_atom(env,"sha224"); - atom_sha256 = enif_make_atom(env,"sha256"); - atom_sha384 = enif_make_atom(env,"sha384"); - atom_sha512 = enif_make_atom(env,"sha512"); - atom_md5 = enif_make_atom(env,"md5"); - atom_ripemd160 = enif_make_atom(env,"ripemd160"); - atom_error = enif_make_atom(env,"error"); - atom_rsa_pkcs1_padding = enif_make_atom(env,"rsa_pkcs1_padding"); - atom_rsa_pkcs1_oaep_padding = enif_make_atom(env,"rsa_pkcs1_oaep_padding"); - atom_rsa_no_padding = enif_make_atom(env,"rsa_no_padding"); - atom_undefined = enif_make_atom(env,"undefined"); - atom_ok = enif_make_atom(env,"ok"); - atom_not_prime = enif_make_atom(env,"not_prime"); - atom_not_strong_prime = enif_make_atom(env,"not_strong_prime"); - atom_unable_to_check_generator = enif_make_atom(env,"unable_to_check_generator"); - atom_not_suitable_generator = enif_make_atom(env,"not_suitable_generator"); - atom_check_failed = enif_make_atom(env,"check_failed"); - atom_unknown = enif_make_atom(env,"unknown"); - atom_none = enif_make_atom(env,"none"); - atom_notsup = enif_make_atom(env,"notsup"); - atom_digest = enif_make_atom(env,"digest"); - - init_digest_types(env); - *priv_data = NULL; library_refc++; return 0; @@ -496,11 +499,6 @@ static int upgrade(ErlNifEnv* env, void** priv_data, void** old_priv_data, if (*priv_data != NULL) { return -1; /* Don't know how to do that */ } - if (library_refc == 0) { - /* No support for real library upgrade. The tricky thing is to know - when to (re)set the callbacks for allocation and locking. */ - return -2; - } if (!init(env, load_info)) { return -1; } -- cgit v1.2.3 From 8d502f8f98a89678448d16a8a15e744d65603f01 Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Mon, 22 Oct 2012 17:59:51 +0200 Subject: crypto: Add debug print macros --- lib/crypto/c_src/crypto.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'lib/crypto/c_src/crypto.c') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index c98dacc3eb..ea0bb10cb7 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -363,13 +363,20 @@ static ERL_NIF_TERM atom_none; static ERL_NIF_TERM atom_notsup; static ERL_NIF_TERM atom_digest; +/* +#define PRINTF_ERR0(FMT) enif_fprintf(stderr, FMT "\n") +#define PRINTF_ERR1(FMT, A1) enif_fprintf(stderr, FMT "\n", A1) +*/ +#define PRINTF_ERR0(FMT) +#define PRINTF_ERR1(FMT,A1) + static int change_basename(char* buf, int bufsz, const char* newfile) { char* p = strrchr(buf, '/'); p = (p == NULL) ? buf : p + 1; if ((p - buf) + strlen(newfile) >= bufsz) { - /*fprintf(stderr, "CRYPTO: lib name too long\r\n");*/ + PRINTF_ERR0("CRYPTO: lib name too long"); return 0; } strcpy(p, newfile); @@ -378,7 +385,7 @@ static int change_basename(char* buf, int bufsz, const char* newfile) static void error_handler(void* null, const char* errstr) { - /*fprintf(stderr, "CRYPTO LOADING ERROR: '%s'\r\n", errstr);*/ + PRINTF_ERR1("CRYPTO LOADING ERROR: '%s'", errstr); } static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) @@ -401,7 +408,7 @@ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) || vernum != 201 || enif_get_string(env, tpl_array[1], lib_buf, sizeof(lib_buf), ERL_NIF_LATIN1) <= 0) { - /*enif_fprintf(stderr, "CRYPTO: Invalid load_info '%T'\n", load_info);*/ + PRINTF_ERR1("CRYPTO: Invalid load_info '%T'", load_info); return 0; } if (library_refc > 0) { @@ -461,7 +468,7 @@ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) ccb = (*funcp)(nlocks); if (!ccb || ccb->sizeof_me != sizeof(*ccb)) { - /*fprintf(stderr, "Invalid 'crypto_callbacks'\r\n");*/ + PRINTF_ERR0("Invalid 'crypto_callbacks'"); return 0; } -- cgit v1.2.3 From b7b4abaeac8e559a3a4f587d46dc0014332b517e Mon Sep 17 00:00:00 2001 From: Sverker Eriksson Date: Mon, 22 Oct 2012 18:22:23 +0200 Subject: crypto: Link crypto_callback statically if static linking of openssl is used. --- lib/crypto/c_src/crypto.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) (limited to 'lib/crypto/c_src/crypto.c') diff --git a/lib/crypto/c_src/crypto.c b/lib/crypto/c_src/crypto.c index ea0bb10cb7..5dc088dcff 100644 --- a/lib/crypto/c_src/crypto.c +++ b/lib/crypto/c_src/crypto.c @@ -370,6 +370,7 @@ static ERL_NIF_TERM atom_digest; #define PRINTF_ERR0(FMT) #define PRINTF_ERR1(FMT,A1) +#ifdef HAVE_DYNAMIC_CRYPTO_LIB static int change_basename(char* buf, int bufsz, const char* newfile) { char* p = strrchr(buf, '/'); @@ -387,12 +388,11 @@ static void error_handler(void* null, const char* errstr) { PRINTF_ERR1("CRYPTO LOADING ERROR: '%s'", errstr); } +#endif /* HAVE_DYNAMIC_CRYPTO_LIB */ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) { ErlNifSysInfo sys_info; - const char callback_lib[] = "crypto_callback"; - void* handle; get_crypto_callbacks_t* funcp; struct crypto_callbacks* ccb; int nlocks = 0; @@ -445,17 +445,23 @@ static int init(ErlNifEnv* env, ERL_NIF_TERM load_info) init_digest_types(env); - if (!change_basename(lib_buf, sizeof(lib_buf), callback_lib)) { - return 0; - } - - if (!(handle = enif_dlopen(lib_buf, &error_handler, NULL))) { - return 0; - } - if (!(funcp = (get_crypto_callbacks_t*) enif_dlsym(handle, "get_crypto_callbacks", - &error_handler, NULL))) { - return 0; +#ifdef HAVE_DYNAMIC_CRYPTO_LIB + { + void* handle; + if (!change_basename(lib_buf, sizeof(lib_buf), "crypto_callback")) { + return 0; + } + if (!(handle = enif_dlopen(lib_buf, &error_handler, NULL))) { + return 0; + } + if (!(funcp = (get_crypto_callbacks_t*) enif_dlsym(handle, "get_crypto_callbacks", + &error_handler, NULL))) { + return 0; + } } +#else /* !HAVE_DYNAMIC_CRYPTO_LIB */ + funcp = &get_crypto_callbacks; +#endif #ifdef OPENSSL_THREADS enif_system_info(&sys_info, sizeof(sys_info)); -- cgit v1.2.3