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.

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
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ exports.createSecureContext = function createSecureContext(options, context) {
}
}

// Do not keep read/write buffers in free list
if (options.singleUse) {
c.singleUse = true;
c.context.setFreeListLength(0);
}

return c;
};

Expand Down
13 changes: 13 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,9 +295,17 @@ TLSSocket.prototype._wrapHandle = function(handle) {
}
});

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

return res;
};

TLSSocket.prototype._destroySSL = function _destroySSL() {
this.ssl.destroySSL();
if (this.ssl._secureContext.singleUse)
this.ssl._secureContext.context.close();
};

TLSSocket.prototype._init = function(socket, wrap) {
var self = this;
var options = this._tlsOptions;
Expand Down Expand Up @@ -416,6 +424,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 Expand Up @@ -853,6 +864,8 @@ exports.connect = function(/* [port, host], options, cb */) {
};

options = util._extend(defaults, options || {});
if (!options.keepAlive)
options.singleUse = true;

assert(typeof options.checkServerIdentity === 'function');

Expand Down
20 changes: 20 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 @@ -264,6 +265,7 @@ void SecureContext::Initialize(Environment* env, Handle<Object> target) {
env->SetProtoMethod(t, "loadPKCS12", SecureContext::LoadPKCS12);
env->SetProtoMethod(t, "getTicketKeys", SecureContext::GetTicketKeys);
env->SetProtoMethod(t, "setTicketKeys", SecureContext::SetTicketKeys);
env->SetProtoMethod(t, "setFreeListLength", SecureContext::SetFreeListLength);
env->SetProtoMethod(t, "getCertificate", SecureContext::GetCertificate<true>);
env->SetProtoMethod(t, "getIssuer", SecureContext::GetCertificate<false>);

Expand Down Expand Up @@ -932,6 +934,13 @@ void SecureContext::SetTicketKeys(const FunctionCallbackInfo<Value>& args) {
}


void SecureContext::SetFreeListLength(const FunctionCallbackInfo<Value>& args) {
SecureContext* wrap = Unwrap<SecureContext>(args.Holder());

wrap->ctx_->freelist_max_len = args[0]->Int32Value();
}


void SecureContext::CtxGetter(Local<String> property,
const PropertyCallbackInfo<Value>& info) {
HandleScope scope(info.GetIsolate());
Expand Down Expand Up @@ -1871,6 +1880,17 @@ void SSLWrap<Base>::SSLGetter(Local<String> property,
}


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

SSL_free(ssl_);
env_->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize);
ssl_ = nullptr;
}


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

Expand Down
19 changes: 15 additions & 4 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class SecureContext : public BaseObject {
static const int kMaxSessionSize = 10 * 1024;

protected:
static const int64_t kExternalSize = sizeof(SSL_CTX);

static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand All @@ -84,6 +85,8 @@ class SecureContext : public BaseObject {
static void LoadPKCS12(const v8::FunctionCallbackInfo<v8::Value>& args);
static void GetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SetFreeListLength(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void CtxGetter(v8::Local<v8::String> property,
const v8::PropertyCallbackInfo<v8::Value>& info);

Expand All @@ -97,10 +100,12 @@ class SecureContext : public BaseObject {
cert_(nullptr),
issuer_(nullptr) {
MakeWeak<SecureContext>(this);
env->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize);
}

void FreeCTXMem() {
if (ctx_) {
env()->isolate()->AdjustAmountOfExternalAllocatedMemory(-kExternalSize);
if (ctx_->cert_store == root_cert_store) {
// SSL_CTX_free() will attempt to free the cert_store as well.
// Since we want our root_cert_store to stay around forever
Expand Down Expand Up @@ -140,14 +145,12 @@ class SSLWrap {
session_callbacks_(false),
new_session_wait_(false) {
ssl_ = SSL_new(sc->ctx_);
env_->isolate()->AdjustAmountOfExternalAllocatedMemory(kExternalSize);
CHECK_NE(ssl_, nullptr);
}

virtual ~SSLWrap() {
if (ssl_ != nullptr) {
SSL_free(ssl_);
ssl_ = nullptr;
}
DestroySSL();
if (next_sess_ != nullptr) {
SSL_SESSION_free(next_sess_);
next_sess_ = nullptr;
Expand All @@ -169,6 +172,12 @@ class SSLWrap {
inline bool is_waiting_new_session() const { return new_session_wait_; }

protected:
// Size allocated by OpenSSL: one for SSL structure, one for SSL3_STATE and
// some for buffers.
// NOTE: Actually it is much more than this
static const int64_t kExternalSize =
sizeof(SSL) + sizeof(SSL3_STATE) + 42 * 1024;

static void InitNPN(SecureContext* sc);
static void AddMethods(Environment* env, v8::Handle<v8::FunctionTemplate> t);

Expand Down Expand Up @@ -221,6 +230,8 @@ class SSLWrap {
static void SSLGetter(v8::Local<v8::String> property,
const v8::PropertyCallbackInfo<v8::Value>& info);

void DestroySSL();
Copy link
Contributor

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
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