From 5fd3ddccbc4fee6096d520c80eff70b5703a44a7 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Mon, 13 Jun 2022 22:52:02 +0300 Subject: [PATCH] ngclient: pick old timestamp if new.ver is equal In the spec version 1.0.30, a new change has been added considering what should happen if there is a new timestamp with the same version. It says the following: "In case they [versions] are equal, discard the new timestamp metadata and abort the update cycle. This is normal and it shouldn't raise any error." In other words, if there is a new timestamp with the same version, then stop the update process and use the old timestamp. Those changes reflect these latest specification modifications. Signed-off-by: Martin Vrachev --- tests/test_trusted_metadata_set.py | 18 ++++++++++++++++ tests/test_updater_top_level_update.py | 21 +++++++++++++++++++ tuf/api/exceptions.py | 4 ++++ .../_internal/trusted_metadata_set.py | 4 ++++ tuf/ngclient/updater.py | 8 ++++++- 5 files changed, 54 insertions(+), 1 deletion(-) diff --git a/tests/test_trusted_metadata_set.py b/tests/test_trusted_metadata_set.py index 24e38a7b..d6ef50f3 100644 --- a/tests/test_trusted_metadata_set.py +++ b/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 diff --git a/tests/test_updater_top_level_update.py b/tests/test_updater_top_level_update.py index c92ee57e..be6ce09d 100644 --- a/tests/test_updater_top_level_update.py +++ b/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: diff --git a/tuf/api/exceptions.py b/tuf/api/exceptions.py index 18fe4371..a2a889b0 100644 --- a/tuf/api/exceptions.py +++ b/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.""" diff --git a/tuf/ngclient/_internal/trusted_metadata_set.py b/tuf/ngclient/_internal/trusted_metadata_set.py index d08694f0..fa788d0a 100644 --- a/tuf/ngclient/_internal/trusted_metadata_set.py +++ b/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 diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index 150d195a..7a5b2c25 100644 --- a/tuf/ngclient/updater.py +++ b/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: