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

Check timestamp/snapshot contains snapshot/targets description #226

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 32 additions & 22 deletions tuf-spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -1368,20 +1368,24 @@ it in the next step.

3. **Check for a rollback attack.**

1. The version number of the trusted timestamp metadata file, if
any, MUST be less than the version number of the new timestamp
metadata file. If the new timestamp metadata version is less than the trusted
timestamp metadata version, discard it, abort the update cycle, and
report the potential rollback attack. In case they are equal, discard the new
timestamp metadata and abort the update cycle. This is normal and it
shouldn't raise any error. The reason for aborting the update process is that
there shouldn't be any changes in the content of this, or any other metadata
files too, considering it has the same version as the already trusted one.
erickt marked this conversation as resolved.
Show resolved Hide resolved

2. The version number of the snapshot metadata file in the
trusted timestamp metadata file, if any, MUST be less than or equal to its
version number in the new timestamp metadata file. If not, discard the new
timestamp metadata file, abort the update cycle, and report the failure.
1. The version number of the trusted timestamp metadata file, if any, MUST be
less than the version number of the new timestamp metadata file. If the
new timestamp metadata version is less than the trusted timestamp metadata
version, discard it, abort the update cycle, and report the potential
rollback attack. In case they are equal, discard the new timestamp
metadata and abort the update cycle. This is normal and it shouldn't raise
any error. The reason for aborting the update process is that there
shouldn't be any changes in the content of this, or any other metadata
files too, considering it has the same version as the already trusted one.

2. The new timestamp metadata file MUST only contain the description of the
snapshot metadata file. If not, discard the new snapshot metadata file,
abort the cycle, and report the failure.
Copy link
Member

@joshuagl joshuagl May 19, 2022

Choose a reason for hiding this comment

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

Just to be clear, here we're checking that timestamp's meta lists only snapshot?

Similar use of the new keyword "description" here, WDYT to changing that to "METAFILES object"?

(I've also changes snapshot -> timestamp in the "discard ..." statement)

Suggested change
2. The new timestamp metadata file MUST only contain the description of the
snapshot metadata file. If not, discard the new snapshot metadata file,
abort the cycle, and report the failure.
2. The new timestamp metadata file's `METAFILES` object MUST only contain the
snapshot metadata file. If not, discard the new timestamp metadata file,
abort the cycle, and report the failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used the term description from https://theupdateframework.github.io/specification/latest/#file-formats-timestamp, but I agree it's a little vague. I like yours better.


3. The version number of the snapshot metadata file in the trusted timestamp
erickt marked this conversation as resolved.
Show resolved Hide resolved
metadata file, if any, MUST be less than or equal to its version number in
the new timestamp metadata file. If not, discard the new timestamp
metadata file, abort the update cycle, and report the failure.

4. **Check for a freeze attack.** The expiration timestamp in the
new timestamp metadata file MUST be higher than the fixed update start time.
Expand Down Expand Up @@ -1425,14 +1429,20 @@ it in the next step.
in the trusted timestamp metadata. If the versions do not match, discard the
new snapshot metadata, abort the update cycle, and report the failure.

5. **Check for a rollback attack**. The version number of the targets
metadata file, and all delegated targets metadata files, if any, in the
trusted snapshot metadata file, if any, MUST be less than or equal to its
version number in the new snapshot metadata file. Furthermore, any targets
metadata filename that was listed in the trusted snapshot metadata file, if
any, MUST continue to be listed in the new snapshot metadata file. If any of
these conditions are not met, discard the new snapshot metadata file, abort
the update cycle, and report the failure.
5. **Check for a rollback attack**.

1. The new snapshot metadata file MUST contain the description of the targets
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. The new snapshot metadata file MUST contain the description of the targets
1. The new snapshot metadata file MUST contain the version numbers of the targets

The listed item is the version numbers, not a description.

Copy link
Member

Choose a reason for hiding this comment

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

I assume that "description" was used as a short-hand for the version plus optional length and hashes (fields in a METAFILES object)? As this terminology is not used elsewhere we should certainly be more exploit here.

Copy link
Member

@joshuagl joshuagl May 19, 2022

Choose a reason for hiding this comment

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

Perhaps we could say

The new snapshot metadata file's METAFILES object MUST list all targets metadata (top-level targets.EXT all all delegated roles) including their version numbers, optionally their length and optionally their hashes. If not, discard the new snapshot metadata file, abort the

etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fleshed this out more, with links back to the definitions. How does it read?

metadata file. If not, discard the new snapshot metadata file, abort the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
metadata file. If not, discard the new snapshot metadata file, abort the
metadata files (the top-level targets.EXT and all delegated roles.) If not, discard the
new snapshot metadata file, abort the

Clarifying that this includes all targets files.

cycle, and report the failure.

2. The version number of the targets metadata file, and all delegated targets
metadata files, if any, in the trusted snapshot metadata file, if any,
MUST be less than or equal to its version number in the new snapshot
metadata file. Furthermore, any targets metadata filename that was listed
in the trusted snapshot metadata file, if any, MUST continue to be listed
in the new snapshot metadata file. If any of these conditions are not
met, discard the new snapshot metadata file, abort the update cycle, and
report the failure.

6. **Check for a freeze attack**. The expiration timestamp in the
new snapshot metadata file MUST be higher than the fixed update start time.
Expand Down