src,crypto: refactor crypto_tls.*

RaisinTen · danielleadams · commit cebf40396fe7 · 2022-01-31T22:53:27.000-05:00
By the design of `GetSSLError()`, the V8 API was unnecessarily being accessed in places where it eventually didn't get used. So this refactor inlines the function appropriately in places where it was being used. Also, this replaces uses of `AllocatedBuffers` with `BackingStore`s. Signed-off-by: Darshan Sen <darshan.sen@postman.com> PR-URL: #40675 Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc
@@ -37,10 +37,11 @@
 namespace node {
 
 using v8::Array;
+using v8::ArrayBuffer;
 using v8::ArrayBufferView;
+using v8::BackingStore;
 using v8::Context;
 using v8::DontDelete;
-using v8::EscapableHandleScope;
 using v8::Exception;
 using v8::False;
 using v8::Function;
@@ -313,6 +314,17 @@ inline bool Set(
       OneByteString(env->isolate(), value))
           .IsNothing();
 }
+
+std::string GetBIOError() {
+  std::string ret;
+  ERR_print_errors_cb(
+      [](const char* str, size_t len, void* opaque) {
+        static_cast<std::string*>(opaque)->assign(str, len);
+        return 0;
+      },
+      static_cast<void*>(&ret));
+  return ret;
+}
 }  // namespace
 
 TLSWrap::TLSWrap(Environment* env,
@@ -572,7 +584,8 @@ void TLSWrap::EncOut() {
   // No encrypted output ready to write to the underlying stream.
   if (BIO_pending(enc_out_) == 0) {
     Debug(this, "No pending encrypted output");
-    if (pending_cleartext_input_.size() == 0) {
+    if (!pending_cleartext_input_ ||
+        pending_cleartext_input_->ByteLength() == 0) {
       if (!in_dowrite_) {
         Debug(this, "No pending cleartext input, not inside DoWrite()");
         InvokeQueued(0);
@@ -665,84 +678,9 @@ void TLSWrap::OnStreamAfterWrite(WriteWrap* req_wrap, int status) {
   EncOut();
 }
 
-MaybeLocal<Value> TLSWrap::GetSSLError(int status, int* err, std::string* msg) {
-  EscapableHandleScope scope(env()->isolate());
-
-  // ssl_ is already destroyed in reading EOF by close notify alert.
-  if (ssl_ == nullptr)
-    return MaybeLocal<Value>();
-
-  *err = SSL_get_error(ssl_.get(), status);
-  switch (*err) {
-    case SSL_ERROR_NONE:
-    case SSL_ERROR_WANT_READ:
-    case SSL_ERROR_WANT_WRITE:
-    case SSL_ERROR_WANT_X509_LOOKUP:
-      return MaybeLocal<Value>();
-
-    case SSL_ERROR_ZERO_RETURN:
-      return scope.Escape(env()->zero_return_string());
-
-    case SSL_ERROR_SSL:
-    case SSL_ERROR_SYSCALL:
-      {
-        unsigned long ssl_err = ERR_peek_error();  // NOLINT(runtime/int)
-        BIO* bio = BIO_new(BIO_s_mem());
-        ERR_print_errors(bio);
-
-        BUF_MEM* mem;
-        BIO_get_mem_ptr(bio, &mem);
-
-        Isolate* isolate = env()->isolate();
-        Local<Context> context = isolate->GetCurrentContext();
-
-        Local<String> message = OneByteString(isolate, mem->data, mem->length);
-        Local<Value> exception = Exception::Error(message);
-        Local<Object> obj =
-            exception->ToObject(context).FromMaybe(Local<Object>());
-        if (UNLIKELY(obj.IsEmpty()))
-          return MaybeLocal<Value>();
-
-        const char* ls = ERR_lib_error_string(ssl_err);
-        const char* fs = ERR_func_error_string(ssl_err);
-        const char* rs = ERR_reason_error_string(ssl_err);
-
-        if (!Set(env(), obj, env()->library_string(), ls) ||
-            !Set(env(), obj, env()->function_string(), fs)) {
-          return MaybeLocal<Value>();
-        }
-
-        if (rs != nullptr) {
-          if (!Set(env(), obj, env()->reason_string(), rs))
-            return MaybeLocal<Value>();
-
-          // SSL has no API to recover the error name from the number, so we
-          // transform reason strings like "this error" to "ERR_SSL_THIS_ERROR",
-          // which ends up being close to the original error macro name.
-          std::string code(rs);
-
-          for (auto& c : code)
-            c = (c == ' ') ? '_' : ToUpper(c);
-
-          if (!Set(env(), obj,
-                   env()->code_string(),
-                   ("ERR_SSL_" + code).c_str())) {
-            return MaybeLocal<Value>();
-          }
-        }
-
-        if (msg != nullptr)
-          msg->assign(mem->data, mem->data + mem->length);
-
-        BIO_free_all(bio);
-
-        return scope.Escape(exception);
-      }
-
-    default:
-      UNREACHABLE();
-  }
-  UNREACHABLE();
+int TLSWrap::GetSSLError(int status) const {
+  // ssl_ might already be destroyed for reading EOF from a close notify alert.
+  return ssl_ != nullptr ? SSL_get_error(ssl_.get(), status) : 0;
 }
 
 void TLSWrap::ClearOut() {
@@ -809,24 +747,61 @@ void TLSWrap::ClearOut() {
   // See node#1642 and SSL_read(3SSL) for details.
   if (read <= 0) {
     HandleScope handle_scope(env()->isolate());
-    int err;
+    Local<Value> error;
+    int err = GetSSLError(read);
+    switch (err) {
+      case SSL_ERROR_ZERO_RETURN:
+        // Ignore ZERO_RETURN after EOF, it is basically not an error.
+        if (eof_) return;
+        error = env()->zero_return_string();
+        break;
 
-    Local<Value> arg = GetSSLError(read, &err, nullptr)
-        .FromMaybe(Local<Value>());
+      case SSL_ERROR_SSL:
+      case SSL_ERROR_SYSCALL:
+        {
+          unsigned long ssl_err = ERR_peek_error();  // NOLINT(runtime/int)
+
+          Local<Context> context = env()->isolate()->GetCurrentContext();
+          if (UNLIKELY(context.IsEmpty())) return;
+          const std::string error_str = GetBIOError();
+          Local<String> message = OneByteString(
+              env()->isolate(), error_str.c_str(), error_str.size());
+          if (UNLIKELY(message.IsEmpty())) return;
+          error = Exception::Error(message);
+          if (UNLIKELY(error.IsEmpty())) return;
+          Local<Object> obj;
+          if (UNLIKELY(!error->ToObject(context).ToLocal(&obj))) return;
+
+          const char* ls = ERR_lib_error_string(ssl_err);
+          const char* fs = ERR_func_error_string(ssl_err);
+          const char* rs = ERR_reason_error_string(ssl_err);
+          if (!Set(env(), obj, env()->library_string(), ls) ||
+              !Set(env(), obj, env()->function_string(), fs) ||
+              !Set(env(), obj, env()->reason_string(), rs, false)) return;
+          // SSL has no API to recover the error name from the number, so we
+          // transform reason strings like "this error" to "ERR_SSL_THIS_ERROR",
+          // which ends up being close to the original error macro name.
+          std::string code(rs);
+          // TODO(RaisinTen): Pass an appropriate execution policy when it is
+          // implemented in our supported compilers.
+          std::transform(code.begin(), code.end(), code.begin(),
+                         [](char c) { return c == ' ' ? '_' : ToUpper(c); });
+          if (!Set(env(), obj,
+                   env()->code_string(), ("ERR_SSL_" + code).c_str())) return;
+        }
+        break;
 
-    // Ignore ZERO_RETURN after EOF, it is basically not a error
-    if (err == SSL_ERROR_ZERO_RETURN && eof_)
-      return;
+      default:
+        return;
+    }
 
-    if (LIKELY(!arg.IsEmpty())) {
-      Debug(this, "Got SSL error (%d), calling onerror", err);
-      // When TLS Alert are stored in wbio,
-      // it should be flushed to socket before destroyed.
-      if (BIO_pending(enc_out_) != 0)
-        EncOut();
+    Debug(this, "Got SSL error (%d), calling onerror", err);
+    // When TLS Alert are stored in wbio,
+    // it should be flushed to socket before destroyed.
+    if (BIO_pending(enc_out_) != 0)
+      EncOut();
 
-      MakeCallback(env()->onerror_string(), 1, &arg);
-    }
+    MakeCallback(env()->onerror_string(), 1, &error);
   }
 }
 
@@ -843,18 +818,19 @@ void TLSWrap::ClearIn() {
     return;
   }
 
-  if (pending_cleartext_input_.size() == 0) {
+  if (!pending_cleartext_input_ ||
+      pending_cleartext_input_->ByteLength() == 0) {
     Debug(this, "Returning from ClearIn(), no pending data");
     return;
   }
 
-  AllocatedBuffer data = std::move(pending_cleartext_input_);
+  std::unique_ptr<BackingStore> bs = std::move(pending_cleartext_input_);
   MarkPopErrorOnReturn mark_pop_error_on_return;
 
-  NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(data.size());
-  int written = SSL_write(ssl_.get(), data.data(), data.size());
-  Debug(this, "Writing %zu bytes, written = %d", data.size(), written);
-  CHECK(written == -1 || written == static_cast<int>(data.size()));
+  NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(bs->ByteLength());
+  int written = SSL_write(ssl_.get(), bs->Data(), bs->ByteLength());
+  Debug(this, "Writing %zu bytes, written = %d", bs->ByteLength(), written);
+  CHECK(written == -1 || written == static_cast<int>(bs->ByteLength()));
 
   // All written
   if (written != -1) {
@@ -863,24 +839,20 @@ void TLSWrap::ClearIn() {
   }
 
   // Error or partial write
-  HandleScope handle_scope(env()->isolate());
-  Context::Scope context_scope(env()->context());
-
-  int err;
-  std::string error_str;
-  MaybeLocal<Value> arg = GetSSLError(written, &err, &error_str);
-  if (!arg.IsEmpty()) {
+  int err = GetSSLError(written);
+  if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) {
     Debug(this, "Got SSL error (%d)", err);
     write_callback_scheduled_ = true;
     // TODO(@sam-github) Should forward an error object with
     // .code/.function/.etc, if possible.
-    return InvokeQueued(UV_EPROTO, error_str.c_str());
+    InvokeQueued(UV_EPROTO, GetBIOError().c_str());
+    return;
   }
 
   Debug(this, "Pushing data back");
   // Push back the not-yet-written data. This can be skipped in the error
   // case because no further writes would succeed anyway.
-  pending_cleartext_input_ = std::move(data);
+  pending_cleartext_input_ = std::move(bs);
 }
 
 std::string TLSWrap::diagnostic_name() const {
@@ -998,7 +970,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
     return 0;
   }
 
-  AllocatedBuffer data;
+  std::unique_ptr<BackingStore> bs;
   MarkPopErrorOnReturn mark_pop_error_on_return;
 
   int written = 0;
@@ -1012,36 +984,39 @@ int TLSWrap::DoWrite(WriteWrap* w,
   // and copying it when it could just be used.
 
   if (nonempty_count != 1) {
-    data = AllocatedBuffer::AllocateManaged(env(), length);
+    {
+      NoArrayBufferZeroFillScope no_zero_fill_scope(env()->isolate_data());
+      bs = ArrayBuffer::NewBackingStore(env()->isolate(), length);
+    }
     size_t offset = 0;
     for (i = 0; i < count; i++) {
-      memcpy(data.data() + offset, bufs[i].base, bufs[i].len);
+      memcpy(static_cast<char*>(bs->Data()) + offset,
+             bufs[i].base, bufs[i].len);
       offset += bufs[i].len;
     }
 
     NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(length);
-    written = SSL_write(ssl_.get(), data.data(), length);
+    written = SSL_write(ssl_.get(), bs->Data(), length);
   } else {
     // Only one buffer: try to write directly, only store if it fails
     uv_buf_t* buf = &bufs[nonempty_i];
     NodeBIO::FromBIO(enc_out_)->set_allocate_tls_hint(buf->len);
     written = SSL_write(ssl_.get(), buf->base, buf->len);
 
     if (written == -1) {
-      data = AllocatedBuffer::AllocateManaged(env(), length);
-      memcpy(data.data(), buf->base, buf->len);
+      NoArrayBufferZeroFillScope no_zero_fill_scope(env()->isolate_data());
+      bs = ArrayBuffer::NewBackingStore(env()->isolate(), length);
+      memcpy(bs->Data(), buf->base, buf->len);
     }
   }
 
   CHECK(written == -1 || written == static_cast<int>(length));
   Debug(this, "Writing %zu bytes, written = %d", length, written);
 
   if (written == -1) {
-    int err;
-    MaybeLocal<Value> arg = GetSSLError(written, &err, &error_);
-
     // If we stopped writing because of an error, it's fatal, discard the data.
-    if (!arg.IsEmpty()) {
+    int err = GetSSLError(written);
+    if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) {
       // TODO(@jasnell): What are we doing with the error?
       Debug(this, "Got SSL error (%d), returning UV_EPROTO", err);
       current_write_.reset();
@@ -1050,8 +1025,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
 
     Debug(this, "Saving data for later write");
     // Otherwise, save unwritten data so it can be written later by ClearIn().
-    CHECK_EQ(pending_cleartext_input_.size(), 0);
-    pending_cleartext_input_ = std::move(data);
+    CHECK(!pending_cleartext_input_ ||
+          pending_cleartext_input_->ByteLength() == 0);
+    pending_cleartext_input_ = std::move(bs);
   }
 
   // Write any encrypted/handshake output that may be ready.
@@ -1491,9 +1467,8 @@ void TLSWrap::MemoryInfo(MemoryTracker* tracker) const {
   tracker->TrackField("ocsp_response", ocsp_response_);
   tracker->TrackField("sni_context", sni_context_);
   tracker->TrackField("error", error_);
-  tracker->TrackFieldWithSize("pending_cleartext_input",
-                              pending_cleartext_input_.size(),
-                              "AllocatedBuffer");
+  if (pending_cleartext_input_)
+    tracker->TrackField("pending_cleartext_input", pending_cleartext_input_);
   if (enc_in_ != nullptr)
     tracker->TrackField("enc_in", NodeBIO::FromBIO(enc_in_));
   if (enc_out_ != nullptr)
@@ -1724,13 +1699,13 @@ void TLSWrap::VerifyError(const FunctionCallbackInfo<Value>& args) {
   const char* reason = X509_verify_cert_error_string(x509_verify_error);
   const char* code = X509ErrorCode(x509_verify_error);
 
-  Local<Object> exception =
+  Local<Object> error =
       Exception::Error(OneByteString(env->isolate(), reason))
           ->ToObject(env->isolate()->GetCurrentContext())
               .FromMaybe(Local<Object>());
 
-  if (Set(env, exception, env->code_string(), code))
-    args.GetReturnValue().Set(exception);
+  if (Set(env, error, env->code_string(), code))
+    args.GetReturnValue().Set(error);
 }
 
 void TLSWrap::GetCipher(const FunctionCallbackInfo<Value>& args) {
diff --git a/src/crypto/crypto_tls.h b/src/crypto/crypto_tls.h
@@ -27,7 +27,6 @@
 #include "crypto/crypto_context.h"
 #include "crypto/crypto_clienthello.h"
 
-#include "allocated_buffer.h"
 #include "async_wrap.h"
 #include "stream_wrap.h"
 #include "v8.h"
@@ -167,7 +166,7 @@ class TLSWrap : public AsyncWrap,
 
   int SetCACerts(SecureContext* sc);
 
-  v8::MaybeLocal<v8::Value> GetSSLError(int status, int* err, std::string* msg);
+  int GetSSLError(int status) const;
 
   static int SelectSNIContextCallback(SSL* s, int* ad, void* arg);
 
@@ -254,7 +253,7 @@ class TLSWrap : public AsyncWrap,
   BIO* enc_in_ = nullptr;   // StreamListener fills this for SSL_read().
   BIO* enc_out_ = nullptr;  // SSL_write()/handshake fills this for EncOut().
   // Waiting for ClearIn() to pass to SSL_write().
-  AllocatedBuffer pending_cleartext_input_;
+  std::unique_ptr<v8::BackingStore> pending_cleartext_input_;
   size_t write_size_ = 0;
   BaseObjectPtr<AsyncWrap> current_write_;
   BaseObjectPtr<AsyncWrap> current_empty_write_;
