Reverify downloaded target's hash before installation.

Tested at a low level with the fake package manager and at a slightly
higher level with OSTree.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>

src/libaktualizr/package_manager/CMakeLists.txt

@@ -38,7 +38,7 @@ if(BUILD_OSTREE)
                       ${PROJECT_SOURCE_DIR} ${PROJECT_BINARY_DIR})
     add_dependencies(build_tests make_ostree_sysroot)
 
-    add_aktualizr_test(NAME ostree SOURCES ostreemanager_test.cc PROJECT_WORKING_DIRECTORY NO_VALGRIND
+    add_aktualizr_test(NAME ostreemanager SOURCES ostreemanager_test.cc PROJECT_WORKING_DIRECTORY NO_VALGRIND
                        ARGS ${PROJECT_BINARY_DIR}/ostree_repo)
 
     if(BUILD_DOCKERAPP)

src/libaktualizr/package_manager/ostreemanager.cc

@@ -65,7 +65,7 @@ data::InstallationResult OstreeManager::pull(const boost::filesystem::path &sysr
                                              const std::string &ostree_server, const KeyManager &keys,
                                              const Uptane::Target &target, const api::FlowControlToken *token,
                                              OstreeProgressCb progress_cb) {
-  std::string refhash = target.sha256Hash();
+  const std::string refhash = target.sha256Hash();
   const char *const commit_ids[] = {refhash.c_str()};
   GError *error = nullptr;
   GVariantBuilder builder;
@@ -239,6 +239,35 @@ bool OstreeManager::fetchTarget(const Uptane::Target &target, Uptane::Fetcher &f
   return OstreeManager::pull(config.sysroot, config.ostree_server, keys, target, token, progress_cb).success;
 }
 
+bool OstreeManager::verifyTarget(const Uptane::Target &target) const {
+  const std::string refhash = target.sha256Hash();
+  GError *error = nullptr;
+
+  GObjectUniquePtr<OstreeSysroot> sysroot = OstreeManager::LoadSysroot(config.sysroot);
+  GObjectUniquePtr<OstreeRepo> repo = LoadRepo(sysroot.get(), &error);
+  if (error != nullptr) {
+    LOG_ERROR << "Could not get OSTree repo";
+    g_error_free(error);
+    return false;
+  }
+
+  GHashTable *ref_list = nullptr;
+  if (ostree_repo_list_commit_objects_starting_with(repo.get(), refhash.c_str(), &ref_list, nullptr, &error) != 0) {
+    guint length = g_hash_table_size(ref_list);
+    g_hash_table_destroy(ref_list);  // OSTree creates the table with destroy notifiers, so no memory leaks expected
+    // should never be greater than 1, but use >= for robustness
+    if (length >= 1) {
+      return true;
+    }
+  }
+  if (error != nullptr) {
+    g_error_free(error);
+    error = nullptr;
+  }
+
+  return false;
+}
+
 Json::Value OstreeManager::getInstalledPackages() const {
   std::string packages_str = Utils::readFile(config.packages_file);
   std::vector<std::string> package_lines;

src/libaktualizr/package_manager/ostreemanager.h

@@ -51,6 +51,7 @@ class OstreeManager : public PackageManagerInterface {
   data::InstallationResult finalizeInstall(const Uptane::Target &target) const override;
   bool fetchTarget(const Uptane::Target &target, Uptane::Fetcher &fetcher, const KeyManager &keys,
                    FetcherProgressCb progress_cb, const api::FlowControlToken *token) override;
+  bool verifyTarget(const Uptane::Target &target) const override;
 
   GObjectUniquePtr<OstreeDeployment> getStagedDeployment() const;
   static GObjectUniquePtr<OstreeSysroot> LoadSysroot(const boost::filesystem::path &path);

src/libaktualizr/package_manager/packagemanagerfake_test.cc

@@ -14,6 +14,43 @@
 #include "utilities/types.h"
 #include "utilities/utils.h"
 
+/*
+ * Verify a stored target.
+ */
+TEST(PackageManagerFake, Verify) {
+  TemporaryDirectory temp_dir;
+  Config config;
+  config.pacman.type = PackageManager::kNone;
+  config.storage.path = temp_dir.Path();
+  std::shared_ptr<INvStorage> storage = INvStorage::newStorage(config.storage);
+
+  Uptane::EcuMap primary_ecu{{Uptane::EcuSerial("primary"), Uptane::HardwareIdentifier("primary_hw")}};
+  Uptane::Target target_good("some-pkg", primary_ecu, {Uptane::Hash(Uptane::Hash::Type::kSha256, "hash-good")}, 4, "");
+  Uptane::Target target_bad("some-pkg", primary_ecu, {Uptane::Hash(Uptane::Hash::Type::kSha256, "hash-bad")}, 4, "");
+
+  PackageManagerFake fakepm(config.pacman, storage, nullptr, nullptr);
+  EXPECT_FALSE(fakepm.verifyTarget(target_good));
+
+  // Write the target with a different hash.
+  {
+    std::unique_ptr<StorageTargetWHandle> fhandle = storage->allocateTargetFile(false, target_bad);
+    const uint8_t wb[] = "bad ";
+    fhandle->wfeed(wb, 4);
+    fhandle->wcommit();
+  }
+  EXPECT_FALSE(fakepm.verifyTarget(target_good));
+
+  // Write the target with the expected hash.
+  storage->removeTargetFile(target_good.filename());
+  {
+    std::unique_ptr<StorageTargetWHandle> fhandle = storage->allocateTargetFile(false, target_good);
+    const uint8_t wb[] = "good";
+    fhandle->wfeed(wb, 4);
+    fhandle->wcommit();
+  }
+  EXPECT_TRUE(fakepm.verifyTarget(target_good));
+}
+
 TEST(PackageManagerFake, FinalizeAfterReboot) {
   TemporaryDirectory temp_dir;
   Config config;

src/libaktualizr/package_manager/packagemanagerinterface.cc

@@ -89,7 +89,7 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
     }
     auto target_exists = storage_->checkTargetFile(target);
     if (target_exists && (*target_exists).first == target.length()) {
-      LOG_INFO << "Image already downloaded skipping download";
+      LOG_INFO << "Image already downloaded; skipping download";
       return true;
     }
     if (target.length() == 0) {
@@ -144,3 +144,8 @@ bool PackageManagerInterface::fetchTarget(const Uptane::Target& target, Uptane::
   }
   return result;
 }
+
+bool PackageManagerInterface::verifyTarget(const Uptane::Target& target) const {
+  auto target_exists = storage_->checkTargetFile(target);
+  return !!target_exists;
+}

src/libaktualizr/package_manager/packagemanagerinterface.h

@@ -33,6 +33,7 @@ class PackageManagerInterface {
   virtual bool imageUpdated() = 0;
   virtual bool fetchTarget(const Uptane::Target& target, Uptane::Fetcher& fetcher, const KeyManager& keys,
                            FetcherProgressCb progress_cb, const api::FlowControlToken* token);
+  virtual bool verifyTarget(const Uptane::Target& target) const;
 
   // only returns the version
   Json::Value getManifest(const Uptane::EcuSerial& ecu_serial) const {

src/libaktualizr/primary/aktualizr_fullostree_test.cc

@@ -59,15 +59,18 @@ TEST(Aktualizr, FullOstreeUpdate) {
   LOG_INFO << "conf: " << conf;
 
   {
-    Aktualizr aktualizr(conf);
-
+    UptaneTestCommon::TestAktualizr aktualizr(conf);
     aktualizr.Initialize();
 
     result::UpdateCheck update_result = aktualizr.CheckUpdates().get();
     ASSERT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+    // Verify the target has not yet been downloaded.
+    EXPECT_FALSE(aktualizr.uptane_client()->package_manager_->verifyTarget(update_result.updates[0]));
 
     result::Download download_result = aktualizr.Download(update_result.updates).get();
     EXPECT_EQ(download_result.status, result::DownloadStatus::kSuccess);
+    // Verify the target has been downloaded.
+    EXPECT_TRUE(aktualizr.uptane_client()->package_manager_->verifyTarget(update_result.updates[0]));
 
     result::Install install_result = aktualizr.Install(update_result.updates).get();
     EXPECT_EQ(install_result.ecu_reports.size(), 1);
@@ -90,6 +93,14 @@ TEST(Aktualizr, FullOstreeUpdate) {
     // check new version
     const auto target = aktualizr.uptane_client()->package_manager_->getCurrent();
     EXPECT_EQ(target.sha256Hash(), new_rev);
+    // Verify again.
+    EXPECT_TRUE(aktualizr.uptane_client()->package_manager_->verifyTarget(target));
+
+    // Verify a bogus target is not present.
+    Uptane::EcuMap primary_ecu{{Uptane::EcuSerial(conf.provision.primary_ecu_serial),
+                                Uptane::HardwareIdentifier(conf.provision.primary_ecu_hardware_id)}};
+    Uptane::Target target_bad("some-pkg", primary_ecu, {Uptane::Hash(Uptane::Hash::Type::kSha256, "hash-bad")}, 4, "");
+    EXPECT_FALSE(aktualizr.uptane_client()->package_manager_->verifyTarget(target_bad));
   }
 }
 
@@ -101,7 +112,7 @@ int main(int argc, char **argv) {
 
   if (argc != 3) {
     std::cerr << "Error: " << argv[0] << " requires the path to the uptane-generator utility "
-              << "and an OStree sysroot\n";
+              << "and an OSTree sysroot\n";
     return EXIT_FAILURE;
   }
   uptane_generator_path = argv[1];

src/libaktualizr/primary/sotauptaneclient.cc

@@ -796,28 +796,41 @@ result::UpdateStatus SotaUptaneClient::checkUpdatesOffline(const std::vector<Upt
 
 result::Install SotaUptaneClient::uptaneInstall(const std::vector<Uptane::Target> &updates) {
   result::Install result;
+  const std::string &correlation_id = director_repo.getCorrelationId();
 
   // clear all old results first
   storage->clearInstallationResults();
 
+  // Recheck the Uptane metadata and make sure the requested updates are
+  // consistent with the stored metadata.
   result::UpdateStatus update_status = checkUpdatesOffline(updates);
   if (update_status != result::UpdateStatus::kUpdatesAvailable) {
     if (update_status == result::UpdateStatus::kNoUpdatesAvailable) {
       result.dev_report = {false, data::ResultCode::Numeric::kAlreadyProcessed, ""};
     } else {
       result.dev_report = {false, data::ResultCode::Numeric::kInternalError, ""};
     }
+    storage->storeDeviceInstallationResult(result.dev_report, "Stored Uptane metadata is invalid", correlation_id);
     sendEvent<event::AllInstallsComplete>(result);
     return result;
   }
 
+  // Recheck the downloaded update hashes.
+  for (const auto &update : updates) {
+    if (!package_manager_->verifyTarget(update)) {
+      result.dev_report = {false, data::ResultCode::Numeric::kInternalError, ""};
+      storage->storeDeviceInstallationResult(result.dev_report, "Downloaded target's hash is invalid", correlation_id);
+      sendEvent<event::AllInstallsComplete>(result);
+      return result;
+    }
+  }
+
   // Uptane step 5 (send time to all ECUs) is not implemented yet.
   Uptane::EcuSerial primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
   std::vector<Uptane::Target> primary_updates = findForEcu(updates, primary_ecu_serial);
   //   6 - send metadata to all the ECUs
   sendMetadataToEcus(updates);
 
-  const std::string &correlation_id = director_repo.getCorrelationId();
   //   7 - send images to ECUs (deploy for OSTree)
   if (primary_updates.size() != 0u) {
     // assuming one OSTree OS per primary => there can be only one OSTree update