tls,crypto: move NPN protcol data to hidden value

cherry-pick 7eee372 from v6-staging. This fix is to be consistent implementation with ALPN. Tow NPN protocol data in the persistent memebers move to hidden variables in the wrap object. PR-URL: #2564 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
nodejs · Feb 1, 2017 · ef547f3 · ef547f3
1 parent e750f14
commit ef547f3
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 27 deletions.
diff --git a/src/env.h b/src/env.h
@@ -134,6 +134,7 @@ namespace node {
   V(netmask_string, "netmask")                                                \
   V(nice_string, "nice")                                                      \
   V(nlink_string, "nlink")                                                    \
+  V(npn_buffer_string, "npnBuffer")                                           \
   V(nsname_string, "nsname")                                                  \
   V(ocsp_request_string, "OCSPRequest")                                       \
   V(offset_string, "offset")                                                  \
@@ -184,6 +185,7 @@ namespace node {
   V(serial_string, "serial")                                                  \
   V(scavenge_string, "scavenge")                                              \
   V(scopeid_string, "scopeid")                                                \
+  V(selected_npn_buffer_string, "selectedNpnBuffer")                          \
   V(sent_shutdown_string, "sentShutdown")                                     \
   V(serial_number_string, "serialNumber")                                     \
   V(service_string, "service")                                                \

diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -1960,14 +1960,17 @@ int SSLWrap<Base>::AdvertiseNextProtoCallback(SSL* s,
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
 
-  if (w->npn_protos_.IsEmpty()) {
+  Local<Value> npn_buffer =
+      w->object()->GetHiddenValue(env->npn_buffer_string());
+
+  if (npn_buffer.IsEmpty()) {
     // No initialization - no NPN protocols
     *data = reinterpret_cast<const unsigned char*>("");
     *len = 0;
   } else {
-    Local<Object> obj = PersistentToLocal(env->isolate(), w->npn_protos_);
-    *data = reinterpret_cast<const unsigned char*>(Buffer::Data(obj));
-    *len = Buffer::Length(obj);
+    CHECK(Buffer::HasInstance(npn_buffer));
+    *data = reinterpret_cast<const unsigned char*>(Buffer::Data(npn_buffer));
+    *len = Buffer::Length(npn_buffer);
   }
 
   return SSL_TLSEXT_ERR_OK;
@@ -1986,25 +1989,27 @@ int SSLWrap<Base>::SelectNextProtoCallback(SSL* s,
   HandleScope handle_scope(env->isolate());
   Context::Scope context_scope(env->context());
 
-  // Release old protocol handler if present
-  w->selected_npn_proto_.Reset();
+  Local<Value> npn_buffer =
+      w->object()->GetHiddenValue(env->npn_buffer_string());
 
-  if (w->npn_protos_.IsEmpty()) {
+  if (npn_buffer.IsEmpty()) {
     // We should at least select one protocol
     // If server is using NPN
     *out = reinterpret_cast<unsigned char*>(const_cast<char*>("http/1.1"));
     *outlen = 8;
 
     // set status: unsupported
-    w->selected_npn_proto_.Reset(env->isolate(), False(env->isolate()));
+    bool r = w->object()->SetHiddenValue(env->selected_npn_buffer_string(),
+                                         False(env->isolate()));
+    CHECK(r);
 
     return SSL_TLSEXT_ERR_OK;
   }
 
-  Local<Object> obj = PersistentToLocal(env->isolate(), w->npn_protos_);
+  CHECK(Buffer::HasInstance(npn_buffer));
   const unsigned char* npn_protos =
-      reinterpret_cast<const unsigned char*>(Buffer::Data(obj));
-  size_t len = Buffer::Length(obj);
+      reinterpret_cast<const unsigned char*>(Buffer::Data(npn_buffer));
+  size_t len = Buffer::Length(npn_buffer);
 
   int status = SSL_select_next_proto(out, outlen, in, inlen, npn_protos, len);
   Local<Value> result;
@@ -2022,8 +2027,9 @@ int SSLWrap<Base>::SelectNextProtoCallback(SSL* s,
       break;
   }
 
-  if (!result.IsEmpty())
-    w->selected_npn_proto_.Reset(env->isolate(), result);
+  bool r = w->object()->SetHiddenValue(env->selected_npn_buffer_string(),
+                                       result);
+  CHECK(r);
 
   return SSL_TLSEXT_ERR_OK;
 }
@@ -2036,9 +2042,12 @@ void SSLWrap<Base>::GetNegotiatedProto(
   ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder());
 
   if (w->is_client()) {
-    if (w->selected_npn_proto_.IsEmpty() == false) {
-      args.GetReturnValue().Set(w->selected_npn_proto_);
-    }
+    Local<Value> selected_npn_buffer =
+        w->object()->GetHiddenValue(w->env()->selected_npn_buffer_string());
+
+    if (selected_npn_buffer.IsEmpty() == false)
+      args.GetReturnValue().Set(selected_npn_buffer);
+
     return;
   }
 
@@ -2062,9 +2071,11 @@ void SSLWrap<Base>::SetNPNProtocols(const FunctionCallbackInfo<Value>& args) {
   Environment* env = w->ssl_env();
 
   if (args.Length() < 1 || !Buffer::HasInstance(args[0]))
-    return w->env()->ThrowTypeError("Must give a Buffer as first argument");
+    return env->ThrowTypeError("Must give a Buffer as first argument");
 
-  w->npn_protos_.Reset(args.GetIsolate(), args[0].As<Object>());
+  Local<Value> npn_buffer =  Local<Value>::New(env->isolate(), args[0]);
+  bool r = w->object()->SetHiddenValue(env->npn_buffer_string(), npn_buffer);
+  CHECK(r);
 }
 #endif  // OPENSSL_NPN_NEGOTIATED
 

diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -193,10 +193,6 @@ class SSLWrap {
       next_sess_ = nullptr;
     }
 
-#ifdef OPENSSL_NPN_NEGOTIATED
-    npn_protos_.Reset();
-    selected_npn_proto_.Reset();
-#endif
 #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
     sni_context_.Reset();
 #endif
@@ -313,11 +309,6 @@ class SSLWrap {
   v8::Persistent<v8::Object> ocsp_response_;
 #endif  // NODE__HAVE_TLSEXT_STATUS_CB
 
-#ifdef OPENSSL_NPN_NEGOTIATED
-  v8::Persistent<v8::Object> npn_protos_;
-  v8::Persistent<v8::Value> selected_npn_proto_;
-#endif  // OPENSSL_NPN_NEGOTIATED
-
 #ifdef SSL_CTRL_SET_TLSEXT_SERVERNAME_CB
   v8::Persistent<v8::Value> sni_context_;
 #endif