Fix/ota 831/retry download

src/libaktualizr/http/httpclient.cc

@@ -96,7 +96,7 @@ HttpClient::~HttpClient() {
 }
 
 HttpResponse HttpClient::get(const std::string& url, int64_t maxsize) {
-  CURL* curl_get = curl_easy_duphandle(curl);
+  CURL* curl_get = Utils::curlDupHandleWrapper(curl, pkcs11_key);
 
   // TODO: it is a workaround for an unidentified bug in libcurl. Ideally the bug itself should be fixed.
   if (pkcs11_key) {
@@ -168,27 +168,19 @@ void HttpClient::setCerts(const std::string& ca, CryptoSource ca_source, const s
 }
 
 HttpResponse HttpClient::post(const std::string& url, const Json::Value& data) {
-  curlEasySetoptWrapper(curl, CURLOPT_URL, url.c_str());
-  curlEasySetoptWrapper(curl, CURLOPT_POST, 1);
+  CURL* curl_post = Utils::curlDupHandleWrapper(curl, pkcs11_key);
+  curlEasySetoptWrapper(curl_post, CURLOPT_URL, url.c_str());
+  curlEasySetoptWrapper(curl_post, CURLOPT_POST, 1);
   std::string data_str = Json::FastWriter().write(data);
-  curlEasySetoptWrapper(curl, CURLOPT_POSTFIELDS, data_str.c_str());
+  curlEasySetoptWrapper(curl_post, CURLOPT_POSTFIELDS, data_str.c_str());
   LOG_TRACE << "post request body:" << data;
-  return perform(curl, RETRY_TIMES, HttpInterface::kPostRespLimit);
+  auto result = perform(curl_post, RETRY_TIMES, HttpInterface::kPostRespLimit);
+  curl_easy_cleanup(curl_post);
+  return result;
 }
 
 HttpResponse HttpClient::put(const std::string& url, const Json::Value& data) {
-  CURL* curl_put = curl_easy_duphandle(curl);
-
-  // TODO: it is a workaround for an unidentified bug in libcurl. Ideally the bug itself should be fixed.
-  if (pkcs11_key) {
-    curlEasySetoptWrapper(curl_put, CURLOPT_SSLENGINE, "pkcs11");
-    curlEasySetoptWrapper(curl_put, CURLOPT_SSLKEYTYPE, "ENG");
-  }
-
-  if (pkcs11_cert) {
-    curlEasySetoptWrapper(curl_put, CURLOPT_SSLCERTTYPE, "ENG");
-  }
-
+  CURL* curl_put = Utils::curlDupHandleWrapper(curl, pkcs11_key);
   curlEasySetoptWrapper(curl_put, CURLOPT_URL, url.c_str());
   std::string data_str = Json::FastWriter().write(data);
   curlEasySetoptWrapper(curl_put, CURLOPT_POSTFIELDS, data_str.c_str());
@@ -222,17 +214,7 @@ HttpResponse HttpClient::perform(CURL* curl_handler, int retry_times, int64_t si
 }
 
 HttpResponse HttpClient::download(const std::string& url, curl_write_callback callback, void* userp, size_t from) {
-  CURL* curl_download = curl_easy_duphandle(curl);
-
-  // TODO: it is a workaround for an unidentified bug in libcurl. Ideally the bug itself should be fixed.
-  if (pkcs11_key) {
-    curlEasySetoptWrapper(curl_download, CURLOPT_SSLENGINE, "pkcs11");
-    curlEasySetoptWrapper(curl_download, CURLOPT_SSLKEYTYPE, "ENG");
-  }
-
-  if (pkcs11_cert) {
-    curlEasySetoptWrapper(curl_download, CURLOPT_SSLCERTTYPE, "ENG");
-  }
+  CURL* curl_download = Utils::curlDupHandleWrapper(curl, pkcs11_key);
 
   curlEasySetoptWrapper(curl_download, CURLOPT_URL, url.c_str());
   curlEasySetoptWrapper(curl_download, CURLOPT_HTTPGET, 1L);

src/libaktualizr/primary/reportqueue.cc

@@ -54,7 +54,7 @@ void ReportQueue::flushQueue() {
     // 404 implies the server does not support this feature. Nothing we can
     // do, just move along.
     if (response.isOk() || response.http_status_code == 404) {
-      report_array = Json::arrayValue;
+      report_array.clear();
     }
   }
 }

src/libaktualizr/primary/sotauptaneclient.cc

@@ -726,7 +726,21 @@ std::pair<bool, Uptane::Target> SotaUptaneClient::downloadImage(Uptane::Target t
     report_queue->enqueue(std_::make_unique<EcuDownloadStartedReport>(ecu.first, correlation_id));
   }
 
-  bool success = uptane_fetcher->fetchVerifyTarget(target);
+  bool success = false;
+  int tries = 3;
+  std::chrono::milliseconds wait(500);
+  while ((tries--) != 0) {
+    success = uptane_fetcher->fetchVerifyTarget(target);
+    if (success) {
+      break;
+    } else if (tries != 0) {
+      std::this_thread::sleep_for(wait);
+      wait *= 2;
+    }
+  }
+  if (!success) {
+    LOG_ERROR << "Download unsuccessful after " << tries << " attempts.";
+  }
 
   // send this asynchronously before `sendEvent`, so that the report timestamp
   // would not be delayed by callbacks on events

src/libaktualizr/primary/sotauptaneclient.h

@@ -71,6 +71,7 @@ class SotaUptaneClient {
   FRIEND_TEST(UptaneCI, OneCycleUpdate);
   FRIEND_TEST(UptaneCI, CheckKeys);
   FRIEND_TEST(UptaneKey, Check);  // Note hacky name
+  FRIEND_TEST(UptaneNetwork, DownloadFailure);
   FRIEND_TEST(aktualizr_secondary_uptane, credentialsPassing);
   friend class CheckForUpdate;       // for load tests
   friend class ProvisionDeviceTask;  // for load tests

src/libaktualizr/uptane/fetcher.cc

@@ -49,7 +49,7 @@ static size_t DownloadHandler(char* contents, size_t size, size_t nmemb, void* u
     (*(ds->events_channel))(event);
   }
   if (ds->fetcher->isPaused()) {
-    return written_size + 1;  // Abort downloading, because pause is requested.
+    return written_size + 1;  // Abort downloading because pause is requested.
   }
   return written_size;
 }
@@ -126,7 +126,6 @@ bool Fetcher::fetchVerifyTarget(const Target& target) {
             http->download(config.uptane.repo_server + "/targets/" + Utils::urlEncode(target.filename()),
                            DownloadHandler, &ds, ds.downloaded_length);
         if (!response.isOk() && !pause_) {
-          fhandle->wabort();
           if (response.curl_code == CURLE_WRITE_ERROR) {
             throw OversizedTarget(target.filename());
           }

src/libaktualizr/uptane/uptane_network_test.cc

@@ -15,9 +15,11 @@
 #include "http/httpclient.h"
 #include "logging/logging.h"
 #include "primary/initializer.h"
+#include "primary/sotauptaneclient.h"
 #include "storage/invstorage.h"
 #include "test_utils.h"
 #include "uptane/uptanerepository.h"
+#include "uptane_test_common.h"
 #include "utilities/utils.h"
 
 Config conf("tests/config/basic.toml");
@@ -113,6 +115,30 @@ TEST(UptaneNetwork, no_errors_sqlite) {
   EXPECT_TRUE(doTestInit(StorageType::kSqlite, "noerrors", "noerrors"));
 }
 
+TEST(UptaneNetwork, DownloadFailure) {
+  TemporaryDirectory temp_dir;
+  conf.storage.path = temp_dir.Path();
+  conf.provision.expiry_days = "download_failure";
+  conf.provision.primary_ecu_serial = "download_failure";
+  conf.provision.primary_ecu_hardware_id = "hardware_id";
+
+  auto storage = INvStorage::newStorage(conf.storage);
+  auto http = std::make_shared<HttpClient>();
+  auto up = SotaUptaneClient::newTestClient(conf, storage, http);
+  EXPECT_NO_THROW(up->initialize());
+
+  Json::Value ot_json;
+  ot_json["custom"]["ecuIdentifiers"][conf.provision.primary_ecu_serial]["hardwareId"] =
+      conf.provision.primary_ecu_hardware_id;
+  ot_json["custom"]["targetFormat"] = "binary";
+  ot_json["length"] = 2048;
+  ot_json["hashes"]["sha256"] = "d03b1a2081755f3a5429854cc3e700f8cbf125db2bd77098ae79a7d783256a7d";
+  Uptane::Target package_to_install{conf.provision.primary_ecu_serial, ot_json};
+
+  std::pair<bool, Uptane::Target> result = up->downloadImage(package_to_install);
+  EXPECT_TRUE(result.first);
+}
+
 #ifndef __NO_MAIN__
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);

src/libaktualizr/uptane/uptane_test.cc

@@ -30,18 +30,6 @@
 #endif
 #endif
 
-std::vector<Uptane::Target> makePackage(const std::string &serial, const std::string &hw_id) {
-  std::vector<Uptane::Target> packages_to_install;
-  Json::Value ot_json;
-  ot_json["custom"]["ecuIdentifiers"][serial]["hardwareId"] = hw_id;
-  ot_json["custom"]["targetFormat"] = "OSTREE";
-  ot_json["length"] = 0;
-  ot_json["hashes"]["sha256"] = serial;
-  packages_to_install.emplace_back(serial, ot_json);
-  return packages_to_install;
-}
-
-/* Validate a TUF root. */
 TEST(Uptane, Verify) {
   TemporaryDirectory temp_dir;
   auto http = std::make_shared<HttpFake>(temp_dir.Path());
@@ -261,7 +249,7 @@ TEST(Uptane, InstallFake) {
   auto storage = INvStorage::newStorage(conf.storage);
   auto up = SotaUptaneClient::newTestClient(conf, storage, http);
   EXPECT_NO_THROW(up->initialize());
-  std::vector<Uptane::Target> packages_to_install = makePackage("testecuserial", "testecuhwid");
+  std::vector<Uptane::Target> packages_to_install = UptaneTestCommon::makePackage("testecuserial", "testecuhwid");
   up->uptaneInstall(packages_to_install);
 
   // Make sure operation_result and filepath were correctly written and formatted.
@@ -467,7 +455,7 @@ TEST(Uptane, ProvisionOnServer) {
   // Testing downloading is not strictly necessary here and will not work due to
   // the metadata concerns anyway.
   std::vector<Uptane::Target> packages_to_install =
-      makePackage(config.provision.primary_ecu_serial, config.provision.primary_ecu_hardware_id);
+      UptaneTestCommon::makePackage(config.provision.primary_ecu_serial, config.provision.primary_ecu_hardware_id);
   DownloadResult result = up->downloadImages(packages_to_install);
   EXPECT_EQ(result.updates.size(), 0);
   EXPECT_EQ(result.status, DownloadStatus::kError);

src/libaktualizr/utilities/utils.cc

@@ -683,6 +683,19 @@ std::string Utils::urlEncode(const std::string &input) {
   return res;
 }
 
+CURL *Utils::curlDupHandleWrapper(CURL *const curl_in, const bool using_pkcs11) {
+  CURL *curl = curl_easy_duphandle(curl_in);
+
+  // This is a workaround for a bug in curl. It has been fixed in
+  // 75a845d8cfa71688d59d43788c35829b25b6d6af (curl 7.61.1), but that is not
+  // the default in most distributions yet, so we will continue to use the
+  // workaround.
+  if (using_pkcs11) {
+    curlEasySetoptWrapper(curl, CURLOPT_SSLENGINE, "pkcs11");
+  }
+  return curl;
+}
+
 class SafeTempRoot {
  public:
   SafeTempRoot(const SafeTempRoot &) = delete;

src/libaktualizr/utilities/utils.h

@@ -45,6 +45,7 @@ struct Utils {
   static std::vector<boost::filesystem::path> glob(const std::string &pat);
   static void createDirectories(const boost::filesystem::path &path, mode_t mode);
   static std::string urlEncode(const std::string &input);
+  static CURL *curlDupHandleWrapper(CURL *curl_in, bool using_pkcs11);
 };
 
 /**

tests/fake_http_server/fake_api_server.py

@@ -22,10 +22,7 @@ def do_POST(self):
             self.wfile.write(token)
     def do_HEAD(self):
         self.send_response(200)
-        self.end_headers()       
-
-
-
+        self.end_headers()
 
 
 class ReUseHTTPServer(HTTPServer):
@@ -40,6 +37,6 @@ def server_bind(self):
 try:
     httpd.serve_forever()
 except KeyboardInterrupt as k:
-    print("fake_uptane_server.py exiting...")
+    print("fake_api_server.py exiting...")
     pass
 

tests/fake_http_server/fake_uptane_server.py

@@ -8,7 +8,24 @@
 
 class Handler(BaseHTTPRequestHandler):
     def do_GET(self):
-        pass
+        if self.path.startswith("/repo/targets/download_failure"):
+            if "Range" in self.headers: #restored connection
+                r = self.headers["Range"]
+                r_from = int(r.split("=")[1].split("-")[0])
+                if r_from == 1024:
+                    self.send_response(206)
+                    self.send_header('Content-Range', 'bytes %d-%d/%d' %(r_from, 2047, 2048))
+                    self.send_header('Content-Length', 1024)
+                else:
+                    self.send_response(503) #Error, we should ask to restore from 1024 byte
+                    self.end_headers()
+                    return
+            else: # First time drop connection
+                self.send_response(200)
+                self.send_header('Content-Length', 2048)
+            self.end_headers()
+            for i in range(1024):
+              self.wfile.write(b'@')
 
     def do_POST(self):
         length = int(self.headers.get('content-length'))
@@ -31,22 +48,28 @@ def do_devicesRegister(self, data):
         elif data["ttl"].startswith("status_"):
             self.send_response(int(data["ttl"][7:]))
             self.end_headers()
-        elif data["ttl"] == "noerrors":
+        elif data["ttl"] == "noerrors" or data["ttl"] == "download_failure":
             self.send_response(200)
             self.end_headers()
             f = open('tests/test_data/cred.p12', 'rb')
             self.wfile.write(f.read())
+        else:
+            self.send_response(404)
+            self.end_headers()
 
     def do_ecuRegister(self, data):
         if data["primary_ecu_serial"] == "drop_request":
             return
         elif data["primary_ecu_serial"].startswith("status_"):
             self.send_response(int(data["primary_ecu_serial"][7:]))
             self.end_headers()
-        elif data["primary_ecu_serial"] == "noerrors":
+        elif data["primary_ecu_serial"] == "noerrors" or data["primary_ecu_serial"] == "download_failure":
             self.send_response(200)
             self.end_headers()
             self.wfile.write(b"{}")
+        else:
+            self.send_response(404)
+            self.end_headers()
 
 
 class ReUseHTTPServer(HTTPServer):

tests/uptane_test_common.h

@@ -2,9 +2,13 @@
 #define UPTANE_TEST_COMMON_H_
 
 #include <string>
+#include <vector>
+
+#include "json/json.h"
 
 #include "config/config.h"
 #include "uptane/secondaryconfig.h"
+#include "uptane/tuf.h"
 #include "utilities/utils.h"
 
 struct UptaneTestCommon {
@@ -24,6 +28,17 @@ struct UptaneTestCommon {
     config.uptane.secondary_configs.push_back(ecu_config);
     return ecu_config;
   }
+
+  static std::vector<Uptane::Target> makePackage(const std::string &serial, const std::string &hw_id) {
+    std::vector<Uptane::Target> packages_to_install;
+    Json::Value ot_json;
+    ot_json["custom"]["ecuIdentifiers"][serial]["hardwareId"] = hw_id;
+    ot_json["custom"]["targetFormat"] = "OSTREE";
+    ot_json["length"] = 0;
+    ot_json["hashes"]["sha256"] = serial;
+    packages_to_install.emplace_back(serial, ot_json);
+    return packages_to_install;
+  }
 };
 
 #endif // UPTANE_TEST_COMMON_H_