Repository: nodejs/node
Pull request: src: update crypto objects to use DictionaryTemplate

More v8::DictionaryTemplate use

Changed files:
filename: src/crypto/crypto_common.cc
patch: @@ -37,7 +37,7 @@ using ncrypto::X509Pointer;
 using ncrypto::X509View;
 using v8::ArrayBuffer;
 using v8::BackingStoreInitializationMode;
-using v8::Context;
+using v8::DictionaryTemplate;
 using v8::EscapableHandleScope;
 using v8::Integer;
 using v8::Local;
@@ -180,76 +180,69 @@ Local<Value> maybeString(Environment* env,
 MaybeLocal<Object> GetCipherInfo(Environment* env, const SSLPointer& ssl) {
   if (ssl.getCipher() == nullptr) return MaybeLocal<Object>();
   EscapableHandleScope scope(env->isolate());
-  Local<Object> info = Object::New(env->isolate());
-
-  if (info->Set(env->context(),
-                env->name_string(),
-                maybeString(env, ssl.getCipherName()))
-          .IsNothing() ||
-      info->Set(env->context(),
-                env->standard_name_string(),
-                maybeString(env, ssl.getCipherStandardName()))
-          .IsNothing() ||
-      info->Set(env->context(),
-                env->version_string(),
-                maybeString(env, ssl.getCipherVersion()))
-          .IsNothing()) {
-    return MaybeLocal<Object>();
+
+  auto tmpl = env->cipherinfo_template();
+  if (tmpl.IsEmpty()) {
+    static constexpr std::string_view names[] = {
+        "name", "standardName", "version"};
+    tmpl = DictionaryTemplate::New(env->isolate(), names);
+    env->set_cipherinfo_template(tmpl);
   }
 
-  return scope.Escape(info);
+  MaybeLocal<Value> values[] = {
+      maybeString(env, ssl.getCipherName()),
+      maybeString(env, ssl.getCipherStandardName()),
+      maybeString(env, ssl.getCipherVersion()),
+  };
+
+  return scope.EscapeMaybe(NewDictionaryInstance(env->context(), tmpl, values));
 }
 
 MaybeLocal<Object> GetEphemeralKey(Environment* env, const SSLPointer& ssl) {
   CHECK(!ssl.isServer());
 
   EscapableHandleScope scope(env->isolate());
-  Local<Object> info = Object::New(env->isolate());
+
+  auto tmpl = env->ephemeral_key_template();
+  if (tmpl.IsEmpty()) {
+    static constexpr std::string_view names[] = {"type", "name", "size"};
+    tmpl = DictionaryTemplate::New(env->isolate(), names);
+    env->set_ephemeral_key_template(tmpl);
+  }
+
+  MaybeLocal<Value> values[] = {
+      Undefined(env->isolate()),  // type
+      Undefined(env->isolate()),  // name
+      Undefined(env->isolate()),  // size
+  };
   EVPKeyPointer key = ssl.getPeerTempKey();
-  if (!key) return scope.Escape(info);
-
-  Local<Context> context = env->context();
-
-  int kid = key.id();
-  switch (kid) {
-    case EVP_PKEY_DH:
-      if (info->Set(context, env->type_string(), env->dh_string())
-              .IsNothing() ||
-          info->Set(context,
-                    env->size_string(),
-                    Integer::New(env->isolate(), key.bits()))
-              .IsNothing()) {
-        return MaybeLocal<Object>();
+  if (EVPKeyPointer key = ssl.getPeerTempKey()) {
+    int kid = key.id();
+    switch (kid) {
+      case EVP_PKEY_DH: {
+        values[0] = env->dh_string();
+        values[2] = Integer::New(env->isolate(), key.bits());
+        break;
       }
-      break;
-    case EVP_PKEY_EC:
-    case EVP_PKEY_X25519:
-    case EVP_PKEY_X448:
-      {
+      case EVP_PKEY_EC:
+      case EVP_PKEY_X25519:
+      case EVP_PKEY_X448: {
         const char* curve_name;
         if (kid == EVP_PKEY_EC) {
           int nid = ECKeyPointer::GetGroupName(key);
           curve_name = OBJ_nid2sn(nid);
         } else {
           curve_name = OBJ_nid2sn(kid);
         }
-        if (info->Set(context, env->type_string(), env->ecdh_string())
-                .IsNothing() ||
-            info->Set(context,
-                      env->name_string(),
-                      OneByteString(env->isolate(), curve_name))
-                .IsNothing() ||
-            info->Set(context,
-                      env->size_string(),
-                      Integer::New(env->isolate(), key.bits()))
-                .IsNothing()) {
-          return MaybeLocal<Object>();
-        }
+        values[0] = env->ecdh_string();
+        values[1] = OneByteString(env->isolate(), curve_name);
+        values[2] = Integer::New(env->isolate(), key.bits());
+        break;
       }
-      break;
+    }
   }
 
-  return scope.Escape(info);
+  return scope.EscapeMaybe(NewDictionaryInstance(env->context(), tmpl, values));
 }
 
 MaybeLocal<Object> ECPointToBuffer(Environment* env,

filename: src/crypto/crypto_x509.cc
patch: @@ -33,6 +33,7 @@ using v8::BackingStoreOnFailureMode;
 using v8::Boolean;
 using v8::Context;
 using v8::Date;
+using v8::DictionaryTemplate;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -46,6 +47,7 @@ using v8::NewStringType;
 using v8::Object;
 using v8::String;
 using v8::Uint32;
+using v8::Undefined;
 using v8::Value;
 
 namespace crypto {
@@ -735,116 +737,86 @@ MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
 
 MaybeLocal<Object> X509ToObject(Environment* env, const X509View& cert) {
   EscapableHandleScope scope(env->isolate());
-  Local<Object> info = Object::New(env->isolate());
-
-  if (!Set<Value>(env,
-                  info,
-                  env->subject_string(),
-                  GetX509NameObject(env, cert.getSubjectName())) ||
-      !Set<Value>(env,
-                  info,
-                  env->issuer_string(),
-                  GetX509NameObject(env, cert.getIssuerName())) ||
-      !Set<Value>(env,
-                  info,
-                  env->subjectaltname_string(),
-                  GetSubjectAltNameString(env, cert)) ||
-      !Set<Value>(env,
-                  info,
-                  env->infoaccess_string(),
-                  GetInfoAccessString(env, cert)) ||
-      !Set<Boolean>(env,
-                    info,
-                    env->ca_string(),
-                    Boolean::New(env->isolate(), cert.isCA()))) [[unlikely]] {
-    return {};
-  }
 
-  if (!cert.ifRsa([&](const ncrypto::Rsa& rsa) {
-        auto pub_key = rsa.getPublicKey();
-        if (!Set<Value>(env,
-                        info,
-                        env->modulus_string(),
-                        GetModulusString(env, pub_key.n)) ||
-            !Set<Value>(env,
-                        info,
-                        env->bits_string(),
-                        Integer::New(env->isolate(),
-                                     BignumPointer::GetBitCount(pub_key.n))) ||
-            !Set<Value>(env,
-                        info,
-                        env->exponent_string(),
-                        GetExponentString(env, pub_key.e)) ||
-            !Set<Object>(env, info, env->pubkey_string(), GetPubKey(env, rsa)))
-            [[unlikely]] {
-          return false;
-        }
-        return true;
-      })) [[unlikely]] {
-    return {};
+  auto tmpl = env->x509_dictionary_template();
+  if (tmpl.IsEmpty()) {
+    static constexpr std::string_view names[] = {
+        "subject",
+        "issuer",
+        "subjectaltname",
+        "infoAccess",
+        "ca",
+        "modulus",
+        "exponent",
+        "pubkey",
+        "bits",
+        "valid_from",
+        "valid_to",
+        "fingerprint",
+        "fingerprint256",
+        "fingerprint512",
+        "ext_key_usage",
+        "serialNumber",
+        "raw",
+        "asn1Curve",
+        "nistCurve",
+    };
+    tmpl = DictionaryTemplate::New(env->isolate(), names);
+    env->set_x509_dictionary_template(tmpl);
   }
 
-  if (!cert.ifEc([&](const ncrypto::Ec& ec) {
-        const auto group = ec.getGroup();
-
-        if (!Set<Value>(
-                env, info, env->bits_string(), GetECGroupBits(env, group)) ||
-            !Set<Value>(
-                env, info, env->pubkey_string(), GetECPubKey(env, group, ec)))
-            [[unlikely]] {
-          return false;
-        }
-
-        const int nid = ec.getCurve();
-        if (nid != 0) [[likely]] {
-          // Curve is well-known, get its OID and NIST nick-name (if it has
-          // one).
-
-          if (!Set<Value>(env,
-                          info,
-                          env->asn1curve_string(),
-                          GetCurveName<OBJ_nid2sn>(env, nid)) ||
-              !Set<Value>(env,
-                          info,
-                          env->nistcurve_string(),
-                          GetCurveName<EC_curve_nid2nist>(env, nid)))
-              [[unlikely]] {
-            return false;
-          }
-        }
-        // Unnamed curves can be described by their mathematical properties,
-        // but aren't used much (at all?) with X.509/TLS. Support later if
-        // needed.
-        return true;
-      })) [[unlikely]] {
-    return {};
-  }
+  MaybeLocal<Value> values[] = {
+      GetX509NameObject(env, cert.getSubjectName()),
+      GetX509NameObject(env, cert.getIssuerName()),
+      GetSubjectAltNameString(env, cert),
+      GetInfoAccessString(env, cert),
+      Boolean::New(env->isolate(), cert.isCA()),
+      Undefined(env->isolate()),  // modulus
+      Undefined(env->isolate()),  // exponent
+      Undefined(env->isolate()),  // pubkey
+      Undefined(env->isolate()),  // bits
+      GetValidFrom(env, cert),
+      GetValidTo(env, cert),
+      GetFingerprintDigest(env, Digest::SHA1, cert),
+      GetFingerprintDigest(env, Digest::SHA256, cert),
+      GetFingerprintDigest(env, Digest::SHA512, cert),
+      GetKeyUsage(env, cert),
+      GetSerialNumber(env, cert),
+      GetDer(env, cert),
+      Undefined(env->isolate()),  // asn1curve
+      Undefined(env->isolate()),  // nistcurve
+  };
+
+  cert.ifRsa([&](const ncrypto::Rsa& rsa) {
+    auto pub_key = rsa.getPublicKey();
+    values[5] = GetModulusString(env, pub_key.n);   // modulus
+    values[6] = GetExponentString(env, pub_key.e);  // exponent
+    values[7] = GetPubKey(env, rsa);                // pubkey
+    values[8] = Integer::New(env->isolate(),
+                             BignumPointer::GetBitCount(pub_key.n));  // bits
+    // TODO(@jasnell): The true response is a left-over from the original
+    // non DictionaryTemplate-based implementation. It can be removed later.
+    return true;
+  });
 
-  if (!Set<Value>(
-          env, info, env->valid_from_string(), GetValidFrom(env, cert)) ||
-      !Set<Value>(env, info, env->valid_to_string(), GetValidTo(env, cert)) ||
-      !Set<Value>(env,
-                  info,
-                  env->fingerprint_string(),
-                  GetFingerprintDigest(env, Digest::SHA1, cert)) ||
-      !Set<Value>(env,
-                  info,
-                  env->fingerprint256_string(),
-                  GetFingerprintDigest(env, Digest::SHA256, cert)) ||
-      !Set<Value>(env,
-                  info,
-                  env->fingerprint512_string(),
-                  GetFingerprintDigest(env, Digest::SHA512, cert)) ||
-      !Set<Value>(
-          env, info, env->ext_key_usage_string(), GetKeyUsage(env, cert)) ||
-      !Set<Value>(
-          env, info, env->serial_number_string(), GetSerialNumber(env, cert)) ||
-      !Set<Value>(env, info, env->raw_string(), GetDer(env, cert)))
-      [[unlikely]] {
-    return {};
-  }
+  cert.ifEc([&](const ncrypto::Ec& ec) {
+    const auto group = ec.getGroup();
+    values[7] = GetECPubKey(env, group, ec);  // pubkey
+    values[8] = GetECGroupBits(env, group);   // bits
+    const int nid = ec.getCurve();
+    if (nid != 0) {
+      // Curve is well-known, get its OID and NIST nick-name (if it has
+      // one).
+      values[17] = GetCurveName<OBJ_nid2sn>(env, nid);         // asn1curve
+      values[18] = GetCurveName<EC_curve_nid2nist>(env, nid);  // nistcurve
+    }
+    // Unnamed curves can be described by their mathematical properties,
+    // but aren't used much (at all?) with X.509/TLS. Support later if
+    // needed.
+    return true;
+  });
 
-  return scope.Escape(info);
+  return scope.EscapeMaybe(NewDictionaryInstance(env->context(), tmpl, values));
 }
 }  // namespace
 

filename: src/env_properties.h
patch: @@ -77,18 +77,15 @@
   V(allow_unknown_named_params_string, "allowUnknownNamedParameters")          \
   V(alpn_callback_string, "ALPNCallback")                                      \
   V(args_string, "args")                                                       \
-  V(asn1curve_string, "asn1Curve")                                             \
   V(async_ids_stack_string, "async_ids_stack")                                 \
   V(attributes_string, "attributes")                                           \
   V(backup_string, "backup")                                                   \
   V(base_string, "base")                                                       \
   V(base_url_string, "baseURL")                                                \
-  V(bits_string, "bits")                                                       \
   V(buffer_string, "buffer")                                                   \
   V(bytes_parsed_string, "bytesParsed")                                        \
   V(bytes_read_string, "bytesRead")                                            \
   V(bytes_written_string, "bytesWritten")                                      \
-  V(ca_string, "ca")                                                           \
   V(cached_data_produced_string, "cachedDataProduced")                         \
   V(cached_data_rejected_string, "cachedDataRejected")                         \
   V(cached_data_string, "cachedData")                                          \
@@ -175,7 +172,6 @@
   V(expire_string, "expire")                                                   \
   V(exponent_string, "exponent")                                               \
   V(exports_string, "exports")                                                 \
-  V(ext_key_usage_string, "ext_key_usage")                                     \
   V(external_stream_string, "_externalStream")                                 \
   V(family_string, "family")                                                   \
   V(fatal_exception_string, "_fatalException")                                 \
@@ -184,9 +180,6 @@
   V(file_string, "file")                                                       \
   V(filename_string, "filename")                                               \
   V(filter_string, "filter")                                                   \
-  V(fingerprint256_string, "fingerprint256")                                   \
-  V(fingerprint512_string, "fingerprint512")                                   \
-  V(fingerprint_string, "fingerprint")                                         \
   V(flags_string, "flags")                                                     \
   V(flowlabel_string, "flowlabel")                                             \
   V(frames_received_string, "framesReceived")                                  \
@@ -215,14 +208,12 @@
   V(identity_string, "identity")                                               \
   V(ignore_case_string, "ignoreCase")                                          \
   V(ignore_string, "ignore")                                                   \
-  V(infoaccess_string, "infoAccess")                                           \
   V(inherit_string, "inherit")                                                 \
   V(input_string, "input")                                                     \
   V(inverse_string, "inverse")                                                 \
   V(ipv4_string, "IPv4")                                                       \
   V(ipv6_string, "IPv6")                                                       \
   V(isclosing_string, "isClosing")                                             \
-  V(issuer_string, "issuer")                                                   \
   V(issuercert_string, "issuerCertificate")                                    \
   V(iterator_string, "Iterator")                                               \
   V(jwk_akp_string, "AKP")                                                     \
@@ -266,12 +257,10 @@
   V(minttl_string, "minttl")                                                   \
   V(mode_string, "mode")                                                       \
   V(module_string, "module")                                                   \
-  V(modulus_string, "modulus")                                                 \
   V(modulus_length_string, "modulusLength")                                    \
   V(name_string, "name")                                                       \
   V(named_curve_string, "namedCurve")                                          \
   V(next_string, "next")                                                       \
-  V(nistcurve_string, "nistCurve")                                             \
   V(node_string, "node")                                                       \
   V(nsname_string, "nsname")                                                   \
   V(object_string, "Object")                                                   \
@@ -329,7 +318,6 @@
   V(protocol_string, "protocol")                                               \
   V(prototype_string, "prototype")                                             \
   V(psk_string, "psk")                                                         \
-  V(pubkey_string, "pubkey")                                                   \
   V(public_exponent_string, "publicExponent")                                  \
   V(rate_string, "rate")                                                       \
   V(raw_string, "raw")                                                         \
@@ -355,7 +343,6 @@
   V(salt_length_string, "saltLength")                                          \
   V(search_string, "search")                                                   \
   V(selector_string, "selector")                                               \
-  V(serial_number_string, "serialNumber")                                      \
   V(serial_string, "serial")                                                   \
   V(servername_string, "servername")                                           \
   V(service_string, "service")                                                 \
@@ -381,8 +368,6 @@
   V(step_string, "step")                                                       \
   V(stream_average_duration_string, "streamAverageDuration")                   \
   V(stream_count_string, "streamCount")                                        \
-  V(subject_string, "subject")                                                 \
-  V(subjectaltname_string, "subjectaltname")                                   \
   V(synthetic_string, "synthetic")                                             \
   V(syscall_string, "syscall")                                                 \
   V(table_string, "table")                                                     \
@@ -405,8 +390,6 @@
   V(unknown_string, "<unknown>")                                               \
   V(url_string, "url")                                                         \
   V(username_string, "username")                                               \
-  V(valid_from_string, "valid_from")                                           \
-  V(valid_to_string, "valid_to")                                               \
   V(value_string, "value")                                                     \
   V(verify_error_string, "verifyError")                                        \
   V(version_string, "version")                                                 \
@@ -425,12 +408,14 @@
   V(blob_reader_constructor_template, v8::FunctionTemplate)                    \
   V(blocklist_constructor_template, v8::FunctionTemplate)                      \
   V(callsite_template, v8::DictionaryTemplate)                                 \
+  V(cipherinfo_template, v8::DictionaryTemplate)                               \
   V(contextify_global_template, v8::ObjectTemplate)                            \
   V(contextify_wrapper_template, v8::ObjectTemplate)                           \
   V(cpu_usage_template, v8::DictionaryTemplate)                                \
   V(crypto_key_object_handle_constructor, v8::FunctionTemplate)                \
   V(env_proxy_template, v8::ObjectTemplate)                                    \
   V(env_proxy_ctor_template, v8::FunctionTemplate)                             \
+  V(ephemeral_key_template, v8::DictionaryTemplate)                            \
   V(dir_instance_template, v8::ObjectTemplate)                                 \
   V(fd_constructor_template, v8::ObjectTemplate)                               \
   V(fdclose_constructor_template, v8::ObjectTemplate)                          \
@@ -475,7 +460,8 @@
   V(worker_cpu_usage_taker_template, v8::ObjectTemplate)                       \
   V(worker_heap_snapshot_taker_template, v8::ObjectTemplate)                   \
   V(worker_heap_statistics_taker_template, v8::ObjectTemplate)                 \
-  V(x509_constructor_template, v8::FunctionTemplate)
+  V(x509_constructor_template, v8::FunctionTemplate)                           \
+  V(x509_dictionary_template, v8::DictionaryTemplate)
 
 #define PER_REALM_STRONG_PERSISTENT_VALUES(V)                                  \
   V(async_hooks_after_function, v8::Function)                                  \

filename: test/parallel/test-tls-multi-key.js
patch: @@ -161,6 +161,7 @@ function test(options) {
         standardName: 'TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384',
         version: 'TLSv1.2',
       });
+
       assert.strictEqual(ecdsa.getPeerCertificate().subject.CN, eccCN);
       assert.strictEqual(ecdsa.getPeerCertificate().asn1Curve, 'prime256v1');
       ecdsa.end();