1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

crypto: fix X509Certificate toLegacyObject

Browse Source 
PR-URL: https://github.com/nodejs/node/pull/42124
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This commit is contained in:
Tobias Nießen 2022-03-09 17:36:40 +01:00 committed by GitHub
parent eacd45656a
commit 36fb79030e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 61 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/internal/crypto/x509.js
View File

@ -56,6 +56,8 @@ const {
kHandle,
} = require('internal/crypto/util');
let lazyTranslatePeerCertificate;
const kInternalState = Symbol('kInternalState');
function isX509Certificate(value) {
@ -345,7 +347,11 @@ class X509Certificate extends JSTransferable {
}
toLegacyObject() {
return this[kHandle].toLegacy();
// TODO(tniessen): do not depend on translatePeerCertificate here, return
// the correct legacy representation from the binding
lazyTranslatePeerCertificate ??=
require('_tls_common').translatePeerCertificate;
return lazyTranslatePeerCertificate(this[kHandle].toLegacy());
}
}

38
src/crypto/crypto_common.cc
View File

@ -1284,8 +1284,7 @@ MaybeLocal<Value> GetPeerCert(
MaybeLocal<Object> X509ToObject(
Environment* env,
X509* cert,
bool names_as_string) {
X509* cert) {
EscapableHandleScope scope(env->isolate());
Local<Context> context = env->context();
Local<Object> info = Object::New(env->isolate());
@ -1293,34 +1292,15 @@ MaybeLocal<Object> X509ToObject(
BIOPointer bio(BIO_new(BIO_s_mem()));
CHECK(bio);
if (names_as_string) {
// TODO(tniessen): this branch should not have to exist. It is only here
// because toLegacyObject() does not actually return a legacy object, and
// instead represents subject and issuer as strings.
if (!Set<Value>(context,
info,
env->subject_string(),
GetSubject(env, bio, cert)) ||
!Set<Value>(context,
info,
env->issuer_string(),
GetIssuerString(env, bio, cert))) {
return MaybeLocal<Object>();
}
} else {
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))) {
return MaybeLocal<Object>();
}
}
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, bio, cert)) ||

3
src/crypto/crypto_common.h
View File

@ -109,8 +109,7 @@ v8::MaybeLocal<v8::Object> ECPointToBuffer(
v8::MaybeLocal<v8::Object> X509ToObject(
Environment* env,
X509* cert,
bool names_as_string = false);
X509* cert);
v8::MaybeLocal<v8::Value> GetValidTo(
Environment* env,

2
src/crypto/crypto_x509.cc
View File

@ -477,7 +477,7 @@ void X509Certificate::ToLegacy(const FunctionCallbackInfo<Value>& args) {
X509Certificate* cert;
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
Local<Value> ret;
if (X509ToObject(env, cert->get(), true).ToLocal(&ret))
if (X509ToObject(env, cert->get()).ToLocal(&ret))
args.GetReturnValue().Set(ret);
}

49
test/parallel/test-crypto-x509.js
View File

@ -198,27 +198,28 @@ const der = Buffer.from(
// Verify that legacy encoding works
const legacyObjectCheck = {
subject: 'C=US\n' +
'ST=CA\n' +
'L=SF\n' +
'O=Joyent\n' +
'OU=Node.js\n' +
'CN=agent1\n' +
'emailAddress=ry@tinyclouds.org',
issuer:
'C=US\n' +
'ST=CA\n' +
'L=SF\n' +
'O=Joyent\n' +
'OU=Node.js\n' +
'CN=ca1\n' +
'emailAddress=ry@tinyclouds.org',
infoAccess:
common.hasOpenSSL3 ?
'OCSP - URI:http://ocsp.nodejs.org/\n' +
'CA Issuers - URI:http://ca.nodejs.org/ca.cert' :
'OCSP - URI:http://ocsp.nodejs.org/\n' +
'CA Issuers - URI:http://ca.nodejs.org/ca.cert\n',
subject: Object.assign(Object.create(null), {
C: 'US',
ST: 'CA',
L: 'SF',
O: 'Joyent',
OU: 'Node.js',
CN: 'agent1',
emailAddress: 'ry@tinyclouds.org',
}),
issuer: Object.assign(Object.create(null), {
C: 'US',
ST: 'CA',
L: 'SF',
O: 'Joyent',
OU: 'Node.js',
CN: 'ca1',
emailAddress: 'ry@tinyclouds.org',
}),
infoAccess: Object.assign(Object.create(null), {
'OCSP - URI': ['http://ocsp.nodejs.org/'],
'CA Issuers - URI': ['http://ca.nodejs.org/ca.cert']
}),
modulus: 'EF5440701637E28ABB038E5641F828D834C342A9D25EDBB86A2BF' +
'6FBD809CB8E037A98B71708E001242E4DEB54C6164885F599DD87' +
'A23215745955BE20417E33C4D0D1B80C9DA3DE419A2607195D2FB' +
@ -243,9 +244,9 @@ const der = Buffer.from(
const legacyObject = x509.toLegacyObject();
assert.deepStrictEqual(legacyObject.raw, x509.raw);
assert.strictEqual(legacyObject.subject, legacyObjectCheck.subject);
assert.strictEqual(legacyObject.issuer, legacyObjectCheck.issuer);
assert.strictEqual(legacyObject.infoAccess, legacyObjectCheck.infoAccess);
assert.deepStrictEqual(legacyObject.subject, legacyObjectCheck.subject);
assert.deepStrictEqual(legacyObject.issuer, legacyObjectCheck.issuer);
assert.deepStrictEqual(legacyObject.infoAccess, legacyObjectCheck.infoAccess);
assert.strictEqual(legacyObject.modulus, legacyObjectCheck.modulus);
assert.strictEqual(legacyObject.bits, legacyObjectCheck.bits);
assert.strictEqual(legacyObject.exponent, legacyObjectCheck.exponent);

18
test/parallel/test-x509-escaping.js
View File

@ -241,6 +241,15 @@ const { hasOpenSSL3 } = common;
assert.deepStrictEqual(peerCert.infoAccess,
Object.assign(Object.create(null),
expected.legacy));
// toLegacyObject() should also produce the same properties. However,
// the X509Certificate is not aware of the chain, so we need to add
// the circular issuerCertificate reference manually for the assertion
// to be true.
const obj = cert.toLegacyObject();
assert.strictEqual(obj.issuerCertificate, undefined);
obj.issuerCertificate = obj;
assert.deepStrictEqual(peerCert, obj);
},
}, common.mustCall());
}));
@ -350,6 +359,15 @@ const { hasOpenSSL3 } = common;
// self-signed. Otherwise, OpenSSL would have already rejected the
// certificate while connecting to the TLS server.
assert.deepStrictEqual(peerCert.issuer, expectedObject);
// toLegacyObject() should also produce the same properties. However,
// the X509Certificate is not aware of the chain, so we need to add
// the circular issuerCertificate reference manually for the assertion
// to be true.
const obj = cert.toLegacyObject();
assert.strictEqual(obj.issuerCertificate, undefined);
obj.issuerCertificate = obj;
assert.deepStrictEqual(peerCert, obj);
},
}, common.mustCall());
}));