src: shift even moar x509 to ncrypto

PR-URL: https://github.com/nodejs/node/pull/54340
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

deps/ncrypto/ncrypto.cc

@@ -908,6 +908,24 @@ X509View::CheckMatch X509View::checkIp(const std::string_view ip, int flags) con
   }
 }
 
+X509View X509View::From(const SSLPointer& ssl) {
+  ClearErrorOnReturn clear_error_on_return;
+  if (!ssl) return {};
+  return X509View(SSL_get_certificate(ssl.get()));
+}
+
+X509View X509View::From(const SSLCtxPointer& ctx) {
+  ClearErrorOnReturn clear_error_on_return;
+  if (!ctx) return {};
+  return X509View(SSL_CTX_get0_certificate(ctx.get()));
+}
+
+X509Pointer X509View::clone() const {
+  ClearErrorOnReturn clear_error_on_return;
+  if (!cert_) return {};
+  return X509Pointer(X509_dup(const_cast<X509*>(cert_)));
+}
+
 Result<X509Pointer, int> X509Pointer::Parse(Buffer<const unsigned char> buffer) {
   ClearErrorOnReturn clearErrorOnReturn;
   BIOPointer bio(BIO_new_mem_buf(buffer.data, buffer.len));
@@ -922,4 +940,27 @@ Result<X509Pointer, int> X509Pointer::Parse(Buffer<const unsigned char> buffer)
 
   return Result<X509Pointer, int>(ERR_get_error());
 }
+
+
+X509Pointer X509Pointer::IssuerFrom(const SSLPointer& ssl, const X509View& view) {
+  return IssuerFrom(SSL_get_SSL_CTX(ssl.get()), view);
+}
+
+X509Pointer X509Pointer::IssuerFrom(const SSL_CTX* ctx, const X509View& cert) {
+  X509_STORE* store = SSL_CTX_get_cert_store(ctx);
+  DeleteFnPtr<X509_STORE_CTX, X509_STORE_CTX_free> store_ctx(
+      X509_STORE_CTX_new());
+  X509Pointer result;
+  X509* issuer;
+  if (store_ctx.get() != nullptr &&
+      X509_STORE_CTX_init(store_ctx.get(), store, nullptr, nullptr) == 1 &&
+      X509_STORE_CTX_get1_issuer(&issuer, store_ctx.get(), cert.get()) == 1) {
+    result.reset(issuer);
+  }
+  return result;
+}
+
+X509Pointer X509Pointer::PeerFrom(const SSLPointer& ssl) {
+  return X509Pointer(SSL_get_peer_certificate(ssl.get()));
+}
 }  // namespace ncrypto

deps/ncrypto/ncrypto.h

@@ -311,14 +311,21 @@ class BignumPointer final {
   DeleteFnPtr<BIGNUM, BN_clear_free> bn_;
 };
 
+class X509Pointer;
+
 class X509View final {
  public:
+  static X509View From(const SSLPointer& ssl);
+  static X509View From(const SSLCtxPointer& ctx);
+
   X509View() = default;
   inline explicit X509View(const X509* cert) : cert_(cert) {}
   X509View(const X509View& other) = default;
   X509View& operator=(const X509View& other) = default;
   NCRYPTO_DISALLOW_MOVE(X509View)
 
+  inline X509* get() const { return const_cast<X509*>(cert_); }
+
   inline bool operator==(std::nullptr_t) noexcept { return cert_ == nullptr; }
   inline operator bool() const { return cert_ != nullptr; }
 
@@ -340,6 +347,8 @@ class X509View final {
   bool checkPrivateKey(const EVPKeyPointer& pkey) const;
   bool checkPublicKey(const EVPKeyPointer& pkey) const;
 
+  X509Pointer clone() const;
+
   enum class CheckMatch {
     NO_MATCH,
     MATCH,
@@ -358,6 +367,9 @@ class X509View final {
 class X509Pointer final {
  public:
   static Result<X509Pointer, int> Parse(Buffer<const unsigned char> buffer);
+  static X509Pointer IssuerFrom(const SSLPointer& ssl, const X509View& view);
+  static X509Pointer IssuerFrom(const SSL_CTX* ctx, const X509View& view);
+  static X509Pointer PeerFrom(const SSLPointer& ssl);
 
   X509Pointer() = default;
   explicit X509Pointer(X509* cert);

src/crypto/crypto_common.cc

@@ -1,6 +1,7 @@
 #include "crypto/crypto_common.h"
 #include "base_object-inl.h"
 #include "crypto/crypto_util.h"
+#include "crypto/crypto_x509.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
 #include "nbytes.h"
@@ -29,40 +30,17 @@ namespace node {
 using v8::Array;
 using v8::ArrayBuffer;
 using v8::BackingStore;
-using v8::Boolean;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::Integer;
 using v8::Local;
 using v8::MaybeLocal;
-using v8::NewStringType;
 using v8::Object;
 using v8::String;
 using v8::Undefined;
 using v8::Value;
 
 namespace crypto {
-static constexpr int kX509NameFlagsMultiline =
-    ASN1_STRFLGS_ESC_2253 |
-    ASN1_STRFLGS_ESC_CTRL |
-    ASN1_STRFLGS_UTF8_CONVERT |
-    XN_FLAG_SEP_MULTILINE |
-    XN_FLAG_FN_SN;
-
-X509Pointer SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert) {
-  X509_STORE* store = SSL_CTX_get_cert_store(ctx);
-  DeleteFnPtr<X509_STORE_CTX, X509_STORE_CTX_free> store_ctx(
-      X509_STORE_CTX_new());
-  X509Pointer result;
-  X509* issuer;
-  if (store_ctx.get() != nullptr &&
-      X509_STORE_CTX_init(store_ctx.get(), store, nullptr, nullptr) == 1 &&
-      X509_STORE_CTX_get1_issuer(&issuer, store_ctx.get(), cert) == 1) {
-    result.reset(issuer);
-  }
-  return result;
-}
-
 void LogSecret(
     const SSLPointer& ssl,
     const char* name,
@@ -120,8 +98,7 @@ long VerifyPeerCertificate(  // NOLINT(runtime/int)
     const SSLPointer& ssl,
     long def) {  // NOLINT(runtime/int)
   long err = def;  // NOLINT(runtime/int)
-  if (X509* peer_cert = SSL_get_peer_certificate(ssl.get())) {
+  if (X509Pointer::PeerFrom(ssl)) {
-    X509_free(peer_cert);
     err = SSL_get_verify_result(ssl.get());
   } else {
     const SSL_CIPHER* curr_cipher = SSL_get_current_cipher(ssl.get());
@@ -140,13 +117,14 @@ long VerifyPeerCertificate(  // NOLINT(runtime/int)
 
 bool UseSNIContext(
     const SSLPointer& ssl, BaseObjectPtr<SecureContext> context) {
+  auto x509 = ncrypto::X509View::From(context->ctx());
+  if (!x509) return false;
   SSL_CTX* ctx = context->ctx().get();
-  X509* x509 = SSL_CTX_get0_certificate(ctx);
   EVP_PKEY* pkey = SSL_CTX_get0_privatekey(ctx);
   STACK_OF(X509)* chain;
 
   int err = SSL_CTX_get0_chain_certs(ctx, &chain);
-  if (err == 1) err = SSL_use_certificate(ssl.get(), x509);
+  if (err == 1) err = SSL_use_certificate(ssl.get(), x509.get());
   if (err == 1) err = SSL_use_PrivateKey(ssl.get(), pkey);
   if (err == 1 && chain != nullptr) err = SSL_set1_chain(ssl.get(), chain);
   return err == 1;
@@ -263,25 +241,9 @@ MaybeLocal<Value> GetValidationErrorCode(Environment* env, int err) {
 
 MaybeLocal<Value> GetCert(Environment* env, const SSLPointer& ssl) {
   ClearErrorOnReturn clear_error_on_return;
-  X509* cert = SSL_get_certificate(ssl.get());
+  ncrypto::X509View cert(SSL_get_certificate(ssl.get()));
-  if (cert == nullptr)
+  if (!cert) return Undefined(env->isolate());
-    return Undefined(env->isolate());
+  return X509Certificate::toObject(env, cert);
-
-  MaybeLocal<Object> maybe_cert = X509ToObject(env, cert);
-  return maybe_cert.FromMaybe<Value>(Local<Value>());
-}
-
-Local<Value> ToV8Value(Environment* env, const BIOPointer& bio) {
-  BUF_MEM* mem;
-  BIO_get_mem_ptr(bio.get(), &mem);
-  MaybeLocal<String> ret =
-      String::NewFromUtf8(
-          env->isolate(),
-          mem->data,
-          NewStringType::kNormal,
-          mem->length);
-  CHECK_EQ(BIO_reset(bio.get()), 1);
-  return ret.FromMaybe(Local<Value>());
 }
 
 namespace {
@@ -330,28 +292,28 @@ StackOfX509 CloneSSLCerts(X509Pointer&& cert,
   return peer_certs;
 }
 
-MaybeLocal<Object> AddIssuerChainToObject(
+MaybeLocal<Object> AddIssuerChainToObject(X509Pointer* cert,
-    X509Pointer* cert,
+                                          Local<Object> object,
-    Local<Object> object,
+                                          StackOfX509&& peer_certs,
-    StackOfX509&& peer_certs,
+                                          Environment* const env) {
-    Environment* const env) {
-  Local<Context> context = env->isolate()->GetCurrentContext();
   cert->reset(sk_X509_delete(peer_certs.get(), 0));
   for (;;) {
     int i;
     for (i = 0; i < sk_X509_num(peer_certs.get()); i++) {
-      X509* ca = sk_X509_value(peer_certs.get(), i);
+      ncrypto::X509View ca(sk_X509_value(peer_certs.get(), i));
-      if (X509_check_issued(ca, cert->get()) != X509_V_OK)
+      if (!cert->view().isIssuedBy(ca)) continue;
-        continue;
 
-      Local<Object> ca_info;
+      Local<Value> ca_info;
-      MaybeLocal<Object> maybe_ca_info = X509ToObject(env, ca);
+      if (!X509Certificate::toObject(env, ca).ToLocal(&ca_info)) return {};
-      if (!maybe_ca_info.ToLocal(&ca_info))
+      CHECK(ca_info->IsObject());
-        return MaybeLocal<Object>();
 
-      if (!Set<Object>(context, object, env->issuercert_string(), ca_info))
+      if (!Set<Object>(env->context(),
-        return MaybeLocal<Object>();
+                       object,
-      object = ca_info;
+                       env->issuercert_string(),
+                       ca_info.As<Object>())) {
+        return {};
+      }
+      object = ca_info.As<Object>();
 
       // NOTE: Intentionally freeing cert that is not used anymore.
       // Delete cert and continue aggregating issuers.
@@ -371,20 +333,22 @@ MaybeLocal<Object> GetLastIssuedCert(
     const SSLPointer& ssl,
     Local<Object> issuer_chain,
     Environment* const env) {
-  Local<Context> context = env->isolate()->GetCurrentContext();
+  Local<Value> ca_info;
-  while (X509_check_issued(cert->get(), cert->get()) != X509_V_OK) {
+  while (!cert->view().isIssuedBy(cert->view())) {
-    X509Pointer ca;
+    auto ca = X509Pointer::IssuerFrom(ssl, cert->view());
-    if (!(ca = SSL_CTX_get_issuer(SSL_get_SSL_CTX(ssl.get()), cert->get())))
+    if (!ca) break;
-      break;
 
-    Local<Object> ca_info;
+    if (!X509Certificate::toObject(env, ca.view()).ToLocal(&ca_info)) return {};
-    MaybeLocal<Object> maybe_ca_info = X509ToObject(env, ca.get());
-    if (!maybe_ca_info.ToLocal(&ca_info))
-      return MaybeLocal<Object>();
 
-    if (!Set<Object>(context, issuer_chain, env->issuercert_string(), ca_info))
+    CHECK(ca_info->IsObject());
-      return MaybeLocal<Object>();
+
-    issuer_chain = ca_info;
+    if (!Set<Object>(env->context(),
+                     issuer_chain,
+                     env->issuercert_string(),
+                     ca_info.As<Object>())) {
+      return {};
+    }
+    issuer_chain = ca_info.As<Object>();
 
     // For self-signed certificates whose keyUsage field does not include
     // keyCertSign, X509_check_issued() will return false. Avoid going into an
@@ -398,144 +362,8 @@ MaybeLocal<Object> GetLastIssuedCert(
   return MaybeLocal<Object>(issuer_chain);
 }
 
-void AddFingerprintDigest(
-    const unsigned char* md,
-    unsigned int md_size,
-    char fingerprint[3 * EVP_MAX_MD_SIZE]) {
-  unsigned int i;
-  const char hex[] = "0123456789ABCDEF";
-
-  for (i = 0; i < md_size; i++) {
-    fingerprint[3*i] = hex[(md[i] & 0xf0) >> 4];
-    fingerprint[(3*i)+1] = hex[(md[i] & 0x0f)];
-    fingerprint[(3*i)+2] = ':';
-  }
-
-  DCHECK_GT(md_size, 0);
-  fingerprint[(3 * (md_size - 1)) + 2] = '\0';
-}
-
-template <const char* (*nid2string)(int nid)>
-MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
-  const char* name = nid2string(nid);
-  return name != nullptr ?
-      MaybeLocal<Value>(OneByteString(env->isolate(), name)) :
-      MaybeLocal<Value>(Undefined(env->isolate()));
-}
-
-MaybeLocal<Value> GetECPubKey(Environment* env,
-                              const EC_GROUP* group,
-                              OSSL3_CONST EC_KEY* ec) {
-  const EC_POINT* pubkey = EC_KEY_get0_public_key(ec);
-  if (pubkey == nullptr)
-    return Undefined(env->isolate());
-
-  return ECPointToBuffer(env, group, pubkey, EC_KEY_get_conv_form(ec), nullptr)
-      .FromMaybe(Local<Object>());
-}
-
-MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
-  if (group == nullptr)
-    return Undefined(env->isolate());
-
-  int bits = EC_GROUP_order_bits(group);
-  if (bits <= 0)
-    return Undefined(env->isolate());
-
-  return Integer::New(env->isolate(), bits);
-}
-
-MaybeLocal<Object> GetPubKey(Environment* env, OSSL3_CONST RSA* rsa) {
-  int size = i2d_RSA_PUBKEY(rsa, nullptr);
-  CHECK_GE(size, 0);
-
-  std::unique_ptr<BackingStore> bs;
-  {
-    NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
-    bs = ArrayBuffer::NewBackingStore(env->isolate(), size);
-  }
-
-  unsigned char* serialized = reinterpret_cast<unsigned char*>(bs->Data());
-  CHECK_GE(i2d_RSA_PUBKEY(rsa, &serialized), 0);
-
-  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
-  return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Object>());
-}
-
-MaybeLocal<Value> GetExponentString(
-    Environment* env,
-    const BIOPointer& bio,
-    const BIGNUM* e) {
-  uint64_t exponent_word = static_cast<uint64_t>(BignumPointer::GetWord(e));
-  BIO_printf(bio.get(), "0x%" PRIx64, exponent_word);
-  return ToV8Value(env, bio);
-}
-
-Local<Value> GetBits(Environment* env, const BIGNUM* n) {
-  return Integer::New(env->isolate(), BignumPointer::GetBitCount(n));
-}
-
-MaybeLocal<Value> GetModulusString(
-    Environment* env,
-    const BIOPointer& bio,
-    const BIGNUM* n) {
-  BN_print(bio.get(), n);
-  return ToV8Value(env, bio);
-}
 }  // namespace
 
-MaybeLocal<Value> GetRawDERCertificate(Environment* env, X509* cert) {
-  int size = i2d_X509(cert, nullptr);
-
-  std::unique_ptr<BackingStore> bs;
-  {
-    NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
-    bs = ArrayBuffer::NewBackingStore(env->isolate(), size);
-  }
-
-  unsigned char* serialized = reinterpret_cast<unsigned char*>(bs->Data());
-  CHECK_GE(i2d_X509(cert, &serialized), 0);
-
-  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
-  return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Object>());
-}
-
-MaybeLocal<Value> GetSerialNumber(Environment* env, X509* cert) {
-  if (ASN1_INTEGER* serial_number = X509_get_serialNumber(cert)) {
-    if (auto bn = BignumPointer(ASN1_INTEGER_to_BN(serial_number, nullptr))) {
-      if (auto hex = bn.toHex()) {
-        return OneByteString(env->isolate(),
-                             static_cast<unsigned char*>(hex.get()));
-      }
-    }
-  }
-
-  return Undefined(env->isolate());
-}
-
-MaybeLocal<Value> GetKeyUsage(Environment* env, X509* cert) {
-  StackOfASN1 eku(static_cast<STACK_OF(ASN1_OBJECT)*>(
-      X509_get_ext_d2i(cert, NID_ext_key_usage, nullptr, nullptr)));
-  if (eku) {
-    const int count = sk_ASN1_OBJECT_num(eku.get());
-    MaybeStackBuffer<Local<Value>, 16> ext_key_usage(count);
-    char buf[256];
-
-    int j = 0;
-    for (int i = 0; i < count; i++) {
-      if (OBJ_obj2txt(buf,
-                      sizeof(buf),
-                      sk_ASN1_OBJECT_value(eku.get(), i), 1) >= 0) {
-        ext_key_usage[j++] = OneByteString(env->isolate(), buf);
-      }
-    }
-
-    return Array::New(env->isolate(), ext_key_usage.out(), count);
-  }
-
-  return Undefined(env->isolate());
-}
-
 MaybeLocal<Value> GetCurrentCipherName(Environment* env,
                                        const SSLPointer& ssl) {
   return GetCipherName(env, SSL_get_current_cipher(ssl.get()));
@@ -546,199 +374,6 @@ MaybeLocal<Value> GetCurrentCipherVersion(Environment* env,
   return GetCipherVersion(env, SSL_get_current_cipher(ssl.get()));
 }
 
-MaybeLocal<Value> GetFingerprintDigest(
-    Environment* env,
-    const EVP_MD* method,
-    X509* cert) {
-  unsigned char md[EVP_MAX_MD_SIZE];
-  unsigned int md_size;
-  char fingerprint[EVP_MAX_MD_SIZE * 3];
-
-  if (X509_digest(cert, method, md, &md_size)) {
-    AddFingerprintDigest(md, md_size, fingerprint);
-    return OneByteString(env->isolate(), fingerprint);
-  }
-  return Undefined(env->isolate());
-}
-
-MaybeLocal<Value> GetValidTo(
-    Environment* env,
-    X509* cert,
-    const BIOPointer& bio) {
-  ASN1_TIME_print(bio.get(), X509_get0_notAfter(cert));
-  return ToV8Value(env, bio);
-}
-
-MaybeLocal<Value> GetValidFrom(
-    Environment* env,
-    X509* cert,
-    const BIOPointer& bio) {
-  ASN1_TIME_print(bio.get(), X509_get0_notBefore(cert));
-  return ToV8Value(env, bio);
-}
-
-v8::MaybeLocal<v8::Value> GetSubjectAltNameString(Environment* env,
-                                                  X509* cert,
-                                                  const BIOPointer& bio) {
-  int index = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1);
-  if (index < 0)
-    return Undefined(env->isolate());
-
-  X509_EXTENSION* ext = X509_get_ext(cert, index);
-  CHECK_NOT_NULL(ext);
-
-  if (!ncrypto::SafeX509SubjectAltNamePrint(bio, ext)) {
-    CHECK_EQ(BIO_reset(bio.get()), 1);
-    return v8::Null(env->isolate());
-  }
-
-  return ToV8Value(env, bio);
-}
-
-v8::MaybeLocal<v8::Value> GetInfoAccessString(Environment* env,
-                                              X509* cert,
-                                              const BIOPointer& bio) {
-  int index = X509_get_ext_by_NID(cert, NID_info_access, -1);
-  if (index < 0)
-    return Undefined(env->isolate());
-
-  X509_EXTENSION* ext = X509_get_ext(cert, index);
-  CHECK_NOT_NULL(ext);
-
-  if (!ncrypto::SafeX509InfoAccessPrint(bio, ext)) {
-    CHECK_EQ(BIO_reset(bio.get()), 1);
-    return v8::Null(env->isolate());
-  }
-
-  return ToV8Value(env, bio);
-}
-
-MaybeLocal<Value> GetIssuerString(Environment* env,
-                                  X509* cert,
-                                  const BIOPointer& bio) {
-  X509_NAME* issuer_name = X509_get_issuer_name(cert);
-  if (X509_NAME_print_ex(
-          bio.get(),
-          issuer_name,
-          0,
-          kX509NameFlagsMultiline) <= 0) {
-    CHECK_EQ(BIO_reset(bio.get()), 1);
-    return Undefined(env->isolate());
-  }
-
-  return ToV8Value(env, bio);
-}
-
-MaybeLocal<Value> GetSubject(Environment* env,
-                             X509* cert,
-                             const BIOPointer& bio) {
-  if (X509_NAME_print_ex(
-          bio.get(),
-          X509_get_subject_name(cert),
-          0,
-          kX509NameFlagsMultiline) <= 0) {
-    CHECK_EQ(BIO_reset(bio.get()), 1);
-    return Undefined(env->isolate());
-  }
-
-  return ToV8Value(env, bio);
-}
-
-template <X509_NAME* get_name(const X509*)>
-static MaybeLocal<Value> GetX509NameObject(Environment* env, X509* cert) {
-  X509_NAME* name = get_name(cert);
-  CHECK_NOT_NULL(name);
-
-  int cnt = X509_NAME_entry_count(name);
-  CHECK_GE(cnt, 0);
-
-  Local<Object> result =
-      Object::New(env->isolate(), Null(env->isolate()), nullptr, nullptr, 0);
-  if (result.IsEmpty()) {
-    return MaybeLocal<Value>();
-  }
-
-  for (int i = 0; i < cnt; i++) {
-    X509_NAME_ENTRY* entry = X509_NAME_get_entry(name, i);
-    CHECK_NOT_NULL(entry);
-
-    // We intentionally ignore the value of X509_NAME_ENTRY_set because the
-    // representation as an object does not allow grouping entries into sets
-    // anyway, and multi-value RDNs are rare, i.e., the vast majority of
-    // Relative Distinguished Names contains a single type-value pair only.
-    const ASN1_OBJECT* type = X509_NAME_ENTRY_get_object(entry);
-    const ASN1_STRING* value = X509_NAME_ENTRY_get_data(entry);
-
-    // If OpenSSL knows the type, use the short name of the type as the key, and
-    // the numeric representation of the type's OID otherwise.
-    int type_nid = OBJ_obj2nid(type);
-    char type_buf[80];
-    const char* type_str;
-    if (type_nid != NID_undef) {
-      type_str = OBJ_nid2sn(type_nid);
-      CHECK_NOT_NULL(type_str);
-    } else {
-      OBJ_obj2txt(type_buf, sizeof(type_buf), type, true);
-      type_str = type_buf;
-    }
-
-    Local<String> v8_name;
-    if (!String::NewFromUtf8(env->isolate(), type_str).ToLocal(&v8_name)) {
-      return MaybeLocal<Value>();
-    }
-
-    // The previous implementation used X509_NAME_print_ex, which escapes some
-    // characters in the value. The old implementation did not decode/unescape
-    // values correctly though, leading to ambiguous and incorrect
-    // representations. The new implementation only converts to Unicode and does
-    // not escape anything.
-    unsigned char* value_str;
-    int value_str_size = ASN1_STRING_to_UTF8(&value_str, value);
-    if (value_str_size < 0) {
-      return Undefined(env->isolate());
-    }
-    auto free_value_str = OnScopeLeave([&]() { OPENSSL_free(value_str); });
-
-    Local<String> v8_value;
-    if (!String::NewFromUtf8(env->isolate(),
-                             reinterpret_cast<const char*>(value_str),
-                             NewStringType::kNormal,
-                             value_str_size)
-             .ToLocal(&v8_value)) {
-      return MaybeLocal<Value>();
-    }
-
-    // For backward compatibility, we only create arrays if multiple values
-    // exist for the same key. That is not great but there is not much we can
-    // change here without breaking things. Note that this creates nested data
-    // structures, yet still does not allow representing Distinguished Names
-    // accurately.
-    bool multiple;
-    if (!result->HasOwnProperty(env->context(), v8_name).To(&multiple)) {
-      return MaybeLocal<Value>();
-    } else if (multiple) {
-      Local<Value> accum;
-      if (!result->Get(env->context(), v8_name).ToLocal(&accum)) {
-        return MaybeLocal<Value>();
-      }
-      if (!accum->IsArray()) {
-        accum = Array::New(env->isolate(), &accum, 1);
-        if (result->Set(env->context(), v8_name, accum).IsNothing()) {
-          return MaybeLocal<Value>();
-        }
-      }
-      Local<Array> array = accum.As<Array>();
-      if (array->Set(env->context(), array->Length(), v8_value).IsNothing()) {
-        return MaybeLocal<Value>();
-      }
-    } else if (result->Set(env->context(), v8_name, v8_value).IsNothing()) {
-      return MaybeLocal<Value>();
-    }
-  }
-
-  return result;
-}
-
 template <MaybeLocal<Value> (*Get)(Environment* env, const SSL_CIPHER* cipher)>
 MaybeLocal<Value> GetCurrentCipherValue(Environment* env,
                                         const SSLPointer& ssl) {
@@ -897,8 +532,6 @@ MaybeLocal<Value> GetPeerCert(
     bool abbreviated,
     bool is_server) {
   ClearErrorOnReturn clear_error_on_return;
-  Local<Object> result;
-  MaybeLocal<Object> maybe_cert;
 
   // NOTE: This is because of the odd OpenSSL behavior. On client `cert_chain`
   // contains the `peer_certificate`, but on server it doesn't.
@@ -909,9 +542,11 @@ MaybeLocal<Value> GetPeerCert(
 
   // Short result requested.
   if (abbreviated) {
-    maybe_cert =
+    if (cert) {
-        X509ToObject(env, cert ? cert.get() : sk_X509_value(ssl_certs, 0));
+      return X509Certificate::toObject(env, cert.view());
-    return maybe_cert.ToLocal(&result) ? result : MaybeLocal<Value>();
+    }
+    return X509Certificate::toObject(
+        env, ncrypto::X509View(sk_X509_value(ssl_certs, 0)));
   }
 
   StackOfX509 peer_certs = CloneSSLCerts(std::move(cert), ssl_certs);
@@ -919,23 +554,18 @@ MaybeLocal<Value> GetPeerCert(
     return Undefined(env->isolate());
 
   // First and main certificate.
-  X509Pointer first_cert(sk_X509_value(peer_certs.get(), 0));
+  Local<Value> result;
+  ncrypto::X509View first_cert(sk_X509_value(peer_certs.get(), 0));
   CHECK(first_cert);
-  maybe_cert = X509ToObject(env, first_cert.release());
+  if (!X509Certificate::toObject(env, first_cert).ToLocal(&result)) return {};
-  if (!maybe_cert.ToLocal(&result))
+  CHECK(result->IsObject());
-    return MaybeLocal<Value>();
 
   Local<Object> issuer_chain;
   MaybeLocal<Object> maybe_issuer_chain;
 
-  maybe_issuer_chain =
+  maybe_issuer_chain = AddIssuerChainToObject(
-      AddIssuerChainToObject(
+      &cert, result.As<Object>(), std::move(peer_certs), env);
-          &cert,
+  if (!maybe_issuer_chain.ToLocal(&issuer_chain)) return {};
-          result,
-          std::move(peer_certs),
-          env);
-  if (!maybe_issuer_chain.ToLocal(&issuer_chain))
-    return MaybeLocal<Value>();
 
   maybe_issuer_chain =
       GetLastIssuedCert(
@@ -945,155 +575,19 @@ MaybeLocal<Value> GetPeerCert(
           env);
 
   issuer_chain.Clear();
-  if (!maybe_issuer_chain.ToLocal(&issuer_chain))
+  if (!maybe_issuer_chain.ToLocal(&issuer_chain)) return {};
-    return MaybeLocal<Value>();
 
   // Last certificate should be self-signed.
-  if (X509_check_issued(cert.get(), cert.get()) == X509_V_OK &&
+  if (cert.view().isIssuedBy(cert.view()) &&
       !Set<Object>(env->context(),
-           issuer_chain,
+                   issuer_chain,
-           env->issuercert_string(),
+                   env->issuercert_string(),
-           issuer_chain)) {
+                   issuer_chain)) {
-    return MaybeLocal<Value>();
+    return {};
   }
 
   return result;
 }
 
-MaybeLocal<Object> X509ToObject(
-    Environment* env,
-    X509* cert) {
-  EscapableHandleScope scope(env->isolate());
-  Local<Context> context = env->context();
-  Local<Object> info = Object::New(env->isolate());
-
-  BIOPointer bio(BIO_new(BIO_s_mem()));
-  CHECK(bio);
-
-  // X509_check_ca() returns a range of values. Only 1 means "is a CA"
-  auto is_ca = Boolean::New(env->isolate(), 1 == X509_check_ca(cert));
-  if (!Set<Value>(context,
-                  info,
-                  env->subject_string(),
-                  GetX509NameObject<X509_get_subject_name>(env, cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->issuer_string(),
-                  GetX509NameObject<X509_get_issuer_name>(env, cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->subjectaltname_string(),
-                  GetSubjectAltNameString(env, cert, bio)) ||
-      !Set<Value>(context,
-                  info,
-                  env->infoaccess_string(),
-                  GetInfoAccessString(env, cert, bio)) ||
-      !Set<Boolean>(context, info, env->ca_string(), is_ca)) {
-    return MaybeLocal<Object>();
-  }
-
-  OSSL3_CONST EVP_PKEY* pkey = X509_get0_pubkey(cert);
-  OSSL3_CONST RSA* rsa = nullptr;
-  OSSL3_CONST EC_KEY* ec = nullptr;
-  if (pkey != nullptr) {
-    switch (EVP_PKEY_id(pkey)) {
-      case EVP_PKEY_RSA:
-        rsa = EVP_PKEY_get0_RSA(pkey);
-        break;
-      case EVP_PKEY_EC:
-        ec = EVP_PKEY_get0_EC_KEY(pkey);
-        break;
-    }
-  }
-
-  if (rsa) {
-    const BIGNUM* n;
-    const BIGNUM* e;
-    RSA_get0_key(rsa, &n, &e, nullptr);
-    if (!Set<Value>(context,
-                    info,
-                    env->modulus_string(),
-                    GetModulusString(env, bio, n)) ||
-        !Set<Value>(context, info, env->bits_string(), GetBits(env, n)) ||
-        !Set<Value>(context,
-                    info,
-                    env->exponent_string(),
-                    GetExponentString(env, bio, e)) ||
-        !Set<Object>(context,
-                     info,
-                     env->pubkey_string(),
-                     GetPubKey(env, rsa))) {
-      return MaybeLocal<Object>();
-    }
-  } else if (ec) {
-    const EC_GROUP* group = EC_KEY_get0_group(ec);
-
-    if (!Set<Value>(
-            context, info, env->bits_string(), GetECGroupBits(env, group)) ||
-        !Set<Value>(
-            context, info, env->pubkey_string(), GetECPubKey(env, group, ec))) {
-      return MaybeLocal<Object>();
-    }
-
-    const int nid = EC_GROUP_get_curve_name(group);
-    if (nid != 0) {
-      // Curve is well-known, get its OID and NIST nick-name (if it has one).
-
-      if (!Set<Value>(context,
-                      info,
-                      env->asn1curve_string(),
-                      GetCurveName<OBJ_nid2sn>(env, nid)) ||
-          !Set<Value>(context,
-                      info,
-                      env->nistcurve_string(),
-                      GetCurveName<EC_curve_nid2nist>(env, nid))) {
-        return MaybeLocal<Object>();
-      }
-    } else {
-      // Unnamed curves can be described by their mathematical properties,
-      // but aren't used much (at all?) with X.509/TLS. Support later if needed.
-    }
-  }
-
-  if (!Set<Value>(context,
-                  info,
-                  env->valid_from_string(),
-                  GetValidFrom(env, cert, bio)) ||
-      !Set<Value>(context,
-                  info,
-                  env->valid_to_string(),
-                  GetValidTo(env, cert, bio))) {
-    return MaybeLocal<Object>();
-  }
-
-  // bio is no longer needed
-  bio.reset();
-
-  if (!Set<Value>(context,
-                  info,
-                  env->fingerprint_string(),
-                  GetFingerprintDigest(env, EVP_sha1(), cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->fingerprint256_string(),
-                  GetFingerprintDigest(env, EVP_sha256(), cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->fingerprint512_string(),
-                  GetFingerprintDigest(env, EVP_sha512(), cert)) ||
-      !Set<Value>(
-          context, info, env->ext_key_usage_string(), GetKeyUsage(env, cert)) ||
-      !Set<Value>(context,
-                  info,
-                  env->serial_number_string(),
-                  GetSerialNumber(env, cert)) ||
-      !Set<Value>(
-          context, info, env->raw_string(), GetRawDERCertificate(env, cert))) {
-    return MaybeLocal<Object>();
-  }
-
-  return scope.Escape(info);
-}
-
 }  // namespace crypto
 }  // namespace node

src/crypto/crypto_common.h

@@ -27,10 +27,6 @@ struct StackOfX509Deleter {
 };
 using StackOfX509 = std::unique_ptr<STACK_OF(X509), StackOfX509Deleter>;
 
-using StackOfASN1 = ncrypto::StackOfASN1;
-
-X509Pointer SSL_CTX_get_issuer(SSL_CTX* ctx, X509* cert);
-
 void LogSecret(
     const SSLPointer& ssl,
     const char* name,
@@ -95,53 +91,11 @@ v8::MaybeLocal<v8::Object> ECPointToBuffer(
     point_conversion_form_t form,
     const char** error);
 
-v8::MaybeLocal<v8::Object> X509ToObject(
-    Environment* env,
-    X509* cert);
-
-v8::MaybeLocal<v8::Value> GetValidTo(
-    Environment* env,
-    X509* cert,
-    const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetValidFrom(
-    Environment* env,
-    X509* cert,
-    const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetFingerprintDigest(
-    Environment* env,
-    const EVP_MD* method,
-    X509* cert);
-
-v8::MaybeLocal<v8::Value> GetKeyUsage(Environment* env, X509* cert);
 v8::MaybeLocal<v8::Value> GetCurrentCipherName(Environment* env,
                                                const SSLPointer& ssl);
 v8::MaybeLocal<v8::Value> GetCurrentCipherVersion(Environment* env,
                                                   const SSLPointer& ssl);
 
-v8::MaybeLocal<v8::Value> GetSerialNumber(Environment* env, X509* cert);
-
-v8::MaybeLocal<v8::Value> GetRawDERCertificate(Environment* env, X509* cert);
-
-v8::Local<v8::Value> ToV8Value(Environment* env, const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetSubject(Environment* env,
-                                     X509* cert,
-                                     const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetIssuerString(Environment* env,
-                                          X509* cert,
-                                          const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetSubjectAltNameString(Environment* env,
-                                                  X509* cert,
-                                                  const BIOPointer& bio);
-
-v8::MaybeLocal<v8::Value> GetInfoAccessString(Environment* env,
-                                              X509* cert,
-                                              const BIOPointer& bio);
-
 }  // namespace crypto
 }  // namespace node
 

src/crypto/crypto_context.cc

@@ -121,7 +121,7 @@ int SSL_CTX_use_certificate_chain(SSL_CTX* ctx,
       // TODO(tniessen): SSL_CTX_get_issuer does not allow the caller to
       // distinguish between a failed operation and an empty result. Fix that
       // and then handle the potential error properly here (set ret to 0).
-      *issuer_ = SSL_CTX_get_issuer(ctx, x.get());
+      *issuer_ = X509Pointer::IssuerFrom(ctx, x.view());
       // NOTE: get_cert_store doesn't increment reference count,
       // no need to free `store`
     } else {

src/crypto/crypto_x509.cc

@@ -1,9 +1,8 @@
-#include "crypto_x509.h"
+#include "crypto/crypto_x509.h"
 #include "base_object-inl.h"
-#include "crypto_bio.h"
+#include "crypto/crypto_common.h"
-#include "crypto_common.h"
+#include "crypto/crypto_keys.h"
-#include "crypto_context.h"
+#include "crypto/crypto_util.h"
-#include "crypto_keys.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"
 #include "ncrypto.h"
@@ -19,11 +18,14 @@ namespace node {
 using v8::Array;
 using v8::ArrayBuffer;
 using v8::ArrayBufferView;
+using v8::BackingStore;
+using v8::Boolean;
 using v8::Context;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
+using v8::Integer;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
@@ -57,13 +59,43 @@ void ManagedX509::MemoryInfo(MemoryTracker* tracker) const {
 }
 
 namespace {
+void AddFingerprintDigest(const unsigned char* md,
+                          unsigned int md_size,
+                          char fingerprint[3 * EVP_MAX_MD_SIZE]) {
+  unsigned int i;
+  static constexpr char hex[] = "0123456789ABCDEF";
+
+  for (i = 0; i < md_size; i++) {
+    fingerprint[3 * i] = hex[(md[i] & 0xf0) >> 4];
+    fingerprint[(3 * i) + 1] = hex[(md[i] & 0x0f)];
+    fingerprint[(3 * i) + 2] = ':';
+  }
+
+  DCHECK_GT(md_size, 0);
+  fingerprint[(3 * (md_size - 1)) + 2] = '\0';
+}
+
+MaybeLocal<Value> GetFingerprintDigest(Environment* env,
+                                       const EVP_MD* method,
+                                       const ncrypto::X509View& cert) {
+  unsigned char md[EVP_MAX_MD_SIZE];
+  unsigned int md_size;
+  char fingerprint[EVP_MAX_MD_SIZE * 3];
+
+  if (X509_digest(cert.get(), method, md, &md_size)) {
+    AddFingerprintDigest(md, md_size, fingerprint);
+    return OneByteString(env->isolate(), fingerprint);
+  }
+  return Undefined(env->isolate());
+}
+
 template <const EVP_MD* (*algo)()>
 void Fingerprint(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (GetFingerprintDigest(env, algo(), cert->get()).ToLocal(&ret)) {
+  if (GetFingerprintDigest(env, algo(), cert->view()).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -82,23 +114,164 @@ MaybeLocal<Value> ToV8Value(Local<Context> context, BIOPointer&& bio) {
   return ret;
 }
 
-MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer&& bio) {
+MaybeLocal<Value> ToV8Value(Local<Context> context, const ASN1_OBJECT* obj) {
+  // If OpenSSL knows the type, use the short name of the type as the key, and
+  // the numeric representation of the type's OID otherwise.
+  int nid = OBJ_obj2nid(obj);
+  char buf[80];
+  const char* str;
+  if (nid != NID_undef) {
+    str = OBJ_nid2sn(nid);
+    CHECK_NOT_NULL(str);
+  } else {
+    OBJ_obj2txt(buf, sizeof(buf), obj, true);
+    str = buf;
+  }
+
+  Local<Value> result;
+  if (!String::NewFromUtf8(context->GetIsolate(), str).ToLocal(&result)) {
+    return {};
+  }
+  return result;
+}
+
+MaybeLocal<Value> ToV8Value(Local<Context> context, const ASN1_STRING* str) {
+  // The previous implementation used X509_NAME_print_ex, which escapes some
+  // characters in the value. The old implementation did not decode/unescape
+  // values correctly though, leading to ambiguous and incorrect
+  // representations. The new implementation only converts to Unicode and does
+  // not escape anything.
+  unsigned char* value_str;
+  int value_str_size = ASN1_STRING_to_UTF8(&value_str, str);
+  if (value_str_size < 0) {
+    return Undefined(context->GetIsolate());
+  }
+  ncrypto::DataPointer free_value_str(value_str, value_str_size);
+
+  Local<Value> result;
+  if (!String::NewFromUtf8(context->GetIsolate(),
+                           reinterpret_cast<const char*>(value_str),
+                           NewStringType::kNormal,
+                           value_str_size)
+           .ToLocal(&result)) {
+    return {};
+  }
+  return result;
+}
+
+MaybeLocal<Value> ToV8Value(Local<Context> context, const BIOPointer& bio) {
   if (!bio) return {};
   BUF_MEM* mem;
   BIO_get_mem_ptr(bio.get(), &mem);
+  Local<Value> ret;
+  if (!String::NewFromUtf8(context->GetIsolate(),
+                           mem->data,
+                           NewStringType::kNormal,
+                           mem->length)
+           .ToLocal(&ret))
+    return {};
+  return ret;
+}
+
+MaybeLocal<Value> ToBuffer(Environment* env, BIOPointer* bio) {
+  if (bio == nullptr || !*bio) return {};
+  BUF_MEM* mem;
+  BIO_get_mem_ptr(bio->get(), &mem);
   auto backing = ArrayBuffer::NewBackingStore(
       mem->data,
       mem->length,
       [](void*, size_t, void* data) {
         BIOPointer free_me(static_cast<BIO*>(data));
       },
-      bio.release());
+      bio->release());
   auto ab = ArrayBuffer::New(env->isolate(), std::move(backing));
   Local<Value> ret;
   if (!Buffer::New(env, ab, 0, ab->ByteLength()).ToLocal(&ret)) return {};
   return ret;
 }
 
+MaybeLocal<Value> GetDer(Environment* env, const ncrypto::X509View& view) {
+  Local<Value> ret;
+  auto bio = view.toDER();
+  if (!bio) return Undefined(env->isolate());
+  if (!ToBuffer(env, &bio).ToLocal(&ret)) {
+    return {};
+  }
+  return ret;
+}
+
+MaybeLocal<Value> GetSubjectAltNameString(Environment* env,
+                                          const ncrypto::X509View& view) {
+  Local<Value> ret;
+  auto bio = view.getSubjectAltName();
+  if (!bio) return Undefined(env->isolate());
+  if (!ToV8Value(env->context(), bio).ToLocal(&ret)) return {};
+  return ret;
+}
+
+MaybeLocal<Value> GetInfoAccessString(Environment* env,
+                                      const ncrypto::X509View& view) {
+  Local<Value> ret;
+  auto bio = view.getInfoAccess();
+  if (!bio) return Undefined(env->isolate());
+  if (!ToV8Value(env->context(), bio).ToLocal(&ret)) {
+    return {};
+  }
+  return ret;
+}
+
+MaybeLocal<Value> GetValidFrom(Environment* env,
+                               const ncrypto::X509View& view) {
+  Local<Value> ret;
+  auto bio = view.getValidFrom();
+  if (!bio) return Undefined(env->isolate());
+  if (!ToV8Value(env->context(), bio).ToLocal(&ret)) {
+    return {};
+  }
+  return ret;
+}
+
+MaybeLocal<Value> GetValidTo(Environment* env, const ncrypto::X509View& view) {
+  Local<Value> ret;
+  auto bio = view.getValidTo();
+  if (!bio) return Undefined(env->isolate());
+  if (!ToV8Value(env->context(), bio).ToLocal(&ret)) {
+    return {};
+  }
+  return ret;
+}
+
+MaybeLocal<Value> GetSerialNumber(Environment* env,
+                                  const ncrypto::X509View& view) {
+  if (auto serial = view.getSerialNumber()) {
+    return OneByteString(env->isolate(),
+                         static_cast<unsigned char*>(serial.get()));
+  }
+  return Undefined(env->isolate());
+}
+
+MaybeLocal<Value> GetKeyUsage(Environment* env, const ncrypto::X509View& cert) {
+  ncrypto::StackOfASN1 eku(static_cast<STACK_OF(ASN1_OBJECT)*>(
+      X509_get_ext_d2i(cert.get(), NID_ext_key_usage, nullptr, nullptr)));
+  if (eku) {
+    const int count = sk_ASN1_OBJECT_num(eku.get());
+    MaybeStackBuffer<Local<Value>, 16> ext_key_usage(count);
+    char buf[256];
+
+    int j = 0;
+    for (int i = 0; i < count; i++) {
+      if (OBJ_obj2txt(
+              buf, sizeof(buf), sk_ASN1_OBJECT_value(eku.get(), i), 1) >= 0) {
+        ext_key_usage[j++] = OneByteString(env->isolate(), buf);
+      }
+    }
+
+    return Array::New(env->isolate(), ext_key_usage.out(), count);
+  }
+
+  return Undefined(env->isolate());
+}
+
 void Pem(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
@@ -114,7 +287,7 @@ void Der(const FunctionCallbackInfo<Value>& args) {
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (ToBuffer(env, cert->view().toDER()).ToLocal(&ret)) {
+  if (GetDer(env, cert->view()).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -134,8 +307,7 @@ void SubjectAltName(const FunctionCallbackInfo<Value>& args) {
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (ToV8Value(env->context(), cert->view().getSubjectAltName())
+  if (GetSubjectAltNameString(env, cert->view()).ToLocal(&ret)) {
-          .ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -155,7 +327,7 @@ void InfoAccess(const FunctionCallbackInfo<Value>& args) {
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (ToV8Value(env->context(), cert->view().getInfoAccess()).ToLocal(&ret)) {
+  if (GetInfoAccessString(env, cert->view()).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -165,7 +337,7 @@ void ValidFrom(const FunctionCallbackInfo<Value>& args) {
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (ToV8Value(env->context(), cert->view().getValidFrom()).ToLocal(&ret)) {
+  if (GetValidFrom(env, cert->view()).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -175,7 +347,7 @@ void ValidTo(const FunctionCallbackInfo<Value>& args) {
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   Local<Value> ret;
-  if (ToV8Value(env->context(), cert->view().getValidTo()).ToLocal(&ret)) {
+  if (GetValidTo(env, cert->view()).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
@@ -184,9 +356,9 @@ void SerialNumber(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
-  if (auto serial = cert->view().getSerialNumber()) {
+  Local<Value> ret;
-    args.GetReturnValue().Set(OneByteString(
+  if (GetSerialNumber(env, cert->view()).ToLocal(&ret)) {
-        env->isolate(), static_cast<unsigned char*>(serial.get())));
+    args.GetReturnValue().Set(ret);
   }
 }
 
@@ -215,24 +387,10 @@ void KeyUsage(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   X509Certificate* cert;
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
-
+  Local<Value> ret;
-  auto eku = cert->view().getKeyUsage();
+  if (GetKeyUsage(env, cert->view()).ToLocal(&ret)) {
-  if (!eku) return;
+    args.GetReturnValue().Set(ret);
-
-  const int count = sk_ASN1_OBJECT_num(eku.get());
-  MaybeStackBuffer<Local<Value>, 16> ext_key_usage(count);
-  char buf[256];
-
-  int j = 0;
-  for (int i = 0; i < count; i++) {
-    if (OBJ_obj2txt(buf, sizeof(buf), sk_ASN1_OBJECT_value(eku.get(), i), 1) >=
-        0) {
-      ext_key_usage[j++] = OneByteString(env->isolate(), buf);
-    }
   }
-
-  args.GetReturnValue().Set(
-      Array::New(env->isolate(), ext_key_usage.out(), count));
 }
 
 void CheckCA(const FunctionCallbackInfo<Value>& args) {
@@ -384,10 +542,280 @@ void ToLegacy(const FunctionCallbackInfo<Value>& args) {
   ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
   ClearErrorOnReturn clear_error_on_return;
   Local<Value> ret;
-  if (X509ToObject(env, cert->get()).ToLocal(&ret)) {
+  if (cert->toObject(env).ToLocal(&ret)) {
     args.GetReturnValue().Set(ret);
   }
 }
+
+template <typename T>
+bool Set(Environment* env,
+         Local<Object> target,
+         Local<Value> name,
+         MaybeLocal<T> maybe_value) {
+  Local<Value> value;
+  if (!maybe_value.ToLocal(&value)) return false;
+
+  // Undefined is ignored, but still considered successful
+  if (value->IsUndefined()) return true;
+
+  return !target->Set(env->context(), name, value).IsNothing();
+}
+
+template <typename T>
+bool Set(Environment* env,
+         Local<Object> target,
+         uint32_t index,
+         MaybeLocal<T> maybe_value) {
+  Local<Value> value;
+  if (!maybe_value.ToLocal(&value)) return false;
+
+  // Undefined is ignored, but still considered successful
+  if (value->IsUndefined()) return true;
+
+  return !target->Set(env->context(), index, value).IsNothing();
+}
+
+// Convert an X509_NAME* into a JavaScript object.
+// Each entry of the name is converted into a property of the object.
+// The property value may be a single string or an array of strings.
+template <X509_NAME* get_name(const X509*)>
+static MaybeLocal<Value> GetX509NameObject(Environment* env,
+                                           const ncrypto::X509View& cert) {
+  X509_NAME* name = get_name(cert.get());
+  CHECK_NOT_NULL(name);
+
+  int cnt = X509_NAME_entry_count(name);
+  CHECK_GE(cnt, 0);
+
+  Local<Value> v8_name;
+  Local<Value> v8_value;
+  // Note the the resulting object uses a null prototype.
+  Local<Object> result =
+      Object::New(env->isolate(), Null(env->isolate()), nullptr, nullptr, 0);
+  if (result.IsEmpty()) return {};
+
+  for (int i = 0; i < cnt; i++) {
+    X509_NAME_ENTRY* entry = X509_NAME_get_entry(name, i);
+    CHECK_NOT_NULL(entry);
+
+    if (!ToV8Value(env->context(), X509_NAME_ENTRY_get_object(entry))
+             .ToLocal(&v8_name) ||
+        !ToV8Value(env->context(), X509_NAME_ENTRY_get_data(entry))
+             .ToLocal(&v8_value)) {
+      return {};
+    }
+
+    // For backward compatibility, we only create arrays if multiple values
+    // exist for the same key. That is not great but there is not much we can
+    // change here without breaking things. Note that this creates nested data
+    // structures, yet still does not allow representing Distinguished Names
+    // accurately.
+    bool multiple;
+    if (!result->Has(env->context(), v8_name).To(&multiple)) {
+      return {};
+    }
+
+    if (multiple) {
+      Local<Value> accum;
+      if (!result->Get(env->context(), v8_name).ToLocal(&accum)) {
+        return {};
+      }
+      if (!accum->IsArray()) {
+        Local<Value> items[] = {
+            accum,
+            v8_value,
+        };
+        accum = Array::New(env->isolate(), items, arraysize(items));
+        if (!Set<Value>(env, result, v8_name, accum)) {
+          return {};
+        }
+      } else {
+        Local<Array> array = accum.As<Array>();
+        if (!Set<Value>(env, array, array->Length(), v8_value)) {
+          return {};
+        }
+      }
+      continue;
+    }
+
+    if (!Set<Value>(env, result, v8_name, v8_value)) {
+      return {};
+    }
+  }
+
+  return result;
+}
+
+MaybeLocal<Object> GetPubKey(Environment* env, OSSL3_CONST RSA* rsa) {
+  int size = i2d_RSA_PUBKEY(rsa, nullptr);
+  CHECK_GE(size, 0);
+
+  std::unique_ptr<BackingStore> bs;
+  {
+    NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
+    bs = ArrayBuffer::NewBackingStore(env->isolate(), size);
+  }
+
+  unsigned char* serialized = reinterpret_cast<unsigned char*>(bs->Data());
+  CHECK_GE(i2d_RSA_PUBKEY(rsa, &serialized), 0);
+
+  Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
+  return Buffer::New(env, ab, 0, ab->ByteLength()).FromMaybe(Local<Object>());
+}
+
+MaybeLocal<Value> GetModulusString(Environment* env, const BIGNUM* n) {
+  BIOPointer bio(BIO_new(BIO_s_mem()));
+  BN_print(bio.get(), n);
+  return ToV8Value(env->context(), bio);
+}
+
+MaybeLocal<Value> GetExponentString(Environment* env, const BIGNUM* e) {
+  uint64_t exponent_word = static_cast<uint64_t>(BignumPointer::GetWord(e));
+  BIOPointer bio(BIO_new(BIO_s_mem()));
+  BIO_printf(bio.get(), "0x%" PRIx64, exponent_word);
+  return ToV8Value(env->context(), bio);
+}
+
+MaybeLocal<Value> GetECPubKey(Environment* env,
+                              const EC_GROUP* group,
+                              OSSL3_CONST EC_KEY* ec) {
+  const EC_POINT* pubkey = EC_KEY_get0_public_key(ec);
+  if (pubkey == nullptr) return Undefined(env->isolate());
+
+  return ECPointToBuffer(env, group, pubkey, EC_KEY_get_conv_form(ec), nullptr)
+      .FromMaybe(Local<Object>());
+}
+
+MaybeLocal<Value> GetECGroupBits(Environment* env, const EC_GROUP* group) {
+  if (group == nullptr) return Undefined(env->isolate());
+
+  int bits = EC_GROUP_order_bits(group);
+  if (bits <= 0) return Undefined(env->isolate());
+
+  return Integer::New(env->isolate(), bits);
+}
+
+template <const char* (*nid2string)(int nid)>
+MaybeLocal<Value> GetCurveName(Environment* env, const int nid) {
+  const char* name = nid2string(nid);
+  return name != nullptr
+             ? MaybeLocal<Value>(OneByteString(env->isolate(), name))
+             : MaybeLocal<Value>(Undefined(env->isolate()));
+}
+
+MaybeLocal<Object> X509ToObject(Environment* env,
+                                const ncrypto::X509View& cert) {
+  EscapableHandleScope scope(env->isolate());
+  Local<Object> info = Object::New(env->isolate());
+
+  if (!Set<Value>(env,
+                  info,
+                  env->subject_string(),
+                  GetX509NameObject<X509_get_subject_name>(env, cert)) ||
+      !Set<Value>(env,
+                  info,
+                  env->issuer_string(),
+                  GetX509NameObject<X509_get_issuer_name>(env, cert)) ||
+      !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()))) {
+    return {};
+  }
+
+  OSSL3_CONST EVP_PKEY* pkey = X509_get0_pubkey(cert.get());
+  OSSL3_CONST RSA* rsa = nullptr;
+  OSSL3_CONST EC_KEY* ec = nullptr;
+  if (pkey != nullptr) {
+    switch (EVP_PKEY_id(pkey)) {
+      case EVP_PKEY_RSA:
+        rsa = EVP_PKEY_get0_RSA(pkey);
+        break;
+      case EVP_PKEY_EC:
+        ec = EVP_PKEY_get0_EC_KEY(pkey);
+        break;
+    }
+  }
+
+  if (rsa) {
+    const BIGNUM* n;
+    const BIGNUM* e;
+    RSA_get0_key(rsa, &n, &e, nullptr);
+    if (!Set<Value>(
+            env, info, env->modulus_string(), GetModulusString(env, n)) ||
+        !Set<Value>(
+            env,
+            info,
+            env->bits_string(),
+            Integer::New(env->isolate(), BignumPointer::GetBitCount(n))) ||
+        !Set<Value>(
+            env, info, env->exponent_string(), GetExponentString(env, e)) ||
+        !Set<Object>(env, info, env->pubkey_string(), GetPubKey(env, rsa))) {
+      return {};
+    }
+  } else if (ec) {
+    const EC_GROUP* group = EC_KEY_get0_group(ec);
+
+    if (!Set<Value>(
+            env, info, env->bits_string(), GetECGroupBits(env, group)) ||
+        !Set<Value>(
+            env, info, env->pubkey_string(), GetECPubKey(env, group, ec))) {
+      return {};
+    }
+
+    const int nid = EC_GROUP_get_curve_name(group);
+    if (nid != 0) {
+      // 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))) {
+        return {};
+      }
+    } else {
+      // Unnamed curves can be described by their mathematical properties,
+      // but aren't used much (at all?) with X.509/TLS. Support later if needed.
+    }
+  }
+
+  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, EVP_sha1(), cert)) ||
+      !Set<Value>(env,
+                  info,
+                  env->fingerprint256_string(),
+                  GetFingerprintDigest(env, EVP_sha256(), cert)) ||
+      !Set<Value>(env,
+                  info,
+                  env->fingerprint512_string(),
+                  GetFingerprintDigest(env, EVP_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))) {
+    return {};
+  }
+
+  return scope.Escape(info);
+}
 }  // namespace
 
 Local<FunctionTemplate> X509Certificate::GetConstructorTemplate(
@@ -400,29 +828,33 @@ Local<FunctionTemplate> X509Certificate::GetConstructorTemplate(
         BaseObject::kInternalFieldCount);
     tmpl->SetClassName(
         FIXED_ONE_BYTE_STRING(env->isolate(), "X509Certificate"));
-    SetProtoMethod(isolate, tmpl, "subject", Subject);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "subject", Subject);
-    SetProtoMethod(isolate, tmpl, "subjectAltName", SubjectAltName);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "subjectAltName", SubjectAltName);
-    SetProtoMethod(isolate, tmpl, "infoAccess", InfoAccess);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "infoAccess", InfoAccess);
-    SetProtoMethod(isolate, tmpl, "issuer", Issuer);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "issuer", Issuer);
-    SetProtoMethod(isolate, tmpl, "validTo", ValidTo);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "validTo", ValidTo);
-    SetProtoMethod(isolate, tmpl, "validFrom", ValidFrom);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "validFrom", ValidFrom);
-    SetProtoMethod(isolate, tmpl, "fingerprint", Fingerprint<EVP_sha1>);
+    SetProtoMethodNoSideEffect(
-    SetProtoMethod(isolate, tmpl, "fingerprint256", Fingerprint<EVP_sha256>);
+        isolate, tmpl, "fingerprint", Fingerprint<EVP_sha1>);
-    SetProtoMethod(isolate, tmpl, "fingerprint512", Fingerprint<EVP_sha512>);
+    SetProtoMethodNoSideEffect(
-    SetProtoMethod(isolate, tmpl, "keyUsage", KeyUsage);
+        isolate, tmpl, "fingerprint256", Fingerprint<EVP_sha256>);
-    SetProtoMethod(isolate, tmpl, "serialNumber", SerialNumber);
+    SetProtoMethodNoSideEffect(
-    SetProtoMethod(isolate, tmpl, "pem", Pem);
+        isolate, tmpl, "fingerprint512", Fingerprint<EVP_sha512>);
-    SetProtoMethod(isolate, tmpl, "raw", Der);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "keyUsage", KeyUsage);
-    SetProtoMethod(isolate, tmpl, "publicKey", PublicKey);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "serialNumber", SerialNumber);
-    SetProtoMethod(isolate, tmpl, "checkCA", CheckCA);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "pem", Pem);
-    SetProtoMethod(isolate, tmpl, "checkHost", CheckHost);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "raw", Der);
-    SetProtoMethod(isolate, tmpl, "checkEmail", CheckEmail);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "publicKey", PublicKey);
-    SetProtoMethod(isolate, tmpl, "checkIP", CheckIP);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "checkCA", CheckCA);
-    SetProtoMethod(isolate, tmpl, "checkIssued", CheckIssued);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "checkHost", CheckHost);
-    SetProtoMethod(isolate, tmpl, "checkPrivateKey", CheckPrivateKey);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "checkEmail", CheckEmail);
-    SetProtoMethod(isolate, tmpl, "verify", CheckPublicKey);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "checkIP", CheckIP);
-    SetProtoMethod(isolate, tmpl, "toLegacy", ToLegacy);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "checkIssued", CheckIssued);
-    SetProtoMethod(isolate, tmpl, "getIssuerCert", GetIssuerCert);
+    SetProtoMethodNoSideEffect(
+        isolate, tmpl, "checkPrivateKey", CheckPrivateKey);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "verify", CheckPublicKey);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "toLegacy", ToLegacy);
+    SetProtoMethodNoSideEffect(isolate, tmpl, "getIssuerCert", GetIssuerCert);
     env->set_x509_constructor_template(tmpl);
   }
   return tmpl;
@@ -457,12 +889,9 @@ MaybeLocal<Object> X509Certificate::New(Environment* env,
 
 MaybeLocal<Object> X509Certificate::GetCert(Environment* env,
                                             const SSLPointer& ssl) {
-  ClearErrorOnReturn clear_error_on_return;
+  auto cert = ncrypto::X509View::From(ssl);
-  X509* cert = SSL_get_certificate(ssl.get());
+  if (!cert) return {};
-  if (cert == nullptr) return MaybeLocal<Object>();
+  return New(env, cert.clone());
-
-  X509Pointer ptr(X509_dup(cert));
-  return New(env, std::move(ptr));
 }
 
 MaybeLocal<Object> X509Certificate::GetPeerCert(Environment* env,
@@ -471,16 +900,16 @@ MaybeLocal<Object> X509Certificate::GetPeerCert(Environment* env,
   ClearErrorOnReturn clear_error_on_return;
   MaybeLocal<Object> maybe_cert;
 
-  bool is_server =
+  X509Pointer cert;
-      static_cast<int>(flag) & static_cast<int>(GetPeerCertificateFlag::SERVER);
+  if ((flag & GetPeerCertificateFlag::SERVER) ==
+      GetPeerCertificateFlag::SERVER) {
+    cert = X509Pointer::PeerFrom(ssl);
+  }
 
-  X509Pointer cert(is_server ? SSL_get_peer_certificate(ssl.get()) : nullptr);
   STACK_OF(X509)* ssl_certs = SSL_get_peer_cert_chain(ssl.get());
   if (!cert && (ssl_certs == nullptr || sk_X509_num(ssl_certs) == 0))
     return MaybeLocal<Object>();
 
-  std::vector<Local<Value>> certs;
-
   if (!cert) {
     cert.reset(sk_X509_value(ssl_certs, 0));
     sk_X509_delete(ssl_certs, 0);
@@ -490,13 +919,14 @@ MaybeLocal<Object> X509Certificate::GetPeerCert(Environment* env,
                                 : New(env, std::move(cert));
 }
 
-template <MaybeLocal<Value> Property(Environment* env, X509* cert)>
+v8::MaybeLocal<v8::Value> X509Certificate::toObject(Environment* env) {
-static void ReturnProperty(const FunctionCallbackInfo<Value>& args) {
+  return toObject(env, view());
-  Environment* env = Environment::GetCurrent(args);
+}
-  X509Certificate* cert;
+
-  ASSIGN_OR_RETURN_UNWRAP(&cert, args.This());
+v8::MaybeLocal<v8::Value> X509Certificate::toObject(
-  Local<Value> ret;
+    Environment* env, const ncrypto::X509View& cert) {
-  if (Property(env, cert->get()).ToLocal(&ret)) args.GetReturnValue().Set(ret);
+  if (!cert) return {};
+  return X509ToObject(env, cert).FromMaybe(Local<Value>());
 }
 
 X509Certificate::X509Certificate(
@@ -551,7 +981,6 @@ std::unique_ptr<worker::TransferData> X509Certificate::CloneForMessaging()
   return std::make_unique<X509CertificateTransferData>(cert_);
 }
 
-
 void X509Certificate::Initialize(Environment* env, Local<Object> target) {
   SetMethod(env->context(), target, "parseX509", Parse);
 

src/crypto/crypto_x509.h

@@ -82,6 +82,10 @@ class X509Certificate final : public BaseObject {
   inline ncrypto::X509View view() const { return *cert_; }
   X509* get() { return cert_->get(); }
 
+  v8::MaybeLocal<v8::Value> toObject(Environment* env);
+  static v8::MaybeLocal<v8::Value> toObject(Environment* env,
+                                            const ncrypto::X509View& cert);
+
   void MemoryInfo(MemoryTracker* tracker) const override;
   SET_MEMORY_INFO_NAME(X509Certificate)
   SET_SELF_SIZE(X509Certificate)
@@ -119,6 +123,20 @@ class X509Certificate final : public BaseObject {
   BaseObjectPtr<X509Certificate> issuer_cert_;
 };
 
+inline X509Certificate::GetPeerCertificateFlag operator|(
+    X509Certificate::GetPeerCertificateFlag lhs,
+    X509Certificate::GetPeerCertificateFlag rhs) {
+  return static_cast<X509Certificate::GetPeerCertificateFlag>(
+      static_cast<int>(lhs) | static_cast<int>(rhs));
+}
+
+inline X509Certificate::GetPeerCertificateFlag operator&(
+    X509Certificate::GetPeerCertificateFlag lhs,
+    X509Certificate::GetPeerCertificateFlag rhs) {
+  return static_cast<X509Certificate::GetPeerCertificateFlag>(
+      static_cast<int>(lhs) & static_cast<int>(rhs));
+}
+
 }  // namespace crypto
 }  // namespace node
 

test/parallel/test-tls-empty-sni-context.js

@@ -16,7 +16,7 @@ const options = {
 const server = tls.createServer(options, (c) => {
   assert.fail('Should not be called');
 }).on('tlsClientError', common.mustCall((err, c) => {
-  assert.match(err.message, /passed a null parameter/i);
+  assert.match(err.message, /no suitable signature algorithm/i);
   server.close();
 })).listen(0, common.mustCall(() => {
   const c = tls.connect({