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.

Sign up for GitHub

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

Jump to bottom

ngclient: recent rollback checks are buggy #1563

Closed
jku opened this issue Sep 2, 2021 · 1 comment
Closed

ngclient: recent rollback checks are buggy #1563

jku opened this issue Sep 2, 2021 · 1 comment
Assignees
@jku
Labels
ngclient

Comments

@jku
Copy link
Member

jku commented Sep 2, 2021

I recently added the rollback protection checks in ngclient and am now improving ngclient testing situation. With hindsight the order is wrong: I've found an issue with the rollback checks

  • TrustedMetadataSet now considers intermediate timestamp and snapshot valid even if it is expired or version does not match meta version: it only checks final validity when the next metadata type is loaded
  • Updater currently decides if it needs to download a new metadata version based whether the local metadata was loaded correctly by TrustedMetadataSet

as a result updater thinks it does not need to download a new version of e.g. snapshot but then targets update fails because the final snapshot is not the correct version...

This should not be too hard to fix, but I'm trying to think if we can somehow combine this with #1523: it feels like a similar issue
@jku
Copy link
Member Author

jku commented Sep 2, 2021 •
edited
Loading

Some findings:

  • the decision to check meta version and expiry at "next" metadata update is correct: otherwise Updater could forget to do it
  • but updater does need a way to know if the just loaded metadata is a valid final metadata or just valid intermediate metadata: this applies to both snapshot and timestamp
  • from ngclient: review snapshot hashes/length check #1523 Updater needs to tell TrustedMetadataSet if the snapshot is local one or not (IOW has it been verified at some point or not)

so I guess this issue and #1523 are not related...

@jku jku self-assigned this Sep 2, 2021
jku pushed a commit to jku/python-tuf that referenced this issue Sep 2, 2021
ngclient: Fix rollback checks
c1be3eb 
The rollback checks themselves work, but they create a situation
where Updater does not realize that it needs to download e.g. a new
snapshot because the local snapshot is valid as _intermediate_ snapshot
(that can be used for rollback protection but nothing else), but is not
valid as final snapshot.

Raise in the end of update_snapshot and update_timestamp if the files
are not valid final metadata: this way the intermediate metadata does
get loaded but Updater also knows it is not the final metadata.

Fixes theupdateframework#1563

Signed-off-by: Jussi Kukkonen <[email protected]>
jku pushed a commit to jku/python-tuf that referenced this issue Sep 6, 2021
ngclient: Fix rollback checks
3b0729e 
The rollback checks themselves work, but they create a situation
where Updater does not realize that it needs to download e.g. a new
snapshot because the local snapshot is valid as _intermediate_ snapshot
(that can be used for rollback protection but nothing else), but is not
valid as final snapshot.

Raise in the end of update_snapshot and update_timestamp if the files
are not valid final metadata: this way the intermediate metadata does
get loaded but Updater also knows it is not the final metadata.

Fixes theupdateframework#1563

Signed-off-by: Jussi Kukkonen <[email protected]>
jku pushed a commit to jku/python-tuf that referenced this issue Sep 6, 2021
ngclient: Fix rollback checks
181acb0 
The rollback checks themselves work, but they create a situation
where Updater does not realize that it needs to download e.g. a new
snapshot because the local snapshot is valid as _intermediate_ snapshot
(that can be used for rollback protection but nothing else), but is not
valid as final snapshot.

Raise in the end of update_snapshot and update_timestamp if the files
are not valid final metadata: this way the intermediate metadata does
get loaded but Updater also knows it is not the final metadata.

This modifies the existing tests but does not yet test the situation
described in the first paragraph.

Fixes theupdateframework#1563

Signed-off-by: Jussi Kukkonen <[email protected]>
jku pushed a commit to jku/python-tuf that referenced this issue Sep 6, 2021
ngclient: Fix rollback checks
b2ecee0 
The rollback checks themselves work, but they create a situation
where Updater does not realize that it needs to download e.g. a new
snapshot because the local snapshot is valid as _intermediate_ snapshot
(that can be used for rollback protection but nothing else), but is not
valid as final snapshot.

Raise in the end of update_snapshot and update_timestamp if the files
are not valid final metadata: this way the intermediate metadata does
get loaded but Updater also knows it is not the final metadata.

This modifies the existing tests but does not yet test the situation
described in the first paragraph.

Fixes theupdateframework#1563

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku linked a pull request Sep 6, 2021 that will close this issue
ngclient: Fix intermediate metadata loading with rollback checks #1565
Closed
@jku jku closed this as completed in d018279 Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jku jku

Labels
ngclient
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ngclient: Fix intermediate metadata loading with rollback checks jku/python-tuf
2 participants
@jku @MVrachev