diff --git a/atom/browser/api/atom_api_session.cc b/atom/browser/api/atom_api_session.cc index 45bb9c2cf3bf..0c850888e8e7 100644 --- a/atom/browser/api/atom_api_session.cc +++ b/atom/browser/api/atom_api_session.cc @@ -204,6 +204,18 @@ struct Converter { } }; +template<> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + atom::VerifyRequestParams val) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate); + dict.Set("hostname", val.hostname); + dict.Set("certificate", val.certificate); + dict.Set("verificationResult", val.default_result); + return dict.GetHandle(); + } +}; + } // namespace mate namespace atom { @@ -738,7 +750,7 @@ void Session::BuildPrototype(v8::Isolate* isolate, .SetMethod("setDownloadPath", &Session::SetDownloadPath) .SetMethod("enableNetworkEmulation", &Session::EnableNetworkEmulation) .SetMethod("disableNetworkEmulation", &Session::DisableNetworkEmulation) - .SetMethod("setCertificateVerifyProc", &Session::SetCertVerifyProc) + .SetMethod("_setCertificateVerifyProc", &Session::SetCertVerifyProc) .SetMethod("setPermissionRequestHandler", &Session::SetPermissionRequestHandler) .SetMethod("clearHostResolverCache", &Session::ClearHostResolverCache) diff --git a/atom/browser/net/atom_cert_verifier.cc b/atom/browser/net/atom_cert_verifier.cc index 61c7439e2733..5dee107eb36e 100644 --- a/atom/browser/net/atom_cert_verifier.cc +++ b/atom/browser/net/atom_cert_verifier.cc @@ -7,6 +7,9 @@ #include "atom/browser/browser.h" #include "atom/browser/net/atom_ct_delegate.h" #include "atom/common/native_mate_converters/net_converter.h" +#include "base/containers/linked_list.h" +#include "base/memory/ptr_util.h" +#include "base/memory/weak_ptr.h" #include "content/public/browser/browser_thread.h" #include "net/base/net_errors.h" #include "net/cert/cert_verify_result.h" @@ -19,17 +22,130 @@ namespace atom { namespace { -void OnResult( - net::CertVerifyResult* verify_result, - const net::CompletionCallback& callback, - bool result) { - BrowserThread::PostTask( - BrowserThread::IO, FROM_HERE, - base::Bind(callback, result ? net::OK : net::ERR_FAILED)); -} +class Response : public base::LinkNode { + public: + Response(net::CertVerifyResult* verify_result, + const net::CompletionCallback& callback) + : verify_result_(verify_result), callback_(callback) {} + net::CertVerifyResult* verify_result() { return verify_result_; } + net::CompletionCallback callback() { return callback_; } + + private: + net::CertVerifyResult* verify_result_; + net::CompletionCallback callback_; + + DISALLOW_COPY_AND_ASSIGN(Response); +}; } // namespace +class CertVerifierRequest : public AtomCertVerifier::Request { + public: + CertVerifierRequest(const AtomCertVerifier::RequestParams& params, + AtomCertVerifier* cert_verifier) + : params_(params), + cert_verifier_(cert_verifier), + error_(net::ERR_IO_PENDING), + custom_response_(net::ERR_IO_PENDING), + first_response_(true), + weak_ptr_factory_(this) {} + + ~CertVerifierRequest() { + cert_verifier_->RemoveRequest(params_); + default_verifier_request_.reset(); + while (!response_list_.empty() && !first_response_) { + base::LinkNode* response_node = response_list_.head(); + response_node->RemoveFromList(); + Response* response = response_node->value(); + RunResponse(response); + } + cert_verifier_ = nullptr; + weak_ptr_factory_.InvalidateWeakPtrs(); + } + + void RunResponse(Response* response) { + if (custom_response_ == net::ERR_ABORTED) { + *(response->verify_result()) = result_; + response->callback().Run(error_); + } else { + response->verify_result()->Reset(); + response->verify_result()->verified_cert = params_.certificate(); + cert_verifier_->ct_delegate()->AddCTExcludedHost(params_.hostname()); + response->callback().Run(custom_response_); + } + delete response; + } + + void Start(net::CRLSet* crl_set, + const net::NetLogWithSource& net_log) { + int error = cert_verifier_->default_verifier()->Verify( + params_, crl_set, &result_, + base::Bind(&CertVerifierRequest::OnDefaultVerificationDone, + weak_ptr_factory_.GetWeakPtr()), + &default_verifier_request_, net_log); + if (error != net::ERR_IO_PENDING) + OnDefaultVerificationDone(error); + } + + void OnDefaultVerificationDone(int error) { + error_ = error; + std::unique_ptr request(new VerifyRequestParams()); + request->hostname = params_.hostname(); + request->default_result = net::ErrorToString(error); + request->certificate = params_.certificate(); + BrowserThread::PostTask( + BrowserThread::UI, FROM_HERE, + base::Bind(&CertVerifierRequest::OnVerifyRequestInUI, + weak_ptr_factory_.GetWeakPtr(), + cert_verifier_->verify_proc(), + base::Passed(&request))); + } + + void OnVerifyRequestInUI(const AtomCertVerifier::VerifyProc& verify_proc, + std::unique_ptr request) { + verify_proc.Run(*(request.get()), + base::Bind(&CertVerifierRequest::OnResponseInUI, + weak_ptr_factory_.GetWeakPtr())); + } + + void OnResponseInUI(int result) { + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + base::Bind(&CertVerifierRequest::NotifyResponseInIO, + weak_ptr_factory_.GetWeakPtr(), result)); + } + + void NotifyResponseInIO(int result) { + custom_response_ = result; + first_response_ = false; + // Responding to first request in the list will initiate destruction of + // the class, respond to others in the list inside destructor. + base::LinkNode* response_node = response_list_.head(); + response_node->RemoveFromList(); + Response* response = response_node->value(); + RunResponse(response); + } + + void AddResponseListener(net::CertVerifyResult* verify_result, + const net::CompletionCallback& callback) { + response_list_.Append(new Response(verify_result, callback)); + } + + const AtomCertVerifier::RequestParams& params() const { return params_; } + + private: + using ResponseList = base::LinkedList; + + const AtomCertVerifier::RequestParams params_; + AtomCertVerifier* cert_verifier_; + int error_; + int custom_response_; + bool first_response_; + ResponseList response_list_; + net::CertVerifyResult result_; + std::unique_ptr default_verifier_request_; + base::WeakPtrFactory weak_ptr_factory_; +}; + AtomCertVerifier::AtomCertVerifier(AtomCTDelegate* ct_delegate) : default_cert_verifier_(net::CertVerifier::CreateDefault()), ct_delegate_(ct_delegate) {} @@ -51,23 +167,43 @@ int AtomCertVerifier::Verify( if (verify_proc_.is_null()) { ct_delegate_->ClearCTExcludedHostsList(); - return default_cert_verifier_->Verify( - params, crl_set, verify_result, callback, out_req, net_log); + return default_cert_verifier_->Verify(params, crl_set, verify_result, + callback, out_req, net_log); + } else { + CertVerifierRequest* request = FindRequest(params); + if (!request) { + out_req->reset(); + std::unique_ptr new_request = + base::MakeUnique(params, this); + new_request->Start(crl_set, net_log); + request = new_request.get(); + *out_req = std::move(new_request); + inflight_requests_[params] = request; + } + request->AddResponseListener(verify_result, callback); + + return net::ERR_IO_PENDING; } - - verify_result->Reset(); - verify_result->verified_cert = params.certificate(); - ct_delegate_->AddCTExcludedHost(params.hostname()); - - BrowserThread::PostTask( - BrowserThread::UI, FROM_HERE, - base::Bind(verify_proc_, params.hostname(), params.certificate(), - base::Bind(OnResult, verify_result, callback))); - return net::ERR_IO_PENDING; } bool AtomCertVerifier::SupportsOCSPStapling() { - return true; + if (verify_proc_.is_null()) + return default_cert_verifier_->SupportsOCSPStapling(); + return false; +} + +void AtomCertVerifier::RemoveRequest(const RequestParams& params) { + auto it = inflight_requests_.find(params); + if (it != inflight_requests_.end()) + inflight_requests_.erase(it); +} + +CertVerifierRequest* AtomCertVerifier::FindRequest( + const RequestParams& params) { + auto it = inflight_requests_.find(params); + if (it != inflight_requests_.end()) + return it->second; + return nullptr; } } // namespace atom diff --git a/atom/browser/net/atom_cert_verifier.h b/atom/browser/net/atom_cert_verifier.h index e321f7d3d933..09fa0f2778e8 100644 --- a/atom/browser/net/atom_cert_verifier.h +++ b/atom/browser/net/atom_cert_verifier.h @@ -5,6 +5,7 @@ #ifndef ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_ #define ATOM_BROWSER_NET_ATOM_CERT_VERIFIER_H_ +#include #include #include @@ -13,19 +14,30 @@ namespace atom { class AtomCTDelegate; +class CertVerifierRequest; + +struct VerifyRequestParams { + std::string hostname; + std::string default_result; + scoped_refptr certificate; +}; class AtomCertVerifier : public net::CertVerifier { public: explicit AtomCertVerifier(AtomCTDelegate* ct_delegate); virtual ~AtomCertVerifier(); - using VerifyProc = - base::Callback, - const base::Callback&)>; + using VerifyProc = base::Callback; void SetVerifyProc(const VerifyProc& proc); + const VerifyProc verify_proc() const { return verify_proc_; } + AtomCTDelegate* ct_delegate() const { return ct_delegate_; } + net::CertVerifier* default_verifier() const { + return default_cert_verifier_.get(); + } + protected: // net::CertVerifier: int Verify(const RequestParams& params, @@ -37,6 +49,12 @@ class AtomCertVerifier : public net::CertVerifier { bool SupportsOCSPStapling() override; private: + friend class CertVerifierRequest; + + void RemoveRequest(const RequestParams& params); + CertVerifierRequest* FindRequest(const RequestParams& params); + + std::map inflight_requests_; VerifyProc verify_proc_; std::unique_ptr default_cert_verifier_; AtomCTDelegate* ct_delegate_; diff --git a/docs/api/session.md b/docs/api/session.md index 58de08d39abb..d0814dfdb376 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -250,15 +250,22 @@ the original network configuration. #### `ses.setCertificateVerifyProc(proc)` * `proc` Function - * `hostname` String - * `certificate` [Certificate](structures/certificate.md) + * `request` Object + * `hostname` String + * `certificate` [Certificate](structures/certificate.md) + * `error` String - Verification result from chromium. * `callback` Function - * `isTrusted` Boolean - Determines if the certificate should be trusted + * `verificationResult` Integer - Value can be one of certificate error codes + from [here](https://code.google.com/p/chromium/codesearch#chromium/src/net/base/net_error_list.h). + Apart from the certificate error codes, the following special codes can be used. + * `0` - Indicates success and disables Certificate Transperancy verification. + * `-2` - Indicates failure. + * `-3` - Uses the verification result from chromium. Sets the certificate verify proc for `session`, the `proc` will be called with -`proc(hostname, certificate, callback)` whenever a server certificate -verification is requested. Calling `callback(true)` accepts the certificate, -calling `callback(false)` rejects it. +`proc(request, callback)` whenever a server certificate +verification is requested. Calling `callback(0)` accepts the certificate, +calling `callback(-2)` rejects it. Calling `setCertificateVerifyProc(null)` will revert back to default certificate verify proc. diff --git a/docs/tutorial/planned-breaking-changes.md b/docs/tutorial/planned-breaking-changes.md index e2d0e3a939a1..57505d0b66fb 100644 --- a/docs/tutorial/planned-breaking-changes.md +++ b/docs/tutorial/planned-breaking-changes.md @@ -91,6 +91,19 @@ process.versions.electron read-only properties for consistency with the other `process.versions` properties set by Node. +## `session` + +```js +// Deprecated +ses.setCertificateVerifyProc(function (hostname, certificate, callback) { + callback(true) +}) +// Replace with +ses.setCertificateVerifyProc(function (request, callback) { + callback(0) +}) +``` + ## `Tray` ```js diff --git a/lib/browser/api/session.js b/lib/browser/api/session.js index ac47244db326..33f3b47dee0e 100644 --- a/lib/browser/api/session.js +++ b/lib/browser/api/session.js @@ -20,3 +20,16 @@ Object.setPrototypeOf(Cookies.prototype, EventEmitter.prototype) Session.prototype._init = function () { app.emit('session-created', this) } + +Session.prototype.setCertificateVerifyProc = function (verifyProc) { + if (verifyProc != null && verifyProc.length > 2) { + // TODO(kevinsawicki): Remove in 2.0, deprecate before then with warnings + this._setCertificateVerifyProc(({hostname, certificate, verificationResult}, cb) => { + verifyProc(hostname, certificate, (result) => { + cb(result ? 0 : -2) + }) + }) + } else { + this._setCertificateVerifyProc(verifyProc) + } +} diff --git a/lib/browser/rpc-server.js b/lib/browser/rpc-server.js index 1dac51645926..2cafc9343925 100644 --- a/lib/browser/rpc-server.js +++ b/lib/browser/rpc-server.js @@ -222,6 +222,7 @@ const unwrapArgs = function (sender, args) { removeRemoteListenersAndLogWarning(meta, args, callIntoRenderer) } } + Object.defineProperty(callIntoRenderer, 'length', { value: meta.length }) v8Util.setRemoteCallbackFreer(callIntoRenderer, meta.id, sender) rendererFunctions.set(objectId, callIntoRenderer) diff --git a/lib/renderer/api/remote.js b/lib/renderer/api/remote.js index c3c8e3b89cd1..8258a91a6619 100644 --- a/lib/renderer/api/remote.js +++ b/lib/renderer/api/remote.js @@ -79,7 +79,8 @@ const wrapArgs = function (args, visited) { return { type: 'function', id: callbacksRegistry.add(value), - location: v8Util.getHiddenValue(value, 'location') + location: v8Util.getHiddenValue(value, 'location'), + length: value.length } } else { return { diff --git a/spec/api-ipc-spec.js b/spec/api-ipc-spec.js index b1ca29c6a158..372855619d61 100644 --- a/spec/api-ipc-spec.js +++ b/spec/api-ipc-spec.js @@ -85,6 +85,13 @@ describe('ipc module', function () { assert.equal(foo.baz(), 123) }) + it('includes the length of functions specified as arguments', function () { + var a = remote.require(path.join(fixtures, 'module', 'function-with-args.js')) + assert.equal(a(function (a, b, c, d, f) {}), 5) + assert.equal(a((a) => {}), 1) + assert.equal(a((...args) => {}), 0) + }) + it('handles circular references in arrays and objects', function () { var a = remote.require(path.join(fixtures, 'module', 'circular.js')) diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 1f31ff7123e5..f8e0e27dc34b 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -557,8 +557,9 @@ describe('session module', function () { }) it('accepts the request when the callback is called with true', function (done) { - session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { - callback(true) + session.defaultSession.setCertificateVerifyProc(function ({hostname, certificate, verificationResult}, callback) { + assert.equal(verificationResult, 'net::ERR_CERT_AUTHORITY_INVALID') + callback(0) }) w.webContents.once('did-finish-load', function () { @@ -568,8 +569,37 @@ describe('session module', function () { w.loadURL(`https://127.0.0.1:${server.address().port}`) }) + describe('deprecated function signature', function () { + it('supports accepting the request', function (done) { + session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { + assert.equal(hostname, '127.0.0.1') + callback(true) + }) + + w.webContents.once('did-finish-load', function () { + assert.equal(w.webContents.getTitle(), 'hello') + done() + }) + w.loadURL(`https://127.0.0.1:${server.address().port}`) + }) + + it('supports rejecting the request', function (done) { + session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { + assert.equal(hostname, '127.0.0.1') + callback(false) + }) + + var url = `https://127.0.0.1:${server.address().port}` + w.webContents.once('did-finish-load', function () { + assert.equal(w.webContents.getTitle(), url) + done() + }) + w.loadURL(url) + }) + }) + it('rejects the request when the callback is called with false', function (done) { - session.defaultSession.setCertificateVerifyProc(function (hostname, certificate, callback) { + session.defaultSession.setCertificateVerifyProc(function ({hostname, certificate, verificationResult}, callback) { assert.equal(hostname, '127.0.0.1') assert.equal(certificate.issuerName, 'Intermediate CA') assert.equal(certificate.subjectName, 'localhost') @@ -580,7 +610,8 @@ describe('session module', function () { 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) + assert.equal(verificationResult, 'net::ERR_CERT_AUTHORITY_INVALID') + callback(-2) }) var url = `https://127.0.0.1:${server.address().port}` diff --git a/spec/fixtures/module/function-with-args.js b/spec/fixtures/module/function-with-args.js new file mode 100644 index 000000000000..ed636e5988a2 --- /dev/null +++ b/spec/fixtures/module/function-with-args.js @@ -0,0 +1,3 @@ +module.exports = function (cb) { + return cb.length +}