src: use smart pointers for NodeBIO

PR-URL: #21984 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]>

Author: addaleax

src/node_crypto.cc

@@ -483,7 +483,7 @@ void SecureContext::Init(const FunctionCallbackInfo<Value>& args){
 
 // Takes a string or buffer and loads it into a BIO.
 // Caller responsible for BIO_free_all-ing the returned object.
-staticBIO*LoadBIO(Environment* env, Local<Value> v){
+staticBIOPointerLoadBIO(Environment* env, Local<Value> 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);

src/node_crypto_bio.cc

@@ -38,36 +38,32 @@ namespace crypto{
 #endif
 
 
-BIO*NodeBIO::New(){
+BIOPointerNodeBIO::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.
-returnBIO_new(const_cast<BIO_METHOD*>(GetMethod()));
+ BIOPointer bio(BIO_new(const_cast<BIO_METHOD*>(GetMethod())));
+if (bio && env != nullptr)
+NodeBIO::FromBIO(bio.get())->env_ = env;
+return bio;
 }
 
 
-BIO*NodeBIO::NewFixed(constchar* data, size_t len){
-BIO* bio = New();
+BIOPointerNodeBIO::NewFixed(constchar* 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<int>(len) ||
-BIO_set_mem_eof_return(bio, 0) != 1){
-BIO_free(bio);
-returnnullptr;
+BIO_write(bio.get(), data, len) != static_cast<int>(len) ||
+BIO_set_mem_eof_return(bio.get(), 0) != 1){
+returnBIOPointer();
  }
 
 return bio;
 }
 
 
-voidNodeBIO::AssignEnvironment(Environment* env){
- env_ = env;
-}
-
-
 intNodeBIO::New(BIO* bio){
 BIO_set_data(bio, newNodeBIO());
-
 BIO_set_init(bio, 1);
 
 return1;

src/node_crypto_bio.h

@@ -24,6 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include"node_crypto.h"
 #include"openssl/bio.h"
 #include"env-inl.h"
 #include"util-inl.h"
@@ -32,25 +33,21 @@
 namespacenode{
 namespacecrypto{
 
+// 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<BIO>).
 classNodeBIO : publicMemoryRetainer{
 public:
-NodeBIO() : env_(nullptr),
-initial_(kInitialBufferLength),
- length_(0),
- eof_return_(-1),
- read_head_(nullptr),
- write_head_(nullptr){
- }
-
 ~NodeBIO();
 
-staticBIO*New();
+staticBIOPointerNew(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(constchar* data, size_t len);
-
-voidAssignEnvironment(Environment* env);
+static BIOPointer NewFixed(constchar* data, size_t len,
+ Environment* env = nullptr);
 
 // Move read head to next buffer if needed
 voidTryMoveReadHead();
@@ -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

src/tls_wrap.cc

@@ -112,11 +112,9 @@ void TLSWrap::NewSessionDoneCb(){
 
 
 voidTLSWrap::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_);