From 64369cd07fa2b90dbe4aa978bd7baf32721b1cae Mon Sep 17 00:00:00 2001 From: joshaber Date: Thu, 30 Mar 2017 17:25:44 -0400 Subject: [PATCH 01/36] Show a certificate trust panel --- atom/browser/api/atom_api_certificate_trust.h | 33 +++++++++++++ .../api/atom_api_certificate_trust_mac.mm | 48 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 atom/browser/api/atom_api_certificate_trust.h create mode 100644 atom/browser/api/atom_api_certificate_trust_mac.mm diff --git a/atom/browser/api/atom_api_certificate_trust.h b/atom/browser/api/atom_api_certificate_trust.h new file mode 100644 index 000000000000..c3e6994b3baf --- /dev/null +++ b/atom/browser/api/atom_api_certificate_trust.h @@ -0,0 +1,33 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ +#define ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ + +#include + +#include "base/callback_forward.h" + +namespace net { +class X509Certificate; +} // namespace net + +namespace atom { + +class NativeWindow; + +namespace api { + +typedef base::Callback ShowTrustCallback; + +void ShowCertificateTrustUI(atom::NativeWindow* parent_window, + const net::X509Certificate& cert, + std::string message, + const ShowTrustCallback& callback); + +} // namespace api + +} // namespace atom + +#endif // ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/api/atom_api_certificate_trust_mac.mm new file mode 100644 index 000000000000..c0109d9f45a7 --- /dev/null +++ b/atom/browser/api/atom_api_certificate_trust_mac.mm @@ -0,0 +1,48 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/api/atom_api_certificate_trust.h" + +#import +#import +#import + +#include "atom/browser/native_window.h" +#include "base/files/file_util.h" +#include "base/mac/foundation_util.h" +#include "base/mac/mac_util.h" +#include "base/mac/scoped_cftyperef.h" +#include "base/strings/sys_string_conversions.h" +#include "net/cert/x509_certificate.h" + +namespace atom { + +namespace api { + +void ShowCertificateTrustUI(atom::NativeWindow* parent_window, + const net::X509Certificate& cert, + std::string message, + const ShowTrustCallback& callback) { + auto sec_policy = SecPolicyCreateBasicX509(); + SecTrustRef trust = nullptr; + SecTrustCreateWithCertificates(cert.CreateOSCertChainForCert(), sec_policy, &trust); + // CFRelease(sec_policy); + + NSWindow* window = parent_window ? + parent_window->GetNativeWindow() : + NULL; + + auto msg = base::SysUTF8ToNSString(message); + + SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; + [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:NULL trust:trust message:msg]; + + callback.Run(true); + // CFRelease(trust); + // [panel release]; +} + +} // namespace api + +} // namespace atom From 653b2d15c325638d4a9aca0cba15bb6dcc35a352 Mon Sep 17 00:00:00 2001 From: joshaber Date: Thu, 30 Mar 2017 17:25:55 -0400 Subject: [PATCH 02/36] Expose the certificate trust panel as part of app --- atom/browser/api/atom_api_app.cc | 12 ++++++++++++ atom/browser/api/atom_api_app.h | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 56a11daf69bd..3a8f4ef0f8b5 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -7,6 +7,7 @@ #include #include +#include "atom/browser/api/atom_api_certificate_trust.h" #include "atom/browser/api/atom_api_menu.h" #include "atom/browser/api/atom_api_session.h" #include "atom/browser/api/atom_api_web_contents.h" @@ -810,6 +811,16 @@ void App::OnCertificateManagerModelCreated( } #endif +#if defined(OS_MACOSX) +void App::ShowCertificateTrust(atom::NativeWindow* parent_window, + const net::X509Certificate& cert, + std::string message, + const ShowTrustCallback& callback, + mate::Arguments* args) { + ShowCertificateTrustUI(parent_window, cert, message, callback); +} +#endif + #if defined(OS_WIN) v8::Local App::GetJumpListSettings() { JumpList jump_list(Browser::Get()->GetAppUserModelID()); @@ -949,6 +960,7 @@ void App::BuildPrototype( base::Bind(&Browser::GetCurrentActivityType, browser)) .SetMethod("setAboutPanelOptions", base::Bind(&Browser::SetAboutPanelOptions, browser)) + // .SetMethod("showCertificateTrust", &App::ShowCertificateTrust) #endif #if defined(OS_WIN) .SetMethod("setUserTasks", base::Bind(&Browser::SetUserTasks, browser)) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 8b276f334d5c..367709cf0c6f 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -8,6 +8,7 @@ #include #include +#include "atom/browser/api/atom_api_certificate_trust.h" #include "atom/browser/api/event_emitter.h" #include "atom/browser/atom_browser_client.h" #include "atom/browser/browser.h" @@ -19,6 +20,7 @@ #include "content/public/browser/gpu_data_manager_observer.h" #include "native_mate/handle.h" #include "net/base/completion_callback.h" +#include "net/cert/x509_certificate.h" #if defined(USE_NSS_CERTS) #include "chrome/browser/certificate_manager_model.h" @@ -151,6 +153,15 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr certificate_manager_model_; #endif + +#if defined(OS_MACOSX) + void ShowCertificateTrust(atom::NativeWindow* parent_window, + const net::X509Certificate& cert, + std::string message, + const ShowTrustCallback& callback, + mate::Arguments* args); +#endif + // Tracks tasks requesting file icons. base::CancelableTaskTracker cancelable_task_tracker_; From 7b044ffe0e723c67e77ab509754655f72df7cfd8 Mon Sep 17 00:00:00 2001 From: joshaber Date: Thu, 30 Mar 2017 17:26:03 -0400 Subject: [PATCH 03/36] Compile these lovely files --- filenames.gypi | 2 ++ 1 file changed, 2 insertions(+) diff --git a/filenames.gypi b/filenames.gypi index 935d7033153a..f2a78ad4d4b6 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -103,6 +103,8 @@ 'atom/browser/api/atom_api_app.h', 'atom/browser/api/atom_api_auto_updater.cc', 'atom/browser/api/atom_api_auto_updater.h', + 'atom/browser/api/atom_api_certificate_trust_mac.mm', + 'atom/browser/api/atom_api_certificate_trust.h', 'atom/browser/api/atom_api_content_tracing.cc', 'atom/browser/api/atom_api_cookies.cc', 'atom/browser/api/atom_api_cookies.h', From edf61d41ba3a3efed0b45f80e252cdb75b154895 Mon Sep 17 00:00:00 2001 From: joshaber Date: Thu, 30 Mar 2017 17:26:11 -0400 Subject: [PATCH 04/36] Link against the Security frameworks --- electron.gyp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/electron.gyp b/electron.gyp index 78eed743cc36..71c53fe6f591 100644 --- a/electron.gyp +++ b/electron.gyp @@ -549,6 +549,8 @@ '$(SDKROOT)/System/Library/Frameworks/Carbon.framework', '$(SDKROOT)/System/Library/Frameworks/QuartzCore.framework', '$(SDKROOT)/System/Library/Frameworks/Quartz.framework', + '$(SDKROOT)/System/Library/Frameworks/Security.framework', + '$(SDKROOT)/System/Library/Frameworks/SecurityInterface.framework', ], }, 'mac_bundle': 1, From deae70de4ddc063d976373f25e3eed11baadb449 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 13:53:31 -0400 Subject: [PATCH 05/36] Dummy out the certificate FromV8 converter --- atom/common/native_mate_converters/net_converter.cc | 6 ++++++ atom/common/native_mate_converters/net_converter.h | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 94fff2ff6027..477e382fdca3 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -73,6 +73,12 @@ v8::Local Converter>::ToV8( return dict.GetHandle(); } +bool Converter>::FromV8(v8::Isolate* isolate, + v8::Local val, + scoped_refptr* out) { + return true; +} + // static v8::Local Converter::ToV8( v8::Isolate* isolate, const net::CertPrincipal& val) { diff --git a/atom/common/native_mate_converters/net_converter.h b/atom/common/native_mate_converters/net_converter.h index 33117ca974f1..9e3128fdb546 100644 --- a/atom/common/native_mate_converters/net_converter.h +++ b/atom/common/native_mate_converters/net_converter.h @@ -33,6 +33,10 @@ template<> struct Converter> { static v8::Local ToV8(v8::Isolate* isolate, const scoped_refptr& val); + + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + scoped_refptr* out); }; template<> From 16cc79354c5dcc8758c043185a16196d64c14bb6 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 13:53:42 -0400 Subject: [PATCH 06/36] Errrrybody's a scoped_refptr now --- atom/browser/api/atom_api_app.cc | 4 ++-- atom/browser/api/atom_api_app.h | 2 +- atom/browser/api/atom_api_certificate_trust.h | 3 ++- atom/browser/api/atom_api_certificate_trust_mac.mm | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 3a8f4ef0f8b5..41e7655a66f1 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -813,7 +813,7 @@ void App::OnCertificateManagerModelCreated( #if defined(OS_MACOSX) void App::ShowCertificateTrust(atom::NativeWindow* parent_window, - const net::X509Certificate& cert, + const scoped_refptr& cert, std::string message, const ShowTrustCallback& callback, mate::Arguments* args) { @@ -960,7 +960,7 @@ void App::BuildPrototype( base::Bind(&Browser::GetCurrentActivityType, browser)) .SetMethod("setAboutPanelOptions", base::Bind(&Browser::SetAboutPanelOptions, browser)) - // .SetMethod("showCertificateTrust", &App::ShowCertificateTrust) + .SetMethod("showCertificateTrust", &App::ShowCertificateTrust) #endif #if defined(OS_WIN) .SetMethod("setUserTasks", base::Bind(&Browser::SetUserTasks, browser)) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 367709cf0c6f..19640cb84886 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -156,7 +156,7 @@ class App : public AtomBrowserClient::Delegate, #if defined(OS_MACOSX) void ShowCertificateTrust(atom::NativeWindow* parent_window, - const net::X509Certificate& cert, + const scoped_refptr& cert, std::string message, const ShowTrustCallback& callback, mate::Arguments* args); diff --git a/atom/browser/api/atom_api_certificate_trust.h b/atom/browser/api/atom_api_certificate_trust.h index c3e6994b3baf..1de7d653bb18 100644 --- a/atom/browser/api/atom_api_certificate_trust.h +++ b/atom/browser/api/atom_api_certificate_trust.h @@ -8,6 +8,7 @@ #include #include "base/callback_forward.h" +#include "base/memory/ref_counted.h" namespace net { class X509Certificate; @@ -22,7 +23,7 @@ namespace api { typedef base::Callback ShowTrustCallback; void ShowCertificateTrustUI(atom::NativeWindow* parent_window, - const net::X509Certificate& cert, + const scoped_refptr& cert, std::string message, const ShowTrustCallback& callback); diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/api/atom_api_certificate_trust_mac.mm index c0109d9f45a7..dc12ada27f84 100644 --- a/atom/browser/api/atom_api_certificate_trust_mac.mm +++ b/atom/browser/api/atom_api_certificate_trust_mac.mm @@ -21,12 +21,12 @@ namespace atom { namespace api { void ShowCertificateTrustUI(atom::NativeWindow* parent_window, - const net::X509Certificate& cert, + const scoped_refptr& cert, std::string message, const ShowTrustCallback& callback) { auto sec_policy = SecPolicyCreateBasicX509(); SecTrustRef trust = nullptr; - SecTrustCreateWithCertificates(cert.CreateOSCertChainForCert(), sec_policy, &trust); + SecTrustCreateWithCertificates(cert->CreateOSCertChainForCert(), sec_policy, &trust); // CFRelease(sec_policy); NSWindow* window = parent_window ? From 141a5ad73f2ecdc4305ef9a7572a3ae72333257e Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 14:16:26 -0400 Subject: [PATCH 07/36] Maybe this is a valid conversion? --- atom/common/native_mate_converters/net_converter.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 477e382fdca3..8e8f6c001498 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -73,9 +73,16 @@ v8::Local Converter>::ToV8( return dict.GetHandle(); } -bool Converter>::FromV8(v8::Isolate* isolate, - v8::Local val, - scoped_refptr* out) { +bool Converter>::FromV8( + v8::Isolate* isolate, v8::Local val, + scoped_refptr* out) { + mate::Dictionary dict; + if (!ConvertFromV8(isolate, val, &dict)) + return false; + + std::string data; + dict.Get("data", &data); + *out = net::X509Certificate::CreateFromBytes(data.c_str(), data.length()); return true; } From 302ca8669dd35121ab1a2bb5126ba6c3e48ff792 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 15:44:47 -0400 Subject: [PATCH 08/36] Slightly better conversion --- atom/common/native_mate_converters/net_converter.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 8e8f6c001498..59f897c8a74b 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -82,7 +82,18 @@ bool Converter>::FromV8( std::string data; dict.Get("data", &data); - *out = net::X509Certificate::CreateFromBytes(data.c_str(), data.length()); + + auto certificate_list = net::X509Certificate::CreateCertificateListFromBytes( + data.c_str(), data.length(), + net::X509Certificate::FORMAT_SINGLE_CERTIFICATE); + if (certificate_list.empty()) + return false; + + auto certificate = certificate_list.front(); + if (!certificate) + return false; + + *out = certificate; return true; } From 69defc5166ac69bd05c8640630f1375e7968dee0 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 21:27:33 -0400 Subject: [PATCH 09/36] Encode all the intermediates --- atom/common/native_mate_converters/net_converter.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 59f897c8a74b..5ceb23a04b8d 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -59,7 +59,8 @@ v8::Local Converter>::ToV8( net::HashValue( val->CalculateFingerprint256(val->os_cert_handle())).ToString()); - if (!val->GetIntermediateCertificates().empty()) { + auto intermediates = val->GetIntermediateCertificates(); + if (!intermediates.empty()) { net::X509Certificate::OSCertHandles issuer_intermediates( val->GetIntermediateCertificates().begin() + 1, val->GetIntermediateCertificates().end()); @@ -68,6 +69,16 @@ v8::Local Converter>::ToV8( val->GetIntermediateCertificates().front(), issuer_intermediates); dict.Set("issuerCert", issuer_cert); + + std::vector intermediates_encoded; + for (size_t i = 0; i < intermediates.size(); i++) { + auto native_cert = intermediates[i]; + std::string encoded; + net::X509Certificate::GetPEMEncoded(native_cert, &encoded); + intermediates_encoded.push_back(encoded); + } + + dict.Set("intermediates", intermediates_encoded); } return dict.GetHandle(); From bde2a597f362fd3b1b4a024367442f8d71c9bc91 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 21:27:49 -0400 Subject: [PATCH 10/36] Decode all the intermediates --- .../native_mate_converters/net_converter.cc | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 5ceb23a04b8d..bfd346835b75 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -84,6 +84,23 @@ v8::Local Converter>::ToV8( return dict.GetHandle(); } +bool CertFromData(const std::string& data, + scoped_refptr* out) { + auto cert_list = net::X509Certificate::CreateCertificateListFromBytes( + data.c_str(), data.length(), + net::X509Certificate::FORMAT_SINGLE_CERTIFICATE); + if (cert_list.empty()) + return false; + + auto leaf_cert = cert_list.front(); + if (!leaf_cert) + return false; + + *out = leaf_cert; + + return true; +} + bool Converter>::FromV8( v8::Isolate* isolate, v8::Local val, scoped_refptr* out) { @@ -93,18 +110,28 @@ bool Converter>::FromV8( std::string data; dict.Get("data", &data); - - auto certificate_list = net::X509Certificate::CreateCertificateListFromBytes( - data.c_str(), data.length(), - net::X509Certificate::FORMAT_SINGLE_CERTIFICATE); - if (certificate_list.empty()) + scoped_refptr leaf_cert; + if (!CertFromData(data, &leaf_cert)) return false; - auto certificate = certificate_list.front(); - if (!certificate) + std::vector intermediates_encoded; + dict.Get("intermediates", &intermediates_encoded); + std::vector intermediates; + for (size_t i = 0; i < intermediates_encoded.size(); i++) { + auto data = intermediates_encoded[i]; + scoped_refptr cert; + if (!CertFromData(data, &cert)) + return false; + + intermediates.push_back(cert->os_cert_handle()); + } + + auto cert = net::X509Certificate::CreateFromHandle( + leaf_cert->os_cert_handle(), intermediates); + if (!cert) return false; - *out = certificate; + *out = cert; return true; } From 4f3d3557cc244741340f12684c06d91ea5a9488b Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 21:51:29 -0400 Subject: [PATCH 11/36] Notify that the cert changed --- .../browser/api/atom_api_certificate_trust_mac.mm | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/api/atom_api_certificate_trust_mac.mm index dc12ada27f84..5057ef55ab51 100644 --- a/atom/browser/api/atom_api_certificate_trust_mac.mm +++ b/atom/browser/api/atom_api_certificate_trust_mac.mm @@ -15,6 +15,7 @@ #include "base/mac/scoped_cftyperef.h" #include "base/strings/sys_string_conversions.h" #include "net/cert/x509_certificate.h" +#include "net/cert/cert_database.h" namespace atom { @@ -29,14 +30,20 @@ void ShowCertificateTrustUI(atom::NativeWindow* parent_window, SecTrustCreateWithCertificates(cert->CreateOSCertChainForCert(), sec_policy, &trust); // CFRelease(sec_policy); - NSWindow* window = parent_window ? - parent_window->GetNativeWindow() : - NULL; + // NSWindow* window = parent_window ? + // parent_window->GetNativeWindow() : + // NULL; auto msg = base::SysUTF8ToNSString(message); SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; - [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:NULL trust:trust message:msg]; + [panel runModalForTrust:trust message:msg]; + // [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:NULL trust:trust message:msg]; + + auto cert_db = net::CertDatabase::GetInstance(); + // This forces Chromium to reload the certificate since it might be trusted + // now. + cert_db->NotifyObserversCertDBChanged(cert.get()); callback.Run(true); // CFRelease(trust); From ee7389bb6ddfe5f6486f8a07c2ac1298cbbb950c Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 31 Mar 2017 22:57:56 -0400 Subject: [PATCH 12/36] Rename to make VS happy --- atom/common/native_mate_converters/net_converter.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index bfd346835b75..2d3095a63ea0 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -118,9 +118,9 @@ bool Converter>::FromV8( dict.Get("intermediates", &intermediates_encoded); std::vector intermediates; for (size_t i = 0; i < intermediates_encoded.size(); i++) { - auto data = intermediates_encoded[i]; + auto intermediate_data = intermediates_encoded[i]; scoped_refptr cert; - if (!CertFromData(data, &cert)) + if (!CertFromData(intermediate_data, &cert)) return false; intermediates.push_back(cert->os_cert_handle()); From 1ed443ea294ad4a84e1100c90ea5acf3df2e29f3 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 13:21:44 -0400 Subject: [PATCH 13/36] Do the callback & release dance --- .../api/atom_api_certificate_trust_mac.mm | 59 +++++++++++++------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/api/atom_api_certificate_trust_mac.mm index 5057ef55ab51..d767330c9453 100644 --- a/atom/browser/api/atom_api_certificate_trust_mac.mm +++ b/atom/browser/api/atom_api_certificate_trust_mac.mm @@ -17,6 +17,26 @@ #include "net/cert/x509_certificate.h" #include "net/cert/cert_database.h" +@interface Trampoline : NSObject + +- (void)createPanelDidEnd:(NSWindow *)sheet + returnCode:(int)returnCode + contextInfo:(void *)contextInfo; + +@end + +@implementation Trampoline + +- (void)createPanelDidEnd:(NSWindow *)sheet + returnCode:(int)returnCode + contextInfo:(void *)contextInfo { + void (^block)(int) = (void (^)(int))contextInfo; + block(returnCode); + [(id)block autorelease]; +} + +@end + namespace atom { namespace api { @@ -26,28 +46,33 @@ void ShowCertificateTrustUI(atom::NativeWindow* parent_window, std::string message, const ShowTrustCallback& callback) { auto sec_policy = SecPolicyCreateBasicX509(); + auto cert_chain = cert->CreateOSCertChainForCert(); SecTrustRef trust = nullptr; - SecTrustCreateWithCertificates(cert->CreateOSCertChainForCert(), sec_policy, &trust); - // CFRelease(sec_policy); - - // NSWindow* window = parent_window ? - // parent_window->GetNativeWindow() : - // NULL; - - auto msg = base::SysUTF8ToNSString(message); + SecTrustCreateWithCertificates(cert_chain, sec_policy, &trust); SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; - [panel runModalForTrust:trust message:msg]; - // [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:NULL trust:trust message:msg]; - auto cert_db = net::CertDatabase::GetInstance(); - // This forces Chromium to reload the certificate since it might be trusted - // now. - cert_db->NotifyObserversCertDBChanged(cert.get()); + void (^callbackBlock)(int) = [^(int returnCode) { + // if (returnCode == NSFileHandlingPanelOKButton) { + auto cert_db = net::CertDatabase::GetInstance(); + // This forces Chromium to reload the certificate since it might be trusted + // now. + cert_db->NotifyObserversCertDBChanged(cert.get()); + // } - callback.Run(true); - // CFRelease(trust); - // [panel release]; + callback.Run(returnCode); + + [panel autorelease]; + CFRelease(trust); + CFRelease(cert_chain); + CFRelease(sec_policy); + } copy]; + + NSWindow* window = parent_window ? + parent_window->GetNativeWindow() : + NULL; + auto msg = base::SysUTF8ToNSString(message); + [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:callbackBlock trust:trust message:msg]; } } // namespace api From 4bbbcd093b7812b8658b6e585973d132576b0a47 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 14:01:48 -0400 Subject: [PATCH 14/36] Handle the callback and cleanup properly --- .../api/atom_api_certificate_trust_mac.mm | 102 +++++++++++++----- 1 file changed, 73 insertions(+), 29 deletions(-) diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/api/atom_api_certificate_trust_mac.mm index d767330c9453..7f14a72f5016 100644 --- a/atom/browser/api/atom_api_certificate_trust_mac.mm +++ b/atom/browser/api/atom_api_certificate_trust_mac.mm @@ -17,22 +17,71 @@ #include "net/cert/x509_certificate.h" #include "net/cert/cert_database.h" -@interface Trampoline : NSObject +@interface TrustDelegate : NSObject { + @private + atom::api::ShowTrustCallback callback_; + SFCertificateTrustPanel* panel_; + scoped_refptr cert_; + SecTrustRef trust_; + CFArrayRef cert_chain_; + SecPolicyRef sec_policy_; +} -- (void)createPanelDidEnd:(NSWindow *)sheet +- (id)initWithCallback:(const atom::api::ShowTrustCallback&)callback + panel:(SFCertificateTrustPanel*)panel + cert:(const scoped_refptr&)cert + trust:(SecTrustRef)trust + certChain:(CFArrayRef)certChain + secPolicy:(SecPolicyRef)secPolicy; + +- (void)panelDidEnd:(NSWindow *)sheet returnCode:(int)returnCode - contextInfo:(void *)contextInfo; + contextInfo:(void*)contextInfo; @end -@implementation Trampoline +@implementation TrustDelegate -- (void)createPanelDidEnd:(NSWindow *)sheet +- (void)dealloc { + [panel_ release]; + CFRelease(trust_); + CFRelease(cert_chain_); + CFRelease(sec_policy_); + + [super dealloc]; +} + +- (id)initWithCallback:(const atom::api::ShowTrustCallback&)callback + panel:(SFCertificateTrustPanel*)panel + cert:(const scoped_refptr&)cert + trust:(SecTrustRef)trust + certChain:(CFArrayRef)certChain + secPolicy:(SecPolicyRef)secPolicy { + if ((self = [super init])) { + callback_ = callback; + panel_ = panel; + cert_ = cert; + trust_ = trust; + cert_chain_ = certChain; + sec_policy_ = secPolicy; + } + + return self; +} + +- (void)panelDidEnd:(NSWindow *)sheet returnCode:(int)returnCode - contextInfo:(void *)contextInfo { - void (^block)(int) = (void (^)(int))contextInfo; - block(returnCode); - [(id)block autorelease]; + contextInfo:(void*)contextInfo { + if (returnCode == NSFileHandlingPanelOKButton) { + auto cert_db = net::CertDatabase::GetInstance(); + // This forces Chromium to reload the certificate since it might be trusted + // now. + cert_db->NotifyObserversCertDBChanged(cert_.get()); + } + + callback_.Run(returnCode); + + [self autorelease]; } @end @@ -50,29 +99,24 @@ void ShowCertificateTrustUI(atom::NativeWindow* parent_window, SecTrustRef trust = nullptr; SecTrustCreateWithCertificates(cert_chain, sec_policy, &trust); - SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; - - void (^callbackBlock)(int) = [^(int returnCode) { - // if (returnCode == NSFileHandlingPanelOKButton) { - auto cert_db = net::CertDatabase::GetInstance(); - // This forces Chromium to reload the certificate since it might be trusted - // now. - cert_db->NotifyObserversCertDBChanged(cert.get()); - // } - - callback.Run(returnCode); - - [panel autorelease]; - CFRelease(trust); - CFRelease(cert_chain); - CFRelease(sec_policy); - } copy]; - NSWindow* window = parent_window ? parent_window->GetNativeWindow() : - NULL; + nil; auto msg = base::SysUTF8ToNSString(message); - [panel beginSheetForWindow:window modalDelegate:nil didEndSelector:NULL contextInfo:callbackBlock trust:trust message:msg]; + + SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; + auto delegate = [[TrustDelegate alloc] initWithCallback:callback + panel:panel + cert:cert + trust:trust + certChain:cert_chain + secPolicy:sec_policy]; + [panel beginSheetForWindow:window + modalDelegate:delegate + didEndSelector:@selector(panelDidEnd:returnCode:contextInfo:) + contextInfo:nil + trust:trust + message:msg]; } } // namespace api From 6e89cb9d7c8fe28ffb3148b518a070b12c657e8f Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:05:24 -0400 Subject: [PATCH 15/36] Move it into `dialog` --- atom/browser/api/atom_api_app.cc | 12 ------- atom/browser/api/atom_api_app.h | 10 ------ atom/browser/api/atom_api_certificate_trust.h | 34 ------------------- atom/browser/api/atom_api_dialog.cc | 15 ++++++++ atom/browser/ui/certificate_trust.h | 29 ++++++++++++++++ .../certificate_trust_mac.mm} | 25 ++++++-------- filenames.gypi | 4 +-- 7 files changed, 56 insertions(+), 73 deletions(-) delete mode 100644 atom/browser/api/atom_api_certificate_trust.h create mode 100644 atom/browser/ui/certificate_trust.h rename atom/browser/{api/atom_api_certificate_trust_mac.mm => ui/certificate_trust_mac.mm} (83%) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index 41e7655a66f1..56a11daf69bd 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/atom/browser/api/atom_api_app.cc @@ -7,7 +7,6 @@ #include #include -#include "atom/browser/api/atom_api_certificate_trust.h" #include "atom/browser/api/atom_api_menu.h" #include "atom/browser/api/atom_api_session.h" #include "atom/browser/api/atom_api_web_contents.h" @@ -811,16 +810,6 @@ void App::OnCertificateManagerModelCreated( } #endif -#if defined(OS_MACOSX) -void App::ShowCertificateTrust(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const ShowTrustCallback& callback, - mate::Arguments* args) { - ShowCertificateTrustUI(parent_window, cert, message, callback); -} -#endif - #if defined(OS_WIN) v8::Local App::GetJumpListSettings() { JumpList jump_list(Browser::Get()->GetAppUserModelID()); @@ -960,7 +949,6 @@ void App::BuildPrototype( base::Bind(&Browser::GetCurrentActivityType, browser)) .SetMethod("setAboutPanelOptions", base::Bind(&Browser::SetAboutPanelOptions, browser)) - .SetMethod("showCertificateTrust", &App::ShowCertificateTrust) #endif #if defined(OS_WIN) .SetMethod("setUserTasks", base::Bind(&Browser::SetUserTasks, browser)) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index 19640cb84886..f2debdf2e533 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -8,7 +8,6 @@ #include #include -#include "atom/browser/api/atom_api_certificate_trust.h" #include "atom/browser/api/event_emitter.h" #include "atom/browser/atom_browser_client.h" #include "atom/browser/browser.h" @@ -153,15 +152,6 @@ class App : public AtomBrowserClient::Delegate, std::unique_ptr certificate_manager_model_; #endif - -#if defined(OS_MACOSX) - void ShowCertificateTrust(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const ShowTrustCallback& callback, - mate::Arguments* args); -#endif - // Tracks tasks requesting file icons. base::CancelableTaskTracker cancelable_task_tracker_; diff --git a/atom/browser/api/atom_api_certificate_trust.h b/atom/browser/api/atom_api_certificate_trust.h deleted file mode 100644 index 1de7d653bb18..000000000000 --- a/atom/browser/api/atom_api_certificate_trust.h +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright (c) 2017 GitHub, Inc. -// Use of this source code is governed by the MIT license that can be -// found in the LICENSE file. - -#ifndef ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ -#define ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ - -#include - -#include "base/callback_forward.h" -#include "base/memory/ref_counted.h" - -namespace net { -class X509Certificate; -} // namespace net - -namespace atom { - -class NativeWindow; - -namespace api { - -typedef base::Callback ShowTrustCallback; - -void ShowCertificateTrustUI(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const ShowTrustCallback& callback); - -} // namespace api - -} // namespace atom - -#endif // ATOM_BROWSER_API_ATOM_API_CERTIFICATE_TRUST_H_ diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 326834472d24..310b1b233b80 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -8,11 +8,13 @@ #include "atom/browser/api/atom_api_window.h" #include "atom/browser/native_window.h" +#include "atom/browser/ui/certificate_trust.h" #include "atom/browser/ui/file_dialog.h" #include "atom/browser/ui/message_box.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/image_converter.h" +#include "atom/common/native_mate_converters/net_converter.h" #include "native_mate/dictionary.h" #include "atom/common/node_includes.h" @@ -119,6 +121,16 @@ void ShowSaveDialog(const file_dialog::DialogSettings& settings, } } +// #if defined(OS_MACOSX) +void ShowCertificateTrust(atom::NativeWindow* parent_window, + const scoped_refptr& cert, + std::string message, + const certificate_trust::ShowTrustCallback& callback, + mate::Arguments* args) { + certificate_trust::ShowCertificateTrust(parent_window, cert, message, callback); +} +// #endif + void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); @@ -126,6 +138,9 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("showErrorBox", &atom::ShowErrorBox); dict.SetMethod("showOpenDialog", &ShowOpenDialog); dict.SetMethod("showSaveDialog", &ShowSaveDialog); +// #if defined(OS_MACOSX) + dict.SetMethod("showCertificateTrustDialog", &ShowCertificateTrust); +// #endif } } // namespace diff --git a/atom/browser/ui/certificate_trust.h b/atom/browser/ui/certificate_trust.h new file mode 100644 index 000000000000..30271edff072 --- /dev/null +++ b/atom/browser/ui/certificate_trust.h @@ -0,0 +1,29 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_UI_CERTIFICATE_TRUST_H_ +#define ATOM_BROWSER_UI_CERTIFICATE_TRUST_H_ + +#include + +#include "base/callback_forward.h" +#include "base/memory/ref_counted.h" +#include "net/cert/x509_certificate.h" + +namespace atom { +class NativeWindow; +} // namespace atom + +namespace certificate_trust { + +typedef base::Callback ShowTrustCallback; + +void ShowCertificateTrust(atom::NativeWindow* parent_window, + const scoped_refptr& cert, + std::string message, + const ShowTrustCallback& callback); + +} // namespace certificate_trust + +#endif // ATOM_BROWSER_UI_CERTIFICATE_TRUST_H_ diff --git a/atom/browser/api/atom_api_certificate_trust_mac.mm b/atom/browser/ui/certificate_trust_mac.mm similarity index 83% rename from atom/browser/api/atom_api_certificate_trust_mac.mm rename to atom/browser/ui/certificate_trust_mac.mm index 7f14a72f5016..a3774e51c348 100644 --- a/atom/browser/api/atom_api_certificate_trust_mac.mm +++ b/atom/browser/ui/certificate_trust_mac.mm @@ -2,7 +2,7 @@ // Use of this source code is governed by the MIT license that can be // found in the LICENSE file. -#include "atom/browser/api/atom_api_certificate_trust.h" +#include "atom/browser/ui/certificate_trust.h" #import #import @@ -14,12 +14,11 @@ #include "base/mac/mac_util.h" #include "base/mac/scoped_cftyperef.h" #include "base/strings/sys_string_conversions.h" -#include "net/cert/x509_certificate.h" #include "net/cert/cert_database.h" @interface TrustDelegate : NSObject { @private - atom::api::ShowTrustCallback callback_; + certificate_trust::ShowTrustCallback callback_; SFCertificateTrustPanel* panel_; scoped_refptr cert_; SecTrustRef trust_; @@ -27,7 +26,7 @@ SecPolicyRef sec_policy_; } -- (id)initWithCallback:(const atom::api::ShowTrustCallback&)callback +- (id)initWithCallback:(const certificate_trust::ShowTrustCallback&)callback panel:(SFCertificateTrustPanel*)panel cert:(const scoped_refptr&)cert trust:(SecTrustRef)trust @@ -51,7 +50,7 @@ [super dealloc]; } -- (id)initWithCallback:(const atom::api::ShowTrustCallback&)callback +- (id)initWithCallback:(const certificate_trust::ShowTrustCallback&)callback panel:(SFCertificateTrustPanel*)panel cert:(const scoped_refptr&)cert trust:(SecTrustRef)trust @@ -86,14 +85,12 @@ @end -namespace atom { +namespace certificate_trust { -namespace api { - -void ShowCertificateTrustUI(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const ShowTrustCallback& callback) { +void ShowCertificateTrust(atom::NativeWindow* parent_window, + const scoped_refptr& cert, + std::string message, + const ShowTrustCallback& callback) { auto sec_policy = SecPolicyCreateBasicX509(); auto cert_chain = cert->CreateOSCertChainForCert(); SecTrustRef trust = nullptr; @@ -119,6 +116,4 @@ void ShowCertificateTrustUI(atom::NativeWindow* parent_window, message:msg]; } -} // namespace api - -} // namespace atom +} // namespace certificate_trust diff --git a/filenames.gypi b/filenames.gypi index f2a78ad4d4b6..ff7bbaee4d79 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -103,8 +103,6 @@ 'atom/browser/api/atom_api_app.h', 'atom/browser/api/atom_api_auto_updater.cc', 'atom/browser/api/atom_api_auto_updater.h', - 'atom/browser/api/atom_api_certificate_trust_mac.mm', - 'atom/browser/api/atom_api_certificate_trust.h', 'atom/browser/api/atom_api_content_tracing.cc', 'atom/browser/api/atom_api_cookies.cc', 'atom/browser/api/atom_api_cookies.h', @@ -281,6 +279,8 @@ 'atom/browser/ui/accelerator_util_views.cc', 'atom/browser/ui/atom_menu_model.cc', 'atom/browser/ui/atom_menu_model.h', + 'atom/browser/ui/certificate_trust_mac.mm', + 'atom/browser/ui/certificate_trust.h', 'atom/browser/ui/cocoa/atom_menu_controller.h', 'atom/browser/ui/cocoa/atom_menu_controller.mm', 'atom/browser/ui/cocoa/atom_touch_bar.h', From 4ffaf2cd99d44b8d143dcc2a4bf81e7a641144a0 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:09:04 -0400 Subject: [PATCH 16/36] Remove old unnecssary change on `app` --- atom/browser/api/atom_api_app.h | 1 - 1 file changed, 1 deletion(-) diff --git a/atom/browser/api/atom_api_app.h b/atom/browser/api/atom_api_app.h index f2debdf2e533..8b276f334d5c 100644 --- a/atom/browser/api/atom_api_app.h +++ b/atom/browser/api/atom_api_app.h @@ -19,7 +19,6 @@ #include "content/public/browser/gpu_data_manager_observer.h" #include "native_mate/handle.h" #include "net/base/completion_callback.h" -#include "net/cert/x509_certificate.h" #if defined(USE_NSS_CERTS) #include "chrome/browser/certificate_manager_model.h" From bcecba20e6096e3b8ebeef3daa3a4397d201b6c3 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:09:24 -0400 Subject: [PATCH 17/36] Fix indentation --- atom/browser/api/atom_api_dialog.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 310b1b233b80..6c6e49bb7f7d 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -123,10 +123,10 @@ void ShowSaveDialog(const file_dialog::DialogSettings& settings, // #if defined(OS_MACOSX) void ShowCertificateTrust(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const certificate_trust::ShowTrustCallback& callback, - mate::Arguments* args) { + const scoped_refptr& cert, + std::string message, + const certificate_trust::ShowTrustCallback& callback, + mate::Arguments* args) { certificate_trust::ShowCertificateTrust(parent_window, cert, message, callback); } // #endif From 0b7ffd094aace166dea48e9495d01335f732e97f Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:25:06 -0400 Subject: [PATCH 18/36] Expose through the actual JS API too --- lib/browser/api/dialog.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 834a6a6d7d8d..814a8a2af574 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -280,6 +280,10 @@ module.exports = { showErrorBox: function (...args) { return binding.showErrorBox(...args) + }, + + showCertificateTrustDialog: function (...args) { + return binding.showCertificateTrustDialog(...args) } } From 11f5c942ce70d85a573d2aa42701d9aa54329b35 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:25:09 -0400 Subject: [PATCH 19/36] Conditionalize --- atom/browser/api/atom_api_dialog.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 6c6e49bb7f7d..9a93ba703770 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -121,7 +121,7 @@ void ShowSaveDialog(const file_dialog::DialogSettings& settings, } } -// #if defined(OS_MACOSX) +#if defined(OS_MACOSX) void ShowCertificateTrust(atom::NativeWindow* parent_window, const scoped_refptr& cert, std::string message, @@ -129,7 +129,7 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window, mate::Arguments* args) { certificate_trust::ShowCertificateTrust(parent_window, cert, message, callback); } -// #endif +#endif void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { @@ -138,9 +138,9 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("showErrorBox", &atom::ShowErrorBox); dict.SetMethod("showOpenDialog", &ShowOpenDialog); dict.SetMethod("showSaveDialog", &ShowSaveDialog); -// #if defined(OS_MACOSX) +#if defined(OS_MACOSX) dict.SetMethod("showCertificateTrustDialog", &ShowCertificateTrust); -// #endif +#endif } } // namespace From 06643e525a084816fab48eb13053d713358a5b95 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:28:44 -0400 Subject: [PATCH 20/36] Const ref that message --- atom/browser/ui/certificate_trust.h | 2 +- atom/browser/ui/certificate_trust_mac.mm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/ui/certificate_trust.h b/atom/browser/ui/certificate_trust.h index 30271edff072..085d04d1030a 100644 --- a/atom/browser/ui/certificate_trust.h +++ b/atom/browser/ui/certificate_trust.h @@ -21,7 +21,7 @@ typedef base::Callback ShowTrustCallback; void ShowCertificateTrust(atom::NativeWindow* parent_window, const scoped_refptr& cert, - std::string message, + const std::string& message, const ShowTrustCallback& callback); } // namespace certificate_trust diff --git a/atom/browser/ui/certificate_trust_mac.mm b/atom/browser/ui/certificate_trust_mac.mm index a3774e51c348..1e7ad5204f2e 100644 --- a/atom/browser/ui/certificate_trust_mac.mm +++ b/atom/browser/ui/certificate_trust_mac.mm @@ -89,7 +89,7 @@ namespace certificate_trust { void ShowCertificateTrust(atom::NativeWindow* parent_window, const scoped_refptr& cert, - std::string message, + const std::string& message, const ShowTrustCallback& callback) { auto sec_policy = SecPolicyCreateBasicX509(); auto cert_chain = cert->CreateOSCertChainForCert(); From b0ef7ddf44cc7f20f43f4d9cab88ded82e181cab Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:28:51 -0400 Subject: [PATCH 21/36] Use better iteration --- atom/common/native_mate_converters/net_converter.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 2d3095a63ea0..305627ad5bcd 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -71,8 +71,7 @@ v8::Local Converter>::ToV8( dict.Set("issuerCert", issuer_cert); std::vector intermediates_encoded; - for (size_t i = 0; i < intermediates.size(); i++) { - auto native_cert = intermediates[i]; + for (auto& native_cert : intermediates) { std::string encoded; net::X509Certificate::GetPEMEncoded(native_cert, &encoded); intermediates_encoded.push_back(encoded); From da1b0aab3e157c063a7ffc2a94d714d8841261cb Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:39:45 -0400 Subject: [PATCH 22/36] Flesh out some docs --- docs/api/dialog.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/api/dialog.md b/docs/api/dialog.md index 230930abd948..71224b69cd4c 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -175,6 +175,17 @@ it is usually used to report errors in early stage of startup. If called before the app `ready`event on Linux, the message will be emitted to stderr, and no GUI dialog will appear. +### `dialog.showCertificateTrustDialog(browserWindow, certificate, message, callback)` _macOS_ + +* `browserWindow` BrowserWindow +* `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import. +* `message` String - The message to display to the user. +* `callback` Function + * `result` Boolean - Whether the user chose to cancel or continue. + +Displays a modal dialog that shows a message and certificate information, and +gives the user the option of trusting/importing the certificate. + ## Sheets On macOS, dialogs are presented as sheets attached to a window if you provide From b8ff4666c84c249b002cc6fd2aa95bc6b7db6f04 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 15:48:47 -0400 Subject: [PATCH 23/36] Remove stale includes --- atom/browser/ui/certificate_trust_mac.mm | 5 ----- 1 file changed, 5 deletions(-) diff --git a/atom/browser/ui/certificate_trust_mac.mm b/atom/browser/ui/certificate_trust_mac.mm index 1e7ad5204f2e..32dc1ce91e19 100644 --- a/atom/browser/ui/certificate_trust_mac.mm +++ b/atom/browser/ui/certificate_trust_mac.mm @@ -5,14 +5,9 @@ #include "atom/browser/ui/certificate_trust.h" #import -#import #import #include "atom/browser/native_window.h" -#include "base/files/file_util.h" -#include "base/mac/foundation_util.h" -#include "base/mac/mac_util.h" -#include "base/mac/scoped_cftyperef.h" #include "base/strings/sys_string_conversions.h" #include "net/cert/cert_database.h" From 74c0cbddaed65827268a9ad19b607b68cdbc245d Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 16:07:02 -0400 Subject: [PATCH 24/36] Linebreak to keep the linter happy --- atom/browser/api/atom_api_dialog.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 9a93ba703770..090f4c5d8ac5 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -127,7 +127,8 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window, std::string message, const certificate_trust::ShowTrustCallback& callback, mate::Arguments* args) { - certificate_trust::ShowCertificateTrust(parent_window, cert, message, callback); + certificate_trust::ShowCertificateTrust(parent_window, cert, + message, callback); } #endif From 370cf815d9ae4ce85c13b7dc725a76e5651169d6 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 16:27:53 -0400 Subject: [PATCH 25/36] Get rid of `intermediates` and rehydrate from `issuerCert` --- .../native_mate_converters/net_converter.cc | 37 ++++++------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 305627ad5bcd..7453dd009775 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -59,8 +59,7 @@ v8::Local Converter>::ToV8( net::HashValue( val->CalculateFingerprint256(val->os_cert_handle())).ToString()); - auto intermediates = val->GetIntermediateCertificates(); - if (!intermediates.empty()) { + if (!val->GetIntermediateCertificates().empty()) { net::X509Certificate::OSCertHandles issuer_intermediates( val->GetIntermediateCertificates().begin() + 1, val->GetIntermediateCertificates().end()); @@ -69,15 +68,6 @@ v8::Local Converter>::ToV8( val->GetIntermediateCertificates().front(), issuer_intermediates); dict.Set("issuerCert", issuer_cert); - - std::vector intermediates_encoded; - for (auto& native_cert : intermediates) { - std::string encoded; - net::X509Certificate::GetPEMEncoded(native_cert, &encoded); - intermediates_encoded.push_back(encoded); - } - - dict.Set("intermediates", intermediates_encoded); } return dict.GetHandle(); @@ -113,24 +103,21 @@ bool Converter>::FromV8( if (!CertFromData(data, &leaf_cert)) return false; - std::vector intermediates_encoded; - dict.Get("intermediates", &intermediates_encoded); - std::vector intermediates; - for (size_t i = 0; i < intermediates_encoded.size(); i++) { - auto intermediate_data = intermediates_encoded[i]; - scoped_refptr cert; - if (!CertFromData(intermediate_data, &cert)) + scoped_refptr parent; + if (dict.Get("issuerCert", &parent)) { + auto parents = std::vector( + parent->GetIntermediateCertificates()); + parents.insert(parents.begin(), parent->os_cert_handle()); + auto cert = net::X509Certificate::CreateFromHandle( + leaf_cert->os_cert_handle(), parents); + if (!cert) return false; - intermediates.push_back(cert->os_cert_handle()); + *out = cert; + } else { + *out = leaf_cert; } - auto cert = net::X509Certificate::CreateFromHandle( - leaf_cert->os_cert_handle(), intermediates); - if (!cert) - return false; - - *out = cert; return true; } From 398132cfe383f3e8cbc3742011596fcd4dceaed9 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 21:19:00 -0400 Subject: [PATCH 26/36] Fix file sorting --- filenames.gypi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filenames.gypi b/filenames.gypi index ff7bbaee4d79..192d953287d7 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -279,8 +279,8 @@ 'atom/browser/ui/accelerator_util_views.cc', 'atom/browser/ui/atom_menu_model.cc', 'atom/browser/ui/atom_menu_model.h', - 'atom/browser/ui/certificate_trust_mac.mm', 'atom/browser/ui/certificate_trust.h', + 'atom/browser/ui/certificate_trust_mac.mm', 'atom/browser/ui/cocoa/atom_menu_controller.h', 'atom/browser/ui/cocoa/atom_menu_controller.mm', 'atom/browser/ui/cocoa/atom_touch_bar.h', From b3e865a4787b8b0aa844c8466a0b8ca83649314c Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 21:22:14 -0400 Subject: [PATCH 27/36] Fix some style things --- atom/browser/ui/certificate_trust_mac.mm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/browser/ui/certificate_trust_mac.mm b/atom/browser/ui/certificate_trust_mac.mm index 32dc1ce91e19..ad54eaa3831c 100644 --- a/atom/browser/ui/certificate_trust_mac.mm +++ b/atom/browser/ui/certificate_trust_mac.mm @@ -28,7 +28,7 @@ certChain:(CFArrayRef)certChain secPolicy:(SecPolicyRef)secPolicy; -- (void)panelDidEnd:(NSWindow *)sheet +- (void)panelDidEnd:(NSWindow*)sheet returnCode:(int)returnCode contextInfo:(void*)contextInfo; @@ -63,7 +63,7 @@ return self; } -- (void)panelDidEnd:(NSWindow *)sheet +- (void)panelDidEnd:(NSWindow*)sheet returnCode:(int)returnCode contextInfo:(void*)contextInfo { if (returnCode == NSFileHandlingPanelOKButton) { @@ -73,7 +73,7 @@ cert_db->NotifyObserversCertDBChanged(cert_.get()); } - callback_.Run(returnCode); + callback_.Run(returnCode == NSFileHandlingPanelOKButton ? true : false); [self autorelease]; } @@ -96,7 +96,7 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window, nil; auto msg = base::SysUTF8ToNSString(message); - SFCertificateTrustPanel *panel = [[SFCertificateTrustPanel alloc] init]; + auto panel = [[SFCertificateTrustPanel alloc] init]; auto delegate = [[TrustDelegate alloc] initWithCallback:callback panel:panel cert:cert From 2badfbe04fbee7104317cffa7031b8fedf1191e8 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 21:22:37 -0400 Subject: [PATCH 28/36] Remove the intermediate function --- atom/browser/api/atom_api_dialog.cc | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 090f4c5d8ac5..0105a433dbfd 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -121,17 +121,6 @@ void ShowSaveDialog(const file_dialog::DialogSettings& settings, } } -#if defined(OS_MACOSX) -void ShowCertificateTrust(atom::NativeWindow* parent_window, - const scoped_refptr& cert, - std::string message, - const certificate_trust::ShowTrustCallback& callback, - mate::Arguments* args) { - certificate_trust::ShowCertificateTrust(parent_window, cert, - message, callback); -} -#endif - void Initialize(v8::Local exports, v8::Local unused, v8::Local context, void* priv) { mate::Dictionary dict(context->GetIsolate(), exports); @@ -140,7 +129,7 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("showOpenDialog", &ShowOpenDialog); dict.SetMethod("showSaveDialog", &ShowSaveDialog); #if defined(OS_MACOSX) - dict.SetMethod("showCertificateTrustDialog", &ShowCertificateTrust); + dict.SetMethod("showCertificateTrustDialog", &certificate_trust::ShowCertificateTrust); #endif } From e2bda3ca0f8cc1bf982815040df78cb952261e79 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 21:33:21 -0400 Subject: [PATCH 29/36] Use an options object for most of the params --- docs/api/dialog.md | 7 ++++--- lib/browser/api/dialog.js | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docs/api/dialog.md b/docs/api/dialog.md index 71224b69cd4c..17aa7341bb57 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -175,11 +175,12 @@ it is usually used to report errors in early stage of startup. If called before the app `ready`event on Linux, the message will be emitted to stderr, and no GUI dialog will appear. -### `dialog.showCertificateTrustDialog(browserWindow, certificate, message, callback)` _macOS_ +### `dialog.showCertificateTrustDialog(browserWindow, options, callback)` _macOS_ * `browserWindow` BrowserWindow -* `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import. -* `message` String - The message to display to the user. +* `options` Object + * `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import. + * `message` String - The message to display to the user. * `callback` Function * `result` Boolean - Whether the user chose to cancel or continue. diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 814a8a2af574..908ffaf670f4 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -282,8 +282,23 @@ module.exports = { return binding.showErrorBox(...args) }, - showCertificateTrustDialog: function (...args) { - return binding.showCertificateTrustDialog(...args) + showCertificateTrustDialog: function (window, options, callback) { + if (options == null || typeof options !== 'object') { + throw new TypeError('options must be an object') + } + + let {certificate, message} = options + if (certificate == null || typeof options !== 'object') { + throw new TypeError('certificate must be an object') + } + + if (message == null) { + message = '' + } else if (typeof message !== 'string') { + throw new TypeError('message must be a string') + } + + return binding.showCertificateTrustDialog(window, certificate, message, callback) } } From 8d465234e40e601e215220270e5c11eec8469276 Mon Sep 17 00:00:00 2001 From: joshaber Date: Mon, 3 Apr 2017 21:40:46 -0400 Subject: [PATCH 30/36] Appease our linty overlords --- atom/browser/api/atom_api_dialog.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index 0105a433dbfd..4f152e3602df 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -129,7 +129,8 @@ void Initialize(v8::Local exports, v8::Local unused, dict.SetMethod("showOpenDialog", &ShowOpenDialog); dict.SetMethod("showSaveDialog", &ShowSaveDialog); #if defined(OS_MACOSX) - dict.SetMethod("showCertificateTrustDialog", &certificate_trust::ShowCertificateTrust); + dict.SetMethod("showCertificateTrustDialog", + &certificate_trust::ShowCertificateTrust); #endif } From 0cab8a3322450465f70e60b00178af4c8b7d0e07 Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 4 Apr 2017 09:19:23 -0400 Subject: [PATCH 31/36] Put CertFromData in an anon namespace --- .../native_mate_converters/net_converter.cc | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 7453dd009775..401cd5328ceb 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -26,6 +26,27 @@ namespace mate { +namespace { + +bool CertFromData(const std::string& data, + scoped_refptr* out) { + auto cert_list = net::X509Certificate::CreateCertificateListFromBytes( + data.c_str(), data.length(), + net::X509Certificate::FORMAT_SINGLE_CERTIFICATE); + if (cert_list.empty()) + return false; + + auto leaf_cert = cert_list.front(); + if (!leaf_cert) + return false; + + *out = leaf_cert; + + return true; +} + +} + // static v8::Local Converter::ToV8( v8::Isolate* isolate, const net::AuthChallengeInfo* val) { @@ -73,23 +94,6 @@ v8::Local Converter>::ToV8( return dict.GetHandle(); } -bool CertFromData(const std::string& data, - scoped_refptr* out) { - auto cert_list = net::X509Certificate::CreateCertificateListFromBytes( - data.c_str(), data.length(), - net::X509Certificate::FORMAT_SINGLE_CERTIFICATE); - if (cert_list.empty()) - return false; - - auto leaf_cert = cert_list.front(); - if (!leaf_cert) - return false; - - *out = leaf_cert; - - return true; -} - bool Converter>::FromV8( v8::Isolate* isolate, v8::Local val, scoped_refptr* out) { From 146e1ed3ce5a4e8eac8f2dfd95f9e221f90a329f Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 4 Apr 2017 09:21:15 -0400 Subject: [PATCH 32/36] Don't pass the result through It's meaningless on macOS, at least. --- atom/browser/ui/certificate_trust.h | 2 +- atom/browser/ui/certificate_trust_mac.mm | 12 +++++------- docs/api/dialog.md | 1 - 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/atom/browser/ui/certificate_trust.h b/atom/browser/ui/certificate_trust.h index 085d04d1030a..7cbf31ea41fb 100644 --- a/atom/browser/ui/certificate_trust.h +++ b/atom/browser/ui/certificate_trust.h @@ -17,7 +17,7 @@ class NativeWindow; namespace certificate_trust { -typedef base::Callback ShowTrustCallback; +typedef base::Callback ShowTrustCallback; void ShowCertificateTrust(atom::NativeWindow* parent_window, const scoped_refptr& cert, diff --git a/atom/browser/ui/certificate_trust_mac.mm b/atom/browser/ui/certificate_trust_mac.mm index ad54eaa3831c..e0888dd3ea24 100644 --- a/atom/browser/ui/certificate_trust_mac.mm +++ b/atom/browser/ui/certificate_trust_mac.mm @@ -66,14 +66,12 @@ - (void)panelDidEnd:(NSWindow*)sheet returnCode:(int)returnCode contextInfo:(void*)contextInfo { - if (returnCode == NSFileHandlingPanelOKButton) { - auto cert_db = net::CertDatabase::GetInstance(); - // This forces Chromium to reload the certificate since it might be trusted - // now. - cert_db->NotifyObserversCertDBChanged(cert_.get()); - } + auto cert_db = net::CertDatabase::GetInstance(); + // This forces Chromium to reload the certificate since it might be trusted + // now. + cert_db->NotifyObserversCertDBChanged(cert_.get()); - callback_.Run(returnCode == NSFileHandlingPanelOKButton ? true : false); + callback_.Run(); [self autorelease]; } diff --git a/docs/api/dialog.md b/docs/api/dialog.md index 17aa7341bb57..58bbd050845a 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -182,7 +182,6 @@ and no GUI dialog will appear. * `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import. * `message` String - The message to display to the user. * `callback` Function - * `result` Boolean - Whether the user chose to cancel or continue. Displays a modal dialog that shows a message and certificate information, and gives the user the option of trusting/importing the certificate. From 736d6afe730d297f58eca96df367676d7c6b013f Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 4 Apr 2017 09:23:30 -0400 Subject: [PATCH 33/36] As you wish linter --- atom/common/native_mate_converters/net_converter.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/common/native_mate_converters/net_converter.cc b/atom/common/native_mate_converters/net_converter.cc index 401cd5328ceb..b78bc5b8e118 100644 --- a/atom/common/native_mate_converters/net_converter.cc +++ b/atom/common/native_mate_converters/net_converter.cc @@ -45,7 +45,7 @@ bool CertFromData(const std::string& data, return true; } -} +} // namespace // static v8::Local Converter::ToV8( From 2749ded062c820e0680d802698a67eb82c8eab51 Mon Sep 17 00:00:00 2001 From: joshaber Date: Tue, 4 Apr 2017 11:45:27 -0400 Subject: [PATCH 34/36] Fix c&p error --- lib/browser/api/dialog.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 908ffaf670f4..215d3f25dd97 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -288,7 +288,7 @@ module.exports = { } let {certificate, message} = options - if (certificate == null || typeof options !== 'object') { + if (certificate == null || typeof certificate !== 'object') { throw new TypeError('certificate must be an object') } From 2e32525e8f041056c10c908c80f55e1d56f2b426 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 4 Apr 2017 10:49:10 -0700 Subject: [PATCH 35/36] Make browser window optional --- docs/api/dialog.md | 7 +++++-- lib/browser/api/dialog.js | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/api/dialog.md b/docs/api/dialog.md index 58bbd050845a..6515a0feb940 100644 --- a/docs/api/dialog.md +++ b/docs/api/dialog.md @@ -175,9 +175,9 @@ it is usually used to report errors in early stage of startup. If called before the app `ready`event on Linux, the message will be emitted to stderr, and no GUI dialog will appear. -### `dialog.showCertificateTrustDialog(browserWindow, options, callback)` _macOS_ +### `dialog.showCertificateTrustDialog([browserWindow, ]options, callback)` _macOS_ -* `browserWindow` BrowserWindow +* `browserWindow` BrowserWindow (optional) * `options` Object * `certificate` [Certificate](structures/certificate.md) - The certificate to trust/import. * `message` String - The message to display to the user. @@ -186,6 +186,9 @@ and no GUI dialog will appear. Displays a modal dialog that shows a message and certificate information, and gives the user the option of trusting/importing the certificate. +The `browserWindow` argument allows the dialog to attach itself to a parent +window, making it modal. + ## Sheets On macOS, dialogs are presented as sheets attached to a window if you provide diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index 215d3f25dd97..964461804769 100644 --- a/lib/browser/api/dialog.js +++ b/lib/browser/api/dialog.js @@ -282,7 +282,9 @@ module.exports = { return binding.showErrorBox(...args) }, - showCertificateTrustDialog: function (window, options, callback) { + showCertificateTrustDialog: function (...args) { + let [window, options, callback] = parseArgs(...args) + if (options == null || typeof options !== 'object') { throw new TypeError('options must be an object') } From 2bd47eb67262a79016edb2c9aef220ee766ba607 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 4 Apr 2017 10:49:21 -0700 Subject: [PATCH 36/36] Add specs for showCertificateTrustDialog option errors --- spec/api-dialog-spec.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/api-dialog-spec.js b/spec/api-dialog-spec.js index 6b245c2b65f7..601e28ca53ca 100644 --- a/spec/api-dialog-spec.js +++ b/spec/api-dialog-spec.js @@ -93,4 +93,20 @@ describe('dialog module', () => { }, /Error processing argument at index 1/) }) }) + + describe('showCertificateTrustDialog', () => { + it('throws errors when the options are invalid', () => { + assert.throws(() => { + dialog.showCertificateTrustDialog() + }, /options must be an object/) + + assert.throws(() => { + dialog.showCertificateTrustDialog({}) + }, /certificate must be an object/) + + assert.throws(() => { + dialog.showCertificateTrustDialog({certificate: {}, message: false}) + }, /message must be a string/) + }) + }) })