Skip to content

Remove problematic targets rollback attack check #65

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

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Nov 25, 2019

In the client application workflow, remove rollback attack check for top-level targets file, which is (1) redundant and (2) prevents recovery from a fast-forward attack.

(1) rollback attacks, via serving older versions of targets or top-level targets than the previously trusted versions, are already prevented by step 3.3.3 of the client workflow, where version numbers of targets and delegated targets in the new snapshot metadata are asserted to be greater than those in the prior trusted snapshot metadata. This, in combination with the 4.1 check that asserts that hashes and version of the actual targets metadata match the ones in the new trusted snapshot, makes another version number check, i.e the one removed in this PR, obsolete.

(2) fast-forward attack recovery, as described in 1.9, works by having the client remove the trusted timestamp and snapshot metadata after a non-root key rotation, so that the client can overcome the version comparison check, and update from a compromised high version to a recovered lower version. However, 1.9 does not mention removing trusted targets metadata after a key rotation. As a consequence, the additional version number check, removed in this PR, would prevent updating recovered targets metadata after a fast-forward attack.

In the client application workflow, remove rollback attack check
for top-level targets file, which is (1) redundant and (2)
prevents recovery from a fast-forward attack.

(1) rollback attacks, via serving older versions of targets or
top-level targets than the previously trusted versions, are already
prevented by step 3.3.3 of the client workflow, where version
numbers of targets and delegated targets in the new snapshot
metadata are asserted to be greater than those in the prior trusted
snapshot metadata.
This, in combination with the 4.1 check that asserts that hashes and
version of the actual targets metadata match the ones in the new
trusted snapshot, makes another version number check, i.e the one
removed in this commit, obsolete.

(2) fast-forward attack recovery, as described in 1.9, works by
having the client remove the trusted timestamp and snapshot
metadata after a non-root key rotation, so that the client can
overcome the version comparison check, and update from a
compromised high version to a recovered lower version.
However, 1.9 does not mention removing trusted targets metadata
after a key rotation. As a consequence, the additional version
number check, removed in this commit, would prevent updating
recovered targets metadata after a fast-forward attack.
@lukpueh
Copy link
Member Author

lukpueh commented Nov 25, 2019

An alternative fix for (2) would be updating the fast-forward recovery step to also untrust targets, as sketched out in 84103fc.

I do, however, prefer the fix I proposed in this PR here, because it also addresses (1) and thereby makes the spec less complicated.

Copy link
Collaborator

@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 to me, but I'd recommend another review from @trishankatdatadog or @JustinCappos before merging

@lukpueh
Copy link
Member Author

lukpueh commented Dec 11, 2019

Ping @JustinCappos. @mnm678 requested another review.

@lukpueh
Copy link
Member Author

lukpueh commented Dec 11, 2019

As far as I understand @erickt is also in favor of merging this change (see #71).

Copy link
Member

@JustinCappos JustinCappos left a comment

Choose a reason for hiding this comment

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

Yes, this is correct. 3.3 and 1.9 already cover this case.

@JustinCappos JustinCappos merged commit 0f56aee into theupdateframework:master Jan 21, 2020
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.
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