Skip to content

Timestamp rollback check should also check for snapshot rollback #106

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

Merged
merged 4 commits into from
Aug 31, 2020

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Aug 26, 2020

This updates section 5.2.2 to also state that when updating the timestamp role, we should also check to make sure new timestamp role's snapshot version is greater than or equal to the trusted timestamp's snapshot version.

This is an important check when initializing a TUF database. For example, say we had the following TUF metadata:

  • root, which has signed all the below metadata
  • snapshot version 1, which points at a target with a compromised package
  • snapshot version 2, which points at a target with a fixed package
  • timestamp version 2, which points at snapshot version 2

Lets say an attacker has stolen the timestamp keys, they could forge a new timestamp role which points at snapshot version 1. In the normal case where we have a fully initialized TUF database with a trusted root, timestamp version 2, and snapshot version 2, this malicious timestamp would get rejected because of the section 5.3.2 snapshot rollback check.

However, consider a partially initialized repository, where we only trust root and timestamp version 2, and for some reason we're not able to fetch the rest of the metadata. In this setup, we won't have a trusted snapshot available for the 5.3.2 rollback check. Thus, the rollback attack would succeed.

To fix this, I've adopted the language in section 5.3.3, where when updating the snapshot role, we do explicitly check to see if the listed targets or any delegated targets version is always greater than or equal to the trusted versions of those roles.

This updates section 5.2.2 to also state that when updating the
`timestamp` role, we should also check to make sure new `timestamp`
role's `snapshot` version is greater than or equal to the trusted
`timestamp`'s `snapshot` version.

This is an important check when initializing a TUF database. For
example, say we had the following TUF metadata:

* `root`, which has signed all the below metadata
* `snapshot` version 1, which points at a target with a compromised package
* `snapshot` version 2, which points at a target with a fixed package
* `timestamp` version 2, which points at snapshot version 2

Lets say an attacker has stolen the `timestamp` keys, they could forge a
new `timestamp` role which points at `snapshot` version 1. In the normal
case where we have a fully initialized TUF database with a trusted
`root`, `timestamp` version 2, and `snapshot` version 2, this malicious
timestamp would get rejected because of the section 5.3.2 snapshot
rollback check.

However, consider a partially initialized repository, where we only
trust `root` and `timestamp` version 2, and for some reason were not
able to fetch the rest of the metadata. In this setup, we won't have a
trusted `snapshot` available for the 5.3.2 rollback check. Thus, the
rollback attack would succeed.

To fix this, I've adopted the language in section 5.3.3, where when
updating the `snapshot` role, we do explicitly check to see if the listed
`targets` or any delegated `targets` version is always greater than or
equal to the trusted versions of those roles.
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 addition and the detailed commit message, @erickt! This definitely makes sense. It would be nice if we could land it together with the strongly related #86, which clarifies/fleshes out rollback prevention for delegated targets in a similar fashion, and updates the corresponding parts about ffwd-recovery. (mind taking a look?).

I left one only mildly related inline comment here, which is not a blocker for this PR. Still curious what others think.

Expiration has nothing to do with the rollback check, so it's not necessary
to state this in section 5.2 for timestamps, and 5.3 for snapshots.
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.

Many thanks, @erickt! :)

@lukpueh lukpueh merged commit 39c80de into theupdateframework:master Aug 31, 2020
@erickt erickt deleted the rollback branch August 31, 2020 16:04
erickt added a commit to erickt/rust-tuf that referenced this pull request Sep 18, 2020
This implements TUF-1.0.5 section 5.2.2.2, where when updating the
timestamp role, we reject the new timestamp if the snapshot version it
points at is older than the currently trusted snapshot role.

See theupdateframework/specification#106 for more
details.

Closes theupdateframework#294

Change-Id: I4fb68aaf61ad8c0e3ffc0439095f5d00e5baa116
erickt added a commit to erickt/tuf-specification that referenced this pull request Sep 23, 2020
This shares the same justification for removal as theupdateframework#65. Step 3.3.1 was made
redundant by theupdateframework#106, which modified the workflow to add 2.2.2, where
updating the timestamp will also check if the new timestamp contains a
snapshot version that is less than the trusted snapshot version.

This, in combination with the 3.1 check that asserts hashes and version of
the actual snapshot metadata match the ones in the new trusted timestamp,
make another version check, i.e, the one removed in this commit, obsolete.
erickt added a commit to erickt/rust-tuf that referenced this pull request Oct 8, 2020
This implements TUF-1.0.5 section 5.2.2.2, where when updating the
timestamp role, we reject the new timestamp if the snapshot version it
points at is older than the currently trusted snapshot role.

See theupdateframework/specification#106 for more
details.

Closes theupdateframework#294

Change-Id: I4fb68aaf61ad8c0e3ffc0439095f5d00e5baa116
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