Merge b2ecee0 into f02401f

tests/test_trusted_metadata_set.py

@@ -256,11 +256,12 @@ def test_update_timestamp_expired(self):
         def timestamp_expired_modifier(timestamp: Timestamp) -> None:
             timestamp.expires = datetime(1970, 1, 1)
 
-        # intermediate timestamp is allowed to be expired
+        # expired intermediate timestamp is loaded but raises
         timestamp = self.modify_metadata("timestamp", timestamp_expired_modifier)
-        self.trusted_set.update_timestamp(timestamp)
+        with self.assertRaises(exceptions.ExpiredMetadataError):
+            self.trusted_set.update_timestamp(timestamp)
 
-        # update snapshot to trigger final timestamp expiry check
+        # snapshot update does start but fails because timestamp is expired
         with self.assertRaises(exceptions.ExpiredMetadataError):
             self.trusted_set.update_snapshot(self.metadata["snapshot"])
 
@@ -289,10 +290,11 @@ def timestamp_version_modifier(timestamp: Timestamp) -> None:
         timestamp = self.modify_metadata("timestamp", timestamp_version_modifier)
         self.trusted_set.update_timestamp(timestamp)
 
-        #intermediate snapshot is allowed to not match meta version
-        self.trusted_set.update_snapshot(self.metadata["snapshot"])
+        # if intermediate snapshot version is incorrect, load it but also raise
+        with self.assertRaises(exceptions.BadVersionNumberError):
+            self.trusted_set.update_snapshot(self.metadata["snapshot"])
 
-        # final snapshot must match meta version
+        # targets update starts but fails if snapshot version does not match
         with self.assertRaises(exceptions.BadVersionNumberError):
             self.trusted_set.update_targets(self.metadata["targets"])
 
@@ -324,11 +326,12 @@ def test_update_snapshot_expired_new_snapshot(self):
         def snapshot_expired_modifier(snapshot: Snapshot) -> None:
             snapshot.expires = datetime(1970, 1, 1)
 
-        # intermediate snapshot is allowed to be expired
+        # expired intermediate snapshot is loaded but will raise
         snapshot = self.modify_metadata("snapshot", snapshot_expired_modifier)
-        self.trusted_set.update_snapshot(snapshot)
+        with self.assertRaises(exceptions.ExpiredMetadataError):
+            self.trusted_set.update_snapshot(snapshot)
 
-        # update targets to trigger final snapshot expiry check
+        # targets update does start but fails because snapshot is expired
         with self.assertRaises(exceptions.ExpiredMetadataError):
             self.trusted_set.update_targets(self.metadata["targets"])
 
@@ -344,8 +347,10 @@ def version_bump(snapshot: Snapshot) -> None:
         new_timestamp = self.modify_metadata("timestamp", meta_version_bump)
         self.trusted_set.update_timestamp(new_timestamp)
 
-        # load a "local" snapshot, then update to newer one:
-        self.trusted_set.update_snapshot(self.metadata["snapshot"])
+        # load a "local" snapshot with mismatching version (loading happens but
+        # BadVersionNumberError is raised), then update to newer one:
+        with self.assertRaises(exceptions.BadVersionNumberError):
+            self.trusted_set.update_snapshot(self.metadata["snapshot"])
         new_snapshot = self.modify_metadata("snapshot", version_bump)
         self.trusted_set.update_snapshot(new_snapshot)
 

tuf/ngclient/_internal/trusted_metadata_set.py

@@ -178,16 +178,20 @@ def update_root(self, data: bytes) -> None:
     def update_timestamp(self, data: bytes) -> None:
         """Verifies and loads 'data' as new timestamp metadata.
 
-        Note that an expired intermediate timestamp is considered valid so it
-        can be used for rollback checks on newer, final timestamp. Expiry is
-        only checked for the final timestamp in update_snapshot().
+        Note that an intermediate timestamp is allowed to be expired:
+        TrustedMetadataSet will throw an ExpiredMetadataError in this case
+        but the intermediate timestamp will be loaded. This way a newer
+        timestamp can still be loaded (and the intermediate timestamp will
+        be used for rollback protection). Expired timestamp will prevent
+        loading snapshot metadata.
 
         Args:
             data: unverified new timestamp metadata as bytes
 
         Raises:
-            RepositoryError: Metadata failed to load or verify. The actual
-                error type and content will contain more details.
+            RepositoryError: Metadata failed to load or verify as final
+                timestamp. The actual error type and content will contain
+                more details.
         """
         if self.snapshot is not None:
             raise RuntimeError("Cannot update timestamp after snapshot")
@@ -237,21 +241,33 @@ def update_timestamp(self, data: bytes) -> None:
         self._trusted_set["timestamp"] = new_timestamp
         logger.debug("Updated timestamp")
 
+        # timestamp is loaded: raise if it is not valid _final_ timestamp
+        self._check_final_timestamp()
+
+    def _check_final_timestamp(self) -> None:
+        """Raise if timestamp is expired"""
+
+        assert self.timestamp is not None  # nosec
+        if self.timestamp.signed.is_expired(self.reference_time):
+            raise exceptions.ExpiredMetadataError("timestamp.json is expired")
+
     def update_snapshot(self, data: bytes) -> None:
         """Verifies and loads 'data' as new snapshot metadata.
 
-        Note that intermediate snapshot is considered valid even if it is
-        expired or the version does not match the timestamp meta version. This
-        means the intermediate snapshot can be used for rollback checks on
-        newer, final snapshot. Expiry and meta version are only checked for
-        the final snapshot in update_delegated_targets().
+        Note that an intermediate snapshot is allowed to be expired and version
+        is allowed to not match timestamp meta version: TrustedMetadataSet will
+        throw an ExpiredMetadataError/BadVersionNumberError in these cases
+        but the intermediate snapshot will be loaded. This way a newer
+        snapshot can still be loaded (and the intermediate snapshot will
+        be used for rollback protection). Expired snapshot or snapshot that
+        does not match timestamp meta version will prevent loading targets.
 
         Args:
             data: unverified new snapshot metadata as bytes
 
         Raises:
-            RepositoryError: Metadata failed to load or verify. The actual
-                error type and content will contain more details.
+            RepositoryError: data failed to load or verify as final snapshot.
+                The actual error type and content will contain more details.
         """
 
         if self.timestamp is None:
@@ -260,10 +276,8 @@ def update_snapshot(self, data: bytes) -> None:
             raise RuntimeError("Cannot update snapshot after targets")
         logger.debug("Updating snapshot")
 
-        # Local timestamp was allowed to be expired to allow for rollback
-        # checks on new timestamp but now timestamp must not be expired
-        if self.timestamp.signed.is_expired(self.reference_time):
-            raise exceptions.ExpiredMetadataError("timestamp.json is expired")
+        # Snapshot cannot be loaded if final timestamp is expired
+        self._check_final_timestamp()
 
         meta = self.timestamp.signed.meta["snapshot.json"]
 
@@ -314,8 +328,11 @@ def update_snapshot(self, data: bytes) -> None:
         self._trusted_set["snapshot"] = new_snapshot
         logger.debug("Updated snapshot")
 
+        # snapshot is loaded, but we raise if it's not valid _final_ snapshot
+        self._check_final_snapshot()
+
     def _check_final_snapshot(self) -> None:
-        """Check snapshot expiry and version before targets is updated"""
+        """Raise if snapshot is expired or meta version does not match"""
 
         assert self.snapshot is not None  # nosec
         assert self.timestamp is not None  # nosec
@@ -361,9 +378,8 @@ def update_delegated_targets(
         if self.snapshot is None:
             raise RuntimeError("Cannot load targets before snapshot")
 
-        # Local snapshot was allowed to be expired and to not match meta
-        # version to allow for rollback checks on new snapshot but now
-        # snapshot must not be expired and must match meta version
+        # Targets cannot be loaded if final snapshot is expired or its version
+        # does not match meta version in timestamp
         self._check_final_snapshot()
 
         delegator: Optional[Metadata] = self.get(delegator_name)

tuf/ngclient/updater.py

@@ -322,7 +322,7 @@ def _load_timestamp(self) -> None:
             self._trusted_set.update_timestamp(data)
         except (OSError, exceptions.RepositoryError) as e:
             # Local timestamp does not exist or is invalid
-            logger.debug("Failed to load local timestamp %s", e)
+            logger.debug("Local timestamp not valid as final: %s", e)
 
         # Load from remote (whether local load succeeded or not)
         data = self._download_metadata(
@@ -339,7 +339,7 @@ def _load_snapshot(self) -> None:
             logger.debug("Local snapshot is valid: not downloading new one")
         except (OSError, exceptions.RepositoryError) as e:
             # Local snapshot does not exist or is invalid: update from remote
-            logger.debug("Failed to load local snapshot %s", e)
+            logger.debug("Local snapshot not valid as final: %s", e)
 
             assert self._trusted_set.timestamp is not None  # nosec
             metainfo = self._trusted_set.timestamp.signed.meta["snapshot.json"]