Fix communication context SSL leaks

Summary:
A `valgrind` run of a modified client that connects multiple times to the
database now shows no memory leaks, previously the context created with
`SSL_CTX_new` was leaked every time.

Reviewers: teon.banek, mtomic

Reviewed By: teon.banek, mtomic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2052
This commit is contained in:
Matej Ferencevic 2019-05-15 15:31:14 +02:00
parent 52adbc6a8b
commit 6082d31843
2 changed files with 98 additions and 0 deletions

View File

@ -32,6 +32,37 @@ ClientContext::ClientContext(const std::string &key_file,
}
}
ClientContext::ClientContext(ClientContext &&other) noexcept
: use_ssl_(other.use_ssl_), ctx_(other.ctx_) {
other.use_ssl_ = false;
other.ctx_ = nullptr;
}
ClientContext &ClientContext::operator=(ClientContext &&other) noexcept {
if (this == &other) return *this;
// destroy my objects
if (use_ssl_) {
SSL_CTX_free(ctx_);
}
// move other objects to self
use_ssl_ = other.use_ssl_;
ctx_ = other.ctx_;
// reset other objects
other.use_ssl_ = false;
other.ctx_ = nullptr;
return *this;
}
ClientContext::~ClientContext() {
if (use_ssl_) {
SSL_CTX_free(ctx_);
}
}
SSL_CTX *ClientContext::context() { return ctx_; }
bool ClientContext::use_ssl() { return use_ssl_; }
@ -83,6 +114,37 @@ ServerContext::ServerContext(const std::string &key_file,
}
}
ServerContext::ServerContext(ServerContext &&other) noexcept
: use_ssl_(other.use_ssl_), ctx_(other.ctx_) {
other.use_ssl_ = false;
other.ctx_ = nullptr;
}
ServerContext &ServerContext::operator=(ServerContext &&other) noexcept {
if (this == &other) return *this;
// destroy my objects
if (use_ssl_) {
SSL_CTX_free(ctx_);
}
// move other objects to self
use_ssl_ = other.use_ssl_;
ctx_ = other.ctx_;
// reset other objects
other.use_ssl_ = false;
other.ctx_ = nullptr;
return *this;
}
ServerContext::~ServerContext() {
if (use_ssl_) {
SSL_CTX_free(ctx_);
}
}
SSL_CTX *ServerContext::context() { return ctx_; }
bool ServerContext::use_ssl() { return use_ssl_; }

View File

@ -6,6 +6,12 @@
namespace communication {
/**
* This class represents a context that should be used with network clients. One
* context can be reused between multiple clients (note: this mainly depends on
* the underlying OpenSSL implementation [see `SSL_new` in
* `openssl/ssl/ssl_lib.c`]).
*/
class ClientContext final {
public:
/**
@ -23,6 +29,18 @@ class ClientContext final {
*/
ClientContext(const std::string &key_file, const std::string &cert_file);
// This object can't be copied because the underlying SSL implementation is
// messy and ownership can't be handled correctly.
ClientContext(const ClientContext &) = delete;
ClientContext &operator=(const ClientContext &) = delete;
// Move constructor/assignment that handle ownership change correctly.
ClientContext(ClientContext &&other) noexcept;
ClientContext &operator=(ClientContext &&other) noexcept;
// Destructor that handles ownership of the SSL object.
~ClientContext();
SSL_CTX *context();
bool use_ssl();
@ -32,6 +50,12 @@ class ClientContext final {
SSL_CTX *ctx_;
};
/**
* This class represents a context that should be used with network servers. One
* context can be reused between multiple servers (note: this mainly depends on
* the underlying OpenSSL implementation [see `SSL_new` in
* `openssl/ssl/ssl_lib.c`]).
*/
class ServerContext final {
public:
/**
@ -51,6 +75,18 @@ class ServerContext final {
ServerContext(const std::string &key_file, const std::string &cert_file,
const std::string &ca_file = "", bool verify_peer = false);
// This object can't be copied because the underlying SSL implementation is
// messy and ownership can't be handled correctly.
ServerContext(const ServerContext &) = delete;
ServerContext &operator=(const ServerContext &) = delete;
// Move constructor/assignment that handle ownership change correctly.
ServerContext(ServerContext &&other) noexcept;
ServerContext &operator=(ServerContext &&other) noexcept;
// Destructor that handles ownership of the SSL object.
~ServerContext();
SSL_CTX *context();
bool use_ssl();