Add test for duplicate keyids

[AdamKorcz]

Add test that is specified in #86 (comment) without the bonus test.

The two clients seem to disagree on whether root metadata should update if there are duplicate keys. As such, python-tuf does not fail on meeting the threshold, but rather because the root has duplicate keys.

[jku]

Can you be more specific, I can't find code that would fail on duplicates in root.roles["role"].keyids? Do you mean signatures?

oh I see now, you mean the test failing in CI. I'll have a look

EDIT:

        if len(set(keyids)) != len(keyids):
            raise ValueError(f"Nonunique keyids: {keyids}")

python tuf indeed will not deserialize metadata with duplicate keyids... we'll have to decide how we continue. This still seems like an acceptable choice by python-tuf: I wouldn't want to force them to change for the test...

[AdamKorcz]

Can you be more specific, I can't find code that would fail on duplicates in root.roles["role"].keyids? Do you mean signatures?

oh I see now, you mean the test failing in CI. I'll have a look

EDIT:

        if len(set(keyids)) != len(keyids):
            raise ValueError(f"Nonunique keyids: {keyids}")

python tuf indeed will not deserialize metadata with duplicate keyids... we'll have to decide how we continue. This still seems like an acceptable choice by python-tuf: I wouldn't want to force them to change for the test...

Yes, for the record, the test is currently set up so that python-tuf fails, but that does not mean that it - or go-tuf for that matter - is wrong. The interesting part here is that the two clients behave differently.

[jku]

a way to move forward here would be to accept either case for now:

• failure to refresh metadata with duplicate keyids -> Test succeeds
• metadata with duplicate keyids works but keyid is not double counted for threshold -> Test succeeds
• metadata with duplicate keyids works and keyid is double counted for threshold -> Test Fails

The logic here is that while we should try to get a full consensus, the really important thing is to check that multiple keyids will not be double counted by any client

EDIT: The above is actually already tested by test_duplicate_keys_root (almost, I'll file a new issue about the missing bit).

[jku]

I think we can close this one:

• test_duplicate_keys_root is very close to testing the "fake reaching the threshold with duplicate keyids" case
• test_duplicate_keys_root should add duplicate signatures too #147 has been filed to make that test even more effective
• Clients disagree on what to do with duplicate keyids #143 has been filed for the inconsistent behaviour between clients WRT duplicate keyids

I'll mark this "request changes" just to remove the approval that I gave before understanding the issue

@AdamKorcz opinions?

[jku]

marking "request changes" to remove my accidental approval: I think we might just be able to close this without merging

[jku]

Closing, everything is handled by other issues