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

test timestamp fast-forward attack recovery #1735

Conversation

kairoaraujo
Copy link
Collaborator

@kairoaraujo kairoaraujo commented Dec 19, 2021

This test simulates the timestamp fast-forward attack recovery.
It simulates that the timestamp keys were compromised, the attacker
generated a new high version of the timestamp.

The repository generates a new key and rollbacks the timestamp
version to the initial version.

Signed-off-by: Kairo de Araujo kdearaujo@vmware.com

Please fill in the fields below to submit a pull request. The more information
that is provided, the better.

Part of #1713

  • 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

@coveralls
Copy link

coveralls commented Dec 19, 2021

Pull Request Test Coverage Report for Build 1602865766

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.702%

Totals Coverage Status
Change from base Build 1601140259: 0.0%
Covered Lines: 4096
Relevant Lines: 4176

💛 - Coveralls

Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

  • The test seems 100% correct but the first few lines seem unnecessary (for this specific test).
  • Could mention the rollback requirement (timestamp key rotation) in a method docstring

Opinions?

tests/test_updater_top_level_update.py Outdated Show resolved Hide resolved
tests/test_updater_top_level_update.py Outdated Show resolved Hide resolved
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

LGTM, the only nitpicks I have are

  • the docstring doesn't specify that timestamp rollback requires timestamp key rotation (this is relevant because the snapshot rollback requires rotating both online keys and not everyone knows these differences) ... but the code does make it clear
  • PR is marked as fixing the issue but it only fixes 1/3 (as mentioned snapshot and targets are slightly different cases)

Anyway, I think we can merge this as is, those are really minor. If you do tweak the docstring, feel free to squash to a single commit.

This test simulates the timestamp fast-forward attack recovery.
It simulates that the timestamp keys were compromised, the attacker
generated a new high version of the timestamp.

The repository generates a new key and rollbacks the timestamp
version to the initial version.

Signed-off-by: Kairo de Araujo <kdearaujo@vmware.com>
@kairoaraujo kairoaraujo force-pushed the issue#1713/test_fast-forward_recovery branch from e5c5b96 to 76a3e6d Compare December 20, 2021 15:59
@jku jku merged commit 2e5ddd3 into theupdateframework:develop Dec 21, 2021
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.

3 participants