Fix secondary root rotation on metadata expiry

src/aktualizr_info/aktualizr_info_test.cc

@@ -1,6 +1,7 @@
 #include <gtest/gtest.h>
 
 #include <boost/filesystem.hpp>
+#include <boost/filesystem/fstream.hpp>
 
 #include "libaktualizr/config.h"
 #include "storage/sqlstorage.h"

src/aktualizr_secondary/aktualizr_secondary_test.cc

@@ -2,7 +2,9 @@
 #include <gtest/gtest.h>
 
 #include <boost/filesystem.hpp>
+#include <boost/filesystem/string_file.hpp>
 #include <boost/optional/optional_io.hpp>
+#include <fstream>
 
 #include "aktualizr_secondary_file.h"
 #include "crypto/keymanager.h"

src/cert_provider/cert_provider_test.cc

@@ -1,6 +1,7 @@
 #include <gtest/gtest.h>
 
 #include <boost/filesystem.hpp>
+#include <boost/filesystem/fstream.hpp>
 #include <boost/format.hpp>
 
 #include "cert_provider_test.h"

src/libaktualizr/config/config_test.cc

@@ -6,6 +6,7 @@
 #include <boost/algorithm/hex.hpp>
 #include <boost/filesystem.hpp>
 #include <boost/program_options.hpp>
+#include <fstream>
 
 #include "bootstrap/bootstrap.h"
 #include "crypto/crypto.h"

src/uptane_generator/repo.cc

@@ -8,6 +8,7 @@
 #include "crypto/crypto.h"
 #include "director_repo.h"
 #include "image_repo.h"
+#include "json/json.h"
 #include "libaktualizr/campaign.h"
 
 Repo::Repo(Uptane::RepositoryType repo_type, boost::filesystem::path path, const std::string &expires,
@@ -288,7 +289,7 @@ void Repo::readKeys() {
   }
 }
 
-void Repo::refresh(const Uptane::Role &role) {
+void Repo::refresh(const Uptane::Role &role, const TimeStamp &expiry) {
   if (repo_type_ == Uptane::RepositoryType::Director() &&
       (role == Uptane::Role::Timestamp() || role == Uptane::Role::Snapshot())) {
     throw std::runtime_error("The " + role.ToString() + " in the Director repo is not currently supported.");
@@ -307,14 +308,14 @@ void Repo::refresh(const Uptane::Role &role) {
     throw std::runtime_error("Refreshing custom role " + role.ToString() + " is not currently supported.");
   }
 
-  // The only interesting part here is to increment the version. It could be
-  // interesting to allow changing the expiry, too.
   Json::Value meta_raw = Utils::parseJSONFile(meta_path)["signed"];
   const unsigned version = meta_raw["version"].asUInt() + 1;
 
   auto current_expire_time = TimeStamp(meta_raw["expires"].asString());
 
-  if (current_expire_time.IsExpiredAt(TimeStamp::Now())) {
+  if (expiry.IsValid()) {
+    meta_raw["expires"] = expiry.ToString();
+  } else if (current_expire_time.IsExpiredAt(TimeStamp::Now())) {
     time_t new_expiration_time;
     std::time(&new_expiration_time);
     new_expiration_time += 60 * 60;  // make it valid for the next hour
@@ -370,8 +371,9 @@ void Repo::rotate(const Uptane::Role &role, KeyType key_type) {
 
   // Sign Root with old and new key
   auto intermediate_meta = signTuf(role, meta_raw);
-  const std::string signed_meta = Utils::jsonToCanonicalStr(signTuf(old_key, intermediate_meta));
-  Utils::writeFile(meta_path, signed_meta);
+  auto signed_meta = signTuf(old_key, intermediate_meta);
+
+  Utils::writeFile(meta_path, Utils::jsonToCanonicalStr(signed_meta));
 
   std::stringstream root_name;
   root_name << version << ".root.json";

src/uptane_generator/repo.h

@@ -41,7 +41,7 @@ class Repo {
   Json::Value getTarget(const std::string &target_name);
   Json::Value signTuf(const Uptane::Role &role, const Json::Value &json);
   void generateCampaigns() const;
-  void refresh(const Uptane::Role &role);
+  void refresh(const Uptane::Role &role, const TimeStamp &expiry);
   void rotate(const Uptane::Role &role, KeyType key_type = KeyType::kRSA2048);
 
  protected:

src/uptane_generator/uptane_repo.cc

@@ -45,11 +45,11 @@ void UptaneRepo::emptyTargets() { director_repo_.emptyTargets(); }
 void UptaneRepo::oldTargets() { director_repo_.oldTargets(); }
 void UptaneRepo::generateCampaigns() { director_repo_.generateCampaigns(); }
 
-void UptaneRepo::refresh(Uptane::RepositoryType repo_type, const Uptane::Role &role) {
+void UptaneRepo::refresh(Uptane::RepositoryType repo_type, const Uptane::Role &role, const TimeStamp &expiry) {
   if (repo_type == Uptane::RepositoryType::Director()) {
-    director_repo_.refresh(role);
+    director_repo_.refresh(role, expiry);
   } else if (repo_type == Uptane::RepositoryType::Image()) {
-    image_repo_.refresh(role);
+    image_repo_.refresh(role, expiry);
   }
 }
 

src/uptane_generator/uptane_repo.h

@@ -23,7 +23,7 @@ class UptaneRepo {
   void emptyTargets();
   void oldTargets();
   void generateCampaigns();
-  void refresh(Uptane::RepositoryType repo_type, const Uptane::Role &role);
+  void refresh(Uptane::RepositoryType repo_type, const Uptane::Role &role, const TimeStamp &expiry = TimeStamp());
   void rotate(Uptane::RepositoryType repo_type, const Uptane::Role &role, KeyType key_type = KeyType::kRSA2048);
 
  private:

src/virtual_secondary/CMakeLists.txt

@@ -4,13 +4,19 @@ set(HEADERS managedsecondary.h virtualsecondary.h)
 
 set(TARGET virtual_secondary)
 
-add_library(${TARGET} STATIC
-  ${SOURCES}
-)
+add_library(virtual_secondary STATIC ${SOURCES})
 
 target_include_directories(${TARGET} PUBLIC ${PROJECT_SOURCE_DIR}/src/virtual_secondary)
 
-add_aktualizr_test(NAME virtual_secondary SOURCES virtual_secondary_test.cc PROJECT_WORKING_DIRECTORY LIBRARIES uptane_generator_lib)
-target_link_libraries(t_virtual_secondary virtual_secondary)
+add_aktualizr_test(NAME virtual_secondary
+                   SOURCES virtual_secondary_test.cc
+                   PROJECT_WORKING_DIRECTORY 
+                   LIBRARIES uptane_generator_lib virtual_secondary)
 
-aktualizr_source_file_checks(${HEADERS} ${SOURCES} ${TEST_SOURCES})
+add_aktualizr_test(NAME bad_rotation
+                   SOURCES bad_rotation_test.cc
+                   PROJECT_WORKING_DIRECTORY
+                   LIBRARIES uptane_generator_lib virtual_secondary)
+
+
+aktualizr_source_file_checks(${HEADERS} ${SOURCES} ${TEST_SOURCES} ${FUZZ_SECONDARY_SOURCES})

src/virtual_secondary/bad_rotation_test.cc

@@ -0,0 +1,127 @@
+#include <iostream>
+
+#include <gtest/gtest.h>
+
+#include <boost/log/utility/setup/file.hpp>
+#include "boost/filesystem.hpp"
+#include "httpfake.h"
+#include "libaktualizr/secondaryinterface.h"
+#include "libaktualizr/types.h"
+#include "logging/logging.h"
+#include "uptane/tuf.h"
+#include "uptane_repo.h"
+#include "uptane_test_common.h"
+#include "utilities/utils.h"
+#include "virtualsecondary.h"
+
+/*
+ * Reproduction for TOR-3452. Rotate the root metadata to avoid it expiring, then recover
+ */
+TEST(VirtualSecondary, RootRotationExpires) {  // NOLINT
+  TemporaryDirectory temp_dir;
+  TemporaryDirectory meta_dir;
+  auto http = std::make_shared<HttpFake>(temp_dir.Path(), "", meta_dir.Path() / "repo");
+  Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server);
+  logger_set_threshold(boost::log::trivial::trace);
+
+  UptaneRepo uptane_repo{meta_dir.PathString(), "", "2023-03-04T16:43:12Z"};
+  uptane_repo.generateRepo(KeyType::kED25519);
+  uptane_repo.addImage("tests/test_data/firmware.txt", "firmware.txt", "secondary_hw");
+
+  const std::string hwid = "primary_hw";
+
+  Utils::writeFile(meta_dir / "fake_meta/primary_firmware.txt", std::string("asdf"));
+  uptane_repo.addImage(meta_dir / "fake_meta/primary_firmware.txt", "primary_firmware.txt", hwid);
+  Utils::writeFile(meta_dir / "fake_meta/primary_firmware2.txt", std::string("asdf"));
+  uptane_repo.addImage(meta_dir / "fake_meta/primary_firmware2.txt", "primary_firmware2.txt", hwid);
+  uptane_repo.addImage("tests/test_data/firmware_name.txt", "firmware_name.txt", "secondary_hw");
+  uptane_repo.addImage("tests/test_data/firmware.txt", "firmware2.txt", "secondary_hw");
+
+  time_t new_expiration_time;
+  std::time(&new_expiration_time);
+  new_expiration_time += 15;  // make it valid for the next 15 seconds
+  struct tm new_expiration_time_str {};
+  gmtime_r(&new_expiration_time, &new_expiration_time_str);
+
+  auto timestamp = TimeStamp(new_expiration_time_str);
+  uptane_repo.refresh(Uptane::RepositoryType::Image(), Uptane::Role::Root(), timestamp);
+  uptane_repo.refresh(Uptane::RepositoryType::Director(), Uptane::Role::Root(), timestamp);
+
+  result::UpdateCheck update_result;
+  result::Download download_result;
+  result::Install install_result;
+
+  uptane_repo.emptyTargets();
+  uptane_repo.addTarget("firmware_name.txt", "secondary_hw", "secondary_ecu_serial");
+  uptane_repo.signTargets();
+
+  {
+    LOG_INFO << "Starting initial run";
+    auto storage = INvStorage::newStorage(conf.storage);
+    UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+    aktualizr.Initialize();
+    update_result = aktualizr.CheckUpdates().get();
+    ASSERT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+    download_result = aktualizr.Download(update_result.updates).get();
+    ASSERT_EQ(download_result.status, result::DownloadStatus::kSuccess);
+    install_result = aktualizr.Install(download_result.updates).get();
+    EXPECT_TRUE(install_result.dev_report.success);
+  }
+  bool expire = true;
+  if (expire) {
+    LOG_INFO << "Sleeping in a warehouse";
+    for (int x = 0; x < 25; x++) {
+      sleep(1);
+      if (timestamp.IsExpiredAt(TimeStamp::Now())) {
+        break;
+      }
+    }
+    ASSERT_TRUE(timestamp.IsExpiredAt(TimeStamp::Now()));
+  } else {
+    ASSERT_FALSE(timestamp.IsExpiredAt(TimeStamp::Now()));
+  }
+
+  uptane_repo.refresh(Uptane::RepositoryType::Image(), Uptane::Role::Root(), TimeStamp("2024-01-01T16:43:12Z"));
+  uptane_repo.refresh(Uptane::RepositoryType::Director(), Uptane::Role::Root(), TimeStamp("2024-01-01T16:43:12Z"));
+  uptane_repo.refresh(Uptane::RepositoryType::Image(), Uptane::Role::Root(), TimeStamp("2025-01-01T16:43:12Z"));
+  uptane_repo.refresh(Uptane::RepositoryType::Director(), Uptane::Role::Root(), TimeStamp("2025-01-01T16:43:12Z"));
+
+  {
+    LOG_INFO << "Starting second run";
+    auto storage = INvStorage::newStorage(conf.storage);
+    UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+    aktualizr.Initialize();
+
+    uptane_repo.emptyTargets();
+    uptane_repo.addTarget("primary_firmware.txt", hwid, "CA:FE:A6:D2:84:9D");
+    uptane_repo.signTargets();
+    update_result = aktualizr.CheckUpdates().get();
+    ASSERT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+    download_result = aktualizr.Download(update_result.updates).get();
+    ASSERT_EQ(download_result.status, result::DownloadStatus::kSuccess);
+    install_result = aktualizr.Install(download_result.updates).get();
+    EXPECT_TRUE(install_result.dev_report.success);
+
+    uptane_repo.emptyTargets();
+    uptane_repo.addTarget("firmware2.txt", "secondary_hw", "secondary_ecu_serial");
+    uptane_repo.signTargets();
+
+    update_result = aktualizr.CheckUpdates().get();
+    ASSERT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+    download_result = aktualizr.Download(update_result.updates).get();
+    ASSERT_EQ(download_result.status, result::DownloadStatus::kSuccess);
+    install_result = aktualizr.Install(download_result.updates).get();
+    EXPECT_TRUE(install_result.dev_report.success);
+  }
+}
+
+#ifndef __NO_MAIN__
+int main(int argc, char** argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+
+  logger_init();
+  logger_set_threshold(boost::log::trivial::trace);
+
+  return RUN_ALL_TESTS();
+}
+#endif

src/virtual_secondary/managedsecondary.cc

@@ -61,9 +61,16 @@ ManagedSecondary::ManagedSecondary(Primary::ManagedSecondaryConfig sconfig_in) :
 
   try {
     director_repo_->checkMetaOffline(*storage_);
+  } catch (const std::exception &e) {
+    // This is actually safe. We've done enough initialization to get
+    // director_repo_ into a valid configuration
+    LOG_INFO << "No valid Director metadata found in storage: " << e.what();
+  }
+  try {
     image_repo_->checkMetaOffline(*storage_);
   } catch (const std::exception &e) {
-    LOG_INFO << "No valid metadata found in storage.";
+    // See above ^ image_repo_ is OK
+    LOG_INFO << "No valid Image metadata found in storage: " << e.what();
   }
 }
 

tests/CMakeLists.txt

@@ -197,6 +197,9 @@ add_test(NAME test_ip_secondary
          COMMAND ${PROJECT_SOURCE_DIR}/tests/ipsecondary_test.py
          --build-dir ${PROJECT_BINARY_DIR} --src-dir ${PROJECT_SOURCE_DIR})
 set_tests_properties(test_ip_secondary PROPERTIES LABELS "noptest")
+# This test flakes when run in parallel. Force it to run serially
+# TODO Fix this test!
+set_tests_properties(test_ip_secondary PROPERTIES RUN_SERIAL ON)
 
 add_test(NAME test_ip_secondary_rotation
          COMMAND ${PROJECT_SOURCE_DIR}/tests/ipsecondary_rotation_test.py