Don't fetch Snapshot or Targets metadata if we already have the latest.

Also test that we don't fetch anything from the Image repo if the
Director does not indicate that there are updates.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>

src/libaktualizr/primary/CMakeLists.txt

@@ -44,6 +44,9 @@ target_link_libraries(t_custom_url virtual_secondary)
 add_aktualizr_test(NAME target_mismatch SOURCES target_mismatch_test.cc PROJECT_WORKING_DIRECTORY
                    ARGS "$<TARGET_FILE:uptane-generator>" LIBRARIES uptane_generator_lib)
 target_link_libraries(t_target_mismatch virtual_secondary)
+add_aktualizr_test(NAME metadata_fetch SOURCES metadata_fetch_test.cc PROJECT_WORKING_DIRECTORY
+                   ARGS "$<TARGET_FILE:uptane-generator>" LIBRARIES uptane_generator_lib)
+target_link_libraries(t_metadata_fetch virtual_secondary)
 
 add_aktualizr_test(NAME device_cred_prov SOURCES device_cred_prov_test.cc PROJECT_WORKING_DIRECTORY LIBRARIES PUBLIC uptane_generator_lib)
 set_tests_properties(test_device_cred_prov PROPERTIES LABELS "crypto")

src/libaktualizr/primary/metadata_fetch_test.cc

@@ -0,0 +1,146 @@
+#include <gtest/gtest.h>
+
+#include <string>
+
+#include "httpfake.h"
+#include "primary/aktualizr.h"
+#include "test_utils.h"
+#include "uptane_test_common.h"
+
+boost::filesystem::path uptane_generator_path;
+
+class HttpFakeMetaCounter : public HttpFake {
+ public:
+  HttpFakeMetaCounter(const boost::filesystem::path &test_dir_in, const boost::filesystem::path &meta_dir_in)
+      : HttpFake(test_dir_in, "", meta_dir_in) {}
+
+  HttpResponse get(const std::string &url, int64_t maxsize) override {
+    if (url.find("director/root.json") != std::string::npos) {
+      ++director_root_count;
+    }
+    if (url.find("director/targets.json") != std::string::npos) {
+      ++director_targets_count;
+    }
+    if (url.find("repo/root.json") != std::string::npos) {
+      ++images_root_count;
+    }
+    if (url.find("repo/timestamp.json") != std::string::npos) {
+      ++images_timestamp_count;
+    }
+    if (url.find("repo/snapshot.json") != std::string::npos) {
+      ++images_snapshot_count;
+    }
+    if (url.find("repo/targets.json") != std::string::npos) {
+      ++images_targets_count;
+    }
+
+    return HttpFake::get(url, maxsize);
+  }
+
+  int director_root_count{0};
+  int director_targets_count{0};
+  int images_root_count{0};
+  int images_timestamp_count{0};
+  int images_snapshot_count{0};
+  int images_targets_count{0};
+};
+
+/*
+ * Don't download Image repo metadata if Director reports no new targets. Don't
+ * download Snapshot and Targets metadata from the Image repo if the Timestamp
+ * indicates nothing has changed.
+ */
+TEST(Aktualizr, MetadataFetch) {
+  TemporaryDirectory temp_dir;
+  TemporaryDirectory meta_dir;
+  auto http = std::make_shared<HttpFakeMetaCounter>(temp_dir.Path(), meta_dir.Path() / "repo");
+  Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server);
+  logger_set_threshold(boost::log::trivial::trace);
+
+  auto storage = INvStorage::newStorage(conf.storage);
+  UptaneTestCommon::TestAktualizr aktualizr(conf, storage, http);
+  aktualizr.Initialize();
+
+  // No updates scheduled: only download Director Root and Targets metadata.
+  Process uptane_gen(uptane_generator_path.string());
+  uptane_gen.run({"generate", "--path", meta_dir.PathString()});
+
+  result::UpdateCheck update_result = aktualizr.CheckUpdates().get();
+  EXPECT_EQ(update_result.status, result::UpdateStatus::kNoUpdatesAvailable);
+  EXPECT_EQ(http->director_root_count, 1);
+  EXPECT_EQ(http->director_targets_count, 1);
+  EXPECT_EQ(http->images_root_count, 0);
+  EXPECT_EQ(http->images_timestamp_count, 0);
+  EXPECT_EQ(http->images_snapshot_count, 0);
+  EXPECT_EQ(http->images_targets_count, 0);
+
+  // Two images added, but only one update scheduled: all metadata objects
+  // should be fetched once.
+  uptane_gen.run({"image", "--path", meta_dir.PathString(), "--filename", "tests/test_data/firmware.txt",
+                  "--targetname", "firmware.txt", "--hwid", "primary_hw"});
+  uptane_gen.run({"image", "--path", meta_dir.PathString(), "--filename", "tests/test_data/firmware_name.txt",
+                  "--targetname", "firmware_name.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({"adddelegation", "--path", meta_dir.PathString(), "--dname", "role-abc", "--dpattern", "abc/*"});
+  uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});
+
+  update_result = aktualizr.CheckUpdates().get();
+  EXPECT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+  EXPECT_EQ(http->director_root_count, 2);
+  EXPECT_EQ(http->director_targets_count, 2);
+  EXPECT_EQ(http->images_root_count, 1);
+  EXPECT_EQ(http->images_timestamp_count, 1);
+  EXPECT_EQ(http->images_snapshot_count, 1);
+  EXPECT_EQ(http->images_targets_count, 1);
+
+  // Update scheduled with pre-existing image: no need to refetch Image repo
+  // Snapshot or Targets metadata.
+  uptane_gen.run({"addtarget", "--path", meta_dir.PathString(), "--targetname", "firmware_name.txt", "--hwid",
+                  "primary_hw", "--serial", "CA:FE:A6:D2:84:9D"});
+  uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});
+
+  update_result = aktualizr.CheckUpdates().get();
+  EXPECT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+  EXPECT_EQ(http->director_root_count, 3);
+  EXPECT_EQ(http->director_targets_count, 3);
+  EXPECT_EQ(http->images_root_count, 2);
+  EXPECT_EQ(http->images_timestamp_count, 2);
+  EXPECT_EQ(http->images_snapshot_count, 1);
+  EXPECT_EQ(http->images_targets_count, 1);
+
+  // Delegation added to an existing delegation; update scheduled with
+  // pre-existing image: Snapshot must be refetched, but Targets are unchanged.
+  uptane_gen.run({"addtarget", "--path", meta_dir.PathString(), "--targetname", "firmware.txt", "--hwid", "primary_hw",
+                  "--serial", "CA:FE:A6:D2:84:9D"});
+  uptane_gen.run({"adddelegation", "--path", meta_dir.PathString(), "--dname", "role-def", "--dpattern", "def/*",
+                  "--dparent", "role-abc"});
+  uptane_gen.run({"signtargets", "--path", meta_dir.PathString()});
+
+  update_result = aktualizr.CheckUpdates().get();
+  EXPECT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable);
+  EXPECT_EQ(http->director_root_count, 4);
+  EXPECT_EQ(http->director_targets_count, 4);
+  EXPECT_EQ(http->images_root_count, 3);
+  EXPECT_EQ(http->images_timestamp_count, 3);
+  EXPECT_EQ(http->images_snapshot_count, 2);
+  EXPECT_EQ(http->images_targets_count, 1);
+}
+
+#ifndef __NO_MAIN__
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  if (argc != 2) {
+    std::cerr << "Error: " << argv[0] << " requires the path to the uptane-generator utility\n";
+    return EXIT_FAILURE;
+  }
+  uptane_generator_path = argv[1];
+
+  logger_init();
+  logger_set_threshold(boost::log::trivial::trace);
+
+  return RUN_ALL_TESTS();
+}
+#endif
+
+// vim: set tabstop=2 shiftwidth=2 expandtab:

src/libaktualizr/uptane/imagesrepository.cc

@@ -22,6 +22,32 @@ bool ImagesRepository::verifyTimestamp(const std::string& timestamp_raw) {
   return true;
 }
 
+bool ImagesRepository::fetchSnapshot(INvStorage& storage, const IMetadataFetcher& fetcher, const int local_version) {
+  std::string images_snapshot;
+  const int64_t snapshot_size = (snapshotSize() > 0) ? snapshotSize() : kMaxSnapshotSize;
+  if (!fetcher.fetchLatestRole(&images_snapshot, snapshot_size, RepositoryType::Image(), Role::Snapshot())) {
+    return false;
+  }
+  const int remote_version = extractVersionUntrusted(images_snapshot);
+
+  // 6. Check that each Targets metadata filename listed in the previous Snapshot metadata file is also listed in this
+  // Snapshot metadata file. If this condition is not met, discard the new Snapshot metadata file, abort the update
+  // cycle, and report the failure. (Checks for a rollback attack.)
+  // Let's wait for this ticket resolution https://github.com/uptane/uptane-standard/issues/149
+  // https://saeljira.it.here.com/browse/OTA-4121
+  if (!verifySnapshot(images_snapshot)) {
+    return false;
+  }
+
+  if (local_version > remote_version) {
+    return false;
+  } else if (local_version < remote_version) {
+    storage.storeNonRoot(images_snapshot, RepositoryType::Image(), Role::Snapshot());
+  }
+
+  return true;
+}
+
 bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) {
   try {
     const std::string canonical = Utils::jsonToCanonicalStr(Utils::parseJSON(snapshot_raw));
@@ -63,6 +89,32 @@ bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) {
   return true;
 }
 
+bool ImagesRepository::fetchTargets(INvStorage& storage, const IMetadataFetcher& fetcher, const int local_version) {
+  std::string images_targets;
+  const Role targets_role = Role::Targets();
+
+  auto targets_size = getRoleSize(Role::Targets());
+  if (targets_size <= 0) {
+    targets_size = kMaxImagesTargetsSize;
+  }
+  if (!fetcher.fetchLatestRole(&images_targets, targets_size, RepositoryType::Image(), targets_role)) {
+    return false;
+  }
+  const int remote_version = extractVersionUntrusted(images_targets);
+
+  if (!verifyTargets(images_targets)) {
+    return false;
+  }
+
+  if (local_version > remote_version) {
+    return false;
+  } else if (local_version < remote_version) {
+    storage.storeNonRoot(images_targets, RepositoryType::Image(), targets_role);
+  }
+
+  return true;
+}
+
 bool ImagesRepository::verifyRoleHashes(const std::string& role_data, const Uptane::Role& role) const {
   const std::string canonical = Utils::jsonToCanonicalStr(Utils::parseJSON(role_data));
   // Hashes are not required. If present, however, we may as well check them.
@@ -226,35 +278,25 @@ bool ImagesRepository::updateMeta(INvStorage& storage, const IMetadataFetcher& f
 
   // Update Images Snapshot Metadata
   {
-    std::string images_snapshot;
-
-    int64_t snapshot_size = (snapshotSize() > 0) ? snapshotSize() : kMaxSnapshotSize;
-    if (!fetcher.fetchLatestRole(&images_snapshot, snapshot_size, RepositoryType::Image(), Role::Snapshot())) {
-      return false;
-    }
-    int remote_version = extractVersionUntrusted(images_snapshot);
-
+    // First check if we already have the latest version according to the
+    // Timestamp metadata.
+    bool fetch_snapshot = true;
     int local_version;
     std::string images_snapshot_stored;
     if (storage.loadNonRoot(&images_snapshot_stored, RepositoryType::Image(), Role::Snapshot())) {
-      local_version = extractVersionUntrusted(images_snapshot_stored);
+      if (verifySnapshot(images_snapshot_stored)) {
+        fetch_snapshot = false;
+      }
+      local_version = snapshot.version();
     } else {
       local_version = -1;
     }
 
-    // 6. Check that each Targets metadata filename listed in the previous Snapshot metadata file is also listed in this
-    // Snapshot metadata file. If this condition is not met, discard the new Snapshot metadata file, abort the update
-    // cycle, and report the failure. (Checks for a rollback attack.)
-    // Let's wait for this ticket resolution https://github.com/uptane/uptane-standard/issues/149
-    // https://saeljira.it.here.com/browse/OTA-4121
-    if (!verifySnapshot(images_snapshot)) {
-      return false;
-    }
-
-    if (local_version > remote_version) {
-      return false;
-    } else if (local_version < remote_version) {
-      storage.storeNonRoot(images_snapshot, RepositoryType::Image(), Role::Snapshot());
+    // If we don't, attempt to fetch the latest.
+    if (fetch_snapshot) {
+      if (!fetchSnapshot(storage, fetcher, local_version)) {
+        return false;
+      }
     }
 
     if (snapshotExpired()) {
@@ -264,34 +306,25 @@ bool ImagesRepository::updateMeta(INvStorage& storage, const IMetadataFetcher& f
 
   // Update Images Targets Metadata
   {
-    std::string images_targets;
-    Role targets_role = Role::Targets();
-
-    auto targets_size = getRoleSize(Role::Targets());
-    if (targets_size <= 0) {
-      targets_size = kMaxImagesTargetsSize;
-    }
-    if (!fetcher.fetchLatestRole(&images_targets, targets_size, RepositoryType::Image(), targets_role)) {
-      return false;
-    }
-    int remote_version = extractVersionUntrusted(images_targets);
-
+    // First check if we already have the latest version according to the
+    // Snapshot metadata.
+    bool fetch_targets = true;
     int local_version;
     std::string images_targets_stored;
-    if (storage.loadNonRoot(&images_targets_stored, RepositoryType::Image(), targets_role)) {
-      local_version = extractVersionUntrusted(images_targets_stored);
+    if (storage.loadNonRoot(&images_targets_stored, RepositoryType::Image(), Role::Targets())) {
+      if (verifyTargets(images_targets_stored)) {
+        fetch_targets = false;
+      }
+      local_version = targets->version();
     } else {
       local_version = -1;
     }
 
-    if (!verifyTargets(images_targets)) {
-      return false;
-    }
-
-    if (local_version > remote_version) {
-      return false;
-    } else if (local_version < remote_version) {
-      storage.storeNonRoot(images_targets, RepositoryType::Image(), targets_role);
+    // If we don't, attempt to fetch the latest.
+    if (fetch_targets) {
+      if (!fetchTargets(storage, fetcher, local_version)) {
+        return false;
+      }
     }
 
     if (targetsExpired()) {

src/libaktualizr/uptane/imagesrepository.h

@@ -40,6 +40,9 @@ class ImagesRepository : public RepositoryCommon {
   bool updateMeta(INvStorage& storage, const IMetadataFetcher& fetcher) override;
 
  private:
+  bool fetchSnapshot(INvStorage& storage, const IMetadataFetcher& fetcher, int local_version);
+  bool fetchTargets(INvStorage& storage, const IMetadataFetcher& fetcher, int local_version);
+
   std::shared_ptr<Uptane::Targets> targets;
   Uptane::TimestampMeta timestamp;
   Uptane::Snapshot snapshot;