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

Update metadata version comparison rules in client workflow #209

Merged
merged 5 commits into from
Apr 28, 2022
Merged

Update metadata version comparison rules in client workflow #209

merged 5 commits into from
Apr 28, 2022

Conversation

rdimitrov
Copy link
Contributor

@rdimitrov rdimitrov commented Feb 16, 2022 •
edited
Loading

The client workflow has a set of version comparisons rules for how
to update metadata files. The following PR addresses the differences
coming from the fact that when updating not all metadata files should
be treated equally.

Fixes #207 and is related to #114

Notes:

  • root should be updated only when there's a newer version
  • timestamp should be updated only when there's a newer version AND the referenced snapshot version is not less than the one in the already trusted timestamp
  • snapshot should be updated only when its version is the same as the one referenced in the trusted timestamp AND the version of the referenced targets metadata (incl. delegated targets files) is equal or newer than the ones in the trusted snapshot.
  • targets should be updated only when its version is the same as the one referenced in the trusted snapshot
  • In regards to How should we handle metadata that has the same version but different content? #114, for timestamp, the decision is to take into account the new timestamp metadata file only if its version is not less than or equal to the trusted one, so this PR addresses one of the use cases mentioned in the issue.

@rdimitrov
Update metadata version comparison rules in client workflow
b534008 
The client workflow has a set of version comparison rules for how
to update metadata files. The following PR addresses the differences
coming from the fact that when updating not all metadata files should
be treated equally.

Fixes theupdateframework#207 and is related to theupdateframework#114

Signed-off-by: Radoslav Dimitrov <[email protected]>
@rdimitrov rdimitrov force-pushed the dimitrovr/version-check-fix branch from 94c5930 to b534008 Compare February 16, 2022 15:46
mnm678
mnm678 previously approved these changes Apr 27, 2022
View reviewed changes
joshuagl
joshuagl previously approved these changes Apr 28, 2022
View reviewed changes
Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

This has long been a source of confusion in the spec. Thank you for the fix!

@joshuagl joshuagl dismissed stale reviews from mnm678 and themself via 24638a6 April 28, 2022 20:23
@joshuagl joshuagl requested a review from mnm678 April 28, 2022 20:24
@joshuagl
Copy link
Member

@mnm678 if you're still comfortable with the change could you re-approve? Thank you.

@mnm678 mnm678 merged commit 7f356e4 into theupdateframework:master Apr 28, 2022
@rdimitrov rdimitrov mentioned this pull request Apr 29, 2022
Open
@rdimitrov rdimitrov deleted the dimitrovr/version-check-fix branch April 29, 2022 13:27
@erickt erickt mentioned this pull request May 19, 2022
Open
This was referenced Jun 8, 2022
Open
Open
@jku jku mentioned this pull request Aug 9, 2022
Open
znewman01 added a commit to znewman01/go-tuf that referenced this pull request Sep 20, 2022
@znewman01
fix: abandon updates if timestamp.json isn't new
f393a21 
Adds a new test for this case: if a client sees a new `timestamp.json`
file with the same version as its current `timestamp.json` file, it
should do nothing (no update, but also no error).

A few other tests were implicitly relying on the fact that the client
did a full update each time, so they've been updated to commit a new
timestamp.

This updates go-tuf for TUF specification v1.0.30 (fixes theupdateframework#321). The
only substantive change was
[theupdateframework/specification#209][tuf-spec-209], which clarifies
the intended behavior for updating metadata files.

Updates for other roles were already in compliance:

- Root metadata: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L258
- Timestamp, checking snapshot version: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L751
- Snapshot, must match version from timestamp: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L667
- Snapshot, no rollback of targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L685
- Targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L643

[tuf-spec-209]: (theupdateframework/specification#209).

Signed-off-by: Zachary Newman <[email protected]>
@znewman01 znewman01 mentioned this pull request Sep 20, 2022
Merged
5 tasks
znewman01 added a commit to znewman01/go-tuf that referenced this pull request Sep 20, 2022
@znewman01
fix: abandon updates if timestamp.json isn't new
189f7d1 
Adds a new test for this case: if a client sees a new `timestamp.json`
file with the same version as its current `timestamp.json` file, it
should do nothing (no update, but also no error).

A few other tests were implicitly relying on the fact that the client
did a full update each time, so they've been updated to commit a new
timestamp.

This updates go-tuf for TUF specification v1.0.30 (fixes theupdateframework#321). The
only substantive change was
[theupdateframework/specification#209][tuf-spec-209], which clarifies
the intended behavior for updating metadata files.

Updates for other roles were already in compliance:

- Root metadata: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L258
- Timestamp, checking snapshot version: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L751
- Snapshot, must match version from timestamp: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L667
- Snapshot, no rollback of targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L685
- Targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L643

[tuf-spec-209]: (theupdateframework/specification#209).

Signed-off-by: Zachary Newman <[email protected]>
znewman01 added a commit to znewman01/go-tuf that referenced this pull request Sep 20, 2022
@znewman01
fix: abandon updates if timestamp.json isn't new
4d59f3b 
Adds a new test for this case: if a client sees a new `timestamp.json`
file with the same version as its current `timestamp.json` file, it
should do nothing (no update, but also no error).

A few other tests were implicitly relying on the fact that the client
did a full update each time, so they've been updated to commit a new
timestamp.

This updates go-tuf for TUF specification v1.0.30 (fixes theupdateframework#321). The
only substantive change was
[theupdateframework/specification#209][tuf-spec-209], which clarifies
the intended behavior for updating metadata files.

Updates for other roles were already in compliance:

- Root metadata: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L258
- Timestamp, checking snapshot version: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L751
- Snapshot, must match version from timestamp: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L667
- Snapshot, no rollback of targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L685
- Targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L643

[tuf-spec-209]: (theupdateframework/specification#209).

Signed-off-by: Zachary Newman <[email protected]>
joshuagl pushed a commit to theupdateframework/go-tuf that referenced this pull request Sep 20, 2022
@znewman01
fix: abandon updates if timestamp.json isn't new (#387)
040092c 
Adds a new test for this case: if a client sees a new `timestamp.json`
file with the same version as its current `timestamp.json` file, it
should do nothing (no update, but also no error).

A few other tests were implicitly relying on the fact that the client
did a full update each time, so they've been updated to commit a new
timestamp.

This updates go-tuf for TUF specification v1.0.30 (fixes #321). The
only substantive change was
[theupdateframework/specification#209][tuf-spec-209], which clarifies
the intended behavior for updating metadata files.

Updates for other roles were already in compliance:

- Root metadata: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L258
- Timestamp, checking snapshot version: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L751
- Snapshot, must match version from timestamp: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L667
- Snapshot, no rollback of targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L685
- Targets: https://github.com/theupdateframework/go-tuf/blob/13eff30efd6c61f165e1bf06e8c0e72f5a0e5703/client/client.go#L643

[tuf-spec-209]: (theupdateframework/specification#209).

Signed-off-by: Zachary Newman <[email protected]>

Signed-off-by: Zachary Newman <[email protected]>
@tedbow tedbow mentioned this pull request Mar 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnm678 mnm678 mnm678 approved these changes

@joshuagl joshuagl joshuagl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Verify and fix the version comparison done for metadata files
3 participants
@rdimitrov @joshuagl @mnm678