Merge pull request #1501 from advancedtelematic/fix/OTA-4119/OTA-4132…

…/root-verification Fix/ota 4119/ota 4132/root verification
advancedtelematic · Jan 6, 2020 · 77df64c · 77df64c
2 parents 2af8b96 + ff053f9
commit 77df64c
Show file tree

Hide file tree

Showing 18 changed files with 172 additions and 166 deletions.
diff --git a/scripts/testupdate_server.py b/scripts/testupdate_server.py
@@ -1,5 +1,6 @@
 #!/usr/bin/python3
 
+import os.path
 import sys
 import socket
 from http.server import BaseHTTPRequestHandler, HTTPServer
@@ -9,9 +10,14 @@ class Handler(BaseHTTPRequestHandler):
     def do_GET(self):
         local_path = self.path
         print("GET: " + local_path)
+        full_path = self.server.base_dir + "/fake_root/repo/" + local_path
+        if not os.path.exists(full_path):
+            self.send_response(404)
+            self.end_headers()
+            return
         self.send_response(200)
         self.end_headers()
-        with open(self.server.base_dir + "/fake_root/repo/" + local_path, "rb") as fl:
+        with open(full_path, "rb") as fl:
             self.wfile.write(bytearray(fl.read()))
 
     def do_POST(self):

diff --git a/src/aktualizr_secondary/aktualizr_secondary_metadata.cc b/src/aktualizr_secondary/aktualizr_secondary_metadata.cc
@@ -8,24 +8,26 @@ Metadata::Metadata(const Uptane::RawMetaPack& meta_pack)
           {Uptane::Role::TIMESTAMP, meta_pack.image_timestamp},
           {Uptane::Role::SNAPSHOT, meta_pack.image_snapshot},
           {Uptane::Role::TARGETS, meta_pack.image_targets},
-      } {}
+      } {
+  director_root_version = Uptane::Version(Uptane::extractVersionUntrusted(meta_pack.director_root));
+  image_root_version = Uptane::Version(Uptane::extractVersionUntrusted(meta_pack.image_root));
+}
 
 bool Metadata::fetchRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo, const Uptane::Role& role,
                          Uptane::Version version) const {
   (void)maxsize;
-  (void)version;
 
-  return getRoleMetadata(result, repo, role);
+  return getRoleMetadata(result, repo, role, version);
 }
 
 bool Metadata::fetchLatestRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo,
                                const Uptane::Role& role) const {
   (void)maxsize;
-  return getRoleMetadata(result, repo, role);
+  return getRoleMetadata(result, repo, role, Uptane::Version());
 }
 
-bool Metadata::getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo,
-                               const Uptane::Role& role) const {
+bool Metadata::getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo, const Uptane::Role& role,
+                               Uptane::Version version) const {
   const std::unordered_map<std::string, std::string>* metadata_map = nullptr;
 
   if (repo == Uptane::RepositoryType::Director()) {
@@ -45,6 +47,18 @@ bool Metadata::getRoleMetadata(std::string* result, const Uptane::RepositoryType
     return false;
   }
 
+  if (role == Uptane::Role::Root() && version != Uptane::Version()) {
+    if (repo == Uptane::RepositoryType::Director() && director_root_version != version) {
+      LOG_DEBUG << "Requested Director root version " << version << " but only version " << director_root_version
+                << " is available.";
+      return false;
+    } else if (repo == Uptane::RepositoryType::Image() && image_root_version != version) {
+      LOG_DEBUG << "Requested Image repo root version " << version << " but only version " << image_root_version
+                << " is available.";
+      return false;
+    }
+  }
+
   *result = found_meta_it->second;
   return true;
 }
diff --git a/src/aktualizr_secondary/aktualizr_secondary_metadata.h b/src/aktualizr_secondary/aktualizr_secondary_metadata.h
@@ -16,11 +16,14 @@ class Metadata : public Uptane::IMetadataFetcher {
                        const Uptane::Role& role) const override;
 
  protected:
-  virtual bool getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo, const Uptane::Role& role) const;
+  virtual bool getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo, const Uptane::Role& role,
+                               Uptane::Version version) const;
 
  private:
   const std::unordered_map<std::string, std::string> _director_metadata;
   const std::unordered_map<std::string, std::string> _image_metadata;
+  Uptane::Version director_root_version;
+  Uptane::Version image_root_version;
 };
 
 #endif  // AKTUALIZR_SECONDARY_METADATA_H_
diff --git a/src/aktualizr_secondary/uptane_verification_test.cc b/src/aktualizr_secondary/uptane_verification_test.cc
@@ -278,9 +278,9 @@ class SecondaryUptaneVerificationTestNegative
                         const Uptane::Role& role)
         : Metadata(valid_metadata), _repo_type(repo), _role(role) {}
 
-    bool getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo,
-                         const Uptane::Role& role) const override {
-      auto return_val = Metadata::getRoleMetadata(result, repo, role);
+    bool getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo, const Uptane::Role& role,
+                         Uptane::Version version) const override {
+      auto return_val = Metadata::getRoleMetadata(result, repo, role, version);
       if (!(_repo_type == repo && _role == role)) {
         return return_val;
       }

diff --git a/src/libaktualizr/uptane/directorrepository.cc b/src/libaktualizr/uptane/directorrepository.cc
@@ -76,68 +76,9 @@ bool DirectorRepository::updateMeta(INvStorage& storage, const IMetadataFetcher&
 
   // reset director repo to initial state before starting UPTANE iteration
   resetMeta();
-  // Load Initial Director Root Metadata
-  {
-    std::string director_root;
-    if (storage.loadLatestRoot(&director_root, RepositoryType::Director())) {
-      if (!initRoot(director_root)) {
-        return false;
-      }
-    } else {
-      if (!fetcher.fetchRole(&director_root, kMaxRootSize, RepositoryType::Director(), Role::Root(), Version(1))) {
-        return false;
-      }
-      if (!initRoot(director_root)) {
-        return false;
-      }
-      storage.storeRoot(director_root, RepositoryType::Director(), Version(1));
-    }
-  }
-
-  // Update Director Root Metadata
-  {
-    // According to the current design root.json without a number is guaranteed to be the latest version.
-    // Therefore we fetch the latest (root.json), and
-    // if it matches what we already have stored, we're good.
-    // If not, then we have to go fetch the missing ones by name/number until we catch up.
 
-    std::string director_root;
-    if (!fetcher.fetchLatestRole(&director_root, kMaxRootSize, RepositoryType::Director(), Role::Root())) {
-      return false;
-    }
-    int remote_version = extractVersionUntrusted(director_root);
-    if (remote_version == -1) {
-      LOG_ERROR << "Failed to extract a version from Director's root.json: " << director_root;
-      return false;
-    }
-
-    int local_version = rootVersion();
-
-    // if remote_version <= local_version then the director's root metadata are never verified
-    // which leads to two issues
-    // 1. At initial stage (just after provisioning) the root metadata from 1.root.json are not verified
-    // 2. If local_version becomes higher than 1, e.g. 2 than a rollback attack is possible since the business logic
-    // here won't return any error as suggested in #4 of
-    // https://uptane.github.io/uptane-standard/uptane-standard.html#check_root
-    // TODO: https://saeljira.it.here.com/browse/OTA-4119
-    for (int version = local_version + 1; version <= remote_version; ++version) {
-      if (!fetcher.fetchRole(&director_root, kMaxRootSize, RepositoryType::Director(), Role::Root(),
-                             Version(version))) {
-        return false;
-      }
-
-      if (!verifyRoot(director_root)) {
-        return false;
-      }
-      storage.storeRoot(director_root, RepositoryType::Director(), Version(version));
-      storage.clearNonRootMeta(RepositoryType::Director());
-    }
-
-    // Check that the current (or latest securely attested) time is lower than the expiration timestamp in the latest
-    // Root metadata file. (Checks for a freeze attack.)
-    if (rootExpired()) {
-      return false;
-    }
+  if (!updateRoot(storage, fetcher, RepositoryType::Director())) {
+    return false;
   }
 
   // Not supported: 3. Download and check the Timestamp metadata file from the Director repository, following the

diff --git a/src/libaktualizr/uptane/fetcher.cc b/src/libaktualizr/uptane/fetcher.cc
@@ -4,7 +4,6 @@ namespace Uptane {
 
 bool Fetcher::fetchRole(std::string* result, int64_t maxsize, RepositoryType repo, const Uptane::Role& role,
                         Version version) const {
-  // TODO: chain-loading root.json
   std::string url = (repo == RepositoryType::Director()) ? director_server : repo_server;
   if (role.IsDelegation()) {
     url += "/delegations";

diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc
@@ -137,59 +137,9 @@ std::shared_ptr<Uptane::Targets> ImagesRepository::verifyDelegation(const std::s
 
 bool ImagesRepository::updateMeta(INvStorage& storage, const IMetadataFetcher& fetcher) {
   resetMeta();
-  // Load Initial Images Root Metadata
-  {
-    std::string images_root;
-    if (storage.loadLatestRoot(&images_root, RepositoryType::Image())) {
-      if (!initRoot(images_root)) {
-        return false;
-      }
-    } else {
-      if (!fetcher.fetchRole(&images_root, kMaxRootSize, RepositoryType::Image(), Role::Root(), Version(1))) {
-        return false;
-      }
-      if (!initRoot(images_root)) {
-        return false;
-      }
-      storage.storeRoot(images_root, RepositoryType::Image(), Version(1));
-    }
-  }
-
-  // Update Image Root Metadata
-  {
-    std::string images_root;
-    if (!fetcher.fetchLatestRole(&images_root, kMaxRootSize, RepositoryType::Image(), Role::Root())) {
-      return false;
-    }
-    int remote_version = extractVersionUntrusted(images_root);
-    if (remote_version == -1) {
-      LOG_ERROR << "Failed to extract a version from Director's root.json: " << images_root;
-      return false;
-    }
-
-    int local_version = rootVersion();
-
-    // if remote_version <= local_version then the director's root metadata are never verified
-    // which leads to two issues
-    // 1. At initial stage (just after provisioning) the root metadata from 1.root.json are not verified
-    // 2. If local_version becomes higher than 1, e.g. 2 than a rollback attack is possible since the business logic
-    // here won't return any error as suggested in #4 of
-    // https://uptane.github.io/uptane-standard/uptane-standard.html#check_root
-    // TODO: https://saeljira.it.here.com/browse/OTA-4119
-    for (int version = local_version + 1; version <= remote_version; ++version) {
-      if (!fetcher.fetchRole(&images_root, kMaxRootSize, RepositoryType::Image(), Role::Root(), Version(version))) {
-        return false;
-      }
-      if (!verifyRoot(images_root)) {
-        return false;
-      }
-      storage.storeRoot(images_root, RepositoryType::Image(), Version(version));
-      storage.clearNonRootMeta(RepositoryType::Image());
-    }
 
-    if (rootExpired()) {
-      return false;
-    }
+  if (!updateRoot(storage, fetcher, RepositoryType::Image())) {
+    return false;
   }
 
   // Update Images Timestamp Metadata

diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h
@@ -119,6 +119,8 @@ class Version {
   explicit Version(int v) : version_(v) {}
   std::string RoleFileName(const Role &role) const;
   int version() const { return version_; }
+  bool operator==(const Version &rhs) const { return version_ == rhs.version_; }
+  bool operator!=(const Version &rhs) const { return version_ != rhs.version_; }
 
  private:
   static const int ANY_VERSION = -1;

diff --git a/src/libaktualizr/uptane/uptane_test.cc b/src/libaktualizr/uptane/uptane_test.cc
@@ -1190,15 +1190,22 @@ class HttpFakeUnstable : public HttpFake {
   HttpFakeUnstable(const boost::filesystem::path &test_dir_in) : HttpFake(test_dir_in, "hasupdates") {}
   HttpResponse get(const std::string &url, int64_t maxsize) override {
     if (unstable_valid_count >= unstable_valid_num) {
-      ++unstable_valid_num;
-      unstable_valid_count = 0;
       return HttpResponse({}, 503, CURLE_OK, "");
     } else {
-      ++unstable_valid_count;
+      // This if is a hack only required as long as we have to explicitly fetch
+      // this to make the Director recognize new devices as "online".
+      if (url.find("director/root.json") == std::string::npos) {
+        ++unstable_valid_count;
+      }
       return HttpFake::get(url, maxsize);
     }
   }
 
+  void setUnstableValidNum(int num) {
+    unstable_valid_num = num;
+    unstable_valid_count = 0;
+  }
+
   int unstable_valid_num{0};
   int unstable_valid_count{0};
 };
@@ -1208,7 +1215,10 @@ class HttpFakeUnstable : public HttpFake {
  * Check metadata from the director.
  * Identify targets for known ECUs.
  * Fetch metadata from the images repo.
- * Check metadata from the images repo. */
+ * Check metadata from the images repo.
+ *
+ * This is a bit fragile because it depends upon a precise number of HTTP get
+ * requests being made. If that changes, this test will need to be adjusted. */
 TEST(Uptane, restoreVerify) {
   TemporaryDirectory temp_dir;
   auto http = std::make_shared<HttpFakeUnstable>(temp_dir.Path());
@@ -1232,32 +1242,38 @@ TEST(Uptane, restoreVerify) {
   EXPECT_FALSE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Director()));
 
   // 2nd attempt, get director root.json
+  http->setUnstableValidNum(1);
   EXPECT_FALSE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Director()));
   EXPECT_FALSE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Director(), Uptane::Role::Targets()));
 
   // 3rd attempt, get director targets.json
+  http->setUnstableValidNum(2);
   EXPECT_FALSE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Director()));
   EXPECT_TRUE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Director(), Uptane::Role::Targets()));
   EXPECT_FALSE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Image()));
 
   // 4th attempt, get images root.json
+  http->setUnstableValidNum(3);
   EXPECT_FALSE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Image()));
   EXPECT_FALSE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Timestamp()));
 
   // 5th attempt, get images timestamp.json
+  http->setUnstableValidNum(4);
   EXPECT_FALSE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Timestamp()));
   EXPECT_FALSE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Snapshot()));
 
   // 6th attempt, get images snapshot.json
+  http->setUnstableValidNum(5);
   EXPECT_FALSE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Snapshot()));
   EXPECT_FALSE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Targets()));
 
   // 7th attempt, get images targets.json, successful iteration
+  http->setUnstableValidNum(6);
   EXPECT_TRUE(sota_client->uptaneIteration(nullptr, nullptr));
   EXPECT_TRUE(storage->loadNonRoot(nullptr, Uptane::RepositoryType::Image(), Uptane::Role::Targets()));
 }