Update specification #433

vladimir-v-diaz · 2017-02-27T21:24:57Z

Add compression_algorithms, terminating, and consistent_snapshot attributes
Update text describing the format of PATHPATTERN
Update text for handling overlapping targets between delegations
Remove notion of a full rolename

Thanks is given to @ecordell (Evan Cordell) and the CoreOS team for their contribution to chained Root files.

….json

Remove note about priority tag schemes and our investigation of several of these schemes

We previously used a full rolename when delegations resembled a tree

vladimir-v-diaz · 2017-02-27T21:29:42Z

@trishankkarthik Can you please review this pull request for correctness? There is more work to be done on the specification, but I'd like to update it gradually. Smaller pull requests is also a goal. I will begin by adding missing features introduced by Diplomat, Mercury, etc., and will later edit the entire thing to assist adopters (the current text can be improved to remove ambiguities).

coveralls · 2017-02-27T21:52:42Z

Coverage remained the same at 97.936% when pulling 6665d25 on update_specification into 40aaf93 on develop.

trishankkarthik · 2017-02-27T22:01:05Z

@vladimir-v-diaz Sure thing, yeah, as soon as I get some downtime!

trishankkarthik

Hi Vlad, these are my comments. Let me know if you'd like to discuss in person!

trishankkarthik · 2017-02-27T22:17:21Z

docs/tuf-spec.txt

+ optionally be compressed with this algorithm. Compressed versions of
+ metadata are not listed in snapshot.json.
+
+ CONSISTENT_SNAPSHOT is a boolean indicating whether the repository supports


What are the consequences of this boolean value being either True or False? Does the metadata change? Do clients download files differently? How does it change how the repository administrators delete metadata and packages?

trishankkarthik · 2017-03-10T20:03:56Z

docs/tuf-spec.txt

@@ -530,6 +532,14 @@ Version 1.0 (Draft)
 , ... }
 }

+ COMPRESSION_ALGORITHM specifies one of the compression algorithms supported


Why are the compression algorithms specified here? What are the benefits?

Listing the compression algorithms used by the repository removes the need to list all compressed versions of metadata in Snapshot, reducing its size and minimizing code complexity. Should the benefits of listing the compression algorithm(s) be stated in the specification, or are you simply curious about the change?

There were a few other reasons, which are captured in an email thread with David Lawrence @endophage (see email thread with title, "role names"). I'll quote some part of his request to remove compressed metadata from Snapshot, etc.

I feel very strongly that data compression should be implemented at layer 6 and TUF should not be validating compressed files. As is, a malicious publisher could intentionally publish decompression bomb. The TUF user would download and validate the hash and size of the compressed file but be vulnerable when they decompress. By implementing compression at layer 6, the client will decompress the stream as it arrives and never read more than Length bytes from the incoming stream.

If we were talking about GBs of data there would be a strong argument for compression on disk, but TUF data is (and necessarily so) in the order of KB and the storage gains from compression are minimal. Compression at layer 6 still provides the benefit of reduced network transfer.

...

Overall, I think consistently not using file extensions when referencing TUF metadata within the metadata files, and specifying any compression is done at layer 6 overall is a security improvement on 2 fronts. Firstly it makes the metadata more consistent, reducing code complexity, which makes auditing easier and chance of bugs lower. Secondly, it better protects the user against decompression bombs. Note that in the Python implementation the code attempts to read the entire gzipped file and as such is vulnerable to a decompression bomb: utils.py:934: fileobject = six.StringIO(gzip.open(filepath).read().decode('utf-8’))

Like @JustinCappos, I'm also not sure that this change is straightforward.

This might have implications for snapshot metadata. There are use cases where, besides version numbers, the hashes of targets and delegated targets metadata files might be listed in the snapshot metadata. Do we include the hashes of both the compressed and uncompressed metadata files in there?

trishankkarthik · 2017-03-10T20:06:09Z

docs/tuf-spec.txt

- "targets/projects"
+
+ ROLENAME is the name of the delegated role. For example,
+ "projects".


So does this mean that a ROLENAME must only be alphanumeric? How does this new ROLENAME format change how metadata files are placed on the repository? Is this format change backwards-compatible with old clients?

Section 3.1.2.1 explains how delegated metadata is placed on the repository.

This change is certainly backwards-incompatible, but so are many of the changes in this version 1.0 of the specification (e.g., terminating delegations, version numbers in Snapshot, etc.)

I'm not sure that we can set restrictions on the rolename, which must ultimately match the filename of the role as saved to the underlying file system. Should we state in the specification that we do not place restrictions on rolenames, and supported characters in rolenames are left to adopters?

trishankkarthik · 2017-03-10T20:07:48Z

docs/tuf-spec.txt

+ terminating delegations instruct the client not to consider future trust
+ statements that match the delegation's pattern. This stops the delegation
+ processing once this delegation (and its descendants) have been processed.
+ (Handling this case is conceptually similar to the use of the cut operator


I think we can safely remove the parenthetical note, most people won't care :)

trishankkarthik · 2017-03-10T20:08:58Z

docs/tuf-spec.txt

+ shell-style wildcards. For example, the path pattern "targets/*.tgz" would
+ match file paths "targets/foo.tgz" and "targets/bar.tgz", but not
+ "targets/foo.txt". Likewise, path pattern "foo-version-?.tgz" matches
+ foo-version-2.tgz", but not "foo-version-alpha.tgz".


One more matching example here would help. Right now, it looks as if "?" matches only "numbers".

trishankkarthik · 2017-03-10T20:09:54Z

docs/tuf-spec.txt

+ order to accommodate this scheme, the "roles" key in the DELEGATIONS object
+ above points to an array, instead of a hash table, of delegated roles.
+
+ Another scheme would have the clients prefer the delegated role with the


I think we can remove this paragraph. It's an interesting, historical footnote, but has nothing to do with how TUF works now.

trishankkarthik · 2017-03-10T20:10:31Z

docs/tuf-spec.txt

+ "targets/foo.txt". Likewise, path pattern "foo-version-?.tgz" matches
+ foo-version-2.tgz", but not "foo-version-alpha.tgz".
+
+ Several schemes exist to resolve conflicts between delegated roles that


Could we please remove the mention of "several schemes", and simply note that we use prioritized delegations ala Diplomat to resolve conflicts? Thanks!

trishankkarthik · 2017-03-10T20:11:50Z

docs/tuf-spec.txt

@@ -1004,8 +1028,8 @@ Version 1.0 (Draft)
 required, the root.json file is versioned and accessible by version number,
 e.g. 3.root.json. Clients update the set of trusted root keys by requesting
 the current root.json and all previous root.json versions, until one is
- found that has been signed by keys the client already trusts. This is to 
- ensure that outdated clients remain able to update, without requiring all 
+ found that has been signed by keys the client already trusts. This is to


Very good change! If I'm not mistaken, CoreOS proposed this change, no? If so, let's credit them here?

Thanks is now given (in the PR summary) to Evan and the CoreOS team. In a separate edit to the specification, a "contributions" section on features that have been contributed by others will be added to the specification.

I think thanks is also given to the author(s) in the pull request that implements the feature.

in text for terminating delegations.

One more matching example here would help. Right now, it looks as if "?" matches only "numbers".

coveralls · 2017-04-24T19:27:00Z

Coverage remained the same at 97.936% when pulling 3532fb8 on update_specification into 40aaf93 on develop.

vladimir-v-diaz · 2017-04-24T20:18:46Z

@trishankkarthik I have gone through all of your comments. Please give the pull request, or my tacked-on commits, another pass when you have the time.

coveralls · 2017-04-24T20:38:06Z

Coverage remained the same at 97.936% when pulling 7eae7e3 on update_specification into 40aaf93 on develop.

JustinCappos · 2017-04-24T21:11:10Z

I'm slightly confused about why this change is showing up here. Wouldn't this require a TAP? Was this added prior to 1.0?

…

On Mon, Apr 24, 2017 at 12:09 PM, Vladimir Diaz ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In docs/tuf-spec.txt <#433 (comment)> : > @@ -530,6 +532,14 @@ Version 1.0 (Draft) , ... } } + COMPRESSION_ALGORITHM specifies one of the compression algorithms supported Listing the compression algorithms used by the repository removes the need to list all compressed versions of metadata in Snapshot, reducing its size and minimizing code complexity. Should the benefits of listing the compression algorithm(s) be stated in the specification, or are you simply curious for the change? There were a few other reasons, which are captured in an email thread with David Lawrence @endophage <https://github.com/endophage> (see email thread with title, "role names"). I'll quote some part of his request to remove compressed metadata in Snapshot, etc. I feel very strongly that data compression should be implemented at layer 6 and TUF should not be validating compressed files. As is, a malicious publisher could intentionally publish decompression bomb. The TUF user would download and validate the hash and size of the compressed file but be vulnerable when they decompress. By implementing compression at layer 6, the client will decompress the stream as it arrives and never read more than Length bytes from the incoming stream. If we were talking about GBs of data there would be a strong argument for compression on disk, but TUF data is (and necessarily so) in the order of KB and the storage gains from compression are minimal. Compression at layer 6 still provides the benefit of reduced network transfer. ... Overall, I think consistently not using file extensions when referencing TUF metadata within the metadata files, and specifying any compression is done at layer 6 overall is a security improvement on 2 fronts. Firstly it makes the metadata more consistent, reducing code complexity, which makes auditing easier and chance of bugs lower. Secondly, it better protects the user against decompression bombs. Note that in the Python implementation the code attempts to read the entire gzipped file and as such is vulnerable to a decompression bomb: utils.py:934: fileobject = six.StringIO(gzip.open(filepath).read().decode('utf-8’)) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#433 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA0XD3rGeaQXYe4rnmTcWsMJzI-HtC3Cks5rzMkkgaJpZM4MNpVN> .

vladimir-v-diaz · 2017-04-25T15:48:30Z

Trishank and I went over my comments. We need to verify whether
(1) the compression algorithms can be excluded from the Snapshot file
(2) restrictions should be set for rolenames. For example, exclude unicode

vladimir-v-diaz · 2017-04-25T16:02:33Z

I'm slightly confused about why this change is showing up here. Wouldn't
this require a TAP? Was this added prior to 1.0?

The change to exclude compressed version of metadata from Snapshot was discussed (via email with the Docker folks) and implemented before the TAP process was adopted.

vladimir-v-diaz · 2017-06-02T15:21:45Z

Trishank and I went over my comments. We need to verify whether
(1) the compression algorithms can be excluded from the Snapshot file
(2) restrictions should be set for rolenames. For example, exclude unicode

I have excluded the compression algorithm field from Root.json, and will open an issue for (2). Merging!

coveralls · 2017-06-02T15:43:56Z

Coverage remained the same at 97.936% when pulling dae2b7a on update_specification into c1e72e0 on develop.

vladimir-v-diaz added 5 commits February 27, 2017 14:48

Add compression_algorithms and consistent_snapshot attributes to root…

c105077

….json

Update text describing format of PATHPATTERN

1a00c4c

Update text for handling overlapping targets between delegations.

bbd3288

Remove note about priority tag schemes and our investigation of several of these schemes

Add 'terminating' attribute and text explaining its use

6cca34b

Remove notion of a full rolename

6665d25

We previously used a full rolename when delegations resembled a tree

trishankkarthik reviewed Mar 10, 2017

View reviewed changes

awwad force-pushed the develop branch 2 times, most recently from 61d04eb to 40aaf93 Compare March 22, 2017 16:11

vladimir-v-diaz added 3 commits April 24, 2017 12:25

Remove parenthetical note

adf48f9

in text for terminating delegations.

Add matching example of shell-style wildcard

116cb39

One more matching example here would help. Right now, it looks as if "?" matches only "numbers".

Remove alternative schemes for overlapping targets

3532fb8

Explain that section 7 covers consistent snapshots

7eae7e3

vladimir-v-diaz mentioned this pull request Jun 2, 2017

Clarification needed on terminating delegations #453

Closed

Merge branch 'develop' into update_specification

dae2b7a

vladimir-v-diaz merged commit 8ddc62c into develop Jun 2, 2017

vladimir-v-diaz deleted the update_specification branch July 11, 2017 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update specification #433

Update specification #433

vladimir-v-diaz commented Feb 27, 2017 •

edited

Loading

vladimir-v-diaz commented Feb 27, 2017

coveralls commented Feb 27, 2017

trishankkarthik commented Feb 27, 2017

trishankkarthik left a comment

trishankkarthik Feb 27, 2017

trishankkarthik Mar 10, 2017

vladimir-v-diaz Apr 24, 2017 •

edited

Loading

trishankkarthik Apr 24, 2017

trishankkarthik Mar 10, 2017

vladimir-v-diaz Apr 24, 2017

trishankkarthik Mar 10, 2017

trishankkarthik Mar 10, 2017

trishankkarthik Mar 10, 2017

trishankkarthik Mar 10, 2017

trishankkarthik Mar 10, 2017

vladimir-v-diaz Apr 24, 2017

coveralls commented Apr 24, 2017

vladimir-v-diaz commented Apr 24, 2017

coveralls commented Apr 24, 2017

JustinCappos commented Apr 24, 2017 via email

vladimir-v-diaz commented Apr 25, 2017 •

edited

Loading

vladimir-v-diaz commented Apr 25, 2017

vladimir-v-diaz commented Jun 2, 2017 •

edited

Loading

coveralls commented Jun 2, 2017

Update specification #433

Update specification #433

Conversation

vladimir-v-diaz commented Feb 27, 2017 • edited Loading

vladimir-v-diaz commented Feb 27, 2017

coveralls commented Feb 27, 2017

trishankkarthik commented Feb 27, 2017

trishankkarthik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vladimir-v-diaz Apr 24, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Apr 24, 2017

vladimir-v-diaz commented Apr 24, 2017

coveralls commented Apr 24, 2017

JustinCappos commented Apr 24, 2017 via email

vladimir-v-diaz commented Apr 25, 2017 • edited Loading

vladimir-v-diaz commented Apr 25, 2017

vladimir-v-diaz commented Jun 2, 2017 • edited Loading

coveralls commented Jun 2, 2017

vladimir-v-diaz commented Feb 27, 2017 •

edited

Loading

vladimir-v-diaz Apr 24, 2017 •

edited

Loading

vladimir-v-diaz commented Apr 25, 2017 •

edited

Loading

vladimir-v-diaz commented Jun 2, 2017 •

edited

Loading