9pfs/solanum-vs-hackint-and-charybdis
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

openssl: Avoid use-after-free when rehashing fails to load new files

Browse source 
Commit cf12678 introduced a fix for issue #186 by freeing the old SSL_CTX
structure before constructing a new one, which could disconnect existing
clients otherwise.

Unfortunately, the freeing is done first, which means that if setting up
a new structure fails for any reason, there will be no usable structures
left, but they are still referenced.

This fix moves the freeing to the end of the function, using intermediate
new variables in the meantime. This problem was discovered while testing
against OpenSSL 1.1.0 RC6.
This commit is contained in:
Aaron Jones 2016-08-12 13:34:13 +00:00
parent e5b5dc997d
commit add3f90b9f
No known key found for this signature in database
GPG key ID: EC6F86EE9CD840B5
1 changed files with 43 additions and 29 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

72
librb/src/openssl.c
View file

@ -375,6 +375,9 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
{ {
const char librb_ciphers[] = "kEECDH+HIGH:kEDH+HIGH:HIGH:!aNULL"; const char librb_ciphers[] = "kEECDH+HIGH:kEDH+HIGH:HIGH:!aNULL";
SSL_CTX *ssl_server_ctx_new;
SSL_CTX *ssl_client_ctx_new;
#ifdef LRB_HAVE_TLS_SET_CURVES #ifdef LRB_HAVE_TLS_SET_CURVES
const char librb_curves[] = "P-521:P-384:P-256"; const char librb_curves[] = "P-521:P-384:P-256";
#endif #endif
@ -391,65 +394,61 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
if(cipher_list == NULL) if(cipher_list == NULL)
cipher_list = librb_ciphers; cipher_list = librb_ciphers;
if (ssl_server_ctx)
SSL_CTX_free(ssl_server_ctx);
if (ssl_client_ctx)
SSL_CTX_free(ssl_client_ctx);
#ifdef LRB_HAVE_TLS_METHOD_API #ifdef LRB_HAVE_TLS_METHOD_API
ssl_server_ctx = SSL_CTX_new(TLS_server_method()); ssl_server_ctx_new = SSL_CTX_new(TLS_server_method());
ssl_client_ctx = SSL_CTX_new(TLS_client_method()); ssl_client_ctx_new = SSL_CTX_new(TLS_client_method());
#else #else
ssl_server_ctx = SSL_CTX_new(SSLv23_server_method()); ssl_server_ctx_new = SSL_CTX_new(SSLv23_server_method());
ssl_client_ctx = SSL_CTX_new(SSLv23_client_method()); ssl_client_ctx_new = SSL_CTX_new(SSLv23_client_method());
#endif #endif
if(ssl_server_ctx == NULL) if(ssl_server_ctx_new == NULL)
{ {
rb_lib_log("rb_init_openssl: Unable to initialize OpenSSL server context: %s", rb_lib_log("rb_init_openssl: Unable to initialize OpenSSL server context: %s",
get_ssl_error(ERR_get_error())); get_ssl_error(ERR_get_error()));
return 0; return 0;
} }
if(ssl_client_ctx == NULL) if(ssl_client_ctx_new == NULL)
{ {
rb_lib_log("rb_init_openssl: Unable to initialize OpenSSL client context: %s", rb_lib_log("rb_init_openssl: Unable to initialize OpenSSL client context: %s",
get_ssl_error(ERR_get_error())); get_ssl_error(ERR_get_error()));
SSL_CTX_free(ssl_server_ctx_new);
return 0; return 0;
} }
#ifndef LRB_HAVE_TLS_METHOD_API #ifndef LRB_HAVE_TLS_METHOD_API
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
SSL_CTX_set_options(ssl_client_ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3); SSL_CTX_set_options(ssl_client_ctx_new, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
#endif #endif
#ifdef SSL_OP_SINGLE_DH_USE #ifdef SSL_OP_SINGLE_DH_USE
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_SINGLE_DH_USE); SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_SINGLE_DH_USE);
#endif #endif
#ifdef SSL_OP_SINGLE_ECDH_USE #ifdef SSL_OP_SINGLE_ECDH_USE
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_SINGLE_ECDH_USE); SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_SINGLE_ECDH_USE);
#endif #endif
#ifdef SSL_OP_NO_TICKET #ifdef SSL_OP_NO_TICKET
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_NO_TICKET); SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_NO_TICKET);
SSL_CTX_set_options(ssl_client_ctx, SSL_OP_NO_TICKET); SSL_CTX_set_options(ssl_client_ctx_new, SSL_OP_NO_TICKET);
#endif #endif
#ifdef SSL_OP_CIPHER_SERVER_PREFERENCE #ifdef SSL_OP_CIPHER_SERVER_PREFERENCE
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_CIPHER_SERVER_PREFERENCE); SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_CIPHER_SERVER_PREFERENCE);
#endif #endif
SSL_CTX_set_verify(ssl_server_ctx, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_accept_all_cb); SSL_CTX_set_verify(ssl_server_ctx_new, SSL_VERIFY_PEER | SSL_VERIFY_CLIENT_ONCE, verify_accept_all_cb);
SSL_CTX_set_session_cache_mode(ssl_server_ctx, SSL_SESS_CACHE_OFF); SSL_CTX_set_session_cache_mode(ssl_server_ctx_new, SSL_SESS_CACHE_OFF);
#ifdef LRB_HAVE_TLS_SET_CURVES #ifdef LRB_HAVE_TLS_SET_CURVES
SSL_CTX_set1_curves_list(ssl_server_ctx, librb_curves); SSL_CTX_set1_curves_list(ssl_server_ctx_new, librb_curves);
#endif #endif
#ifdef LRB_HAVE_TLS_ECDH_AUTO #ifdef LRB_HAVE_TLS_ECDH_AUTO
SSL_CTX_set_ecdh_auto(ssl_server_ctx, 1); SSL_CTX_set_ecdh_auto(ssl_server_ctx_new, 1);
#endif #endif
/* /*
@ -458,25 +457,31 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
#if (OPENSSL_VERSION_NUMBER >= 0x10000000L) && (OPENSSL_VERSION_NUMBER < 0x10002000L) && !defined(OPENSSL_NO_ECDH) #if (OPENSSL_VERSION_NUMBER >= 0x10000000L) && (OPENSSL_VERSION_NUMBER < 0x10002000L) && !defined(OPENSSL_NO_ECDH)
EC_KEY *key = EC_KEY_new_by_curve_name(NID_secp384r1); EC_KEY *key = EC_KEY_new_by_curve_name(NID_secp384r1);
if (key) { if (key) {
SSL_CTX_set_tmp_ecdh(ssl_server_ctx, key); SSL_CTX_set_tmp_ecdh(ssl_server_ctx_new, key);
EC_KEY_free(key); EC_KEY_free(key);
} }
#endif #endif
SSL_CTX_set_cipher_list(ssl_server_ctx, cipher_list); SSL_CTX_set_cipher_list(ssl_server_ctx_new, cipher_list);
SSL_CTX_set_cipher_list(ssl_client_ctx, cipher_list); SSL_CTX_set_cipher_list(ssl_client_ctx_new, cipher_list);
if(!SSL_CTX_use_certificate_chain_file(ssl_server_ctx, certfile) || !SSL_CTX_use_certificate_chain_file(ssl_client_ctx, certfile)) if(!SSL_CTX_use_certificate_chain_file(ssl_server_ctx_new, certfile) || !SSL_CTX_use_certificate_chain_file(ssl_client_ctx_new, certfile))
{ {
rb_lib_log("rb_setup_ssl_server: Error loading certificate file [%s]: %s", certfile, rb_lib_log("rb_setup_ssl_server: Error loading certificate file [%s]: %s", certfile,
get_ssl_error(ERR_get_error())); get_ssl_error(ERR_get_error()));
SSL_CTX_free(ssl_server_ctx_new);
SSL_CTX_free(ssl_client_ctx_new);
return 0; return 0;
} }
if(!SSL_CTX_use_PrivateKey_file(ssl_server_ctx, keyfile, SSL_FILETYPE_PEM) || !SSL_CTX_use_PrivateKey_file(ssl_client_ctx, keyfile, SSL_FILETYPE_PEM)) if(!SSL_CTX_use_PrivateKey_file(ssl_server_ctx_new, keyfile, SSL_FILETYPE_PEM) || !SSL_CTX_use_PrivateKey_file(ssl_client_ctx_new, keyfile, SSL_FILETYPE_PEM))
{ {
rb_lib_log("rb_setup_ssl_server: Error loading keyfile [%s]: %s", keyfile, rb_lib_log("rb_setup_ssl_server: Error loading keyfile [%s]: %s", keyfile,
get_ssl_error(ERR_get_error())); get_ssl_error(ERR_get_error()));
SSL_CTX_free(ssl_server_ctx_new);
SSL_CTX_free(ssl_client_ctx_new);
return 0; return 0;
} }
@ -499,12 +504,21 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
} }
else else
{ {
SSL_CTX_set_tmp_dh(ssl_server_ctx, dh); SSL_CTX_set_tmp_dh(ssl_server_ctx_new, dh);
DH_free(dh); DH_free(dh);
fclose(fp); fclose(fp);
} }
} }
if (ssl_server_ctx)
SSL_CTX_free(ssl_server_ctx);
if (ssl_client_ctx)
SSL_CTX_free(ssl_client_ctx);
ssl_server_ctx = ssl_server_ctx_new;
ssl_client_ctx = ssl_client_ctx_new;
return 1; return 1;
} }