Feat/ota 4118/director targets sanity

src/libaktualizr/primary/metadata_fetch_test.cc

@@ -108,6 +108,7 @@ TEST(Aktualizr, MetadataFetch) {
 
   // Update scheduled with pre-existing image: no need to refetch Image repo
   // Snapshot or Targets metadata.
+  uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
   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()});
@@ -125,6 +126,7 @@ TEST(Aktualizr, MetadataFetch) {
 
   // Delegation added to an existing delegation; update scheduled with
   // pre-existing image: Snapshot must be refetched, but Targets are unchanged.
+  uptane_gen.run({"emptytargets", "--path", meta_dir.PathString()});
   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/*",

src/libaktualizr/primary/sotauptaneclient.cc

@@ -451,6 +451,10 @@ bool SotaUptaneClient::getNewTargets(std::vector<Uptane::Target> *new_targets, u
       Uptane::EcuSerial ecu_serial = ecu.first;
       Uptane::HardwareIdentifier hw_id = ecu.second;
 
+      // 5.4.4.6.8. If checking Targets metadata from the Director repository,
+      // and the ECU performing the verification is the Primary ECU, check that
+      // all listed ECU identifiers correspond to ECUs that are actually present
+      // in the vehicle.
       auto hw_id_known = ecuHwId(ecu_serial);
       if (!hw_id_known) {
         LOG_ERROR << "Unknown ECU ID in Director Targets metadata: " << ecu_serial.ToString();

src/libaktualizr/uptane/directorrepository.cc

@@ -8,7 +8,37 @@ void DirectorRepository::resetMeta() {
   latest_targets = Targets();
 }
 
-bool DirectorRepository::targetsExpired() const { return latest_targets.isExpired(TimeStamp::Now()); }
+bool DirectorRepository::targetsExpired() {
+  if (latest_targets.isExpired(TimeStamp::Now())) {
+    last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Targets().ToString());
+    return true;
+  }
+  return false;
+}
+
+bool DirectorRepository::targetsSanityCheck() {
+  //  5.4.4.6.6. If checking Targets metadata from the Director repository,
+  //  verify that there are no delegations.
+  if (!latest_targets.delegated_role_names_.empty()) {
+    last_exception =
+        Uptane::InvalidMetadata(type.toString(), Role::Targets().ToString(), "Found unexpected delegation.");
+    return false;
+  }
+  //  5.4.4.6.7. If checking Targets metadata from the Director repository,
+  //  check that no ECU identifier is represented more than once.
+  std::set<Uptane::EcuSerial> ecu_ids;
+  for (const auto& target : targets.targets) {
+    for (const auto& ecu : target.ecus()) {
+      if (ecu_ids.find(ecu.first) == ecu_ids.end()) {
+        ecu_ids.insert(ecu.first);
+      } else {
+        last_exception = Uptane::InvalidMetadata(type.toString(), Role::Targets().ToString(), "Found repeated ECU ID.");
+        return false;
+      }
+    }
+  }
+  return true;
+}
 
 bool DirectorRepository::usePreviousTargets() const {
   // Don't store the new targets if they are empty and we've previously received
@@ -63,7 +93,10 @@ bool DirectorRepository::checkMetaOffline(INvStorage& storage) {
     }
 
     if (targetsExpired()) {
-      last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Targets().ToString());
+      return false;
+    }
+
+    if (!targetsSanityCheck()) {
       return false;
     }
   }
@@ -107,16 +140,6 @@ bool DirectorRepository::updateMeta(INvStorage& storage, const IMetadataFetcher&
       local_version = -1;
     }
 
-    // Not supported: If the ECU performing the verification is the Primary ECU, it SHOULD also ensure that the Targets
-    // metadata from the Director repository doesn’t contain any ECU identifiers for ECUs not actually present in the
-    // vehicle.
-
-    // Not supported: The following steps of the Director's Targets metadata verification are missing in
-    // DirectorRepository::updateMeta()
-    //  6. If checking Targets metadata from the Director repository, verify that there are no delegations.
-    //  7. If checking Targets metadata from the Director repository, check that no ECU identifier is represented more
-    //  than once.
-    // TODO: https://saeljira.it.here.com/browse/OTA-4118
     if (!verifyTargets(director_targets)) {
       return false;
     }
@@ -128,7 +151,10 @@ bool DirectorRepository::updateMeta(INvStorage& storage, const IMetadataFetcher&
     }
 
     if (targetsExpired()) {
-      last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Targets().ToString());
+      return false;
+    }
+
+    if (!targetsSanityCheck()) {
       return false;
     }
   }

src/libaktualizr/uptane/directorrepository.h

@@ -21,16 +21,16 @@ class DirectorRepository : public RepositoryCommon {
     return targets.getTargets(ecu_id, hw_id);
   }
   const std::string& getCorrelationId() const { return targets.correlation_id(); }
-  bool targetsExpired() const;
   bool checkMetaOffline(INvStorage& storage);
   void dropTargets(INvStorage& storage);
 
-  Exception getLastException() const { return last_exception; }
   bool updateMeta(INvStorage& storage, const IMetadataFetcher& fetcher) override;
   bool matchTargetsWithImageTargets(const Uptane::Targets& image_targets) const;
 
  private:
   void resetMeta();
+  bool targetsExpired();
+  bool targetsSanityCheck();
   bool usePreviousTargets() const;
 
  private:
@@ -40,7 +40,6 @@ class DirectorRepository : public RepositoryCommon {
   // checking expiration but the most recent non-empty list for everything else.
   Uptane::Targets targets;         // Only empty if we've never received non-empty targets.
   Uptane::Targets latest_targets;  // Can be an empty list.
-  Exception last_exception{"", ""};
 };
 
 }  // namespace Uptane

src/libaktualizr/uptane/exceptions.h

@@ -67,7 +67,7 @@ class ExpiredMetadata : public Exception {
 class InvalidMetadata : public Exception {
  public:
   InvalidMetadata(const std::string& reponame, const std::string& role, const std::string& reason)
-      : Exception(reponame, "The " + role + " metadata failed to parse:" + reason) {}
+      : Exception(reponame, "The " + role + " metadata failed to parse: " + reason) {}
   ~InvalidMetadata() noexcept override = default;
 };
 
@@ -94,14 +94,14 @@ class BadKeyId : public Exception {
 class BadEcuId : public Exception {
  public:
   BadEcuId(const std::string& reponame)
-      : Exception(reponame, "The target had an ECU ID that did not match the client's configured ECU id.") {}
+      : Exception(reponame, "The target had an ECU ID that did not match the client's configured ECU ID.") {}
   ~BadEcuId() noexcept override = default;
 };
 
 class BadHardwareId : public Exception {
  public:
   BadHardwareId(const std::string& reponame)
-      : Exception(reponame, "The target had a hardware ID that did not match the client's configured hardware id.") {}
+      : Exception(reponame, "The target had a hardware ID that did not match the client's configured hardware ID.") {}
   ~BadHardwareId() noexcept override = default;
 };
 

src/libaktualizr/uptane/imagerepository.cc

@@ -22,6 +22,14 @@ bool ImageRepository::verifyTimestamp(const std::string& timestamp_raw) {
   return true;
 }
 
+bool ImageRepository::timestampExpired() {
+  if (timestamp.isExpired(TimeStamp::Now())) {
+    last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Timestamp().ToString());
+    return true;
+  }
+  return false;
+}
+
 bool ImageRepository::fetchSnapshot(INvStorage& storage, const IMetadataFetcher& fetcher, const int local_version) {
   std::string image_snapshot;
   const int64_t snapshot_size = (snapshotSize() > 0) ? snapshotSize() : kMaxSnapshotSize;
@@ -94,6 +102,14 @@ bool ImageRepository::verifySnapshot(const std::string& snapshot_raw, bool prefe
   return true;
 }
 
+bool ImageRepository::snapshotExpired() {
+  if (snapshot.isExpired(TimeStamp::Now())) {
+    last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Snapshot().ToString());
+    return true;
+  }
+  return false;
+}
+
 bool ImageRepository::fetchTargets(INvStorage& storage, const IMetadataFetcher& fetcher, const int local_version) {
   std::string image_targets;
   const Role targets_role = Role::Targets();
@@ -196,6 +212,14 @@ std::shared_ptr<Uptane::Targets> ImageRepository::verifyDelegation(const std::st
   return std::shared_ptr<Uptane::Targets>(nullptr);
 }
 
+bool ImageRepository::targetsExpired() {
+  if (targets->isExpired(TimeStamp::Now())) {
+    last_exception = Uptane::ExpiredMetadata(type.toString(), Role::Targets().ToString());
+    return true;
+  }
+  return false;
+}
+
 bool ImageRepository::updateMeta(INvStorage& storage, const IMetadataFetcher& fetcher) {
   resetMeta();
 

src/libaktualizr/uptane/imagerepository.h

@@ -17,16 +17,10 @@ class ImageRepository : public RepositoryCommon {
   void resetMeta();
 
   bool verifyTargets(const std::string& targets_raw, bool prefetch);
-  bool targetsExpired() { return targets->isExpired(TimeStamp::Now()); }
 
   bool verifyTimestamp(const std::string& timestamp_raw);
-  bool timestampExpired() { return timestamp.isExpired(TimeStamp::Now()); }
 
   bool verifySnapshot(const std::string& snapshot_raw, bool prefetch);
-  bool snapshotExpired() { return snapshot.isExpired(TimeStamp::Now()); }
-  int64_t snapshotSize() { return timestamp.snapshot_size(); }
-
-  Exception getLastException() const { return last_exception; }
 
   static std::shared_ptr<Uptane::Targets> verifyDelegation(const std::string& delegation_raw, const Uptane::Role& role,
                                                            const Targets& parent_target);
@@ -40,14 +34,16 @@ class ImageRepository : public RepositoryCommon {
   bool updateMeta(INvStorage& storage, const IMetadataFetcher& fetcher) override;
 
  private:
+  bool timestampExpired();
+  bool snapshotExpired();
+  int64_t snapshotSize() const { return timestamp.snapshot_size(); }
   bool fetchSnapshot(INvStorage& storage, const IMetadataFetcher& fetcher, int local_version);
   bool fetchTargets(INvStorage& storage, const IMetadataFetcher& fetcher, int local_version);
+  bool targetsExpired();
 
   std::shared_ptr<Uptane::Targets> targets;
   Uptane::TimestampMeta timestamp;
   Uptane::Snapshot snapshot;
-
-  Exception last_exception{"", ""};
 };
 
 }  // namespace Uptane

src/libaktualizr/uptane/tuf.cc

@@ -341,7 +341,7 @@ void Uptane::BaseMeta::init(const Json::Value &json) {
   try {
     expiry_ = TimeStamp(json["signed"]["expires"].asString());
   } catch (const TimeStamp::InvalidTimeStamp &exc) {
-    throw Uptane::InvalidMetadata("", "", "Invalid timestamp");
+    throw Uptane::InvalidMetadata("", "", "invalid timestamp");
   }
   original_object_ = json;
 }

src/libaktualizr/uptane/uptane_test.cc

@@ -1340,17 +1340,17 @@ TEST(Uptane, IgnoreUnknownUpdate) {
   auto result = sota_client->fetchMeta();
   EXPECT_EQ(result.status, result::UpdateStatus::kError);
   EXPECT_STREQ(sota_client->getLastException().what(),
-               "The target had an ECU ID that did not match the client's configured ECU id.");
+               "The target had an ECU ID that did not match the client's configured ECU ID.");
   sota_client->last_exception = Uptane::Exception{"", ""};
   result = sota_client->checkUpdates();
   EXPECT_EQ(result.status, result::UpdateStatus::kError);
   EXPECT_STREQ(sota_client->getLastException().what(),
-               "The target had an ECU ID that did not match the client's configured ECU id.");
+               "The target had an ECU ID that did not match the client's configured ECU ID.");
   std::vector<Uptane::Target> packages_to_install = UptaneTestCommon::makePackage("testecuserial", "testecuhwid");
   sota_client->last_exception = Uptane::Exception{"", ""};
   auto report = sota_client->uptaneInstall(packages_to_install);
   EXPECT_STREQ(sota_client->getLastException().what(),
-               "The target had an ECU ID that did not match the client's configured ECU id.");
+               "The target had an ECU ID that did not match the client's configured ECU ID.");
   EXPECT_EQ(report.ecu_reports.size(), 0);
 }
 

src/libaktualizr/uptane/uptanerepository.h

@@ -16,6 +16,7 @@ class RepositoryCommon {
   int rootVersion() { return root.version(); }
   bool rootExpired() { return root.isExpired(TimeStamp::Now()); }
   virtual bool updateMeta(INvStorage &storage, const IMetadataFetcher &fetcher) = 0;
+  Exception getLastException() const { return last_exception; }
 
  protected:
   void resetRoot();
@@ -25,6 +26,7 @@ class RepositoryCommon {
 
   Root root;
   RepositoryType type;
+  Exception last_exception{"", ""};
 };
 }  // namespace Uptane