From 446399c3c11b20d0afa6726a7ad399823b72e4d8 Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Sun, 6 Nov 2016 13:37:07 +0000 Subject: [PATCH 1/9] Expose whole certificate chain to verify proc and certificate-error event. --- atom/common/native_mate_converters/net_converter.cc | 4 ++++ docs/api/structures/certificate.md | 1 + 2 files changed, 5 insertions(+) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 00a06566a292..0310ee46129f 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -45,7 +45,11 @@ v8::Local Converter>::ToV8( std::string encoded_data; net::X509Certificate::GetPEMEncoded( val->os_cert_handle(), &encoded_data); + std::vector encoded_chain; + val->GetPEMEncodedChain(&encoded_chain); + dict.Set("data", encoded_data); + dict.Set("chain", encoded_chain); dict.Set("issuerName", val->issuer().GetDisplayName()); dict.Set("subjectName", val->subject().GetDisplayName()); dict.Set("serialNumber", base::HexEncode(val->serial_number().data(), diff --git a/docs/api/structures/certificate.md b/docs/api/structures/certificate.md index 95e15ceadaf5..164b2bc88f48 100644 --- a/docs/api/structures/certificate.md +++ b/docs/api/structures/certificate.md @@ -1,6 +1,7 @@ # Certificate Object * `data` String - PEM encoded data +* `chain` String[] - PEM encoded chain * `issuerName` String - Issuer's Common Name * `subjectName` String - Subject's Common Name * `serialNumber` String - Hex value represented string From 5d028f9163230f6bc21a2705980e5abf2d14af7b Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Wed, 9 Nov 2016 17:19:35 +0000 Subject: [PATCH 2/9] Expose extra certificate information: full breakdown of issuer and subject principals, as well as full structure of intermediate issuer certificates. --- .../native_mate_converters/net_converter.cc | 32 +++++++++++++++++-- .../native_mate_converters/net_converter.h | 7 ++++ docs/api/structures/certificate-principal.md | 8 +++++ docs/api/structures/certificate.md | 4 ++- 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 docs/api/structures/certificate-principal.md diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 0310ee46129f..af345698bf93 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -45,12 +45,11 @@ v8::Local Converter>::ToV8( std::string encoded_data; net::X509Certificate::GetPEMEncoded( val->os_cert_handle(), &encoded_data); - std::vector encoded_chain; - val->GetPEMEncodedChain(&encoded_chain); dict.Set("data", encoded_data); - dict.Set("chain", encoded_chain); + dict.Set("issuer", mate::ConvertToV8(isolate, val->issuer())); dict.Set("issuerName", val->issuer().GetDisplayName()); + dict.Set("subject", mate::ConvertToV8(isolate, val->subject())); dict.Set("subjectName", val->subject().GetDisplayName()); dict.Set("serialNumber", base::HexEncode(val->serial_number().data(), val->serial_number().size())); @@ -60,6 +59,33 @@ v8::Local Converter>::ToV8( net::HashValue( val->CalculateFingerprint256(val->os_cert_handle())).ToString()); + if (!val->GetIntermediateCertificates().empty()) { + net::X509Certificate::OSCertHandles issuer_intermediates( + val->GetIntermediateCertificates().begin() + 1, + val->GetIntermediateCertificates().end()); + const scoped_refptr& issuer_cert = + net::X509Certificate::CreateFromHandle( + val->GetIntermediateCertificates().front(), + issuer_intermediates); + dict.Set("issuerCert", mate::ConvertToV8(isolate, issuer_cert)); + } + + return dict.GetHandle(); +} + +// static +v8::Local Converter::ToV8( + v8::Isolate* isolate, const net::CertPrincipal& val) { + mate::Dictionary dict(isolate, v8::Object::New(isolate)); + + dict.Set("commonName", val.common_name); + dict.Set("organizations", mate::ConvertToV8(isolate, val.organization_names)); + dict.Set("organizationUnits", + mate::ConvertToV8(isolate, val.organization_unit_names)); + dict.Set("locality", val.locality_name); + dict.Set("state", val.state_or_province_name); + dict.Set("country", val.country_name); + return dict.GetHandle(); } diff --git a/atom/common/native_mate_converters/net_converter.h b/atom/common/native_mate_converters/net_converter.h index 16013e34f986..33117ca974f1 100644 --- a/atom/common/native_mate_converters/net_converter.h +++ b/atom/common/native_mate_converters/net_converter.h @@ -18,6 +18,7 @@ class AuthChallengeInfo; class URLRequest; class X509Certificate; class HttpResponseHeaders; +struct CertPrincipal; } namespace mate { @@ -34,6 +35,12 @@ struct Converter> { const scoped_refptr& val); }; +template<> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const net::CertPrincipal& val); +}; + template <> struct Converter { static v8::Local ToV8(v8::Isolate* isolate, diff --git a/docs/api/structures/certificate-principal.md b/docs/api/structures/certificate-principal.md new file mode 100644 index 000000000000..12c46382aaf3 --- /dev/null +++ b/docs/api/structures/certificate-principal.md @@ -0,0 +1,8 @@ +# CertificatePrincipal Object + +* `commonName` String - Common Name +* `organizations` String[] - Organization names +* `organizationUnits` String[] - Organization Unit names +* `locality` String - Locality +* `state` String - State or province +* `country` String - Country or region diff --git a/docs/api/structures/certificate.md b/docs/api/structures/certificate.md index 164b2bc88f48..a8c66461fd99 100644 --- a/docs/api/structures/certificate.md +++ b/docs/api/structures/certificate.md @@ -1,8 +1,10 @@ # Certificate Object * `data` String - PEM encoded data -* `chain` String[] - PEM encoded chain +* `issuer` [CertificatePrincipal](structures/certificate-principal.md) - Issuer principal * `issuerName` String - Issuer's Common Name +* `issuerCert` Certificate - Issuer certificate (if not self-signed) +* `subject` [CertificatePrincipal](structures/certificate-principal.md) - Subject principal * `subjectName` String - Subject's Common Name * `serialNumber` String - Hex value represented string * `validStart` Number - Start date of the certificate being valid in seconds From 93ce2f780065c1b93c36e1f0f4fd265a1470e742 Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Fri, 11 Nov 2016 09:55:56 +0000 Subject: [PATCH 3/9] Add assertions for new fields in existing specs. --- spec/api-app-spec.js | 8 ++++++++ spec/api-session-spec.js | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 2afce4dc1672..9ff687e9a5f7 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -207,6 +207,14 @@ describe('app module', function () { app.on('select-client-certificate', function (event, webContents, url, list, callback) { assert.equal(list.length, 1) assert.equal(list[0].issuerName, 'Intermediate CA') + assert.equal(list[0].subjectName, 'localhost') + assert(list[0].issuer); + assert.equal(list[0].issuer.commonName, 'Intermediate CA') + assert(list[0].subject); + assert.equal(list[0].subject.commonName, 'localhost') + assert(list[0].issuerCert) + assert(list[0].issuerCert.subject) + assert.equal(list[0].issuerCert.subject.commonName, 'Intermediate CA') callback(list[0]) }) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 8451fc0cc937..37f599f5aa9b 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -553,6 +553,14 @@ describe('session module', function () { session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { assert.equal(hostname, '127.0.0.1') assert.equal(certificate.issuerName, 'Intermediate CA') + assert.equal(certificate.subjectName, 'localhost') + assert(certificate.issuer); + assert.equal(certificate.issuer.commonName, 'Intermediate CA') + assert(certificate.subject); + assert.equal(certificate.subject.commonName, 'localhost') + assert(certificate.issuerCert) + assert(certificate.issuerCert.subject) + assert.equal(certificate.issuerCert.subject.commonName, 'Intermediate CA') callback(false) }) From f767f0f04876ba74f0c3b244d8126c1d00b6b4ba Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Sat, 12 Nov 2016 12:18:38 +0000 Subject: [PATCH 4/9] Remove unnecessary ConvertToV8 calls. --- atom/common/native_mate_converters/net_converter.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index af345698bf93..94fff2ff6027 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -47,9 +47,9 @@ v8::Local Converter>::ToV8( val->os_cert_handle(), &encoded_data); dict.Set("data", encoded_data); - dict.Set("issuer", mate::ConvertToV8(isolate, val->issuer())); + dict.Set("issuer", val->issuer()); dict.Set("issuerName", val->issuer().GetDisplayName()); - dict.Set("subject", mate::ConvertToV8(isolate, val->subject())); + dict.Set("subject", val->subject()); dict.Set("subjectName", val->subject().GetDisplayName()); dict.Set("serialNumber", base::HexEncode(val->serial_number().data(), val->serial_number().size())); @@ -67,7 +67,7 @@ v8::Local Converter>::ToV8( net::X509Certificate::CreateFromHandle( val->GetIntermediateCertificates().front(), issuer_intermediates); - dict.Set("issuerCert", mate::ConvertToV8(isolate, issuer_cert)); + dict.Set("issuerCert", issuer_cert); } return dict.GetHandle(); @@ -79,9 +79,8 @@ v8::Local Converter::ToV8( mate::Dictionary dict(isolate, v8::Object::New(isolate)); dict.Set("commonName", val.common_name); - dict.Set("organizations", mate::ConvertToV8(isolate, val.organization_names)); - dict.Set("organizationUnits", - mate::ConvertToV8(isolate, val.organization_unit_names)); + dict.Set("organizations", val.organization_names); + dict.Set("organizationUnits", val.organization_unit_names); dict.Set("locality", val.locality_name); dict.Set("state", val.state_or_province_name); dict.Set("country", val.country_name); From 2e5c92d41e6f9cbdea9336cc8099fd57f47c7dff Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Mon, 14 Nov 2016 09:13:49 +0000 Subject: [PATCH 5/9] Fix listing issues: remove semicolons. --- spec/api-app-spec.js | 4 ++-- spec/api-session-spec.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 9ff687e9a5f7..6485d85cc5f3 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -208,9 +208,9 @@ describe('app module', function () { assert.equal(list.length, 1) assert.equal(list[0].issuerName, 'Intermediate CA') assert.equal(list[0].subjectName, 'localhost') - assert(list[0].issuer); + assert(list[0].issuer) assert.equal(list[0].issuer.commonName, 'Intermediate CA') - assert(list[0].subject); + assert(list[0].subject) assert.equal(list[0].subject.commonName, 'localhost') assert(list[0].issuerCert) assert(list[0].issuerCert.subject) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 37f599f5aa9b..facabaae1bc9 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -554,9 +554,9 @@ describe('session module', function () { assert.equal(hostname, '127.0.0.1') assert.equal(certificate.issuerName, 'Intermediate CA') assert.equal(certificate.subjectName, 'localhost') - assert(certificate.issuer); + assert(certificate.issuer) assert.equal(certificate.issuer.commonName, 'Intermediate CA') - assert(certificate.subject); + assert(certificate.subject) assert.equal(certificate.subject.commonName, 'localhost') assert(certificate.issuerCert) assert(certificate.issuerCert.subject) From 86321a2c607e96b267496dc69816e3d8b7a4d34d Mon Sep 17 00:00:00 2001 From: Greg Nolle Date: Tue, 15 Nov 2016 09:36:48 +0000 Subject: [PATCH 6/9] Fixed incorrect subject name of test client certificate. --- spec/api-app-spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 6485d85cc5f3..dccf1ad52d57 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -207,11 +207,11 @@ describe('app module', function () { app.on('select-client-certificate', function (event, webContents, url, list, callback) { assert.equal(list.length, 1) assert.equal(list[0].issuerName, 'Intermediate CA') - assert.equal(list[0].subjectName, 'localhost') + assert.equal(list[0].subjectName, 'Client Cert') assert(list[0].issuer) assert.equal(list[0].issuer.commonName, 'Intermediate CA') assert(list[0].subject) - assert.equal(list[0].subject.commonName, 'localhost') + assert.equal(list[0].subject.commonName, 'Client Cert') assert(list[0].issuerCert) assert(list[0].issuerCert.subject) assert.equal(list[0].issuerCert.subject.commonName, 'Intermediate CA') From a1dca8afc9fb5979e06398b4356b12ece63f8519 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 15 Nov 2016 15:53:17 -0800 Subject: [PATCH 7/9] :art: Remove buildup asserts and just use assert.equal --- spec/api-app-spec.js | 4 ---- spec/api-session-spec.js | 4 ---- 2 files changed, 8 deletions(-) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index dccf1ad52d57..16fcd34fadb3 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -208,12 +208,8 @@ describe('app module', function () { assert.equal(list.length, 1) assert.equal(list[0].issuerName, 'Intermediate CA') assert.equal(list[0].subjectName, 'Client Cert') - assert(list[0].issuer) assert.equal(list[0].issuer.commonName, 'Intermediate CA') - assert(list[0].subject) assert.equal(list[0].subject.commonName, 'Client Cert') - assert(list[0].issuerCert) - assert(list[0].issuerCert.subject) assert.equal(list[0].issuerCert.subject.commonName, 'Intermediate CA') callback(list[0]) }) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index facabaae1bc9..0672a4d7b57b 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -554,12 +554,8 @@ describe('session module', function () { assert.equal(hostname, '127.0.0.1') assert.equal(certificate.issuerName, 'Intermediate CA') assert.equal(certificate.subjectName, 'localhost') - assert(certificate.issuer) assert.equal(certificate.issuer.commonName, 'Intermediate CA') - assert(certificate.subject) assert.equal(certificate.subject.commonName, 'localhost') - assert(certificate.issuerCert) - assert(certificate.issuerCert.subject) assert.equal(certificate.issuerCert.subject.commonName, 'Intermediate CA') callback(false) }) From 095d7118480b9ef7db912140db4f50ea9f1c0f70 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 15 Nov 2016 16:24:50 -0800 Subject: [PATCH 8/9] Remove select-client-certificate issueCert assert --- spec/api-app-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index 16fcd34fadb3..d49ef18989c5 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -210,7 +210,6 @@ describe('app module', function () { assert.equal(list[0].subjectName, 'Client Cert') assert.equal(list[0].issuer.commonName, 'Intermediate CA') assert.equal(list[0].subject.commonName, 'Client Cert') - assert.equal(list[0].issuerCert.subject.commonName, 'Intermediate CA') callback(list[0]) }) From fc1ce3eeab2f042a6dd14731c382990f74ca481b Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 15 Nov 2016 16:32:12 -0800 Subject: [PATCH 9/9] Assert certs further up the chain --- spec/api-session-spec.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 0672a4d7b57b..f6e69808b5c8 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -556,7 +556,11 @@ describe('session module', function () { assert.equal(certificate.subjectName, 'localhost') assert.equal(certificate.issuer.commonName, 'Intermediate CA') assert.equal(certificate.subject.commonName, 'localhost') + assert.equal(certificate.issuerCert.issuer.commonName, 'Root CA') assert.equal(certificate.issuerCert.subject.commonName, 'Intermediate CA') + assert.equal(certificate.issuerCert.issuerCert.issuer.commonName, 'Root CA') + assert.equal(certificate.issuerCert.issuerCert.subject.commonName, 'Root CA') + assert.equal(certificate.issuerCert.issuerCert.issuerCert, undefined) callback(false) })