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

Make metadata signatures ordered by keyid #1217

Merged
merged 1 commit into from
Nov 23, 2020

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Nov 19, 2020

Fixes #1154 and parts of #1211.

Description of the changes being introduced by the pull request:
In repository_lib._generate_and_write_metadata sort the set of signing key keyids alphabetically before passing them on to signing functions, to make the order in which signatures are added deterministic. This is above all beneficial for testing.

This commit also adds an exemplary test for signatures on root metadata using the repository_tool interface to setup all the state that is required to test _generate_and_write_metadata.

Kudos to @erickt for proposing this fix, and to him and @davidstrauss for bringing the issue to our attention.

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

In 'repository_lib._generate_and_write_metadata' sort the set of
signing key keyids alphabetically before passing them on to signing
functions, to make the order in which signatures are added
deterministic.

This is above all beneficial for testing.

This commit also adds an exemplary test for signatures on root
metadata using the repository_tool interface to setup all the state
that required to test _generate_and_write_metadata.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh requested a review from mnm678 November 20, 2020 10:16
Copy link
Contributor

@mnm678 mnm678 left a comment

Choose a reason for hiding this comment

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

This looks good, but IIUC the metadata may still be non-deterministic as the body is not canonical json in transit.

@lukpueh
Copy link
Member Author

lukpueh commented Nov 23, 2020

Thanks for the reviews, @jku and @mnm678! And yes, @mnm678, metadata may still be non-deterministic due to JSON not being deterministic. This PR is only about the order of signatures. I'll only close #1154 and leave #1211 open.

@lukpueh lukpueh merged commit 6333a4e into theupdateframework:develop Nov 23, 2020
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.

repository_lib.py lists signatures in an non-deterministic order
3 participants