Expand on the TUF client update workflow, per popular demand. #440

trishankkarthik · 2017-04-26T18:09:00Z

Besides our reference implementation, I do not believe that the TUF specification describes the precise steps that an alternative, compliant implementation should follow to download and verify a target. This PR is a humble attempt at solving this problem.

There are still some details are missing, such as:

Downloading and verifying the complete chain of root metadata files. (Maybe @ecordell or @heartsucker could help here?)
Merge reading consistent snapshots with the client update workflow.
Using the map file, but the pseudocode is available...

Hopefully, I didn't miss something important, or, worse, get something wrong. (I'll blame it on travel lag.) Please take it from here!

coveralls · 2017-04-26T18:36:44Z

Coverage remained the same at 97.936% when pulling f092d2a on trishankkarthik:add-detailed-workflow into 40aaf93 on theupdateframework:develop.

JustinCappos

Quick comments upon first read.

JustinCappos · 2017-04-26T18:45:00Z

docs/tuf-spec.txt

@@ -928,24 +928,113 @@ Version 1.0 (Draft)

 5.1. The client application

- 1. The client application first instructs TUF to check for updates.
+ 1. **Download the root metadata file**, up to some predetermined number of


Please clarify what "predetermined" means here.

JustinCappos · 2017-04-26T18:47:45Z

docs/tuf-spec.txt

+ arbitrarily increase the version numbers of: (1) the timestamp metadata,
+ (2) the snapshot metadata, and / or (3) the targets, or a delegated
+ targets, metadata file in the snapshot metadata. Please see the Mercury
+ paper for more details (to be published soon).


You can probably point to where this will be:

https://ssl.engineering.nyu.edu/papers/kuppusamy_usenix_17.pdf

For the time being, feel free to put the SUBMITTED there, but please label it DRAFT

JustinCappos · 2017-04-26T18:48:49Z

docs/tuf-spec.txt


- 7. TUF downloads and verifies the file and then makes the file available to the
- software update system.
+ 2.1. **Check signatures.** It must have been signed by a threshold of keys


Please avoid 'It', especially when it crosses points, etc.

JustinCappos · 2017-04-26T18:49:50Z

docs/tuf-spec.txt

+ 5.1. If there is no targets metadata about this target, then report that
+ there is no such target.
+
+ 5.2. Otherwise, download the target (up to the number of bytes specified in


It's worth explaining why this is 'up to' instead of 'exactly'

coveralls · 2017-04-26T19:35:40Z

Coverage remained the same at 97.936% when pulling 6236878 on trishankkarthik:add-detailed-workflow into 40aaf93 on theupdateframework:develop.

vladimir-v-diaz · 2017-04-26T20:33:24Z

Should we postpone the specification edits for the map file changes? The PR that implements the map file (TAP 4) hasn't yet been reviewed/merged with the master/develop branch. We can also update the specification in pull request #430.

JustinCappos · 2017-04-26T20:54:43Z

You could, but realistically, TAP 4 is moving quickly toward acceptance. We will need to do TAP 7 first, but I hope we will have all approved in the next two months.

…

On Wed, Apr 26, 2017 at 4:33 PM, Vladimir Diaz ***@***.***> wrote: Should we postpone the specification edits for the map file changes? The PR that implements the map file (TAP 4) hasn't yet been reviewed/merged with the master/develop branch. We can also update the specification in pull request #430 <#430>. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#440 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA0XD72xQXzaW1JYqogQJyFDoqzLeWY2ks5rz6oVgaJpZM4NJOnY> .

heartsucker

I think this should be updated to use RFC 2119 keywords, because there are some places where you say "should" but I think you mean "must."

heartsucker · 2017-04-27T09:09:33Z

docs/tuf-spec.txt


- 2. TUF downloads and verifies timestamp.json.
+ 1.1. **Check signatures.** The root metadata file must have been signed by


I think you should mention key pinning over TOFU for the initial root.json.

Good idea. Where is it best in the specification to discuss this?

Also, where in the specification do you think we should use "SHOULD" or "MUST" (ala RFC 2119)?

For example:

The latest known time should be lower than the expiration timestamp

should say

The latest known time MUST be lower than the expiration timestamp

Also

The version number of the previous root metadata file must be less than or equal

Should have the "must" in all caps

As for where to mention pinning vs. TOFU, I think there should be a section before 5.1 maybe called "client design" that mentions this, and then step 1 could specifically say something like "If a client was initialized with a pinned set of root keys, these keys and only these keys MUST be used to validate the original root.json."

Thanks very much, @heartsucker, will incorporate these suggestions!

heartsucker · 2017-04-29T10:11:15Z

I have another question on this. I seem to have a chicken and egg problem.

Imagine a repo at time T1 with all metadata at versions 1. A client connects and downloads all metadata. At time T2, someone swaps the root metadata keys and all role keys creating all new metadata at versions 2. A client connects at T3 and starts by downloading the timestamp.json. This would be untrusted because the client doesn't know about the version 2 keys.

I think this could include the step saying "if timestamp.json is untrusted, the client downloads root.json an rebuilds the chain of trust, returns to the start and attempts to download timestamp.json again repeating the loop."

ecordell · 2017-04-29T14:00:39Z

@heartsucker The first step of the update process should always be updating the root chain, so that trusted keys are up to date. I think the only change needed here is to clarify that "downloading the root" can mean "downloading n roots, until one is reached that is signed by trusted keys"

trishankkarthik · 2017-04-29T18:54:47Z

Very good question, @heartsucker, about not being able to verify timestamp and / or snapshot metadata due to change of keys, and thanks for the response, @ecordell !

Indeed, partly due to our findings from the submitted Mercury draft, we have made updating the root metadata the first and foremost step, so that there are no problems verifying timestamp and / or snapshot metadata from a change of keys (especially after the repository has been compromised, and recovered). This is also the first and foremost step, partly due to TAP 5.

@heartsucker

Client's should validate new root.json according to the threshold and keys set by its previous version. See @heartsucker comment [here](theupdateframework/rust-tuf#42 (comment))

vladimir-v-diaz · 2017-05-18T16:28:02Z

@trishankkarthik I think this PR is good enough to merge at the moment. We can address the other points raised in the comments in separate pull requests. Please go ahead and make
any last remaining edits before we merge.

Documenting the other needed changes to specification...

The map file (TAP 4) hasn't been finalized yet.
Downloading and verifying chain of root metadata
More detailed outline of root migration (see Clarify procedure for updating to new root.json #444 and added root key migration details #445)

…data.

Ideally, the former should be merged with the latter.

trishankkarthik · 2017-05-19T01:12:24Z

@vladimir-v-diaz @JustinCappos I agree we should merge. But first, I think we should:

Merge Section 7.2 (reading consistent snapshots) with Section 5.1 (client update workflow). It's confusing to have multiple sections about the same thing.
Remove some redundant notes at the end of Section 5.1.

trishankkarthik · 2017-05-19T04:50:57Z

Okay, I merged reading consistent snapshots with the client update workflow. I also added some other details. Someone else please review to make sure I didn't make mistakes from burning the midnight oil.

@vladimir-v-diaz We should fix Section 7.1 (writing consistent snapshots). It seems to have some now inaccurate information. For example, aren't all targets metadata files written as FILENAME.VERSION.EXT instead of FILENAME.DIGEST.EXT? Isn't only the snapshot metadata file written as FILENAME.DIGEST.EXT? The root and timestamp metadata files are written as both FILENAME.EXT and FILENAME.DIGEST.EXT, right?

Also, have we decided whether it's (DIGEST|VERSION).FILENAME.EXT or FILENAME.(DIGEST|VERSION).EXT? Thanks!

trishankkarthik · 2017-05-19T20:33:41Z

Okay, I just added that the expiration of the previous root metadata file does not matter, because we will attempt to update it in the next step.

vladimir-v-diaz · 2017-05-23T14:03:02Z

@trishankkarthik

The code expects metadata filenames to be in <version_number>.rolename.<ext> format.

I think that's the format we last settled on. There were several reasons, one of them being that long digests might exceed the filename limit set by certain filesystems. However, according to Hannes, TAP 8 requires that filenames include the digest. We'll see once the TAP is edited with more detail.

trishankkarthik · 2017-05-23T15:28:38Z

@vladimir-v-diaz Got it. Please feel free to make the change!

trishankkarthik · 2017-05-23T16:32:41Z

docs/tuf-spec.txt

+ without requiring all previous root keys to be kept to sign new root.json
+ metadata.
+
+ In the event that the keys being updated are root keys, it is important to


@vladimir-v-diaz I think this is great! Should we say a bit more about this in Step 1 of Section 5.1?

I think we should. Do we also explain how to recover from a fast-forward attack?

Note: I just reviewed section 7.1 and it seems accurate to me, at least in regard to filenames. Is there a sentence/statement in particular that is inaccurate?

Okay, we do mention fast-forward attacks in the revised 1.5 section!

coveralls · 2017-05-23T16:48:09Z

Coverage remained the same at 97.936% when pulling 06665c0 on trishankkarthik:add-detailed-workflow into 70cf57a on theupdateframework:develop.

vladimir-v-diaz · 2017-05-23T16:50:41Z

I think we might be able to reuse some of @heartsucker's outline of root key migrations here.

Also corrected how files are fetched.

…f into add-detailed-workflow

trishankkarthik · 2017-05-23T21:15:45Z

Okay, so I just worked with @vladimir-v-diaz to update the workflow. Specifically, I added details on how to update to the latest root metadata file (thanks to @ecordell and @heartsucker).

@JustinCappos Please review the entire workflow to make sure there are no problems?

coveralls · 2017-05-23T21:41:21Z

Coverage remained the same at 97.936% when pulling 558fb43 on trishankkarthik:add-detailed-workflow into 70cf57a on theupdateframework:develop.

coveralls · 2017-05-24T02:09:15Z

Coverage remained the same at 97.936% when pulling c5e8c07 on trishankkarthik:add-detailed-workflow into 70cf57a on theupdateframework:develop.

JustinCappos

good change

Expand on the TUF client update workflow, per popular demand.

f092d2a

trishankkarthik requested review from JustinCappos, vladimir-v-diaz and awwad April 26, 2017 18:09

trishankkarthik added this to the TUF 1.0 milestone Apr 26, 2017

trishankkarthik mentioned this pull request Apr 26, 2017

Spec - 4.4 - Client parses snapshot.json with the length/hash of root.json (but not of other metadata) theupdateframework/rust-tuf#14

Closed

JustinCappos reviewed Apr 26, 2017

View reviewed changes

Address comments by @JustinCappos.

6236878

heartsucker reviewed Apr 27, 2017

View reviewed changes

heartsucker mentioned this pull request Apr 28, 2017

Preliminary implementation of Uptane test vectors advancedtelematic/rvi_sota_client#225

Merged

trishankkarthik removed request for vladimir-v-diaz and awwad April 30, 2017 00:30

trishankkarthik self-assigned this Apr 30, 2017

vladimir-v-diaz referenced this pull request May 18, 2017

Clarify procedure for updating to new root.json

63ec868

Client's should validate new root.json according to the threshold and keys set by its previous version. See @heartsucker comment [here](theupdateframework/rust-tuf#42 (comment))

Clarify that the previous snapshot metadata file may be safely expired.

afa804f

vladimir-v-diaz mentioned this pull request May 18, 2017

Clarify procedure for updating to new root.json #444

Merged

Use "MUST" with regard to RFC 2119.

947e366

trishankkarthik requested a review from vladimir-v-diaz May 19, 2017 00:48

trishankkarthik added 3 commits May 18, 2017 20:58

Explain how to obtain and load the previous root metadata file.

8b1f853

Explain why download targets up to the # of bytes in the targets meta…

c5deaa3

…data.

Edit Section 7.2 to reflect Section 5.1.

30536cf

Ideally, the former should be merged with the latter.

trishankkarthik added 3 commits May 19, 2017 00:10

Add subtleties to preorder DFS for targets.

487d27a

Remove what seems like unnecessary, obsolete text.

5666cbc

Merge reading consistent snapshots with the client update workflow.

a2a5931

trishankkarthik added 2 commits May 19, 2017 16:31

Remove what seems like an unnecessary note.

3935141

Add a note about the expiration of the previous root metadata file.

d80ea0b

Merge branch 'develop' into add-detailed-workflow

06665c0

trishankkarthik commented May 23, 2017

View reviewed changes

trishankkarthik added 2 commits May 23, 2017 17:09

Worked with @vladimir-v-diaz to add root migration workflow.

98de3c4

Also corrected how files are fetched.

Merge branch 'add-detailed-workflow' of github.com:trishankkarthik/tu…

558fb43

…f into add-detailed-workflow

Fix bloopers.

c5e8c07

JustinCappos approved these changes May 24, 2017

View reviewed changes

JustinCappos merged commit 85eb899 into theupdateframework:develop May 24, 2017

vladimir-v-diaz mentioned this pull request May 24, 2017

added root key migration details #445

Closed

vladimir-v-diaz mentioned this pull request Feb 5, 2020

Reference the map file in the specification (TAP4) theupdateframework/specification#88

Open

joshuagl mentioned this pull request May 12, 2022

avoid trying to update root role every time theupdateframework/specification#225

Open


		2. TUF downloads and verifies timestamp.json.
		1.1. Check signatures. The root metadata file must have been signed by

Expand on the TUF client update workflow, per popular demand. #440

Expand on the TUF client update workflow, per popular demand. #440

Conversation

trishankkarthik commented Apr 26, 2017 • edited Loading

coveralls commented Apr 26, 2017

JustinCappos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Apr 26, 2017

vladimir-v-diaz commented Apr 26, 2017

JustinCappos commented Apr 26, 2017 via email

heartsucker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heartsucker commented Apr 29, 2017

ecordell commented Apr 29, 2017

trishankkarthik commented Apr 29, 2017 • edited Loading

vladimir-v-diaz commented May 18, 2017

trishankkarthik commented May 19, 2017

trishankkarthik commented May 19, 2017

trishankkarthik commented May 19, 2017

vladimir-v-diaz commented May 23, 2017 • edited Loading

trishankkarthik commented May 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 23, 2017

vladimir-v-diaz commented May 23, 2017

trishankkarthik commented May 23, 2017

coveralls commented May 23, 2017

coveralls commented May 24, 2017

JustinCappos left a comment

Choose a reason for hiding this comment

trishankkarthik commented Apr 26, 2017 •

edited

Loading

trishankkarthik commented Apr 29, 2017 •

edited

Loading

vladimir-v-diaz commented May 23, 2017 •

edited

Loading