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

Fix root self verification to only count a keyid once towards the threshold #1218

Merged

Conversation

joshuagl
Copy link
Member

Fixes #N/A

Description of the changes being introduced by the pull request:

@trishankatdatadog perceptively observed that _verify_root_self_signed(), introduced in #1101, will incorrectly count multiple signatures with the same keyid towards the threshold for new signatures listed inside an updated root metadata file.

This PR introduces:

  1. a test to catch this issue, by having a root metadata file with a threshold of two list the same keyid:sig dict twice in the metadata file's signatures.
  2. a fix to _verify_root_self_signed() to uniquify a list of keyids for which signatures have been verified, and compare only the number of unique keyids against the threshold.

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

tuf/client/updater.py Outdated Show resolved Hide resolved
tuf/client/updater.py Outdated Show resolved Hide resolved
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix and the test, @joshuagl! Please add DCO and we are good to go.

Also, kudos to @trishankatdatadog, for bringing this to our attention.

tests/test_updater_root_rotation_integration.py Outdated Show resolved Hide resolved
When the updater is verifying that the new root metadata is signed by a
threshold of keys defined by the new root metadata itself, multiple
signatures with the same keyid should not be counted more than once
towards the threshold.

Implement a test for this, which currently fails.

Reported-by: Trishank Karthik Kuppusamy <trishank.kuppusamy@datadoghq.com>
Signed-off-by: Joshua Lock <jlock@vmware.com>
When verifying newly downloaded root metadata with the keys listed in the
root metadata being verified, multiple signatures with the same keyid
should not be counted towards the threshold. A keyid should only count
once towards the threshold.

This fixes the _verify_root_self_signed() method introduced in PR theupdateframework#1101 to
ensure that keyids are only counted once when verifying a threshold of new
root signatures.

Signed-off-by: Joshua Lock <jlock@vmware.com>
@joshuagl joshuagl force-pushed the joshuagl/root-self-verify-two branch from 44d48af to 83ac7be Compare November 24, 2020 13:23
@lukpueh lukpueh merged commit 9f8979b into theupdateframework:develop Nov 24, 2020
@joshuagl joshuagl deleted the joshuagl/root-self-verify-two branch November 24, 2020 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants