Skip to content

Commit

Permalink
tls: destroy SSL once it is out of use
Browse files Browse the repository at this point in the history
Do not keep SSL structure in memory once socket is closed. This should
lower the memory usage in many cases.

Fix: #1522
PR-URL: #1529
Reviewed-By: Shigeki Ohtsu <[email protected]>
  • Loading branch information
indutny committed Apr 30, 2015
1 parent 7306252 commit 2d241b3
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 9 deletions.
9 changes: 9 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,15 @@ TLSSocket.prototype._wrapHandle = function(handle) {
}
});

this.on('close', this._destroySSL);

return res;
};

TLSSocket.prototype._destroySSL = function _destroySSL() {
return this.ssl.destroySSL();
};

TLSSocket.prototype._init = function(socket, wrap) {
var self = this;
var options = this._tlsOptions;
Expand Down Expand Up @@ -416,6 +422,9 @@ TLSSocket.prototype.renegotiate = function(options, callback) {
var requestCert = this._requestCert,
rejectUnauthorized = this._rejectUnauthorized;

if (this.destroyed)
return;

if (typeof options.requestCert !== 'undefined')
requestCert = !!options.requestCert;
if (typeof options.rejectUnauthorized !== 'undefined')
Expand Down
11 changes: 11 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ template int SSLWrap<TLSWrap>::SelectNextProtoCallback(
void* arg);
#endif
template int SSLWrap<TLSWrap>::TLSExtStatusCallback(SSL* s, void* arg);
template void SSLWrap<TLSWrap>::DestroySSL();


static void crypto_threadid_cb(CRYPTO_THREADID* tid) {
Expand Down Expand Up @@ -1871,6 +1872,16 @@ void SSLWrap<Base>::SSLGetter(Local<String> property,
}


template <class Base>
void SSLWrap<Base>::DestroySSL() {
if (ssl_ == nullptr)
return;

SSL_free(ssl_);
ssl_ = nullptr;
}


void Connection::OnClientHelloParseEnd(void* arg) {
Connection* conn = static_cast<Connection*>(arg);

Expand Down
7 changes: 3 additions & 4 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,7 @@ class SSLWrap {
}

virtual ~SSLWrap() {
if (ssl_ != nullptr) {
SSL_free(ssl_);
ssl_ = nullptr;
}
DestroySSL();
if (next_sess_ != nullptr) {
SSL_SESSION_free(next_sess_);
next_sess_ = nullptr;
Expand Down Expand Up @@ -221,6 +218,8 @@ class SSLWrap {
static void SSLGetter(v8::Local<v8::String> property,
const v8::PropertyCallbackInfo<v8::Value>& info);

void DestroySSL();

inline Environment* ssl_env() const {
return env_;
}
Expand Down
39 changes: 34 additions & 5 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ void TLSWrap::Receive(const FunctionCallbackInfo<Value>& args) {
uv_buf_t buf;

// Copy given buffer entirely or partiall if handle becomes closed
while (len > 0 && !wrap->IsClosing()) {
while (len > 0 && wrap->IsAlive() && !wrap->IsClosing()) {
wrap->stream_->OnAlloc(len, &buf);
size_t copy = buf.len > len ? len : buf.len;
memcpy(buf.base, data, copy);
Expand Down Expand Up @@ -282,6 +282,9 @@ void TLSWrap::EncOut() {
if (established_ && !write_item_queue_.IsEmpty())
MakePending();

if (ssl_ == nullptr)
return;

// No data to write
if (BIO_pending(enc_out_) == 0) {
if (clear_in_->Length() == 0)
Expand Down Expand Up @@ -396,7 +399,8 @@ void TLSWrap::ClearOut() {
if (eof_)
return;

CHECK_NE(ssl_, nullptr);
if (ssl_ == nullptr)
return;

char out[kClearOutChunkSize];
int read;
Expand Down Expand Up @@ -451,6 +455,9 @@ bool TLSWrap::ClearIn() {
if (!hello_parser_.IsEnded())
return false;

if (ssl_ == nullptr)
return false;

int written = 0;
while (clear_in_->Length() > 0) {
size_t avail = 0;
Expand Down Expand Up @@ -503,7 +510,7 @@ int TLSWrap::GetFD() {


bool TLSWrap::IsAlive() {
return stream_->IsAlive();
return ssl_ != nullptr && stream_->IsAlive();
}


Expand Down Expand Up @@ -573,6 +580,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
return 0;
}

if (ssl_ == nullptr)
return UV_EPROTO;

int written = 0;
for (i = 0; i < count; i++) {
written = SSL_write(ssl_, bufs[i].base, bufs[i].len);
Expand Down Expand Up @@ -660,7 +670,10 @@ void TLSWrap::DoRead(ssize_t nread,
}

// Only client connections can receive data
CHECK_NE(ssl_, nullptr);
if (ssl_ == nullptr) {
OnRead(UV_EPROTO, nullptr);
return;
}

// Commit read data
NodeBIO* enc_in = NodeBIO::FromBIO(enc_in_);
Expand All @@ -680,7 +693,7 @@ void TLSWrap::DoRead(ssize_t nread,


int TLSWrap::DoShutdown(ShutdownWrap* req_wrap) {
if (SSL_shutdown(ssl_) == 0)
if (ssl_ != nullptr && SSL_shutdown(ssl_) == 0)
SSL_shutdown(ssl_);
shutdown_ = true;
EncOut();
Expand All @@ -696,6 +709,9 @@ void TLSWrap::SetVerifyMode(const FunctionCallbackInfo<Value>& args) {
if (args.Length() < 2 || !args[0]->IsBoolean() || !args[1]->IsBoolean())
return env->ThrowTypeError("Bad arguments, expected two booleans");

if (wrap->ssl_ == nullptr)
return env->ThrowTypeError("SetVerifyMode after destroySSL");

int verify_mode;
if (wrap->is_server()) {
bool request_cert = args[0]->IsTrue();
Expand Down Expand Up @@ -735,6 +751,14 @@ void TLSWrap::EnableHelloParser(const FunctionCallbackInfo<Value>& args) {
}


void TLSWrap::DestroySSL(const FunctionCallbackInfo<Value>& args) {
TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());
wrap->SSLWrap<TLSWrap>::DestroySSL();
delete wrap->clear_in_;
wrap->clear_in_ = nullptr;
}


void TLSWrap::OnClientHelloParseEnd(void* arg) {
TLSWrap* c = static_cast<TLSWrap*>(arg);
c->Cycle();
Expand All @@ -747,6 +771,8 @@ void TLSWrap::GetServername(const FunctionCallbackInfo<Value>& args) {

TLSWrap* wrap = Unwrap<TLSWrap>(args.Holder());

CHECK_NE(wrap->ssl_, nullptr);

const char* servername = SSL_get_servername(wrap->ssl_,
TLSEXT_NAMETYPE_host_name);
if (servername != nullptr) {
Expand All @@ -771,6 +797,8 @@ void TLSWrap::SetServername(const FunctionCallbackInfo<Value>& args) {
if (!wrap->is_client())
return;

CHECK_NE(wrap->ssl_, nullptr);

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
node::Utf8Value servername(env->isolate(), args[0].As<String>());
SSL_set_tlsext_host_name(wrap->ssl_, *servername);
Expand Down Expand Up @@ -830,6 +858,7 @@ void TLSWrap::Initialize(Handle<Object> target,
env->SetProtoMethod(t, "setVerifyMode", SetVerifyMode);
env->SetProtoMethod(t, "enableSessionCallbacks", EnableSessionCallbacks);
env->SetProtoMethod(t, "enableHelloParser", EnableHelloParser);
env->SetProtoMethod(t, "destroySSL", DestroySSL);

StreamBase::AddMethods<TLSWrap>(env, t, StreamBase::kFlagHasWritev);
SSLWrap<TLSWrap>::AddMethods(env, t);
Expand Down
1 change: 1 addition & 0 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ class TLSWrap : public crypto::SSLWrap<TLSWrap>,
const v8::FunctionCallbackInfo<v8::Value>& args);
static void EnableHelloParser(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void DestroySSL(const v8::FunctionCallbackInfo<v8::Value>& args);

#ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
static void GetServername(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down

0 comments on commit 2d241b3

Please sign in to comment.