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

Clients disagree on what to do with duplicate keyids #143

Open
jku opened this issue Aug 14, 2024 · 2 comments
Open

Clients disagree on what to do with duplicate keyids #143

jku opened this issue Aug 14, 2024 · 2 comments

Comments

@jku
Copy link
Member

jku commented Aug 14, 2024

We should test role keyids where two ids in the list are the same.

Unfortunately the embedded clients disagree on what to do currently:
#108

  • python-tuf fails to deserialize delegating metadata because there are duplicate keyids
  • go-tuf accepts the delegating metadata, and the delegated roles metadata (as long as threshold is reached)

Either decision seems fairly reasonable but it would be even better if they were consistent.

We should decide what to do here

  • double check the spec: my read on it is that spec does not explicitly require keyids to be unique but the assumption is sort of there
  • maybe file a spec issue?
  • we could also accept either choice in the test suite -- and only fail the test if duplicate keyids are accepted and they can fool the threshold check (EDIT this is done in test_duplicate_keys_root())
@jku jku changed the title add test for non-unique keyids add test for duplicate keyids Aug 14, 2024
@jku jku changed the title add test for duplicate keyids Clients disagree on what to do with duplicate keyids Aug 15, 2024
@jku
Copy link
Member Author

jku commented Aug 15, 2024

@rdimitrov @lukpueh any strong feelings here?

@lukpueh
Copy link
Member

lukpueh commented Aug 19, 2024

We should decide what to do here

  • double check the spec: my read on it is that spec does not explicitly require keyids to be unique but the assumption is sort of there

Hm, a very vague assumption maybe. The closest thing might be this requirement, but I think it can still be fulfilled with duplicate keyids.

Clients MUST ensure that for any KEYID represented in this key list and in other files,
inly one unique key has that KEYID. 

There are a few (albeit ambiguous -- see theupdateframework/specification#308) notes about keyid uniqueness in the context of threshold computation, but they don't cover the keyid list either.

  • maybe file a spec issue?

Sounds good. Even if we don't make unique keyids a requirement, we could at least point out the risk of duplicate keyids

  • we could also accept either choice in the test suite -- and only fail the test if duplicate keyids are accepted and they can fool the threshold check (EDIT this is done in test_duplicate_keys_root())

Great idea. The threshold check is what we care about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants