Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
src: deduplicate X509 getter implementations
Browse files Browse the repository at this point in the history
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: #48563
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Alba Mendez <[email protected]>
tniessen authored and ruyadorno committed Sep 11, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 83fe6b1 commit 1ed0697
Showing 3 changed files with 53 additions and 100 deletions.
46 changes: 19 additions & 27 deletions src/crypto/crypto_common.cc
Original file line number Diff line number Diff line change
@@ -492,7 +492,7 @@ MaybeLocal<Value> GetModulusString(
}
} // namespace

MaybeLocal<Object> GetRawDERCertificate(Environment* env, X509* cert) {
MaybeLocal<Value> GetRawDERCertificate(Environment* env, X509* cert) {
int size = i2d_X509(cert, nullptr);

std::unique_ptr<BackingStore> bs;
@@ -872,10 +872,9 @@ bool SafeX509InfoAccessPrint(const BIOPointer& out, X509_EXTENSION* ext) {
return ok;
}

v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
Environment* env,
const BIOPointer& bio,
X509* cert) {
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());
@@ -891,10 +890,9 @@ v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
return ToV8Value(env, bio);
}

v8::MaybeLocal<v8::Value> GetInfoAccessString(
Environment* env,
const BIOPointer& bio,
X509* cert) {
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());
@@ -910,10 +908,9 @@ v8::MaybeLocal<v8::Value> GetInfoAccessString(
return ToV8Value(env, bio);
}

MaybeLocal<Value> GetIssuerString(
Environment* env,
const BIOPointer& bio,
X509* cert) {
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(),
@@ -927,10 +924,9 @@ MaybeLocal<Value> GetIssuerString(
return ToV8Value(env, bio);
}

MaybeLocal<Value> GetSubject(
Environment* env,
const BIOPointer& bio,
X509* cert) {
MaybeLocal<Value> 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<Object> X509ToObject(
!Set<Value>(context,
info,
env->subjectaltname_string(),
GetSubjectAltNameString(env, bio, cert)) ||
GetSubjectAltNameString(env, cert, bio)) ||
!Set<Value>(context,
info,
env->infoaccess_string(),
GetInfoAccessString(env, bio, cert)) ||
GetInfoAccessString(env, cert, bio)) ||
!Set<Boolean>(context, info, env->ca_string(), is_ca)) {
return MaybeLocal<Object>();
}
@@ -1390,18 +1386,14 @@ MaybeLocal<Object> X509ToObject(
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->ext_key_usage_string(), GetKeyUsage(env, cert)) ||
!Set<Value>(context,
info,
env->serial_number_string(),
GetSerialNumber(env, cert)) ||
!Set<Object>(context,
info,
env->raw_string(),
GetRawDERCertificate(env, cert))) {
!Set<Value>(
context, info, env->raw_string(), GetRawDERCertificate(env, cert))) {
return MaybeLocal<Object>();
}

30 changes: 13 additions & 17 deletions src/crypto/crypto_common.h
Original file line number Diff line number Diff line change
@@ -118,30 +118,26 @@ v8::MaybeLocal<v8::Value> GetCurrentCipherVersion(Environment* env,

v8::MaybeLocal<v8::Value> GetSerialNumber(Environment* env, X509* cert);

v8::MaybeLocal<v8::Object> GetRawDERCertificate(Environment* env, X509* cert);
v8::MaybeLocal<v8::Value> GetRawDERCertificate(Environment* env, X509* cert);

v8::Local<v8::Value> ToV8Value(Environment* env, const BIOPointer& bio);
bool SafeX509SubjectAltNamePrint(const BIOPointer& out, X509_EXTENSION* ext);

v8::MaybeLocal<v8::Value> GetSubject(
Environment* env,
const BIOPointer& bio,
X509* cert);
v8::MaybeLocal<v8::Value> GetSubject(Environment* env,
X509* cert,
const BIOPointer& bio);

v8::MaybeLocal<v8::Value> GetIssuerString(
Environment* env,
const BIOPointer& bio,
X509* cert);
v8::MaybeLocal<v8::Value> GetIssuerString(Environment* env,
X509* cert,
const BIOPointer& bio);

v8::MaybeLocal<v8::Value> GetSubjectAltNameString(
Environment* env,
const BIOPointer& bio,
X509* cert);
v8::MaybeLocal<v8::Value> GetSubjectAltNameString(Environment* env,
X509* cert,
const BIOPointer& bio);

v8::MaybeLocal<v8::Value> GetInfoAccessString(
Environment* env,
const BIOPointer& bio,
X509* cert);
v8::MaybeLocal<v8::Value> GetInfoAccessString(Environment* env,
X509* cert,
const BIOPointer& bio);

} // namespace crypto
} // namespace node
77 changes: 21 additions & 56 deletions src/crypto/crypto_x509.cc
Original file line number Diff line number Diff line change
@@ -204,97 +204,62 @@ void X509Certificate::Parse(const FunctionCallbackInfo<Value>& args) {
args.GetReturnValue().Set(cert);
}

void X509Certificate::Subject(const FunctionCallbackInfo<Value>& args) {
template <MaybeLocal<Value> Property(
Environment* env, X509* cert, const BIOPointer& bio)>
static void ReturnPropertyThroughBIO(const FunctionCallbackInfo<Value>& 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<Value> 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<Value>& args) {
ReturnPropertyThroughBIO<GetSubject>(args);
}

void X509Certificate::Issuer(const FunctionCallbackInfo<Value>& 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<Value> ret;
if (GetIssuerString(env, bio, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnPropertyThroughBIO<GetIssuerString>(args);
}

void X509Certificate::SubjectAltName(const FunctionCallbackInfo<Value>& 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<Value> ret;
if (GetSubjectAltNameString(env, bio, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnPropertyThroughBIO<GetSubjectAltNameString>(args);
}

void X509Certificate::InfoAccess(const FunctionCallbackInfo<Value>& 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<Value> ret;
if (GetInfoAccessString(env, bio, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnPropertyThroughBIO<GetInfoAccessString>(args);
}

void X509Certificate::ValidFrom(const FunctionCallbackInfo<Value>& 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<Value> ret;
if (GetValidFrom(env, cert->get(), bio).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnPropertyThroughBIO<GetValidFrom>(args);
}

void X509Certificate::ValidTo(const FunctionCallbackInfo<Value>& args) {
ReturnPropertyThroughBIO<GetValidTo>(args);
}

template <MaybeLocal<Value> Property(Environment* env, X509* cert)>
static void ReturnProperty(const FunctionCallbackInfo<Value>& 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<Value> 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<Value>& args) {
Environment* env = Environment::GetCurrent(args);
X509Certificate* cert;
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
Local<Value> ret;
if (GetKeyUsage(env, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnProperty<GetKeyUsage>(args);
}

void X509Certificate::SerialNumber(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
X509Certificate* cert;
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
Local<Value> ret;
if (GetSerialNumber(env, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnProperty<GetSerialNumber>(args);
}

void X509Certificate::Raw(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
X509Certificate* cert;
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
Local<Value> ret;
if (GetRawDERCertificate(env, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
ReturnProperty<GetRawDERCertificate>(args);
}

void X509Certificate::PublicKey(const FunctionCallbackInfo<Value>& args) {

0 comments on commit 1ed0697

Please sign in to comment.