diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 1237157da3f775..afe607b4d08a4c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -483,7 +483,7 @@ void SecureContext::Init(const FunctionCallbackInfo& args) { // Takes a string or buffer and loads it into a BIO. // Caller responsible for BIO_free_all-ing the returned object. -static BIO* LoadBIO(Environment* env, Local v) { +static BIOPointer LoadBIO(Environment* env, Local v) { HandleScope scope(env->isolate()); if (v->IsString()) { @@ -738,9 +738,12 @@ static X509_STORE* NewRootCertStore() { if (root_certs_vector.empty()) { for (size_t i = 0; i < arraysize(root_certs); i++) { - BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i])); - X509* x509 = PEM_read_bio_X509(bp, nullptr, NoPasswordCallback, nullptr); - BIO_free(bp); + X509* x509 = + PEM_read_bio_X509(NodeBIO::NewFixed(root_certs[i], + strlen(root_certs[i])).get(), + nullptr, // no re-use of X509 structure + NoPasswordCallback, + nullptr); // no callback data // Parse errors from the built-in roots are fatal. CHECK_NOT_NULL(x509); diff --git a/src/node_crypto_bio.cc b/src/node_crypto_bio.cc index 094bb9cc1f8822..ab68c0a08111f2 100644 --- a/src/node_crypto_bio.cc +++ b/src/node_crypto_bio.cc @@ -38,36 +38,32 @@ namespace crypto { #endif -BIO* NodeBIO::New() { +BIOPointer NodeBIO::New(Environment* env) { // The const_cast doesn't violate const correctness. OpenSSL's usage of // BIO_METHOD is effectively const but BIO_new() takes a non-const argument. - return BIO_new(const_cast(GetMethod())); + BIOPointer bio(BIO_new(const_cast(GetMethod()))); + if (bio && env != nullptr) + NodeBIO::FromBIO(bio.get())->env_ = env; + return bio; } -BIO* NodeBIO::NewFixed(const char* data, size_t len) { - BIO* bio = New(); +BIOPointer NodeBIO::NewFixed(const char* data, size_t len, Environment* env) { + BIOPointer bio = New(env); - if (bio == nullptr || + if (!bio || len > INT_MAX || - BIO_write(bio, data, len) != static_cast(len) || - BIO_set_mem_eof_return(bio, 0) != 1) { - BIO_free(bio); - return nullptr; + BIO_write(bio.get(), data, len) != static_cast(len) || + BIO_set_mem_eof_return(bio.get(), 0) != 1) { + return BIOPointer(); } return bio; } -void NodeBIO::AssignEnvironment(Environment* env) { - env_ = env; -} - - int NodeBIO::New(BIO* bio) { BIO_set_data(bio, new NodeBIO()); - BIO_set_init(bio, 1); return 1; diff --git a/src/node_crypto_bio.h b/src/node_crypto_bio.h index b4aa85f8fa36fa..e2303fe7397b5b 100644 --- a/src/node_crypto_bio.h +++ b/src/node_crypto_bio.h @@ -28,29 +28,26 @@ #include "env-inl.h" #include "util-inl.h" #include "v8.h" +#include "node_crypto.h" namespace node { namespace crypto { +// This class represents buffers for OpenSSL I/O, implemented as a singly-linked +// list of chunks. It can be used both for writing data from Node to OpenSSL +// and back, but only one direction per instance. +// The structure is only accessed, and owned by, the OpenSSL BIOPointer +// (a.k.a. std::unique_ptr). class NodeBIO : public MemoryRetainer { public: - NodeBIO() : env_(nullptr), - initial_(kInitialBufferLength), - length_(0), - eof_return_(-1), - read_head_(nullptr), - write_head_(nullptr) { - } - ~NodeBIO(); - static BIO* New(); + static BIOPointer New(Environment* env = nullptr); // NewFixed takes a copy of `len` bytes from `data` and returns a BIO that, // when read from, returns those bytes followed by EOF. - static BIO* NewFixed(const char* data, size_t len); - - void AssignEnvironment(Environment* env); + static BIOPointer NewFixed(const char* data, size_t len, + Environment* env = nullptr); // Move read head to next buffer if needed void TryMoveReadHead(); @@ -161,12 +158,12 @@ class NodeBIO : public MemoryRetainer { char* data_; }; - Environment* env_; - size_t initial_; - size_t length_; - int eof_return_; - Buffer* read_head_; - Buffer* write_head_; + Environment* env_ = nullptr; + size_t initial_ = kInitialBufferLength; + size_t length_ = 0; + int eof_return_ = -1; + Buffer* read_head_ = nullptr; + Buffer* write_head_ = nullptr; }; } // namespace crypto diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 3efa6adb4edb0e..3e6c66d5e36d07 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -112,11 +112,9 @@ void TLSWrap::NewSessionDoneCb() { void TLSWrap::InitSSL() { - // Initialize SSL - enc_in_ = crypto::NodeBIO::New(); - enc_out_ = crypto::NodeBIO::New(); - crypto::NodeBIO::FromBIO(enc_in_)->AssignEnvironment(env()); - crypto::NodeBIO::FromBIO(enc_out_)->AssignEnvironment(env()); + // Initialize SSL – OpenSSL takes ownership of these. + enc_in_ = crypto::NodeBIO::New(env()).release(); + enc_out_ = crypto::NodeBIO::New(env()).release(); SSL_set_bio(ssl_.get(), enc_in_, enc_out_);