Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata API: prevent Delegation role names to be one of top level metadata roles #1558

Closed
MVrachev opened this issue Sep 1, 2021 · 7 comments · Fixed by #1690
Closed

Metadata API: prevent Delegation role names to be one of top level metadata roles #1558

MVrachev opened this issue Sep 1, 2021 · 7 comments · Fixed by #1690
Assignees
Labels
backlog Issues to address with priority for current development goals good first issue Bite-sized items for first time contributors
Milestone

Comments

@MVrachev
Copy link
Collaborator

MVrachev commented Sep 1, 2021

Description of issue or feature request:

Current behavior:
Delegation role names are not restricted in any way in the spec, but they are targets metadata role names.
That could lead to a problem if delegation role names are one of root, timestamp, snapshot or targets.

Expected behavior:
Make sure that delegation role names aren't one of the top-level metadata roles.

@MVrachev MVrachev added good first issue Bite-sized items for first time contributors backlog Issues to address with priority for current development goals labels Sep 1, 2021
@joshuagl
Copy link
Member

joshuagl commented Sep 6, 2021

I wonder if this kind of logic and role name sanitisation more generally belongs in metadata API? Perhaps it should be part of the (as yet undesigned) repository interface?

@jku
Copy link
Member

jku commented Sep 6, 2021

same check does make sense in a client: we would not want to accidentally overwrite our local root.json because a targets decides to delegate to a "root" role version 9000.

I don't think there are practical risks of that -- the client would have to succeed in downloading and verifying the file as a targets metadata before it would be written to local disk but this still seems like a reasonable check for a client to make

@jku
Copy link
Member

jku commented Sep 22, 2021

the legacy client also prevents empty string as rolename -- this seems like a reasonable take for ngclient too

@joshuagl
Copy link
Member

@MVrachev is this issue covered by #1630?

@MVrachev
Copy link
Collaborator Author

No, as the validation itself happens in the delegator class who have the full list of roles or in the case of #1630 that's Root.
Here the validation should happen in Targets the same way it's done in #1630.

@MVrachev
Copy link
Collaborator Author

When solving this issue please add tests in test_metadata_serialization.py the same way it's done in #1630 specifically see incorect_roots.

@kairoaraujo
Copy link
Collaborator

I am working on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals good first issue Bite-sized items for first time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants