diff --git a/tests/test_metadata_serialization.py b/tests/test_metadata_serialization.py index 1f5867d2ce..7b3894e3e7 100644 --- a/tests/test_metadata_serialization.py +++ b/tests/test_metadata_serialization.py @@ -304,6 +304,22 @@ def test_invalid_delegated_role_serialization(self, test_case_data: str): DelegatedRole.from_dict(copy.copy(case_dict)) + invalid_delegations: DataSet = { + "duplicate role names": '{"keys": {"keyid" : {"keytype": "rsa", "scheme": "rsassa-pss-sha256", "keyval": {"public": "foo"}}}, \ + "roles": [ \ + {"keyids": ["keyid"], "name": "a", "paths": ["fn1", "fn2"], "terminating": false, "threshold": 3}, \ + {"keyids": ["keyid2"], "name": "a", "paths": ["fn3"], "terminating": false, "threshold": 2} \ + ] \ + }', + } + + @run_sub_tests_with_dataset(invalid_delegations) + def test_invalid_delegation_serialization(self, test_case_data: str): + case_dict = json.loads(test_case_data) + with self.assertRaises(ValueError): + Delegations.from_dict(copy.deepcopy(case_dict)) + + valid_delegations: DataSet = { "all": '{"keys": { \ diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index fb4a1daf2c..dd59e82187 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -339,10 +339,7 @@ def verify_delegate( raise ValueError(f"No delegation found for {delegated_role}") keys = self.signed.delegations.keys - roles = self.signed.delegations.roles - # Assume role names are unique in delegations.roles: #1426 - # Find first role in roles with matching name (or None if no match) - role = next((r for r in roles if r.name == delegated_role), None) + role = self.signed.delegations.roles.get(delegated_role) else: raise TypeError("Call is valid only on delegator metadata") @@ -1129,16 +1126,17 @@ class Delegations: Attributes: keys: Dictionary of keyids to Keys. Defines the keys used in 'roles'. - roles: List of DelegatedRoles that define which keys are required to - sign the metadata for a specific role. The roles order also - defines the order that role delegations are considered in. + roles: Ordered dictionary of role names to DelegatedRoles instances. It + defines which keys are required to sign the metadata for a specific + role. The roles order also defines the order that role delegations + are considered in. unrecognized_fields: Dictionary of all unrecognized fields. """ def __init__( self, keys: Dict[str, Key], - roles: List[DelegatedRole], + roles: "OrderedDict[str, DelegatedRole]", unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: self.keys = keys @@ -1153,17 +1151,19 @@ def from_dict(cls, delegations_dict: Dict[str, Any]) -> "Delegations": for keyid, key_dict in keys.items(): keys_res[keyid] = Key.from_dict(keyid, key_dict) roles = delegations_dict.pop("roles") - roles_res = [] + roles_res: "OrderedDict[str, DelegatedRole]" = OrderedDict() for role_dict in roles: new_role = DelegatedRole.from_dict(role_dict) - roles_res.append(new_role) + if new_role.name in roles_res: + raise ValueError(f"Duplicate role {new_role.name}") + roles_res[new_role.name] = new_role # All fields left in the delegations_dict are unrecognized. return cls(keys_res, roles_res, delegations_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" keys = {keyid: key.to_dict() for keyid, key in self.keys.items()} - roles = [role_obj.to_dict() for role_obj in self.roles] + roles = [role_obj.to_dict() for role_obj in self.roles.values()] return { "keys": keys, "roles": roles, diff --git a/tuf/ngclient/updater.py b/tuf/ngclient/updater.py index a3c7189d75..87290bf0aa 100644 --- a/tuf/ngclient/updater.py +++ b/tuf/ngclient/updater.py @@ -420,7 +420,7 @@ def _preorder_depth_first_walk( child_roles_to_visit = [] # NOTE: This may be a slow operation if there are many # delegated roles. - for child_role in role_metadata.delegations.roles: + for child_role in role_metadata.delegations.roles.values(): if child_role.is_delegated_path(target_filepath): logger.debug("Adding child role %s", child_role.name)