From e10b0802f58a1b3212a3bcfb870f3298dd9ffe73 Mon Sep 17 00:00:00 2001 From: Mike Sul Date: Thu, 16 Jan 2020 14:10:05 +0200 Subject: [PATCH] OTA-4174: Use the same manifest generation and signing functionality on both Primary and Secondary Signed-off-by: Mike Sul --- src/libaktualizr/primary/sotauptaneclient.cc | 69 +++++++++----------- src/libaktualizr/primary/sotauptaneclient.h | 15 +++-- src/libaktualizr/uptane/uptane_ci_test.cc | 2 - src/libaktualizr/uptane/uptanerepository.cc | 5 -- src/libaktualizr/uptane/uptanerepository.h | 21 ------ src/virtual_secondary/managedsecondary.cc | 3 + tests/uptane_vector_tests.cc | 1 - 7 files changed, 43 insertions(+), 73 deletions(-) diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index 779448f0e8..4a7f4e9494 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -49,7 +49,7 @@ void SotaUptaneClient::addSecondary(const std::shared_ptrgetCurrent()); } return false; @@ -86,7 +86,7 @@ void SotaUptaneClient::finalizeAfterReboot() { std::vector 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 installed_versions; boost::optional 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> 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(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(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 *new_targets, unsigned int *ecus_count) { std::vector 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 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::vectorsaveEcuInstallationResult(primary_ecu_serial, install_res); // TODO: distinguish this case from regular failure for local and remote // event reporting - report_queue->enqueue(std_::make_unique(uptane_manifest.getPrimaryEcuSerial(), - correlation_id, false)); - sendEvent(uptane_manifest.getPrimaryEcuSerial(), false); + report_queue->enqueue( + std_::make_unique(primaryEcuSerial(), correlation_id, false)); + sendEvent(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 misconfigured_ecus; std::vector 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(std::distance(serials.cbegin(), store_it))] = true; } @@ -1196,7 +1191,7 @@ std::vector SotaUptaneClient::sendImagesToEcus(const std::vector reports; std::vector>> 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]; diff --git a/src/libaktualizr/primary/sotauptaneclient.h b/src/libaktualizr/primary/sotauptaneclient.h index fb5e348636..6da1fb1e29 100644 --- a/src/libaktualizr/primary/sotauptaneclient.h +++ b/src/libaktualizr/primary/sotauptaneclient.h @@ -30,16 +30,16 @@ class SotaUptaneClient { public: - SotaUptaneClient(Config &config_in, const std::shared_ptr &storage_in, - std::shared_ptr http_in, std::shared_ptr events_channel_in) + SotaUptaneClient(Config &config_in, std::shared_ptr storage_in, std::shared_ptr http_in, + std::shared_ptr 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 &storage_in, std::shared_ptr 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 void sendEvent(Args &&... args) { std::shared_ptr event = std::make_shared(std::forward(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 storage; std::shared_ptr http; std::shared_ptr package_manager_; @@ -173,6 +173,7 @@ class SotaUptaneClient { // ecu_serial => secondary* std::map> secondaries; std::mutex download_mutex; + mutable Uptane::EcuSerial primary_ecu_serial_; }; class TargetCompare { diff --git a/src/libaktualizr/uptane/uptane_ci_test.cc b/src/libaktualizr/uptane/uptane_ci_test.cc index 74efdb0fd5..c604c0da91 100644 --- a/src/libaktualizr/uptane/uptane_ci_test.cc +++ b/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(config, storage); EXPECT_NO_THROW(sota_client->initialize()); EXPECT_TRUE(sota_client->putManifestSimple()); diff --git a/src/libaktualizr/uptane/uptanerepository.cc b/src/libaktualizr/uptane/uptanerepository.cc index f4198af00a..31e3d787eb 100644 --- a/src/libaktualizr/uptane/uptanerepository.cc +++ b/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 diff --git a/src/libaktualizr/uptane/uptanerepository.h b/src/libaktualizr/uptane/uptanerepository.h index 7af36a88da..7e01277fc0 100644 --- a/src/libaktualizr/uptane/uptanerepository.h +++ b/src/libaktualizr/uptane/uptanerepository.h @@ -14,27 +14,6 @@ namespace Uptane { -class PrimaryManifest { - public: - PrimaryManifest(const Config &config_in, std::shared_ptr 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 &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 storage_; - KeyManager keys_; -}; - class RepositoryCommon { public: RepositoryCommon(RepositoryType type_in) : type{type_in} {} diff --git a/src/virtual_secondary/managedsecondary.cc b/src/virtual_secondary/managedsecondary.cc index 594d87e5bc..3f7b6b6a9e 100644 --- a/src/virtual_secondary/managedsecondary.cc +++ b/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; diff --git a/tests/uptane_vector_tests.cc b/tests/uptane_vector_tests.cc index a77c4d857a..142511e200 100644 --- a/tests/uptane_vector_tests.cc +++ b/tests/uptane_vector_tests.cc @@ -96,7 +96,6 @@ 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(config, storage); Uptane::EcuSerial ecu_serial(config.provision.primary_ecu_serial); Uptane::HardwareIdentifier hw_id(config.provision.primary_ecu_hardware_id);