Skip to content

Commit

Permalink
webif/ghttp/ssl: Do not leak memory allocated by openssl.
Browse files Browse the repository at this point in the history
This patch does several related things:
1. Moves SSL initialization in one place (using it from two threads
   can cause undefined behaviour).
2. Clean openssl allocated context if ssl initialization in
   webif failed for some reason (not founding certificate file for
   example).
3. Clear memory after openssl error functions are called.

git-svn-id: http://streamboard.de.vu/svn/oscam/trunk@10470 4b0bc96b-bc66-0410-9d44-ebda105a78c1
  • Loading branch information
gfto committed Feb 10, 2015
1 parent ed0a8b9 commit 88a65ac
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 32 deletions.
15 changes: 12 additions & 3 deletions module-ghttp.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,20 @@ static bool _ssl_connect(struct s_client *client, int32_t fd)
if(context->ssl_handle == NULL)
{
ERR_print_errors_fp(stderr);
ERR_remove_state(0);
return false;
}
if(!SSL_set_fd(context->ssl_handle, fd))
{
ERR_print_errors_fp(stderr);
ERR_remove_state(0);
return false;
}
if(SSL_connect(context->ssl_handle) != 1)
{ ERR_print_errors_fp(stderr); }
{
ERR_print_errors_fp(stderr);
ERR_remove_state(0);
}

if(context->ssl_handle)
{
Expand All @@ -89,7 +94,11 @@ int32_t ghttp_client_init(struct s_client *cl)
SSL_load_error_strings();
SSL_library_init();
ghttp_ssl_context = SSL_CTX_new(SSLv23_client_method());
if(ghttp_ssl_context == NULL) { ERR_print_errors_fp(stderr); }
if(ghttp_ssl_context == NULL)
{
ERR_print_errors_fp(stderr);
ERR_remove_state(0);
}
#endif

if(cl->reader->r_port == 0)
Expand Down Expand Up @@ -669,12 +678,12 @@ static void ghttp_cleanup(struct s_client *client)
ll_destroy(&context->ecm_q);
ll_destroy_data(&context->post_contexts);
#ifdef WITH_SSL
ERR_free_strings();
if(context->ssl_handle)
{
SSL_shutdown(context->ssl_handle);
SSL_free(context->ssl_handle);
}
SSL_CTX_free(ghttp_ssl_context);
#endif
NULLFREE(context);
}
Expand Down
29 changes: 11 additions & 18 deletions module-webif-lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -844,11 +844,6 @@ static void SSL_dyn_destroy_function(struct CRYPTO_dynlock_value *l, const char
/* Init necessary structures for SSL in WebIf*/
SSL_CTX *SSL_Webif_Init(void)
{
SSL_library_init();
SSL_load_error_strings();
ERR_load_BIO_strings();
ERR_load_SSL_strings();

SSL_CTX *ctx;

static const char *cs_cert = "oscam.pem";
Expand Down Expand Up @@ -896,30 +891,28 @@ SSL_CTX *SSL_Webif_Init(void)
{ cs_strncpy(path, cfg.http_cert, sizeof(path)); }

if(!ctx)
{
ERR_print_errors_fp(stderr);
return NULL;
}
goto out_err;

if(SSL_CTX_use_certificate_file(ctx, path, SSL_FILETYPE_PEM) <= 0)
{
ERR_print_errors_fp(stderr);
return NULL;
}
goto out_err;

if(SSL_CTX_use_PrivateKey_file(ctx, path, SSL_FILETYPE_PEM) <= 0)
{
ERR_print_errors_fp(stderr);
return NULL;
}
goto out_err;

if(!SSL_CTX_check_private_key(ctx))
{
cs_log("SSL: Private key does not match the certificate public key");
return NULL;
goto out_err;
}

cs_log("load ssl certificate file %s", path);
return ctx;

out_err:
ERR_print_errors_fp(stderr);
ERR_remove_state(0);
SSL_CTX_free(ctx);
return NULL;
}
#endif

Expand Down
19 changes: 8 additions & 11 deletions module-webif.c
Original file line number Diff line number Diff line change
Expand Up @@ -8076,17 +8076,14 @@ static void *http_server(void *UNUSED(d))
// Wait a bit so that we don't close ressources while http threads are active
cs_sleepms(300);
#ifdef WITH_SSL
if(ssl_active)
{
SSL_CTX_free(ctx);
CRYPTO_set_dynlock_create_callback(NULL);
CRYPTO_set_dynlock_lock_callback(NULL);
CRYPTO_set_dynlock_destroy_callback(NULL);
CRYPTO_set_locking_callback(NULL);
CRYPTO_set_id_callback(NULL);
OPENSSL_free(lock_cs);
lock_cs = NULL;
}
SSL_CTX_free(ctx);
CRYPTO_set_dynlock_create_callback(NULL);
CRYPTO_set_dynlock_lock_callback(NULL);
CRYPTO_set_dynlock_destroy_callback(NULL);
CRYPTO_set_locking_callback(NULL);
CRYPTO_set_id_callback(NULL);
OPENSSL_free(lock_cs);
lock_cs = NULL;
#endif
cs_log("HTTP Server stopped");
free_client(cl);
Expand Down
28 changes: 28 additions & 0 deletions oscam.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,32 @@
#include "reader-common.h"
#include "module-gbox.h"

#ifdef WITH_SSL
#include <openssl/crypto.h>
#include <openssl/ssl.h>
#include <openssl/err.h>

static void ssl_init(void)
{
SSL_load_error_strings();
ERR_load_BIO_strings();
ERR_load_SSL_strings();
SSL_library_init();
}

static void ssl_done(void)
{
ERR_remove_state(0);
ERR_free_strings();
EVP_cleanup();
CRYPTO_cleanup_all_ex_data();
}

#else
static void ssl_init(void) { }
static void ssl_done(void) { }
#endif

extern char *config_mak;

/*****************************************************************************
Expand Down Expand Up @@ -1504,6 +1530,7 @@ int32_t main(int32_t argc, char *argv[])
cs_init_statistics();
coolapi_open_all();
init_stat();
ssl_init();

// These initializations *MUST* be called after init_config()
// because modules depend on config values.
Expand Down Expand Up @@ -1654,6 +1681,7 @@ int32_t main(int32_t argc, char *argv[])
free_readerdb();
free_irdeto_guess_tab();
config_free();
ssl_done();

detect_valgrind();
if (!running_under_valgrind)
Expand Down

0 comments on commit 88a65ac

Please sign in to comment.