mirror of
https://github.com/matrix-construct/construct
synced 2024-12-28 00:14:07 +01:00
openssl: Avoid use-after-free when rehashing fails to load new files
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:
parent
09d19cbbd3
commit
3fccd78d32
1 changed files with 43 additions and 29 deletions
72
rb/openssl.c
72
rb/openssl.c
|
@ -373,6 +373,9 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
|
|||
{
|
||||
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
|
||||
const char librb_curves[] = "P-521:P-384:P-256";
|
||||
#endif
|
||||
|
@ -389,65 +392,61 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
|
|||
if(cipher_list == NULL)
|
||||
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
|
||||
ssl_server_ctx = SSL_CTX_new(TLS_server_method());
|
||||
ssl_client_ctx = SSL_CTX_new(TLS_client_method());
|
||||
ssl_server_ctx_new = SSL_CTX_new(TLS_server_method());
|
||||
ssl_client_ctx_new = SSL_CTX_new(TLS_client_method());
|
||||
#else
|
||||
ssl_server_ctx = SSL_CTX_new(SSLv23_server_method());
|
||||
ssl_client_ctx = SSL_CTX_new(SSLv23_client_method());
|
||||
ssl_server_ctx_new = SSL_CTX_new(SSLv23_server_method());
|
||||
ssl_client_ctx_new = SSL_CTX_new(SSLv23_client_method());
|
||||
#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",
|
||||
get_ssl_error(ERR_get_error()));
|
||||
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",
|
||||
get_ssl_error(ERR_get_error()));
|
||||
|
||||
SSL_CTX_free(ssl_server_ctx_new);
|
||||
return 0;
|
||||
}
|
||||
|
||||
#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_client_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_new, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
|
||||
#endif
|
||||
|
||||
#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
|
||||
|
||||
#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
|
||||
|
||||
#ifdef SSL_OP_NO_TICKET
|
||||
SSL_CTX_set_options(ssl_server_ctx, SSL_OP_NO_TICKET);
|
||||
SSL_CTX_set_options(ssl_client_ctx, SSL_OP_NO_TICKET);
|
||||
SSL_CTX_set_options(ssl_server_ctx_new, SSL_OP_NO_TICKET);
|
||||
SSL_CTX_set_options(ssl_client_ctx_new, SSL_OP_NO_TICKET);
|
||||
#endif
|
||||
|
||||
#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
|
||||
|
||||
SSL_CTX_set_verify(ssl_server_ctx, 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_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_new, SSL_SESS_CACHE_OFF);
|
||||
|
||||
#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
|
||||
|
||||
#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
|
||||
|
||||
/*
|
||||
|
@ -456,25 +455,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)
|
||||
EC_KEY *key = EC_KEY_new_by_curve_name(NID_secp384r1);
|
||||
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);
|
||||
}
|
||||
#endif
|
||||
|
||||
SSL_CTX_set_cipher_list(ssl_server_ctx, cipher_list);
|
||||
SSL_CTX_set_cipher_list(ssl_client_ctx, cipher_list);
|
||||
SSL_CTX_set_cipher_list(ssl_server_ctx_new, 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,
|
||||
get_ssl_error(ERR_get_error()));
|
||||
|
||||
SSL_CTX_free(ssl_server_ctx_new);
|
||||
SSL_CTX_free(ssl_client_ctx_new);
|
||||
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,
|
||||
get_ssl_error(ERR_get_error()));
|
||||
|
||||
SSL_CTX_free(ssl_server_ctx_new);
|
||||
SSL_CTX_free(ssl_client_ctx_new);
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
@ -497,12 +502,21 @@ rb_setup_ssl_server(const char *certfile, const char *keyfile, const char *dhfil
|
|||
}
|
||||
else
|
||||
{
|
||||
SSL_CTX_set_tmp_dh(ssl_server_ctx, dh);
|
||||
SSL_CTX_set_tmp_dh(ssl_server_ctx_new, dh);
|
||||
DH_free(dh);
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue