From df363d0010286e8f33a5b74ad858cfbcd011c9af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Mon, 3 Jul 2023 16:31:00 +0200 Subject: [PATCH] src: deduplicate X509 getter implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reorder arguments of internal helper functions such that their order is consistent across X509 property getters. Add ReturnPropertyThroughBIO() and ReturnProperty(). Use these new helpers to deduplicate code across various X509 property getters. PR-URL: https://github.com/nodejs/node/pull/48563 Reviewed-By: Juan José Arboleda Reviewed-By: Alba Mendez --- src/crypto/crypto_common.cc | 46 +++++++++------------- src/crypto/crypto_common.h | 30 +++++++-------- src/crypto/crypto_x509.cc | 77 ++++++++++--------------------------- 3 files changed, 53 insertions(+), 100 deletions(-) diff --git a/src/crypto/crypto_common.cc b/src/crypto/crypto_common.cc index 41e607e9298314..c6120a655ec853 100644 --- a/src/crypto/crypto_common.cc +++ b/src/crypto/crypto_common.cc @@ -492,7 +492,7 @@ MaybeLocal GetModulusString( } } // namespace -MaybeLocal GetRawDERCertificate(Environment* env, X509* cert) { +MaybeLocal GetRawDERCertificate(Environment* env, X509* cert) { int size = i2d_X509(cert, nullptr); std::unique_ptr bs; @@ -872,10 +872,9 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) { return ok; } -v8::MaybeLocal GetSubjectAltNameString( - Environment* env, - const BIOPointer& bio, - X509* cert) { +v8::MaybeLocal 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()); @@ -891,10 +890,9 @@ v8::MaybeLocal GetSubjectAltNameString( return ToV8Value(env, bio); } -v8::MaybeLocal GetInfoAccessString( - Environment* env, - const BIOPointer& bio, - X509* cert) { +v8::MaybeLocal 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()); @@ -910,10 +908,9 @@ v8::MaybeLocal GetInfoAccessString( return ToV8Value(env, bio); } -MaybeLocal GetIssuerString( - Environment* env, - const BIOPointer& bio, - X509* cert) { +MaybeLocal GetIssuerString(Environment* env, + X509* cert, + const BIOPointer& bio) { X509_NAME* issuer_name = X509_get_issuer_name(cert); if (X509_NAME_print_ex( bio.get(), @@ -927,10 +924,9 @@ MaybeLocal GetIssuerString( return ToV8Value(env, bio); } -MaybeLocal GetSubject( - Environment* env, - const BIOPointer& bio, - X509* cert) { +MaybeLocal GetSubject(Environment* env, + X509* cert, + const BIOPointer& bio) { if (X509_NAME_print_ex( bio.get(), X509_get_subject_name(cert), @@ -1283,11 +1279,11 @@ MaybeLocal X509ToObject( !Set(context, info, env->subjectaltname_string(), - GetSubjectAltNameString(env, bio, cert)) || + GetSubjectAltNameString(env, cert, bio)) || !Set(context, info, env->infoaccess_string(), - GetInfoAccessString(env, bio, cert)) || + GetInfoAccessString(env, cert, bio)) || !Set(context, info, env->ca_string(), is_ca)) { return MaybeLocal(); } @@ -1390,18 +1386,14 @@ MaybeLocal X509ToObject( info, env->fingerprint512_string(), GetFingerprintDigest(env, EVP_sha512(), cert)) || - !Set(context, - info, - env->ext_key_usage_string(), - GetKeyUsage(env, cert)) || + !Set( + context, info, env->ext_key_usage_string(), GetKeyUsage(env, cert)) || !Set(context, info, env->serial_number_string(), GetSerialNumber(env, cert)) || - !Set(context, - info, - env->raw_string(), - GetRawDERCertificate(env, cert))) { + !Set( + context, info, env->raw_string(), GetRawDERCertificate(env, cert))) { return MaybeLocal(); } diff --git a/src/crypto/crypto_common.h b/src/crypto/crypto_common.h index 99eb83aae86792..9eb3ff4460badd 100644 --- a/src/crypto/crypto_common.h +++ b/src/crypto/crypto_common.h @@ -118,30 +118,26 @@ v8::MaybeLocal GetCurrentCipherVersion(Environment* env, v8::MaybeLocal GetSerialNumber(Environment* env, X509* cert); -v8::MaybeLocal GetRawDERCertificate(Environment* env, X509* cert); +v8::MaybeLocal GetRawDERCertificate(Environment* env, X509* cert); v8::Local ToV8Value(Environment* env, const BIOPointer& bio); bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext); -v8::MaybeLocal GetSubject( - Environment* env, - const BIOPointer& bio, - X509* cert); +v8::MaybeLocal GetSubject(Environment* env, + X509* cert, + const BIOPointer& bio); -v8::MaybeLocal GetIssuerString( - Environment* env, - const BIOPointer& bio, - X509* cert); +v8::MaybeLocal GetIssuerString(Environment* env, + X509* cert, + const BIOPointer& bio); -v8::MaybeLocal GetSubjectAltNameString( - Environment* env, - const BIOPointer& bio, - X509* cert); +v8::MaybeLocal GetSubjectAltNameString(Environment* env, + X509* cert, + const BIOPointer& bio); -v8::MaybeLocal GetInfoAccessString( - Environment* env, - const BIOPointer& bio, - X509* cert); +v8::MaybeLocal GetInfoAccessString(Environment* env, + X509* cert, + const BIOPointer& bio); } // namespace crypto } // namespace node diff --git a/src/crypto/crypto_x509.cc b/src/crypto/crypto_x509.cc index 58946640b174a2..84f2528a28f8d0 100644 --- a/src/crypto/crypto_x509.cc +++ b/src/crypto/crypto_x509.cc @@ -203,97 +203,62 @@ void X509Certificate::Parse(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(cert); } -void X509Certificate::Subject(const FunctionCallbackInfo& args) { +template Property( + Environment* env, X509* cert, const BIOPointer& bio)> +static void ReturnPropertyThroughBIO(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); X509Certificate* cert; ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); BIOPointer bio(BIO_new(BIO_s_mem())); CHECK(bio); Local ret; - if (GetSubject(env, bio, cert->get()).ToLocal(&ret)) + if (Property(env, cert->get(), bio).ToLocal(&ret)) args.GetReturnValue().Set(ret); } +void X509Certificate::Subject(const FunctionCallbackInfo& args) { + ReturnPropertyThroughBIO(args); +} + void X509Certificate::Issuer(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - BIOPointer bio(BIO_new(BIO_s_mem())); - CHECK(bio); - Local ret; - if (GetIssuerString(env, bio, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnPropertyThroughBIO(args); } void X509Certificate::SubjectAltName(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - BIOPointer bio(BIO_new(BIO_s_mem())); - CHECK(bio); - Local ret; - if (GetSubjectAltNameString(env, bio, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnPropertyThroughBIO(args); } void X509Certificate::InfoAccess(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - BIOPointer bio(BIO_new(BIO_s_mem())); - CHECK(bio); - Local ret; - if (GetInfoAccessString(env, bio, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnPropertyThroughBIO(args); } void X509Certificate::ValidFrom(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - BIOPointer bio(BIO_new(BIO_s_mem())); - CHECK(bio); - Local ret; - if (GetValidFrom(env, cert->get(), bio).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnPropertyThroughBIO(args); } void X509Certificate::ValidTo(const FunctionCallbackInfo& args) { + ReturnPropertyThroughBIO(args); +} + +template Property(Environment* env, X509* cert)> +static void ReturnProperty(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); X509Certificate* cert; ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - BIOPointer bio(BIO_new(BIO_s_mem())); - CHECK(bio); Local ret; - if (GetValidTo(env, cert->get(), bio).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + if (Property(env, cert->get()).ToLocal(&ret)) args.GetReturnValue().Set(ret); } void X509Certificate::KeyUsage(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - Local ret; - if (GetKeyUsage(env, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnProperty(args); } void X509Certificate::SerialNumber(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - Local ret; - if (GetSerialNumber(env, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnProperty(args); } void X509Certificate::Raw(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - X509Certificate* cert; - ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder()); - Local ret; - if (GetRawDERCertificate(env, cert->get()).ToLocal(&ret)) - args.GetReturnValue().Set(ret); + ReturnProperty(args); } void X509Certificate::PublicKey(const FunctionCallbackInfo& args) {