commit 33e91e78bfe98c063b0c3b6d590976e275685686
author Alex Deymo <deymo@google.com> Tue Dec 01 18:26:08 2015 -0300
committer Alex Deymo <deymo@google.com> Fri Dec 04 19:27:34 2015 -0800
tree 1912f6dbc00b546af84865eaf8da1e1b0b5df9db
parent 2b4268c8f89d115b0895b3f0172f42c11d9b1d7c

Fix certificate checker callback lifetime.

OpenSSL's SSL_CTX_set_verify() function allows us to set a callback
called after certificate validation but doesn't provide a way to pass
private data to this callback. CL:183832 was passing the pointer to the
CertificateChecker instance using a global pointer, nevertheless the
lifetime of this pointer was wrong since libcurl can trigger this
callback asynchronously when the SSL certificates are downloaded.

This patch converts the CertificateChecker into a singleton class and
uses the same trick previously used to pass the ServerToCheck value
using different callbacks.

Bug: 25818567
Test: Run an update on edison-userdebug; FEATURES=test emerge-link update_engine

Change-Id: I84cdb2f8c5ac86d1463634e73e867f213f7a2f5a

update_attempter.cc

diff --git a/update_attempter.cc b/update_attempter.cc
index 93fda66..4406149 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -119,15 +119,20 @@
 
 UpdateAttempter::UpdateAttempter(
     SystemState* system_state,
+    CertificateChecker* cert_checker,
     LibCrosProxy* libcros_proxy,
     org::chromium::debugdProxyInterface* debugd_proxy)
     : processor_(new ActionProcessor()),
       system_state_(system_state),
+      cert_checker_(cert_checker),
       chrome_proxy_resolver_(libcros_proxy),
       debugd_proxy_(debugd_proxy) {
 }
 
 UpdateAttempter::~UpdateAttempter() {
+  // CertificateChecker might not be initialized in unittests.
+  if (cert_checker_)
+    cert_checker_->SetObserver(nullptr);
   CleanupCpuSharesManagement();
   // Release ourselves as the ActionProcessor's delegate to prevent
   // re-scheduling the updates due to the processing stopped.
@@ -141,6 +146,9 @@
   prefs_ = system_state_->prefs();
   omaha_request_params_ = system_state_->request_params();
 
+  if (cert_checker_)
+    cert_checker_->SetObserver(this);
+
   // In case of update_engine restart without a reboot we need to restore the
   // reboot needed state.
   if (GetBootTimeAtUpdate(nullptr))
@@ -578,14 +586,9 @@
   processor_->set_delegate(this);
 
   // Actions:
-  std::unique_ptr<CertificateChecker> update_check_checker(
-      new CertificateChecker(prefs_, &openssl_wrapper_,
-                             ServerToCheck::kUpdate));
-  update_check_checker->SetObserver(this);
   std::unique_ptr<LibcurlHttpFetcher> update_check_fetcher(
-      new LibcurlHttpFetcher(GetProxyResolver(),
-                             system_state_->hardware(),
-                             std::move(update_check_checker)));
+      new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware()));
+  update_check_fetcher->set_server_to_check(ServerToCheck::kUpdate);
   // Try harder to connect to the network, esp when not interactive.
   // See comment in libcurl_http_fetcher.cc.
   update_check_fetcher->set_no_network_max_retries(interactive ? 1 : 3);
@@ -608,14 +611,10 @@
                                  GetProxyResolver(),
                                  system_state_->hardware())),
                              false));
-  std::unique_ptr<CertificateChecker> download_checker(
-      new CertificateChecker(prefs_, &openssl_wrapper_,
-                             ServerToCheck::kDownload));
-  download_checker->SetObserver(this);
+
   LibcurlHttpFetcher* download_fetcher =
-      new LibcurlHttpFetcher(GetProxyResolver(),
-                             system_state_->hardware(),
-                             std::move(download_checker));
+      new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware());
+  download_fetcher->set_server_to_check(ServerToCheck::kDownload);
   shared_ptr<DownloadAction> download_action(
       new DownloadAction(prefs_,
                          system_state_,