From 71ec21b0fee77207da6d2566541849555f08eda8 Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Fri, 20 Dec 2019 16:53:28 +0100
Subject: [PATCH 01/11] Refactor and unify root metadata update process.

It is essentially identical for the Director and Image repos, so there
is no need to duplicate the code.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 src/libaktualizr/uptane/directorrepository.cc | 63 +----------------
 src/libaktualizr/uptane/imagesrepository.cc   | 54 +-------------
 src/libaktualizr/uptane/uptanerepository.cc   | 70 ++++++++++++++++++-
 src/libaktualizr/uptane/uptanerepository.h    |  2 +
 4 files changed, 75 insertions(+), 114 deletions(-)

diff --git a/src/libaktualizr/uptane/directorrepository.cc b/src/libaktualizr/uptane/directorrepository.cc
index d083b5d6a4..9ad2e7276f 100644
--- 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/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc
index d8fc3d8919..2e9e232f4d 100644
--- 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/uptanerepository.cc b/src/libaktualizr/uptane/uptanerepository.cc
index 6602eb55cf..fabbd51344 100644
--- a/src/libaktualizr/uptane/uptanerepository.cc
+++ b/src/libaktualizr/uptane/uptanerepository.cc
@@ -32,7 +32,7 @@ bool RepositoryCommon::initRoot(const std::string& root_raw) {
 
 bool RepositoryCommon::verifyRoot(const std::string& root_raw) {
   try {
-    int prev_version = root.version();
+    int prev_version = rootVersion();
     root = Root(type, Utils::parseJSON(root_raw), root);  // double signature verification
     if (root.version() != prev_version + 1) {
       LOG_ERROR << "Version in root metadata doesn't match the expected value";
@@ -47,6 +47,74 @@ bool RepositoryCommon::verifyRoot(const std::string& root_raw) {
 
 void RepositoryCommon::resetRoot() { root = Root(Root::Policy::kAcceptAll); }
 
+bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& fetcher,
+                                  const RepositoryType repo_type) {
+  // Load current (or initial) Root metadata.
+  {
+    std::string root_raw;
+    if (storage.loadLatestRoot(&root_raw, repo_type)) {
+      if (!initRoot(root_raw)) {
+        return false;
+      }
+    } else {
+      if (!fetcher.fetchRole(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(1))) {
+        return false;
+      }
+      if (!initRoot(root_raw)) {
+        return false;
+      }
+      storage.storeRoot(root_raw, repo_type, Version(1));
+    }
+  }
+
+  // Update to latest 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 root_raw;
+    if (!fetcher.fetchLatestRole(&root_raw, kMaxRootSize, repo_type, Role::Root())) {
+      return false;
+    }
+    int remote_version = extractVersionUntrusted(root_raw);
+    if (remote_version == -1) {
+      LOG_ERROR << "Failed to extract a version from " << repo_type.toString() << "'s root.json: " << root_raw;
+      return false;
+    }
+
+    int local_version = rootVersion();
+
+    // if remote_version <= local_version then the 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(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(version))) {
+        return false;
+      }
+
+      if (!verifyRoot(root_raw)) {
+        return false;
+      }
+      storage.storeRoot(root_raw, repo_type, Version(version));
+      storage.clearNonRootMeta(repo_type);
+    }
+
+    // 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;
+    }
+  }
+
+  return true;
+}
+
 Json::Value Manifest::signManifest(const Json::Value& manifest_unsigned) const {
   Json::Value manifest = keys_.signTuf(manifest_unsigned);
   return manifest;
diff --git a/src/libaktualizr/uptane/uptanerepository.h b/src/libaktualizr/uptane/uptanerepository.h
index 118a41fd14..ee9a3898d5 100644
--- a/src/libaktualizr/uptane/uptanerepository.h
+++ b/src/libaktualizr/uptane/uptanerepository.h
@@ -47,6 +47,8 @@ class RepositoryCommon {
 
  protected:
   void resetRoot();
+  bool updateRoot(INvStorage &storage, const IMetadataFetcher &fetcher, RepositoryType repo_type);
+
   Root root;
   RepositoryType type;
 };

From 27dd181734d7c9bd16f4f34dbad6b955642658d9 Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Mon, 23 Dec 2019 12:02:05 +0100
Subject: [PATCH 02/11] Check and fetch root metadata according to the
 standard.

Do not check for an unnumbered version of the root metadata. Just check
for the next available version and move on if it is not found.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 src/libaktualizr/uptane/fetcher.cc          |  1 -
 src/libaktualizr/uptane/uptane_test.cc      | 18 +++++-
 src/libaktualizr/uptane/uptanerepository.cc | 64 ++++++++-------------
 3 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/src/libaktualizr/uptane/fetcher.cc b/src/libaktualizr/uptane/fetcher.cc
index e492c7ee58..ebf38b9a6d 100644
--- 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/uptane_test.cc b/src/libaktualizr/uptane/uptane_test.cc
index 6dd9e4d158..efeff81c06 100644
--- a/src/libaktualizr/uptane/uptane_test.cc
+++ b/src/libaktualizr/uptane/uptane_test.cc
@@ -1190,8 +1190,6 @@ 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;
@@ -1199,6 +1197,11 @@ class HttpFakeUnstable : public HttpFake {
     }
   }
 
+  void setUnstableValidNum(int num) {
+    unstable_valid_num = num;
+    unstable_valid_count = 0;
+  }
+
   int unstable_valid_num{0};
   int unstable_valid_count{0};
 };
@@ -1208,7 +1211,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 +1238,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()));
 }
diff --git a/src/libaktualizr/uptane/uptanerepository.cc b/src/libaktualizr/uptane/uptanerepository.cc
index fabbd51344..f266a0f0a4 100644
--- a/src/libaktualizr/uptane/uptanerepository.cc
+++ b/src/libaktualizr/uptane/uptanerepository.cc
@@ -33,7 +33,15 @@ bool RepositoryCommon::initRoot(const std::string& root_raw) {
 bool RepositoryCommon::verifyRoot(const std::string& root_raw) {
   try {
     int prev_version = rootVersion();
+    // 5.4.4.3.2.3. Version N+1 of the Root metadata file MUST have been signed
+    // by the following: (1) a threshold of keys specified in the latest Root
+    // metadata file (version N), and (2) a threshold of keys specified in the
+    // new Root metadata file being validated (version N+1).
     root = Root(type, Utils::parseJSON(root_raw), root);  // double signature verification
+    // 5.4.4.3.2.4. The version number of the latest Root metadata file (version
+    // N) must be less than or equal to the version number of the new Root
+    // metadata file (version N+1). NOTE: we do not accept an equal version
+    // number. It must increment.
     if (root.version() != prev_version + 1) {
       LOG_ERROR << "Version in root metadata doesn't match the expected value";
       return false;
@@ -49,7 +57,7 @@ void RepositoryCommon::resetRoot() { root = Root(Root::Policy::kAcceptAll); }
 
 bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& fetcher,
                                   const RepositoryType repo_type) {
-  // Load current (or initial) Root metadata.
+  // 5.4.4.3.1. Load the previous Root metadata file.
   {
     std::string root_raw;
     if (storage.loadLatestRoot(&root_raw, repo_type)) {
@@ -67,52 +75,28 @@ bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& f
     }
   }
 
-  // Update to latest 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.
-
+  // 5.4.4.3.2. Update to the latest Root metadata file.
+  for (int version = rootVersion() + 1;; ++version) {
+    // 5.4.4.3.2.2. Try downloading a new version N+1 of the Root metadata file.
     std::string root_raw;
-    if (!fetcher.fetchLatestRole(&root_raw, kMaxRootSize, repo_type, Role::Root())) {
-      return false;
-    }
-    int remote_version = extractVersionUntrusted(root_raw);
-    if (remote_version == -1) {
-      LOG_ERROR << "Failed to extract a version from " << repo_type.toString() << "'s root.json: " << root_raw;
-      return false;
+    if (!fetcher.fetchRole(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(version))) {
+      break;
     }
 
-    int local_version = rootVersion();
-
-    // if remote_version <= local_version then the 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(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(version))) {
-        return false;
-      }
-
-      if (!verifyRoot(root_raw)) {
-        return false;
-      }
-      storage.storeRoot(root_raw, repo_type, Version(version));
-      storage.clearNonRootMeta(repo_type);
-    }
-
-    // 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()) {
+    if (!verifyRoot(root_raw)) {
       return false;
     }
+
+    // 5.4.4.3.2.5. Set the latest Root metadata file to the new Root metadata
+    // file.
+    storage.storeRoot(root_raw, repo_type, Version(version));
+    storage.clearNonRootMeta(repo_type);
   }
 
-  return true;
+  // 5.4.4.3.3. 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.)
+  return !rootExpired();
 }
 
 Json::Value Manifest::signManifest(const Json::Value& manifest_unsigned) const {

From 0fd13c38d9b25950f500c0d4054935e8b227c3de Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Mon, 23 Dec 2019 17:01:54 +0100
Subject: [PATCH 03/11] Allow returning a 404 for missing metadata in the
 Python test servers.

Previously they just threw exceptions.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 scripts/testupdate_server.py               |  8 +++++++-
 tests/fake_http_server/fake_test_server.py | 18 +++++++++---------
 tests/test_fixtures.py                     |  4 ++++
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/scripts/testupdate_server.py b/scripts/testupdate_server.py
index 589b680609..e8f8221c04 100755
--- 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/tests/fake_http_server/fake_test_server.py b/tests/fake_http_server/fake_test_server.py
index 291eada86f..9721686912 100755
--- a/tests/fake_http_server/fake_test_server.py
+++ b/tests/fake_http_server/fake_test_server.py
@@ -45,27 +45,29 @@ def _serve_simple(self, uri):
     def serve_meta(self, uri):
         if self.server.meta_path is None:
             raise RuntimeError("Please supply a path for metadata")
-        self._serve_simple(self.server.meta_path + uri)
+        if not os.path.exists(self.server.meta_path + uri):
+            self.send_response(404)
+            self.end_headers()
+        else:
+            self.send_response(200)
+            self.end_headers()
+            self._serve_simple(self.server.meta_path + uri)
 
     def serve_target(self, filename):
         if self.server.target_path is None:
             raise RuntimeError("Please supply a path for targets")
+        self.send_response(200)
+        self.end_headers()
         self._serve_simple(self.server.target_path + filename)
 
     def do_GET(self):
         if self.path.startswith("/director/") and self.path.endswith(".json"):
-            self.send_response(200)
-            self.end_headers()
             role = self.path[len("/director/"):]
             self.serve_meta("/repo/director/" + role)
         elif self.path.startswith("/repo/") and self.path.endswith(".json"):
-            self.send_response(200)
-            self.end_headers()
             role = self.path[len("/repo/"):]
             self.serve_meta('/repo/repo/' + role)
         elif self.path.startswith("/repo/targets"):
-            self.send_response(200)
-            self.end_headers()
             filename = self.path[len("/repo/targets"):]
             self.serve_target(filename)
 
@@ -115,8 +117,6 @@ def do_GET(self):
                 self.wfile.write(b'aa')
                 sleep(1)
         elif self.path == '/campaigner/campaigns':
-            self.send_response(200)
-            self.end_headers()
             self.serve_meta("/campaigns.json")
         elif self.path == '/user_agent':
             user_agent = self.headers.get('user-agent')
diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py
index 61bfb781f0..b94a03b75e 100644
--- a/tests/test_fixtures.py
+++ b/tests/test_fixtures.py
@@ -365,6 +365,10 @@ def default_handler(self):
             self.end_headers()
 
         def default_get(self):
+            if not os.path.exists(self.file_path):
+                self.send_response(404)
+                self.end_headers()
+                return
             self.send_response(200)
             self.end_headers()
             with open(self.file_path, 'rb') as source:

From dea618a1c153794c23fbfaa21bff24ebc74feada Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Fri, 27 Dec 2019 10:47:44 +0100
Subject: [PATCH 04/11] text_fixtures.py: Fix config paths.

It appears to have been dumping an object into the storage field instead
of the actual path.

Also convert the import paths into absolute paths instead of local. This
makes testing and debugging much easier.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 tests/test_fixtures.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py
index b94a03b75e..31bfb240f5 100644
--- a/tests/test_fixtures.py
+++ b/tests/test_fixtures.py
@@ -48,9 +48,11 @@ def __init__(self, aktualizr_primary_exe, aktualizr_info_exe, id,
 
         with open(path.join(self._storage_dir.name, 'config.toml'), 'w+') as config_file:
             config_file.write(Aktualizr.CONFIG_TEMPLATE.format(server_url=uptane_server.base_url,
-                                                               ca_path=ca, pkey_path=pkey, cert_path=cert,
+                                                               ca_path=os.path.abspath(ca),
+                                                               pkey_path=os.path.abspath(pkey),
+                                                               cert_path=os.path.abspath(cert),
                                                                serial=id[1], hw_ID=id[0],
-                                                               storage_dir=self._storage_dir,
+                                                               storage_dir=self._storage_dir.name,
                                                                db_path=path.join(self._storage_dir.name, 'sql.db'),
                                                                log_level=self._log_level,
                                                                secondary_cfg_file=self._secondary_config_file,
@@ -281,7 +283,7 @@ def __init__(self, aktualizr_secondary_exe, id, port=9050, primary_port=9040):
         with open(path.join(self._storage_dir.name, 'config.toml'), 'w+') as config_file:
             config_file.write(IPSecondary.CONFIG_TEMPLATE.format(serial=id[1], hw_ID=id[0],
                                                                  port=self.port, primary_port=self.primary_port,
-                                                                 storage_dir=self._storage_dir,
+                                                                 storage_dir=self._storage_dir.name,
                                                                  db_path=path.join(self._storage_dir.name, 'db.sql')))
             self._config_file = config_file.name
 
@@ -300,7 +302,6 @@ def __init__(self, aktualizr_secondary_exe, id, port=9050, primary_port=9040):
     path = "{storage_dir}"
     sqldb_path = "{db_path}"
 
-
     [pacman]
     type = "fake"
     '''

From cf01e89a43e515cdeae565daf953b55d0af11fed Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Fri, 27 Dec 2019 12:59:54 +0100
Subject: [PATCH 05/11] Fix Secondary root metadata fetching logic.

It now needs to respect versions.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 .../aktualizr_secondary_metadata.cc           | 26 ++++++++++++++-----
 .../aktualizr_secondary_metadata.h            |  5 +++-
 .../uptane_verification_test.cc               |  6 ++---
 src/libaktualizr/uptane/tuf.h                 |  2 ++
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/aktualizr_secondary/aktualizr_secondary_metadata.cc b/src/aktualizr_secondary/aktualizr_secondary_metadata.cc
index 8dcb466029..7b3fd14866 100644
--- 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
index a199b965b4..bbed5d108a 100644
--- 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
index 3d0c42d354..915bf1ed56 100644
--- 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/tuf.h b/src/libaktualizr/uptane/tuf.h
index c12567f0fa..af90567768 100644
--- 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;

From 13dd90a2d19c5d8ddf9f125b8f63c8343afb8cf3 Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Fri, 27 Dec 2019 14:47:36 +0100
Subject: [PATCH 06/11] test_backend_failure.py: Don't expect root.json to get
 fetched.

aktualizr no longer tries an unnumbered version.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 tests/test_backend_failure.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/test_backend_failure.py b/tests/test_backend_failure.py
index dec669e049..5ce09e1d96 100755
--- a/tests/test_backend_failure.py
+++ b/tests/test_backend_failure.py
@@ -26,7 +26,7 @@
 https://saeljira.it.here.com/browse/OTA-3730
 """
 @with_uptane_backend(start_generic_server=True)
-@with_path(paths=['/1.root.json', '/root.json', '/targets.json'])
+@with_path(paths=['/1.root.json', '/targets.json'])
 @with_director(handlers=[
                             DownloadInterruptionHandler(number_of_failures=1),
                             MalformedJsonHandler(number_of_failures=1),
@@ -55,7 +55,7 @@ def test_backend_failure_sanity_director_update_after_metadata_download_failure(
 Note: Aktualizr doesn't send any installation report in manifest in case of metadata download failure
 """
 @with_uptane_backend(start_generic_server=True)
-@with_path(paths=['/1.root.json', '/root.json', '/timestamp.json', '/snapshot.json', '/targets.json'])
+@with_path(paths=['/1.root.json', '/timestamp.json', '/snapshot.json', '/targets.json'])
 @with_imagerepo(handlers=[
                             DownloadInterruptionHandler(number_of_failures=1),
                             MalformedJsonHandler(number_of_failures=1),
@@ -251,7 +251,7 @@ def test_backend_failure_sanity_customrepo_unsuccessful_update_redirect(aktualiz
   - malformed json is received
 """
 @with_uptane_backend(start_generic_server=True)
-@with_path(paths=['/1.root.json', '/root.json', '/targets.json'])
+@with_path(paths=['/1.root.json', '/targets.json'])
 @with_imagerepo()
 @with_director(handlers=[
                             DownloadInterruptionHandler(number_of_failures=3),
@@ -271,7 +271,7 @@ def test_backend_failure_sanity_director_unsuccessful_download(install_mngr, akt
   - malformed json is received
 """
 @with_uptane_backend(start_generic_server=True)
-@with_path(paths=['/1.root.json', '/root.json', '/timestamp.json', '/snapshot.json', '/targets.json'])
+@with_path(paths=['/1.root.json', '/timestamp.json', '/snapshot.json', '/targets.json'])
 @with_imagerepo(handlers=[
                             DownloadInterruptionHandler(number_of_failures=3),
                             MalformedJsonHandler(number_of_failures=1),

From 57d66615261b955c52ac03981a72b0910cb4676d Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Mon, 23 Dec 2019 16:44:40 +0100
Subject: [PATCH 07/11] Bump tuf-test-vectors to fix root fetching logic.

New tests should cover the root version checking.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 tests/tuf-test-vectors | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tuf-test-vectors b/tests/tuf-test-vectors
index 70c7b23369..4661f6c377 160000
--- a/tests/tuf-test-vectors
+++ b/tests/tuf-test-vectors
@@ -1 +1 @@
-Subproject commit 70c7b23369b4164912d2672574355598a3e4ebdf
+Subproject commit 4661f6c377e52f6dc9ab80f00889c4151f215f3c

From b322fa5a7f9188b399f14bd73c3cb2d3423778b0 Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Mon, 30 Dec 2019 16:47:18 +0100
Subject: [PATCH 08/11] Fetch unnumbered Director root once as a workaround.

This is a hack, but currently the backend expects that the unnumbered
root.json is fetched once. Otherwise a device is not considered
"online". Once that requirement is lifted, this commit should be
reverted.

Also at that time, uptane-generator should be amended to stop creating
an unnumbered root.json.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 src/libaktualizr/uptane/uptane_test.cc      | 6 +++++-
 src/libaktualizr/uptane/uptanerepository.cc | 8 ++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/libaktualizr/uptane/uptane_test.cc b/src/libaktualizr/uptane/uptane_test.cc
index efeff81c06..1461882ede 100644
--- a/src/libaktualizr/uptane/uptane_test.cc
+++ b/src/libaktualizr/uptane/uptane_test.cc
@@ -1192,7 +1192,11 @@ class HttpFakeUnstable : public HttpFake {
     if (unstable_valid_count >= unstable_valid_num) {
       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);
     }
   }
diff --git a/src/libaktualizr/uptane/uptanerepository.cc b/src/libaktualizr/uptane/uptanerepository.cc
index f266a0f0a4..dbd8cbbf68 100644
--- a/src/libaktualizr/uptane/uptanerepository.cc
+++ b/src/libaktualizr/uptane/uptanerepository.cc
@@ -65,6 +65,14 @@ bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& f
         return false;
       }
     } else {
+      // This block is a hack only required as long as we have to explicitly
+      // fetch this to make the Director recognize new devices as "online".
+      if (repo_type == RepositoryType::Director()) {
+        if (!fetcher.fetchLatestRole(&root_raw, kMaxRootSize, repo_type, Role::Root())) {
+          return false;
+        }
+      }
+
       if (!fetcher.fetchRole(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(1))) {
         return false;
       }

From a3a4c5d3ad7ae742e2b8647e3cab83ac7442791b Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Thu, 2 Jan 2020 16:56:27 +0100
Subject: [PATCH 09/11] Copy test credentials to temporary directory before
 using them.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 tests/ipsecondary_test.py |  4 ++--
 tests/test_fixtures.py    | 26 ++++++++++++++++----------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/tests/ipsecondary_test.py b/tests/ipsecondary_test.py
index cfb3bd144b..b33c579f57 100755
--- a/tests/ipsecondary_test.py
+++ b/tests/ipsecondary_test.py
@@ -3,7 +3,7 @@
 import logging
 import argparse
 
-from os import getcwd, chdir
+from os import getcwd, chdir, path
 
 from test_fixtures import with_aktualizr, with_uptane_backend, KeyStore, with_secondary
 
@@ -152,7 +152,7 @@ def test_primary_timeout_after_device_is_registered(uptane_repo, secondary, aktu
     parser.add_argument('-s', '--src-dir', help='source directory', default='.')
     input_params = parser.parse_args()
 
-    KeyStore.base_dir = input_params.src_dir
+    KeyStore.base_dir = path.abspath(input_params.src_dir)
     initial_cwd = getcwd()
     chdir(input_params.build_dir)
 
diff --git a/tests/test_fixtures.py b/tests/test_fixtures.py
index 31bfb240f5..c3f2b01fee 100644
--- a/tests/test_fixtures.py
+++ b/tests/test_fixtures.py
@@ -26,18 +26,19 @@
 class Aktualizr:
 
     def __init__(self, aktualizr_primary_exe, aktualizr_info_exe, id,
-                 uptane_server, ca, pkey, cert, wait_port=9040, wait_timeout=60, log_level=1,
+                 uptane_server, wait_port=9040, wait_timeout=60, log_level=1,
                  secondary=None, output_logs=True,
                  run_mode='once', director=None, image_repo=None,
                  sysroot=None, treehub=None, ostree_mock_path=None, **kwargs):
         self.id = id
-
         self._aktualizr_primary_exe = aktualizr_primary_exe
         self._aktualizr_info_exe = aktualizr_info_exe
         self._storage_dir = tempfile.TemporaryDirectory()
         self._log_level = log_level
         self._sentinel_file = 'need_reboot'
         self.reboot_sentinel_file = os.path.join(self._storage_dir.name, self._sentinel_file)
+        self._import_dir = os.path.join(self._storage_dir.name, 'import')
+        KeyStore.copy_keys(self._import_dir)
 
         with open(path.join(self._storage_dir.name, 'secondary_config.json'), 'w+') as secondary_config_file:
             secondary_cfg = json.loads(Aktualizr.SECONDARY_CONFIG_TEMPLATE.
@@ -48,9 +49,7 @@ def __init__(self, aktualizr_primary_exe, aktualizr_info_exe, id,
 
         with open(path.join(self._storage_dir.name, 'config.toml'), 'w+') as config_file:
             config_file.write(Aktualizr.CONFIG_TEMPLATE.format(server_url=uptane_server.base_url,
-                                                               ca_path=os.path.abspath(ca),
-                                                               pkey_path=os.path.abspath(pkey),
-                                                               cert_path=os.path.abspath(cert),
+                                                               import_path=self._import_dir,
                                                                serial=id[1], hw_ID=id[0],
                                                                storage_dir=self._storage_dir.name,
                                                                db_path=path.join(self._storage_dir.name, 'sql.db'),
@@ -78,9 +77,10 @@ def __init__(self, aktualizr_primary_exe, aktualizr_info_exe, id,
     server = "{server_url}"
 
     [import]
-    tls_cacert_path = "{ca_path}"
-    tls_pkey_path = "{pkey_path}"
-    tls_clientcert_path = "{cert_path}"
+    base_path = "{import_path}"
+    tls_cacert_path = "ca.pem"
+    tls_pkey_path = "pkey.pem"
+    tls_clientcert_path = "client.pem"
 
     [provision]
     primary_ecu_serial = "{serial}"
@@ -256,6 +256,13 @@ def emulate_reboot(self):
 class KeyStore:
     base_dir = "./"
 
+    @staticmethod
+    def copy_keys(dest_path):
+        os.mkdir(dest_path)
+        shutil.copy(KeyStore.ca(), dest_path)
+        shutil.copy(KeyStore.pkey(), dest_path)
+        shutil.copy(KeyStore.cert(), dest_path)
+
     @staticmethod
     def ca():
         return path.join(KeyStore.base_dir, 'tests/test_data/prov_testupdate/ca.pem')
@@ -733,8 +740,7 @@ def decorator(test):
         @wraps(test)
         def wrapper(*args, ostree_mock_path=None, **kwargs):
             aktualizr = Aktualizr(aktualizr_primary_exe=aktualizr_primary_exe,
-                                  aktualizr_info_exe=aktualizr_info_exe,
-                                  id=id, ca=KeyStore.ca(), pkey=KeyStore.pkey(), cert=KeyStore.cert(),
+                                  aktualizr_info_exe=aktualizr_info_exe, id=id,
                                   wait_timeout=wait_timeout, log_level=log_level, output_logs=output_logs,
                                   run_mode=run_mode, ostree_mock_path=ostree_mock_path, **kwargs)
             if start:

From 0419df658f1ff1a1c9c426467a3722b6528df6e4 Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Thu, 2 Jan 2020 16:56:53 +0100
Subject: [PATCH 10/11] Improve python test output readability.

Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 tests/ipsecondary_test.py     |  2 +-
 tests/test_aktualizr_kill.py  |  2 +-
 tests/test_backend_failure.py |  2 +-
 tests/test_update.py          | 12 ++++++------
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/ipsecondary_test.py b/tests/ipsecondary_test.py
index b33c579f57..72d3e1d38d 100755
--- a/tests/ipsecondary_test.py
+++ b/tests/ipsecondary_test.py
@@ -166,7 +166,7 @@ def test_primary_timeout_after_device_is_registered(uptane_repo, secondary, aktu
     for test in test_suite:
         logger.info('>>> Running {}...'.format(test.__name__))
         test_run_result = test()
-        logger.info('>>> {}: {}'.format('OK' if test_run_result else 'Failed', test.__name__))
+        logger.info('>>> {}: {}\n'.format('OK' if test_run_result else 'FAILED', test.__name__))
         test_suite_run_result = test_suite_run_result and test_run_result
 
     chdir(initial_cwd)
diff --git a/tests/test_aktualizr_kill.py b/tests/test_aktualizr_kill.py
index 0d0f2b79b7..248284b9d1 100755
--- a/tests/test_aktualizr_kill.py
+++ b/tests/test_aktualizr_kill.py
@@ -53,7 +53,7 @@ def test_aktualizr_kill(director, aktualizr, **kwargs):
     for test in test_suite:
         logger.info('>>> Running {}...'.format(test.__name__))
         test_run_result = test()
-        logger.info('>>> {}: {}'.format('OK' if test_run_result else 'Failed', test.__name__))
+        logger.info('>>> {}: {}\n'.format('OK' if test_run_result else 'FAILED', test.__name__))
         test_suite_run_result = test_suite_run_result and test_run_result
 
     chdir(initial_cwd)
diff --git a/tests/test_backend_failure.py b/tests/test_backend_failure.py
index 5ce09e1d96..8791df011b 100755
--- a/tests/test_backend_failure.py
+++ b/tests/test_backend_failure.py
@@ -317,7 +317,7 @@ def test_backend_failure_sanity_imagerepo_unsuccessful_download(install_mngr, ak
     for test in test_suite:
         logger.info('>>> Running {}...'.format(test.__name__))
         test_run_result = test()
-        logger.info('>>> {}: {}'.format('OK' if test_run_result else 'Failed', test.__name__))
+        logger.info('>>> {}: {}\n'.format('OK' if test_run_result else 'FAILED', test.__name__))
         test_suite_run_result = test_suite_run_result and test_run_result
 
     chdir(initial_cwd)
diff --git a/tests/test_update.py b/tests/test_update.py
index ffcfe1328e..f2fb11c102 100755
--- a/tests/test_update.py
+++ b/tests/test_update.py
@@ -14,11 +14,11 @@
 
 """
  Test update of Primary and Secondary if their package manager differs, `ostree` and `fake` respectively
- 
+
  Aktualizr/Primary's package manager is set to `ostree`
  Secondary's package manager is set to `fake`
  Primary goal is to verify whether aktualizr succeeds with a binary/fake update of secondary
- while aktualizr/primary is configured with ostree package manager   
+ while aktualizr/primary is configured with ostree package manager
 """
 @with_uptane_backend(start_generic_server=True)
 @with_director()
@@ -40,14 +40,14 @@ def test_primary_ostree_secondary_fake_updates(uptane_repo, secondary, aktualizr
     # check the Primary update, must be in pending state since it requires reboot
     pending_rev = aktualizr.get_primary_pending_version()
     if pending_rev != target_rev:
-        logger.error("Pending version {} != the target one {}".format(pending_rev, target_rev))
+        logger.error("Pending version {} != the target version {}".format(pending_rev, target_rev))
         return False
 
     # check the Secondary update
     current_secondary_image_hash = aktualizr.get_current_image_info(secondary.id)
     if current_secondary_image_hash != secondary_update_hash:
-        logger.error("Secondary current image {} != {} expected one".format(current_secondary_image_hash,
-                                                                            secondary_update_hash))
+        logger.error("Current secondary image {} != expected image {}".format(current_secondary_image_hash,
+                                                                              secondary_update_hash))
         return False
 
     # emulate reboot and run aktualizr once more
@@ -83,7 +83,7 @@ def test_primary_ostree_secondary_fake_updates(uptane_repo, secondary, aktualizr
     for test in test_suite:
         logger.info('>>> Running {}...'.format(test.__name__))
         test_run_result = test()
-        logger.info('>>> {}: {}'.format('OK' if test_run_result else 'Failed', test.__name__))
+        logger.info('>>> {}: {}\n'.format('OK' if test_run_result else 'FAILED', test.__name__))
         test_suite_run_result = test_suite_run_result and test_run_result
 
     chdir(initial_cwd)

From ff053f9820611c7a54ca5baadccef61d35445c4d Mon Sep 17 00:00:00 2001
From: Patrick Vacek <patrickvacek@gmail.com>
Date: Mon, 6 Jan 2020 14:02:26 +0100
Subject: [PATCH 11/11] Set an upper bound of allowed root rotations to 1000.

The idea is to try to better handle the scenario where the server keeps
unceasingly providing valid metadata, which could be a sort of bizarre
attack.

Suggested-by: Laurent Bonnans <laurent.bonnans@here.com>
Signed-off-by: Patrick Vacek <patrickvacek@gmail.com>
---
 src/libaktualizr/uptane/uptanerepository.cc | 2 +-
 src/libaktualizr/uptane/uptanerepository.h  | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libaktualizr/uptane/uptanerepository.cc b/src/libaktualizr/uptane/uptanerepository.cc
index dbd8cbbf68..b9727861a3 100644
--- a/src/libaktualizr/uptane/uptanerepository.cc
+++ b/src/libaktualizr/uptane/uptanerepository.cc
@@ -84,7 +84,7 @@ bool RepositoryCommon::updateRoot(INvStorage& storage, const IMetadataFetcher& f
   }
 
   // 5.4.4.3.2. Update to the latest Root metadata file.
-  for (int version = rootVersion() + 1;; ++version) {
+  for (int version = rootVersion() + 1; version < kMaxRotations; ++version) {
     // 5.4.4.3.2.2. Try downloading a new version N+1 of the Root metadata file.
     std::string root_raw;
     if (!fetcher.fetchRole(&root_raw, kMaxRootSize, repo_type, Role::Root(), Version(version))) {
diff --git a/src/libaktualizr/uptane/uptanerepository.h b/src/libaktualizr/uptane/uptanerepository.h
index ee9a3898d5..12395db2f9 100644
--- a/src/libaktualizr/uptane/uptanerepository.h
+++ b/src/libaktualizr/uptane/uptanerepository.h
@@ -49,6 +49,8 @@ class RepositoryCommon {
   void resetRoot();
   bool updateRoot(INvStorage &storage, const IMetadataFetcher &fetcher, RepositoryType repo_type);
 
+  static const int64_t kMaxRotations = 1000;
+
   Root root;
   RepositoryType type;
 };