Fix/bad metadata handling

src/aktualizr_primary/main.cc

@@ -92,6 +92,12 @@ void processEvent(const std::shared_ptr<event::BaseEvent> &event) {
     // Do nothing; libaktualizr already logs it.
   } else if (event->variant == "UpdateCheckComplete") {
     // Do nothing; libaktualizr already logs it.
+  } else if (event->variant == "AllDownloadsComplete") {
+    const auto downloads_complete = dynamic_cast<event::AllDownloadsComplete *>(event.get());
+    LOG_INFO << "got " << event->variant << " event with status: " << downloads_complete->result.status;
+  } else if (event->variant == "AllInstallsComplete") {
+    const auto installs_complete = dynamic_cast<event::AllInstallsComplete *>(event.get());
+    LOG_INFO << "got " << event->variant << " event with status: " << installs_complete->result.dev_report.result_code;
   } else {
     LOG_INFO << "got " << event->variant << " event";
   }

src/libaktualizr/primary/aktualizr.cc

@@ -35,11 +35,19 @@ bool Aktualizr::IsRegistered() const { return storage_->loadEcuRegistered(); }
 bool Aktualizr::UptaneCycle() {
   result::UpdateCheck update_result = CheckUpdates().get();
   if (update_result.updates.empty()) {
+    if (update_result.status == result::UpdateStatus::kError) {
+      // If the metadata verification failed, inform the backend immediately.
+      SendManifest().get();
+    }
     return true;
   }
 
   result::Download download_result = Download(update_result.updates).get();
   if (download_result.status != result::DownloadStatus::kSuccess || download_result.updates.empty()) {
+    if (download_result.status != result::DownloadStatus::kNothingToDownload) {
+      // If the download failed, inform the backend immediately.
+      SendManifest().get();
+    }
     return true;
   }
 

src/libaktualizr/primary/empty_targets_test.cc

@@ -92,6 +92,56 @@ TEST(Aktualizr, EmptyTargets) {
   }
 }
 
+/* Check that Aktualizr switches back to empty targets after failing to verify
+ * the Target matching. Also check that no updates are reported if any are
+ * invalid. */
+TEST(Aktualizr, EmptyTargetsAfterVerification) {
+  TemporaryDirectory temp_dir;
+  TemporaryDirectory meta_dir;
+  auto http = std::make_shared<HttpRejectEmptyCorrId>(temp_dir.Path(), meta_dir.Path() / "repo");
+  Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server);
+  logger_set_threshold(boost::log::trivial::trace);
+
+  // Add two images: a valid one for the Primary and an invalid for the
+  // Secondary. The Primary will get verified first and should succeed.
+  Process uptane_gen(uptane_generator_path.string());
+  uptane_gen.run({"generate", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
+  uptane_gen.run({"image", "--path", meta_dir.PathString(), "--filename", "tests/test_data/firmware.txt",
+                  "--targetname", "firmware.txt", "--hwid", "primary_hw"});
+  uptane_gen.run({"addtarget", "--path", meta_dir.PathString(), "--targetname", "firmware.txt", "--hwid", "primary_hw",
+                  "--serial", "CA:FE:A6:D2:84:9D"});
+  uptane_gen.run({"image", "--path", meta_dir.PathString(), "--filename", "tests/test_data/firmware_name.txt",
+                  "--targetname", "firmware_name.txt", "--hwid", "bad"});
+  uptane_gen.run({"addtarget", "--path", meta_dir.PathString(), "--targetname", "firmware_name.txt", "--hwid",
+                  "secondary_hw", "--serial", "secondary_ecu_serial"});
+  uptane_gen.run({"signtargets", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
+
+  // failing verification
+  auto storage = INvStorage::newStorage(conf.storage);
+  {
+    UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+    aktualizr.Initialize();
+
+    result::UpdateCheck update_result = aktualizr.CheckUpdates().get();
+    EXPECT_EQ(update_result.status, result::UpdateStatus::kError);
+    EXPECT_EQ(update_result.ecus_count, 0);
+    EXPECT_TRUE(update_result.updates.empty());
+  }
+
+  // Backend reacts to failure: no need to install the target anymore
+  uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
+  uptane_gen.run({"signtargets", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
+
+  // check that no update is available
+  {
+    UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+    aktualizr.Initialize();
+
+    result::UpdateCheck update_result = aktualizr.CheckUpdates().get();
+    EXPECT_EQ(update_result.status, result::UpdateStatus::kNoUpdatesAvailable);
+  }
+}
+
 #ifdef FIU_ENABLE
 
 /* Check that Aktualizr switches back to empty targets after failing a

src/libaktualizr/primary/results.h

@@ -88,6 +88,29 @@ enum class DownloadStatus {
   kError,
 };
 
+inline std::ostream& operator<<(std::ostream& os, const DownloadStatus stat) {
+  std::string stat_str;
+  switch (stat) {
+    case DownloadStatus::kSuccess:
+      stat_str = "Success";
+      break;
+    case DownloadStatus::kPartialSuccess:
+      stat_str = "Partial success";
+      break;
+    case DownloadStatus::kNothingToDownload:
+      stat_str = "Nothing to download";
+      break;
+    case DownloadStatus::kError:
+      stat_str = "Error";
+      break;
+    default:
+      stat_str = "unknown";
+      break;
+  }
+  os << '"' << stat_str << '"';
+  return os;
+}
+
 /**
  * Container for information about downloading an update.
  */

src/libaktualizr/primary/sotauptaneclient.cc

@@ -531,6 +531,8 @@ result::Download SotaUptaneClient::downloadImages(const std::vector<Uptane::Targ
     } else {
       result =
           result::Download(downloaded_targets, result::DownloadStatus::kError, "Error rechecking stored metadata.");
+      storeInstallationFailure(
+          data::InstallationResult(data::ResultCode::Numeric::kInternalError, "Error rechecking stored metadata."));
     }
     sendEvent<event::AllDownloadsComplete>(result);
     return result;
@@ -553,13 +555,8 @@ result::Download SotaUptaneClient::downloadImages(const std::vector<Uptane::Targ
       LOG_ERROR << "Only " << downloaded_targets.size() << " of " << targets.size() << " were successfully downloaded.";
       result = result::Download(downloaded_targets, result::DownloadStatus::kPartialSuccess, "");
     }
-    // Store installation report to inform Director of the download failure.
-    const std::string &correlation_id = director_repo.getCorrelationId();
-    data::InstallationResult device_installation_result =
-        data::InstallationResult(data::ResultCode::Numeric::kDownloadFailed, "Target download failed");
-    storage->storeDeviceInstallationResult(device_installation_result, "", correlation_id);
-    // Fix for OTA-2587, listen to backend again after end of install.
-    director_repo.dropTargets(*storage);
+    storeInstallationFailure(
+        data::InstallationResult(data::ResultCode::Numeric::kDownloadFailed, "Target download failed."));
   }
 
   sendEvent<event::AllDownloadsComplete>(result);
@@ -736,22 +733,25 @@ result::UpdateCheck SotaUptaneClient::checkUpdates() {
 
   if (updates.empty()) {
     LOG_DEBUG << "No new updates found in Uptane metadata.";
-    result = result::UpdateCheck(updates, ecus_count, result::UpdateStatus::kNoUpdatesAvailable,
-                                 Utils::parseJSON(director_targets), "");
+    result =
+        result::UpdateCheck({}, 0, result::UpdateStatus::kNoUpdatesAvailable, Utils::parseJSON(director_targets), "");
     return result;
   }
 
-  // For every target in the Director Targets metadata, walk the delegation
-  // tree (if necessary) and find a matching target in the Images repo
-  // metadata.
+  // 5.4.4.2.10.: Verify that Targets metadata from the Director and Image
+  // repositories match. A Primary ECU MUST perform this check on metadata for
+  // all images listed in the Targets metadata file from the Director
+  // repository.
   for (auto &target : updates) {
     auto images_target = findTargetInDelegationTree(target, false);
     if (images_target == nullptr) {
       // TODO: Could also be a missing target or delegation expiration.
       last_exception = Uptane::TargetMismatch(target.filename());
       LOG_ERROR << "No matching target in images targets metadata for " << target;
-      result = result::UpdateCheck(updates, ecus_count, result::UpdateStatus::kError,
-                                   Utils::parseJSON(director_targets), "Target mismatch.");
+      result = result::UpdateCheck({}, 0, result::UpdateStatus::kError, Utils::parseJSON(director_targets),
+                                   "Target mismatch.");
+      storeInstallationFailure(
+          data::InstallationResult(data::ResultCode::Numeric::kVerificationFailed, "Metadata verification failed."));
       return result;
     }
     // If the URL from the Director is unset, but the URL from the Images repo
@@ -1039,6 +1039,15 @@ void SotaUptaneClient::verifySecondaries() {
   storage->storeMisconfiguredEcus(misconfigured_ecus);
 }
 
+void SotaUptaneClient::storeInstallationFailure(const data::InstallationResult &result) {
+  // Store installation report to inform Director of the update failure before
+  // we actually got to the install step.
+  const std::string &correlation_id = director_repo.getCorrelationId();
+  storage->storeDeviceInstallationResult(result, "", correlation_id);
+  // Fix for OTA-2587, listen to backend again after end of install.
+  director_repo.dropTargets(*storage);
+}
+
 void SotaUptaneClient::rotateSecondaryRoot(Uptane::RepositoryType repo, Uptane::SecondaryInterface &secondary) {
   std::string latest_root;
 

src/libaktualizr/primary/sotauptaneclient.h

@@ -132,6 +132,7 @@ class SotaUptaneClient {
   std::vector<result::Install::EcuReport> sendImagesToEcus(const std::vector<Uptane::Target> &targets);
 
   bool putManifestSimple(const Json::Value &custom = Json::nullValue);
+  void storeInstallationFailure(const data::InstallationResult &result);
   bool getNewTargets(std::vector<Uptane::Target> *new_targets, unsigned int *ecus_count = nullptr);
   void rotateSecondaryRoot(Uptane::RepositoryType repo, Uptane::SecondaryInterface &secondary);
   bool updateDirectorMeta();

src/libaktualizr/utilities/types.cc

@@ -62,7 +62,7 @@ Package Package::fromJson(const std::string &json_str) {
 const std::map<data::ResultCode::Numeric, const char *> data::ResultCode::string_repr{
     {ResultCode::Numeric::kOk, "OK"},
     {ResultCode::Numeric::kAlreadyProcessed, "ALREADY_PROCESSED"},
-    {ResultCode::Numeric::kValidationFailed, "VALIDATION_FAILED"},
+    {ResultCode::Numeric::kVerificationFailed, "VERIFICATION_FAILED"},
     {ResultCode::Numeric::kInstallFailed, "INSTALL_FAILED"},
     {ResultCode::Numeric::kDownloadFailed, "DOWNLOAD_FAILED"},
     {ResultCode::Numeric::kInternalError, "INTERNAL_ERROR"},

src/libaktualizr/utilities/types.h

@@ -126,8 +126,8 @@ struct ResultCode {
     kOk = 0,
     /// Operation has already been processed
     kAlreadyProcessed = 1,
-    /// Update image integrity has been compromised
-    kValidationFailed = 3,
+    /// Metadata verification failed
+    kVerificationFailed = 3,
     /// Package installation failed
     kInstallFailed = 4,
     /// Package download failed