Skip to content

Lower TLS memory footprint #1529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
indutny wants to merge 4 commits into from
Closed

Lower TLS memory footprint #1529

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e066c6
tls: destroy SSL once it is out of use
indutny Apr 26, 2015
6c9620a
crypto: track external memory for SSL structures
indutny Apr 26, 2015
dbac061
tls: zero SSL_CTX freelist for a singleUse socket
indutny Apr 26, 2015
1357735
tls: destroy singleUse context immediately
indutny Apr 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
tls: destroy SSL once it is out of use 
Do not keep SSL structure in memory once socket is closed. This should
lower the memory usage in many cases.

Fix: #1522
  • Loading branch information
@indutny
indutny committed Apr 27, 2015
commit 4e066c6ce608691fbeb90f518c91d6d70522950c
9 changes: 9 additions & 0 deletions lib/_tls_wrap.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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();
Copy link
Copy Markdown
Contributor

@shigeki shigeki Apr 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@indutny This causes a link error with gcc-4.8.4 on my Ubuntu but works fine with gcc-4.9.2 on Ubuntu and with clang on Mac though I've not checked yet whether it comes from a gcc bug or not.

/home/ohtsu/github/shigeki/io.js/out/Release/obj.target/iojs/src/tls_wrap.o: In function `node::crypto::SSLWrap<node::TLSWrap>::~SSLWrap()':
tls_wrap.cc:(.text._ZN4node6crypto7SSLWrapINS_7TLSWrapEED0Ev[_ZN4node6crypto7SSLWrapINS_7TLSWrapEED0Ev]+0x14): undefined reference to `node::crypto::SSLWrap<node::TLSWrap>::DestroySSL()'
/home/ohtsu/github/shigeki/io.js/out/Release/obj.target/iojs/src/tls_wrap.o: In function `node::TLSWrap::~TLSWrap()':
tls_wrap.cc:(.text._ZN4node7TLSWrapD2Ev+0x1fe): undefined reference to `node::crypto::SSLWrap<node::TLSWrap>::DestroySSL()'
collect2: error: ld returned 1 exit status
iojs.target.mk:198: recipe for target '/home/ohtsu/github/shigeki/io.js/out/Release/iojs' failed
make[1]: *** [/home/ohtsu/github/shigeki/io.js/out/Release/iojs] Error 1
make[1]: Leaving directory '/home/ohtsu/github/shigeki/io.js/out'
Makefile:35: recipe for target 'iojs' failed
make: *** [iojs] Error 2
ohtsu@u1504:~/github/shigeki/io.js$ g++ --version
g++ (Ubuntu 4.8.4-1ubuntu15) 4.8.4


inline Environment* ssl_env() const {
return env_;
}
Expand Down
39 changes: 34 additions & 5 deletions src/tls_wrap.cc
View file
Open in desktop
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
View file
Open in desktop
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