Skip to content
This repository was archived by the owner on Mar 29, 2022. It is now read-only.

Conversation

@awwad
Copy link
Contributor

@awwad awwad commented Mar 25, 2019

For both Uptane Standard conformance (Uptane Standard 5.4.4.2) and the resolution to the
Timeserver fast-forward attack (discussed in #173), it is important for clients to
update Director Repository metadata before Image Repository metadata.

This PR:

  • primarily, makes sure that the Director repo is updated first by modifying the update procedure in secondary.py and primary.py.
  • incidentally, also updates the demo and testing mapping metadata (pinned.json) in order to avoid any suggestion that the Director repository is updated after the Image Repository
  • incidentally, it also makes the primary.py and secondary.py top-level metadata refresh
    procedure identical, for future modularization. This includes a rename in primary.py of refresh_toplevel_metadata_from_repositories to just refresh_toplevel_metadata, which is the same name the function has in secondary.py. While both names are accurate in a sense, the former could be misleading in secondary.py.
  • updates testing accordingly

@awwad awwad added the demo label Mar 25, 2019
@awwad awwad self-assigned this Mar 25, 2019
@coveralls
Copy link

coveralls commented Mar 25, 2019

Coverage Status

Coverage increased (+0.03%) to 96.783% when pulling 18c23b5 on fix_repo_update_order into 0c9d0f1 on develop.

@awwad
Copy link
Contributor Author

awwad commented Mar 26, 2019

EDIT: the below is now all resolved

The actual content of this PR has to be dug out of PR #171.... It's a movement from just calling updater.refresh() in fully_validate_metadata() to calling a new function refresh_toplevel_metadata_from_repositories(), which will make multiple calls to updater.refresh() to ensure that the Director repository is called first.

@awwad awwad force-pushed the fix_repo_update_order branch from d4c77d7 to 9ff20fd Compare March 28, 2019 17:28
@awwad awwad changed the title WIP: Fix repo update order (Director first) in demo and tests Fix repo update order (Director first) in demo and tests Mar 28, 2019
@awwad awwad changed the title Fix repo update order (Director first) in demo and tests Fix repo update order (Director first) Mar 28, 2019
@awwad awwad requested a review from lukpueh March 28, 2019 19:23
Copy link
Collaborator

@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.

The function name refactor and explicit update order looks good to me.

Just to make sure, changing the order in the metadata is only to not confuse the human reader, right? At least that's what the code and the PR description suggest. I am just asking, because the commit message in 6d91773ec08ce1e3ef3b73650902ebb9b9018356 reads differently, i.e. that it does make a difference to the update order.

Also, could it be that you missed tests/test_data/pinned.json?

@awwad
Copy link
Contributor Author

awwad commented Mar 29, 2019

Just to make sure, changing the order in the metadata is only to not confuse the human reader, right? At least that's what the code and the PR description suggest. I am just asking, because the commit message in 6d91773 reads differently, i.e. that it does make a difference to the update order.

My first take (when the commit message went in) was that the order was relevant, but it is not, so the change persists in order to prevent confusion, yes. I'll edit the commit history to remove the old comment.

Also, could it be that you missed tests/test_data/pinned.json?

Looks like. :) Fixing.

@awwad awwad force-pushed the fix_repo_update_order branch from 9ff20fd to c4ac50d Compare March 29, 2019 16:23
awwad added 2 commits March 29, 2019 12:33
The director should be updated first, so this might be helpful
to readers.

Signed-off-by: Sebastien Awwad <sebastien.awwad@gmail.com>
and also make the primary and secondary top-level metadata refresh
procedure identical (for future modularization).  This includes a
rename in primary.py of refresh_toplevel_metadata_from_repositories
to just refresh_toplevel_metadata, which is the same name the
function has in secondary.py.  While both names are accurate in a
sense, the former could be misleading in secondary.py.

Signed-off-by: Sebastien Awwad <sebastien.awwad@gmail.com>
@awwad awwad force-pushed the fix_repo_update_order branch from c4ac50d to 18c23b5 Compare March 29, 2019 16:34
@awwad awwad merged commit 18c23b5 into develop Mar 29, 2019
@awwad awwad deleted the fix_repo_update_order branch March 29, 2019 16:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants