From bda0573746a86734de7748f6c47ddb243239d379 Mon Sep 17 00:00:00 2001 From: Phil Wise Date: Wed, 24 Jul 2024 17:55:39 +0100 Subject: [PATCH] Tidy up root rotation logic This wasn't implicated in TOR-3452, but the new version is shorter and has simple control flow. --- src/libaktualizr/primary/sotauptaneclient.cc | 75 +++++++++++--------- 1 file changed, 42 insertions(+), 33 deletions(-) diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index c45c143e7..7cb0ddb52 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -1292,52 +1292,61 @@ data::InstallationResult SotaUptaneClient::rotateSecondaryRoot(Uptane::Repositor LOG_ERROR << "Error reading Root metadata"; return data::InstallationResult(data::ResultCode::Numeric::kInternalError, "Error reading Root metadata"); } - - data::InstallationResult result{data::ResultCode::Numeric::kOk, ""}; const int last_root_version = Uptane::extractVersionUntrusted(latest_root); const int sec_root_version = secondary.getRootVersion((repo == Uptane::RepositoryType::Director())); - // If sec_root_version is 0, assume either the Secondary doesn't have Root - // metadata or doesn't support the Root version request. Continue on and hope - // for the best. + LOG_DEBUG << "Rotating " << repo << " from " << sec_root_version << " to " << (last_root_version - 1); if (sec_root_version < 0) { LOG_WARNING << "Secondary with serial " << secondary.getSerial() << " reported an invalid " << repo << " repo Root version: " << sec_root_version; - result = - data::InstallationResult(data::ResultCode::Numeric::kInternalError, - "Secondary with serial " + secondary.getSerial().ToString() + " reported an invalid " + - repo.ToString() + " repo Root version: " + std::to_string(sec_root_version)); - } else if (sec_root_version > 0 && last_root_version - sec_root_version > 1) { - // Only send intermediate Roots that would otherwise be skipped. The latest - // will be sent with the complete set of the latest metadata. - for (int v = sec_root_version + 1; v < last_root_version; v++) { - std::string root; - if (!storage->loadRoot(&root, repo, Uptane::Version(v))) { - LOG_WARNING << "Couldn't find Root metadata in the storage, trying remote repo"; - try { - uptane_fetcher->fetchRole(&root, Uptane::kMaxRootSize, repo, Uptane::Role::Root(), Uptane::Version(v), - flow_control_); - } catch (const std::exception &e) { - LOG_ERROR << "Root metadata could not be fetched for Secondary with serial " << secondary.getSerial() - << ", skipping to the next Secondary"; - result = data::InstallationResult(data::ResultCode::Numeric::kInternalError, - "Root metadata could not be fetched for Secondary with serial " + - secondary.getSerial().ToString() + ", skipping to the next Secondary"); - break; - } - } + return {data::ResultCode::Numeric::kInternalError, "Secondary with serial " + secondary.getSerial().ToString() + + " reported an invalid " + repo.ToString() + + " repo Root version: " + std::to_string(sec_root_version)}; + } + + // Only send intermediate Roots that would otherwise be skipped. The latest + // will be sent with the complete set of the latest metadata. + for (int version_to_send = sec_root_version + 1; version_to_send < last_root_version; version_to_send++) { + std::string root; + if (!storage->loadRoot(&root, repo, Uptane::Version(version_to_send))) { + LOG_WARNING << "Couldn't find Root metadata in the storage, trying remote repo"; try { - result = secondary.putRoot(root, repo == Uptane::RepositoryType::Director()); - } catch (const std::exception &ex) { - result = data::InstallationResult(data::ResultCode::Numeric::kInternalError, ex.what()); + uptane_fetcher->fetchRole(&root, Uptane::kMaxRootSize, repo, Uptane::Role::Root(), + Uptane::Version(version_to_send), flow_control_); + } catch (const std::exception &e) { + LOG_ERROR << "Root metadata could not be fetched for Secondary with serial " << secondary.getSerial() + << ", skipping to the next Secondary"; + return {data::ResultCode::Numeric::kInternalError, + "Root metadata could not be fetched for Secondary with serial " + secondary.getSerial().ToString() + + ", skipping to the next Secondary"}; } + } + try { + auto result = secondary.putRoot(root, repo == Uptane::RepositoryType::Director()); if (!result.isSuccess()) { + // Old (pre 2024-07-XX) versions would assume that if sec_root_version + // is 0, either the Secondary doesn't have Root metadata or doesn't + // support the Root version request and skip sending any root metadata. + // Unfortunatately this cause TOR-3452 where an expired root metadata + // would cause updates to fail. Instead assume that '0' could mean 'I + // don't have any root versions yet'. If we send version 1 and it is + // rejected, then assume we are in the case that the code originally was + // defending against: the secondary can't rotate root, and treat this + // as a success. The previous code would have returned success in this + // case anyway. + if (version_to_send == 1) { + LOG_WARNING + << "Sending root.1.json to a secondary failed. Assuming it doesn't allow root rotation and continuing."; + return {data::ResultCode::Numeric::kOk, ""}; + } LOG_ERROR << "Sending Root metadata to Secondary with serial " << secondary.getSerial() << " failed: " << result.result_code << " " << result.description; - break; + return result; } + } catch (const std::exception &ex) { + return {data::ResultCode::Numeric::kInternalError, ex.what()}; } } - return result; + return {data::ResultCode::Numeric::kOk, ""}; } // TODO: the function blocks until it updates all the Secondaries. Consider non-blocking operation.