diff --git a/tests/test_trusted_metadata_set.py b/tests/test_trusted_metadata_set.py index a9bce8f833..1fa60052fd 100644 --- a/tests/test_trusted_metadata_set.py +++ b/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) diff --git a/tuf/ngclient/_internal/trusted_metadata_set.py b/tuf/ngclient/_internal/trusted_metadata_set.py index be1b0b44ed..dac579bca3 100644 --- a/tuf/ngclient/_internal/trusted_metadata_set.py +++ b/tuf/ngclient/_internal/trusted_metadata_set.py @@ -13,11 +13,18 @@ (trusted_set["root"]) or, in the case of top-level metadata, using the helper properties (trusted_set.root). -The rules for top-level metadata are - * Metadata is updatable only if metadata it depends on is loaded - * Metadata is not updatable if any metadata depending on it has been loaded - * Metadata must be updated in order: - root -> timestamp -> snapshot -> targets -> (delegated targets) +The rules that TrustedMetadataSet follows for top-level metadata are + * Metadata must be loaded in order: + root -> timestamp -> snapshot -> targets -> (delegated targets). + * Metadata can be loaded even if it is expired (or in the snapshot case if the + meta info does not match): this is called "intermediate metadata". + * Intermediate metadata can _only_ be used to load newer versions of the + same metadata: As an example an expired root can be used to load a new root. + * Metadata is loadable only if metadata before it in loading order is loaded + (and is not intermediate): As an example timestamp can be loaded if a + final (non-expired) root has been loaded. + * Metadata is not loadable if any metadata after it in loading order has been + loaded: As an example new roots cannot be loaded if timestamp is loaded. Exceptions are raised if metadata fails to load in any way. @@ -178,16 +185,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 +248,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 +283,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 +335,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 +385,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) diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index a3c7189d75..318cb85f4b 100644 --- a/tuf/ngclient/updater.py +++ b/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"]