Skip to content

Commit

Permalink
Remove targets/latest_targets distinction
Browse files Browse the repository at this point in the history 
This removes the distinction between 'the last director targets the
server sent us' and 'the last _non-empty_ targets the server sent us'.
This change was introduced in 2019 by advancedtelematic#1186
(aka #02deeb5), we believe to deal with the old director that needed the
client to always send back a end-state manifest for any correlationid it
gave out.

Unfortunately this doesn't work with offline updates. Consider the
following steps:

1) The user sends an online update to a device
2) Aktualizr receives a non-empty targets, and installs it
3) The user plugs in a device and installs an offline update
4) Aktualizr sends the manifest to the server
5) The server recognises that the device has been updated offline, and
   starts sending empty targets to mean 'I don't know what you should be
   running, carry on as before'

Under the old behaviour, Aktualizr would ignore the empty targets and
try and install the whatever was sent in step advancedtelematic#2.

This also removes the targets_meta field from the UpdateCheck. This
appears to be unused and doesn't have any tests. It was introduced in
2018 by advancedtelematic#981, aka
Commit #c3ca9f52. We populate it by fishing the information out of the
Sqlite storage, and this introduces a few more error paths in
SotaUptaneClient.
  • Loading branch information
@cajun-rat
cajun-rat committed Dec 31, 2023
1 parent 60724e5 commit 623598b
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 128 deletions.
9 changes: 2 additions & 7 deletions include/libaktualizr/results.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,11 @@ class UpdateCheck {
public:
UpdateCheck() = default;
UpdateCheck(std::vector<Uptane::Target> updates_in, unsigned int ecus_count_in, UpdateStatus status_in,
Json::Value targets_meta_in, std::string message_in)
: updates(std::move(updates_in)),
ecus_count(ecus_count_in),
status(status_in),
targets_meta(std::move(targets_meta_in)),
message(std::move(message_in)) {}
std::string message_in)
: updates(std::move(updates_in)), ecus_count(ecus_count_in), status(status_in), message(std::move(message_in)) {}
std::vector<Uptane::Target> updates;
unsigned int ecus_count{0};
UpdateStatus status{UpdateStatus::kNoUpdatesAvailable};
Json::Value targets_meta;
std::string message;
};

Expand Down
57 changes: 16 additions & 41 deletions src/libaktualizr/primary/empty_targets_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ class HttpRejectEmptyCorrId : public HttpFake {
};

/*
* Verify that we can successfully install an update after receiving
* subsequent Targets metadata that is empty.
* If we have downloaded an update, and the server decides to empty the
* director targets, then don't try and carry on the update. This is a change
* from previous behaviour, where we tried to persist the old targets (at least
* up until a reboot).
*/
TEST(Aktualizr, EmptyTargets) {
TemporaryDirectory temp_dir;
Expand All @@ -41,55 +43,28 @@ TEST(Aktualizr, EmptyTargets) {
logger_set_threshold(boost::log::trivial::trace);

Process uptane_gen(uptane_generator_path.string());
uptane_gen.run({"generate", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
uptane_gen.run({"generate", "--path", meta_dir.PathString(), "--correlationid", "cid1"});
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({"signtargets", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});

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::kUpdatesAvailable);

result::Download download_result = aktualizr.Download(update_result.updates).get();
EXPECT_EQ(download_result.status, result::DownloadStatus::kSuccess);
UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
aktualizr.Initialize();

uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
uptane_gen.run({"signtargets", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
result::UpdateCheck update_result = aktualizr.CheckUpdates().get();
ASSERT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);

result::UpdateCheck update_result2 = aktualizr.CheckUpdates().get();
EXPECT_EQ(update_result2.status, result::UpdateStatus::kUpdatesAvailable);
result::Download download_result = aktualizr.Download(update_result.updates).get();
EXPECT_EQ(download_result.status, result::DownloadStatus::kSuccess);

result::Install install_result = aktualizr.Install(update_result2.updates).get();
ASSERT_EQ(install_result.ecu_reports.size(), 1);
EXPECT_EQ(install_result.ecu_reports[0].install_res.result_code.num_code,
data::ResultCode::Numeric::kNeedCompletion);

aktualizr.uptane_client()->package_manager_->completeInstall();
}
{
UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
aktualizr.Initialize();

const Json::Value manifest = http->last_manifest["signed"];
const Json::Value manifest_versions = manifest["ecu_version_manifests"];

EXPECT_EQ(manifest["installation_report"]["report"]["items"].size(), 1);
EXPECT_EQ(manifest["installation_report"]["report"]["items"][0]["ecu"].asString(), "CA:FE:A6:D2:84:9D");
EXPECT_TRUE(manifest["installation_report"]["report"]["items"][0]["result"]["success"].asBool());

EXPECT_EQ(manifest_versions["CA:FE:A6:D2:84:9D"]["signed"]["installed_image"]["filepath"].asString(),
"firmware.txt");
EXPECT_EQ(manifest_versions["CA:FE:A6:D2:84:9D"]["signed"]["installed_image"]["fileinfo"]["length"].asUInt(), 17);
uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});

result::UpdateCheck update_result3 = aktualizr.CheckUpdates().get();
EXPECT_EQ(update_result3.status, result::UpdateStatus::kNoUpdatesAvailable);
}
result::UpdateCheck update_result2 = aktualizr.CheckUpdates().get();
EXPECT_EQ(update_result2.status, result::UpdateStatus::kNoUpdatesAvailable);
}

/* Check that Aktualizr switches back to empty targets after failing to verify
Expand Down
44 changes: 8 additions & 36 deletions src/libaktualizr/primary/sotauptaneclient.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -924,8 +924,6 @@ void SotaUptaneClient::sendDeviceData() {
result::UpdateCheck SotaUptaneClient::fetchMeta() {
requiresProvision();

result::UpdateCheck result;

reportNetworkInfo();

if (hasPendingUpdates()) {
Expand All @@ -938,23 +936,19 @@ result::UpdateCheck SotaUptaneClient::fetchMeta() {
// if there are still some pending updates just return, don't check for new updates
// no need in update checking if there are some pending updates
LOG_INFO << "An update is pending. Skipping check for update until installation is complete.";
return result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue,
"There are pending updates, no new updates are checked");
return {{}, 0, result::UpdateStatus::kError, "There are pending updates, no new updates are checked"};
}

// Uptane step 1 (build the vehicle version manifest):
if (!putManifestSimple()) {
LOG_ERROR << "Error sending manifest!";
}
result = checkUpdates();
auto result = checkUpdates();
sendEvent<event::UpdateCheckComplete>(result);

return result;
}

result::UpdateCheck SotaUptaneClient::checkUpdates(UpdateType utype) {
result::UpdateCheck result;

std::vector<Uptane::Target> updates;
unsigned int ecus_count = 0;
try {
Expand All @@ -967,32 +961,15 @@ result::UpdateCheck SotaUptaneClient::checkUpdates(UpdateType utype) {
data::InstallationResult(data::ResultCode::Numeric::kVerificationFailed, "Could not update metadata"));
}
last_exception = std::current_exception();
result = result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue, "Could not update metadata.");
return result;
return {{}, 0, result::UpdateStatus::kError, "Could not update metadata."};
} catch (const std::exception &e) {
last_exception = std::current_exception();
result = result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue, "Could not update metadata.");
return result;
}

std::string director_targets;
if (utype == UpdateType::kOnline &&
!storage->loadNonRoot(&director_targets, Uptane::RepositoryType::Director(), Uptane::Role::Targets())) {
result = result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue, "Could not update metadata.");
return result;
} else if (utype == UpdateType::kOffline &&
!storage->loadNonRoot(&director_targets, Uptane::RepositoryType::Director(),
Uptane::Role::OfflineUpdates())) {
result =
result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue, "Could not update offline metadata.");
return result;
return {{}, 0, result::UpdateStatus::kError, "Could not update metadata."};
}

if (updates.empty()) {
LOG_DEBUG << "No new updates found in Uptane metadata.";
result =
result::UpdateCheck({}, 0, result::UpdateStatus::kNoUpdatesAvailable, Utils::parseJSON(director_targets), "");
return result;
return {{}, 0, result::UpdateStatus::kNoUpdatesAvailable, ""};
}

// 5.4.4.2.10.: Verify that Targets metadata from the Director and Image
Expand Down Expand Up @@ -1020,21 +997,17 @@ result::UpdateCheck SotaUptaneClient::checkUpdates(UpdateType utype) {
} catch (const std::exception &e) {
last_exception = std::current_exception();
LOG_ERROR << e.what();
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;
return {{}, 0, result::UpdateStatus::kError, "Target mismatch."};
}

result = result::UpdateCheck(updates, ecus_count, result::UpdateStatus::kUpdatesAvailable,
Utils::parseJSON(director_targets), "");
if (updates.size() == 1) {
LOG_INFO << "1 new update found in both Director and Image repo metadata.";
} else {
LOG_INFO << updates.size() << " new updates found in both Director and Image repo metadata.";
}
return result;
return {updates, ecus_count, result::UpdateStatus::kUpdatesAvailable, ""};
}

result::UpdateStatus SotaUptaneClient::checkUpdatesOffline(const std::vector<Uptane::Target> &targets,
Expand Down Expand Up @@ -1760,8 +1733,7 @@ result::UpdateCheck SotaUptaneClient::fetchMetaOffUpd(const boost::filesystem::p
// if there are still some pending updates just return, don't check for new updates
// no need in update checking if there are some pending updates
LOG_INFO << "An update is pending. Skipping check for update until installation is complete.";
return result::UpdateCheck({}, 0, result::UpdateStatus::kError, Json::nullValue,
"There are pending updates, no new updates are checked");
return {{}, 0, result::UpdateStatus::kError, "There are pending updates, no new updates are checked"};
}

// // Uptane step 1 (build the vehicle version manifest):
Expand Down
14 changes: 6 additions & 8 deletions src/libaktualizr/uptane/director_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@ TEST(Director, EmptyTargets) {
TemporaryDirectory meta_dir;

Process uptane_gen(uptane_generator_path.string());
uptane_gen.run({"generate", "--path", meta_dir.PathString()});
uptane_gen.run({"generate", "--path", meta_dir.PathString(), "--correlationid", "cid1"});

DirectorRepository director;
EXPECT_NO_THROW(director.initRoot(Uptane::RepositoryType(Uptane::RepositoryType::DIRECTOR),
Utils::readFile(meta_dir.Path() / "repo/director/root.json")));

EXPECT_NO_THROW(director.verifyTargets(Utils::readFile(meta_dir.Path() / "repo/director/targets.json")));
EXPECT_TRUE(director.targets.targets.empty());
EXPECT_TRUE(director.latest_targets.targets.empty());

uptane_gen.run({"image", "--path", meta_dir.PathString(), "--filename", "tests/test_data/firmware.txt",
"--targetname", "firmware.txt", "--hwid", "primary_hw"});
Expand All @@ -36,15 +35,14 @@ TEST(Director, EmptyTargets) {
EXPECT_NO_THROW(director.verifyTargets(Utils::readFile(meta_dir.Path() / "repo/director/targets.json")));
EXPECT_EQ(director.targets.targets.size(), 1);
EXPECT_EQ(director.targets.targets[0].filename(), "firmware.txt");
EXPECT_EQ(director.targets.targets.size(), director.latest_targets.targets.size());
EXPECT_EQ(director.getCorrelationId(), "cid1");

uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
uptane_gen.run({"signtargets", "--path", meta_dir.PathString(), "--correlationid", "abc123"});
uptane_gen.run({"emptytargets", "--path", meta_dir.PathString(), "--correlationid", "cid2"});
uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});

EXPECT_NO_THROW(director.verifyTargets(Utils::readFile(meta_dir.Path() / "repo/director/targets.json")));
EXPECT_EQ(director.targets.targets.size(), 1);
EXPECT_EQ(director.targets.targets[0].filename(), "firmware.txt");
EXPECT_TRUE(director.latest_targets.targets.empty());
EXPECT_TRUE(director.targets.targets.empty());
EXPECT_EQ(director.getCorrelationId(), "cid2");
}

} // namespace Uptane
Expand Down
35 changes: 10 additions & 25 deletions src/libaktualizr/uptane/directorrepository.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,13 @@ namespace Uptane {
void DirectorRepository::resetMeta() {
resetRoot();
targets = Targets();
latest_targets = Targets();
#ifdef BUILD_OFFLINE_UPDATES
offline_snapshot_ = Snapshot();
#endif
}

void DirectorRepository::checkTargetsExpired(UpdateType utype) {
if (latest_targets.isExpired(TimeStamp::Now())) {
if (targets.isExpired(TimeStamp::Now())) {
if (utype == UpdateType::kOffline) {
throw Uptane::ExpiredMetadata(type.ToString(), Role::OFFLINEUPDATES);
} else {
Expand All @@ -32,7 +31,7 @@ void DirectorRepository::checkTargetsExpired(UpdateType utype) {
void DirectorRepository::targetsSanityCheck(UpdateType utype) {
// 5.4.4.6.6. If checking Targets metadata from the Director repository,
// verify that there are no delegations.
if (!latest_targets.delegated_role_names_.empty()) {
if (!targets.delegated_role_names_.empty()) {
if (utype == UpdateType::kOffline) {
throw Uptane::InvalidMetadata(type.ToString(), Role::OFFLINEUPDATES, "Found unexpected delegation.");
} else {
Expand All @@ -59,21 +58,12 @@ void DirectorRepository::targetsSanityCheck(UpdateType utype) {
}
}

bool DirectorRepository::usePreviousTargets() const {
// Don't store the new targets if they are empty and we've previously received
// a non-empty list.
return !targets.targets.empty() && latest_targets.targets.empty();
}

void DirectorRepository::verifyTargets(const std::string& targets_raw) {
try {
// Verify the signature:
latest_targets = Targets(RepositoryType::Director(), Role::Targets(), Utils::parseJSON(targets_raw),
std::make_shared<MetaWithKeys>(root));
if (!usePreviousTargets()) {
targets = latest_targets;
correlation_id_ = latest_targets.correlation_id();
}
targets = Targets(RepositoryType::Director(), Role::Targets(), Utils::parseJSON(targets_raw),
std::make_shared<MetaWithKeys>(root));
correlation_id_ = targets.correlation_id();
} catch (const Uptane::Exception& e) {
LOG_ERROR << "Signature verification for Director Targets metadata failed";
throw;
Expand Down Expand Up @@ -154,7 +144,7 @@ void DirectorRepository::updateMeta(INvStorage& storage, const IMetadataFetcher&
// the database, which can cause some minor confusion.
if (local_version > remote_version) {
throw Uptane::SecurityException(RepositoryType::DIRECTOR, "Rollback attempt");
} else if (local_version < remote_version && !usePreviousTargets()) {
} else if (local_version < remote_version && !targets.targets.empty()) {
storage.storeNonRoot(director_targets, RepositoryType::Director(), Role::Targets());
}

Expand Down Expand Up @@ -315,9 +305,7 @@ void DirectorRepository::updateMetaOffUpd(INvStorage& storage, const OfflineUpda
}

verifyOfflineTargets(director_offline_targets, storage);
if (!usePreviousTargets()) {
storage.storeNonRoot(director_offline_targets, RepositoryType::Director(), Role::OfflineUpdates());
}
storage.storeNonRoot(director_offline_targets, RepositoryType::Director(), Role::OfflineUpdates());

// PURE-2 step 4(iii)
checkTargetsExpired(UpdateType::kOffline);
Expand Down Expand Up @@ -363,12 +351,9 @@ void DirectorRepository::checkOfflineSnapshotExpired() {
void DirectorRepository::verifyOfflineTargets(const std::string& targets_raw, INvStorage& storage) {
// PURE-2 step 4(ii)
try {
latest_targets = Targets(RepositoryType::Director(), Role::OfflineUpdates(), Utils::parseJSON(targets_raw),
std::make_shared<MetaWithKeys>(root));
targets = Targets(RepositoryType::Director(), Role::OfflineUpdates(), Utils::parseJSON(targets_raw),
std::make_shared<MetaWithKeys>(root));
transformOfflineTargets(storage);
if (!usePreviousTargets()) {
targets = latest_targets;
}
} catch (const Uptane::Exception& e) {
LOG_ERROR << "Signature verification for Director Targets metadata failed";
throw;
Expand All @@ -386,7 +371,7 @@ void DirectorRepository::transformOfflineTargets(INvStorage& storage) {
throw std::runtime_error("Unable to load ECU serials");
}

for (Uptane::Target& target : latest_targets.targets) {
for (Uptane::Target& target : targets.targets) {
std::vector<Uptane::HardwareIdentifier> hwids = target.hardwareIds();
for (Uptane::HardwareIdentifier& hwid : hwids) {
for (const auto& s : serials) {
Expand Down
7 changes: 1 addition & 6 deletions src/libaktualizr/uptane/directorrepository.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,8 @@ class DirectorRepository : public RepositoryCommon {
void resetMeta();
void checkTargetsExpired(UpdateType utype);
void targetsSanityCheck(UpdateType utype);
bool usePreviousTargets() const;

// Since the Director can send us an empty targets list to mean "no new
// updates", we have to persist the previous targets list. Use the latest for
// checking expiration but the most recent non-empty list for everything else.
Uptane::Targets targets; // Only empty if we've never received non-empty targets.
Uptane::Targets latest_targets; // Can be an empty list.
Uptane::Targets targets;
/**
* The correlation id of the currently running update.
* This is set when the targets are first downloaded from the server, and
Expand Down
Loading

0 comments on commit 623598b

Please sign in to comment.