OTA-4174: Use the same manifest generation and signing functionality …

…on both Primary and Secondary

Signed-off-by: Mike Sul <[email protected]>

src/libaktualizr/primary/sotauptaneclient.cc

@@ -49,7 +49,7 @@ void SotaUptaneClient::addSecondary(const std::shared_ptr<Uptane::SecondaryInter
 }
 
 bool SotaUptaneClient::isInstalledOnPrimary(const Uptane::Target &target) {
-  if (target.ecus().find(uptane_manifest.getPrimaryEcuSerial()) != target.ecus().end()) {
+  if (target.ecus().find(primaryEcuSerial()) != target.ecus().end()) {
     return target.MatchTarget(package_manager_->getCurrent());
   }
   return false;
@@ -86,7 +86,7 @@ void SotaUptaneClient::finalizeAfterReboot() {
   std::vector<Uptane::Target> updates;
   unsigned int ecus_count = 0;
   if (uptaneOfflineIteration(&updates, &ecus_count)) {
-    const Uptane::EcuSerial &ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+    const Uptane::EcuSerial &ecu_serial = primaryEcuSerial();
 
     std::vector<Uptane::Target> installed_versions;
     boost::optional<Uptane::Target> pending_target;
@@ -124,7 +124,7 @@ void SotaUptaneClient::finalizeAfterReboot() {
 
 data::InstallationResult SotaUptaneClient::PackageInstallSetResult(const Uptane::Target &target) {
   data::InstallationResult result;
-  Uptane::EcuSerial ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  Uptane::EcuSerial ecu_serial = primaryEcuSerial();
 
   // This is to recover more gracefully if the install process was interrupted
   // but ends up booting the new version anyway (e.g: ostree finished
@@ -195,36 +195,32 @@ void SotaUptaneClient::reportAktualizrConfiguration() {
 
 Json::Value SotaUptaneClient::AssembleManifest() {
   Json::Value manifest;  // signed top-level
-  Uptane::EcuSerial primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  Uptane::EcuSerial primary_ecu_serial = primaryEcuSerial();
   manifest["primary_ecu_serial"] = primary_ecu_serial.ToString();
 
   // first part: report current version/state of all ecus
   Json::Value version_manifest;
 
-  Json::Value primary_ecu_version = package_manager_->getManifest(primary_ecu_serial);
+  Json::Value primary_manifest = uptane_manifest->assembleManifest(package_manager_->getCurrent());
   std::vector<std::pair<Uptane::EcuSerial, int64_t>> ecu_cnt;
+  std::string report_counter;
   if (!storage->loadEcuReportCounter(&ecu_cnt) || (ecu_cnt.size() == 0)) {
     LOG_ERROR << "No ECU version report counter, please check the database!";
+    // TODO: consider not sending manifest at all in this case, or maybe retry
   } else {
-    primary_ecu_version["report_counter"] = std::to_string(ecu_cnt[0].second + 1);
+    report_counter = std::to_string(ecu_cnt[0].second + 1);
     storage->saveEcuReportCounter(ecu_cnt[0].first, ecu_cnt[0].second + 1);
   }
-  version_manifest[primary_ecu_serial.ToString()] = uptane_manifest.signManifest(primary_ecu_version);
+  version_manifest[primary_ecu_serial.ToString()] = uptane_manifest->sign(primary_manifest, report_counter);
 
   for (auto it = secondaries.begin(); it != secondaries.end(); it++) {
-    Json::Value secmanifest = it->second->getManifest();
-    if (secmanifest.isMember("signatures") && secmanifest.isMember("signed")) {
-      const auto public_key = it->second->getPublicKey();
-      const std::string canonical = Utils::jsonToCanonicalStr(secmanifest["signed"]);
-      const bool verified = public_key.VerifySignature(secmanifest["signatures"][0]["sig"].asString(), canonical);
-
-      if (verified) {
-        version_manifest[it->first.ToString()] = secmanifest;
-      } else {
-        LOG_ERROR << "Secondary manifest verification failed, manifest: " << secmanifest;
-      }
+    Uptane::Manifest secmanifest = it->second->getManifest();
+
+    if (secmanifest.verifySignature(it->second->getPublicKey())) {
+      version_manifest[it->first.ToString()] = secmanifest;
     } else {
-      LOG_ERROR << "Secondary manifest is corrupted or not signed, manifest: " << secmanifest;
+      // TODO: send a corresponding event/report in this case, https://saeljira.it.here.com/browse/OTA-4305
+      LOG_ERROR << "Secondary manifest is corrupted or not signed, or signature is invalid manifest: " << secmanifest;
     }
   }
   manifest["ecu_version_manifests"] = version_manifest;
@@ -268,8 +264,8 @@ bool SotaUptaneClient::hasPendingUpdates() const { return storage->hasPendingIns
 
 void SotaUptaneClient::initialize() {
   LOG_DEBUG << "Checking if device is provisioned...";
-  KeyManager keys(storage, config.keymanagerConfig());
-  Initializer initializer(config.provision, storage, http, keys, secondaries);
+  auto keys = std::make_shared<KeyManager>(storage, config.keymanagerConfig());
+  Initializer initializer(config.provision, storage, http, *keys, secondaries);
 
   if (!initializer.isSuccessful()) {
     throw std::runtime_error("Fatal error during provisioning or ECU device registration.");
@@ -280,7 +276,8 @@ void SotaUptaneClient::initialize() {
     throw std::runtime_error("Unable to load ECU serials after device registration.");
   }
 
-  uptane_manifest.setPrimaryEcuSerialHwId(serials[0]);
+  uptane_manifest = std::make_shared<Uptane::ManifestIssuer>(keys, serials[0].first);
+  primary_ecu_serial_ = serials[0].first;
   hw_ids.insert(serials[0]);
 
   verifySecondaries();
@@ -395,7 +392,7 @@ void SotaUptaneClient::computeDeviceInstallationResult(data::InstallationResult
 
 bool SotaUptaneClient::getNewTargets(std::vector<Uptane::Target> *new_targets, unsigned int *ecus_count) {
   std::vector<Uptane::Target> targets = director_repo.getTargets().targets;
-  Uptane::EcuSerial primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  Uptane::EcuSerial primary_ecu_serial = primaryEcuSerial();
   if (ecus_count != nullptr) {
     *ecus_count = 0;
   }
@@ -590,7 +587,7 @@ std::pair<bool, Uptane::Target> SotaUptaneClient::downloadImage(const Uptane::Ta
   };
 
   bool success = false;
-  const Uptane::EcuSerial &primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  const Uptane::EcuSerial &primary_ecu_serial = primaryEcuSerial();
 
   if (target.IsForEcu(primary_ecu_serial) || !target.IsOstree()) {
     // TODO: download should be the logical ECU and packman specific
@@ -852,7 +849,7 @@ result::Install SotaUptaneClient::uptaneInstall(const std::vector<Uptane::Target
     return result;
   }
 
-  Uptane::EcuSerial primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  Uptane::EcuSerial primary_ecu_serial = primaryEcuSerial();
   // Recheck the downloaded update hashes.
   for (const auto &update : updates) {
     if (update.IsForEcu(primary_ecu_serial) || !update.IsOstree()) {
@@ -911,12 +908,11 @@ result::Install SotaUptaneClient::uptaneInstall(const std::vector<Uptane::Target
       storage->saveEcuInstallationResult(primary_ecu_serial, install_res);
       // TODO: distinguish this case from regular failure for local and remote
       // event reporting
-      report_queue->enqueue(std_::make_unique<EcuInstallationCompletedReport>(uptane_manifest.getPrimaryEcuSerial(),
-                                                                              correlation_id, false));
-      sendEvent<event::InstallTargetComplete>(uptane_manifest.getPrimaryEcuSerial(), false);
+      report_queue->enqueue(
+          std_::make_unique<EcuInstallationCompletedReport>(primaryEcuSerial(), correlation_id, false));
+      sendEvent<event::InstallTargetComplete>(primaryEcuSerial(), false);
     }
-    result.ecu_reports.emplace(result.ecu_reports.begin(), primary_update, uptane_manifest.getPrimaryEcuSerial(),
-                               install_res);
+    result.ecu_reports.emplace(result.ecu_reports.begin(), primary_update, primaryEcuSerial(), install_res);
     // TODO: other updates for primary
   } else {
     LOG_INFO << "No update to install on primary";
@@ -988,7 +984,7 @@ bool SotaUptaneClient::putManifestSimple(const Json::Value &custom) {
   if (custom != Json::nullValue) {
     manifest["custom"] = custom;
   }
-  auto signed_manifest = uptane_manifest.signManifest(manifest);
+  auto signed_manifest = uptane_manifest->sign(manifest);
   HttpResponse response = http->put(config.uptane.director_server + "/manifest", signed_manifest);
   if (response.isOk()) {
     if (!connected) {
@@ -1022,13 +1018,12 @@ void SotaUptaneClient::verifySecondaries() {
 
   std::vector<MisconfiguredEcu> misconfigured_ecus;
   std::vector<bool> found(serials.size(), false);
-  SerialCompare primary_comp(uptane_manifest.getPrimaryEcuSerial());
+  SerialCompare primary_comp(primaryEcuSerial());
   EcuSerials::const_iterator store_it;
   store_it = std::find_if(serials.cbegin(), serials.cend(), primary_comp);
   if (store_it == serials.cend()) {
-    LOG_ERROR << "Primary ECU serial " << uptane_manifest.getPrimaryEcuSerial() << " not found in storage!";
-    misconfigured_ecus.emplace_back(uptane_manifest.getPrimaryEcuSerial(), Uptane::HardwareIdentifier(""),
-                                    EcuState::kOld);
+    LOG_ERROR << "Primary ECU serial " << primaryEcuSerial() << " not found in storage!";
+    misconfigured_ecus.emplace_back(primaryEcuSerial(), Uptane::HardwareIdentifier(""), EcuState::kOld);
   } else {
     found[static_cast<size_t>(std::distance(serials.cbegin(), store_it))] = true;
   }
@@ -1196,7 +1191,7 @@ std::vector<result::Install::EcuReport> SotaUptaneClient::sendImagesToEcus(const
   std::vector<result::Install::EcuReport> reports;
   std::vector<std::pair<result::Install::EcuReport, std::future<data::ResultCode::Numeric>>> firmwareFutures;
 
-  const Uptane::EcuSerial &primary_ecu_serial = uptane_manifest.getPrimaryEcuSerial();
+  const Uptane::EcuSerial &primary_ecu_serial = primaryEcuSerial();
   // target images should already have been downloaded to metadata_path/targets/
   for (auto targets_it = targets.cbegin(); targets_it != targets.cend(); ++targets_it) {
     for (auto ecus_it = targets_it->ecus().cbegin(); ecus_it != targets_it->ecus().cend(); ++ecus_it) {
@@ -1288,7 +1283,7 @@ void SotaUptaneClient::checkAndUpdatePendingSecondaries() {
   storage->getPendingEcus(&pending_ecus);
 
   for (const auto &pending_ecu : pending_ecus) {
-    if (uptane_manifest.getPrimaryEcuSerial() == pending_ecu.first) {
+    if (primaryEcuSerial() == pending_ecu.first) {
       continue;
     }
     auto &sec = secondaries[pending_ecu.first];

src/libaktualizr/primary/sotauptaneclient.h

@@ -30,16 +30,16 @@
 
 class SotaUptaneClient {
  public:
-  SotaUptaneClient(Config &config_in, const std::shared_ptr<INvStorage> &storage_in,
-                   std::shared_ptr<HttpInterface> http_in, std::shared_ptr<event::Channel> events_channel_in)
+  SotaUptaneClient(Config &config_in, std::shared_ptr<INvStorage> storage_in, std::shared_ptr<HttpInterface> http_in,
+                   std::shared_ptr<event::Channel> events_channel_in)
       : config(config_in),
-        uptane_manifest(config, storage_in),
-        storage(storage_in),
+        storage(std::move(storage_in)),
         http(std::move(http_in)),
         package_manager_(PackageManagerFactory::makePackageManager(config.pacman, config.bootloader, storage, http)),
         uptane_fetcher(new Uptane::Fetcher(config, http)),
         report_queue(new ReportQueue(config, http)),
-        events_channel(std::move(events_channel_in)) {}
+        events_channel(std::move(events_channel_in)),
+        primary_ecu_serial_{Uptane::EcuSerial::Unknown()} {}
 
   SotaUptaneClient(Config &config_in, const std::shared_ptr<INvStorage> &storage_in,
                    std::shared_ptr<HttpInterface> http_in)
@@ -144,7 +144,7 @@ class SotaUptaneClient {
                                                    const Uptane::Target &queried_target, int level, bool terminating,
                                                    bool offline);
   void checkAndUpdatePendingSecondaries();
-
+  const Uptane::EcuSerial &primaryEcuSerial() const { return primary_ecu_serial_; }
   template <class T, class... Args>
   void sendEvent(Args &&... args) {
     std::shared_ptr<event::BaseEvent> event = std::make_shared<T>(std::forward<Args>(args)...);
@@ -158,7 +158,7 @@ class SotaUptaneClient {
   Config &config;
   Uptane::DirectorRepository director_repo;
   Uptane::ImagesRepository images_repo;
-  Uptane::PrimaryManifest uptane_manifest;
+  Uptane::ManifestIssuer::Ptr uptane_manifest;
   std::shared_ptr<INvStorage> storage;
   std::shared_ptr<HttpInterface> http;
   std::shared_ptr<PackageManagerInterface> package_manager_;
@@ -173,6 +173,7 @@ class SotaUptaneClient {
   // ecu_serial => secondary*
   std::map<Uptane::EcuSerial, std::shared_ptr<Uptane::SecondaryInterface>> secondaries;
   std::mutex download_mutex;
+  mutable Uptane::EcuSerial primary_ecu_serial_;
 };
 
 class TargetCompare {

src/libaktualizr/uptane/uptane_ci_test.cc

@@ -34,8 +34,6 @@ TEST(UptaneCI, ProvisionAndPutManifest) {
   config.postUpdateValues();  // re-run copy of urls
 
   auto storage = INvStorage::newStorage(config.storage);
-  Uptane::PrimaryManifest uptane_manifest{config, storage};
-
   auto sota_client = std_::make_unique<UptaneTestCommon::TestUptaneClient>(config, storage);
   EXPECT_NO_THROW(sota_client->initialize());
   EXPECT_TRUE(sota_client->putManifestSimple());

src/libaktualizr/uptane/uptanerepository.cc

@@ -107,9 +107,4 @@ bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& f
   return !rootExpired();
 }
 
-Json::Value PrimaryManifest::signManifest(const Json::Value& manifest_unsigned) const {
-  Json::Value manifest = keys_.signTuf(manifest_unsigned);
-  return manifest;
-}
-
 }  // namespace Uptane

src/libaktualizr/uptane/uptanerepository.h

@@ -14,27 +14,6 @@
 
 namespace Uptane {
 
-class PrimaryManifest {
- public:
-  PrimaryManifest(const Config &config_in, std::shared_ptr<INvStorage> storage_in)
-      : storage_{std::move(storage_in)}, keys_(storage_, config_in.keymanagerConfig()) {}
-
-  Json::Value signManifest(const Json::Value &manifest_unsigned) const;
-
-  void setPrimaryEcuSerialHwId(const std::pair<Uptane::EcuSerial, Uptane::HardwareIdentifier> &serials) {
-    primary_ecu_serial = serials.first;
-    primary_hardware_id = serials.second;
-  }
-
-  EcuSerial getPrimaryEcuSerial() const { return primary_ecu_serial; }
-
- private:
-  Uptane::EcuSerial primary_ecu_serial{Uptane::EcuSerial::Unknown()};
-  Uptane::HardwareIdentifier primary_hardware_id{Uptane::HardwareIdentifier::Unknown()};
-  std::shared_ptr<INvStorage> storage_;
-  KeyManager keys_;
-};
-
 class RepositoryCommon {
  public:
   RepositoryCommon(RepositoryType type_in) : type{type_in} {}

src/virtual_secondary/managedsecondary.cc

@@ -187,6 +187,9 @@ Uptane::Manifest ManagedSecondary::getManifest() const {
   }
 
   Json::Value manifest = Uptane::ManifestIssuer::assembleManifest(firmware_info, getSerial());
+  // consider updating Uptane::ManifestIssuer functionality to fulfill the given use-case
+  // and removing the following code from here so we encapsulate manifest generation
+  // and signing functionality in one place
   manifest["attacks_detected"] = detected_attack;
 
   Json::Value signed_ecu_version;

tests/test_update.py

@@ -21,10 +21,10 @@
  while aktualizr/primary is configured with ostree package manager
 """
 @with_uptane_backend(start_generic_server=True)
+@with_secondary(start=True)
 @with_director()
 @with_treehub()
 @with_sysroot()
-@with_secondary(start=True)
 @with_aktualizr(start=False, run_mode='once', output_logs=True)
 def test_primary_ostree_secondary_fake_updates(uptane_repo, secondary, aktualizr, director,
                                                uptane_server, sysroot, treehub):

tests/uptane_vector_tests.cc

@@ -96,11 +96,11 @@ TEST_P(UptaneVector, Test) {
   logger_set_threshold(boost::log::trivial::trace);
 
   auto storage = INvStorage::newStorage(config.storage);
-  Uptane::PrimaryManifest uptane_manifest{config, storage};
   auto uptane_client = std_::make_unique<SotaUptaneClient>(config, storage);
   Uptane::EcuSerial ecu_serial(config.provision.primary_ecu_serial);
   Uptane::HardwareIdentifier hw_id(config.provision.primary_ecu_hardware_id);
   uptane_client->hw_ids.insert(std::make_pair(ecu_serial, hw_id));
+  uptane_client->primary_ecu_serial_ = ecu_serial;
   Uptane::EcuMap ecu_map{{ecu_serial, hw_id}};
   Uptane::Target target("test_filename", ecu_map, {{Uptane::Hash::Type::kSha256, "sha256"}}, 1, "");
   storage->saveInstalledVersion(ecu_serial.ToString(), target, InstalledVersionUpdateMode::kCurrent);