Merge pull request #2024 from MVrachev/timestamp-eq-version

ngclient: pick old timestamp if new.version is equal

Author: Lukas Pühringer

tests/test_trusted_metadata_set.py

@@ -274,9 +274,25 @@ def version_modifier(timestamp: Timestamp) -> None:
         with self.assertRaises(exceptions.BadVersionNumberError):
             self.trusted_set.update_timestamp(self.metadata[Timestamp.type])
 
+    def test_update_timestamp_with_same_timestamp(self) -> None:
+        # Test that timestamp is NOT updated if:
+        # new_timestamp.version == trusted_timestamp.version
+        self.trusted_set.update_timestamp(self.metadata[Timestamp.type])
+        initial_timestamp = self.trusted_set.timestamp
+
+        # Update timestamp with the same version.
+        with self.assertRaises(exceptions.EqualVersionNumberError):
+            self.trusted_set.update_timestamp((self.metadata[Timestamp.type]))
+
+        # Every object has a unique id() if they are equal, this means timestamp
+        # was not updated.
+        self.assertEqual(id(initial_timestamp), id(self.trusted_set.timestamp))
+
     def test_update_timestamp_snapshot_ver_below_current(self) -> None:
         def bump_snapshot_version(timestamp: Timestamp) -> None:
             timestamp.snapshot_meta.version = 2
+            # The timestamp version must be increased to initiate a update.
+            timestamp.version += 1
 
         # set current known snapshot.json version to 2
         timestamp = self.modify_metadata(Timestamp.type, bump_snapshot_version)
@@ -382,6 +398,8 @@ def snapshot_expired_modifier(snapshot: Snapshot) -> None:
     def test_update_snapshot_successful_rollback_checks(self) -> None:
         def meta_version_bump(timestamp: Timestamp) -> None:
             timestamp.snapshot_meta.version += 1
+            # The timestamp version must be increased to initiate a update.
+            timestamp.version += 1
 
         def version_bump(snapshot: Snapshot) -> None:
             snapshot.version += 1

tests/test_updater_top_level_update.py

@@ -804,6 +804,27 @@ def test_max_metadata_lengths(self) -> None:
         updater = self._init_updater()
         updater.refresh()
 
+    def test_timestamp_eq_versions_check(self) -> None:
+        # Test that a modified timestamp with different content, but the same
+        # version doesn't replace the valid locally stored one.
+
+        # Make a successful update of valid metadata which stores it in cache
+        self._run_refresh()
+        initial_timestamp_meta_ver = self.sim.timestamp.snapshot_meta.version
+
+        # Change timestamp without bumping its version in order to test if a new
+        # timestamp with the same version will be persisted.
+        self.sim.timestamp.snapshot_meta.version = 100
+        self._run_refresh()
+
+        # If the local timestamp md file has the same snapshot_meta.version as
+        # the initial one, then the new modified timestamp has not been stored.
+        timestamp_path = os.path.join(self.metadata_dir, "timestamp.json")
+        timestamp: Metadata[Timestamp] = Metadata.from_file(timestamp_path)
+        self.assertEqual(
+            initial_timestamp_meta_ver, timestamp.signed.snapshot_meta.version
+        )
+
 
 if __name__ == "__main__":
     if "--dump" in sys.argv:

tuf/api/exceptions.py

@@ -29,6 +29,10 @@ class BadVersionNumberError(RepositoryError):
     """An error for metadata that contains an invalid version number."""
 
 
+class EqualVersionNumberError(BadVersionNumberError):
+    """An error for metadata containing a previously verified version number."""
+
+
 class ExpiredMetadataError(RepositoryError):
     """Indicate that a TUF Metadata file has expired."""
 

tuf/ngclient/_internal/trusted_metadata_set.py

@@ -226,6 +226,10 @@ def update_timestamp(self, data: bytes) -> Metadata[Timestamp]:
                     f"New timestamp version {new_timestamp.signed.version} must"
                     f" be >= {self.timestamp.signed.version}"
                 )
+            # Keep using old timestamp if versions are equal.
+            if new_timestamp.signed.version == self.timestamp.signed.version:
+                raise exceptions.EqualVersionNumberError()
+
             # Prevent rolling back snapshot version
             snapshot_meta = self.timestamp.signed.snapshot_meta
             new_snapshot_meta = new_timestamp.signed.snapshot_meta

tuf/ngclient/updater.py

@@ -336,7 +336,13 @@ def _load_timestamp(self) -> None:
         data = self._download_metadata(
             Timestamp.type, self.config.timestamp_max_length
         )
-        self._trusted_set.update_timestamp(data)
+        try:
+            self._trusted_set.update_timestamp(data)
+        except exceptions.EqualVersionNumberError:
+            # If the new timestamp version is the same as current, discard the
+            # new timestamp. This is normal and it shouldn't raise any error.
+            return
+
         self._persist_metadata(Timestamp.type, data)
 
     def _load_snapshot(self) -> None: