TAP 8 Implementation

[mnm678]

This is some initial implementation work for the TAP per the most recent pr. My plan for this pr is to include:

• Rotate file definition
• Use of rotate files in the verification flow
• Testing

cc @joshuagl who mentioned interest in collaborating on this

[coveralls]

Pull Request Test Coverage Report for Build 4028042547

• 104 of 110 (94.55%) changed or added relevant lines in 4 files are covered.
• 5 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.7%) to 97.505%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
tuf/api/metadata.py	52	58	89.66%

Files with Coverage Reduction	New Missed Lines	%
tuf/ngclient/_internal/trusted_metadata_set.py	2	98.72%
tuf/api/metadata.py	3	97.5%

Totals
Change from base Build 3821837306:	-0.7%
Covered Lines:	1437
Relevant Lines:	1461

---

💛 - Coveralls

[jku]

I've not reviewed properly but I'll write down some early high level worries:

• we can't just change the security properties that the implementation has: previously it was not possible for keyowners to change their keys, with this PR it suddenly is...
• we already know that TUF is a little chatty in the no-op case: even if client has all the metadata it needs, there are two requests. This PR seems to bump that minimum to 6 requests -- I don't quite understand why but that's what the test results look like
• including snapshot and timestamp in the rotated roles seems not very useful, but more importantly a "layering violation": if you want to include rotate files in snapshot, then the rotation should only apply to targets
• the TAP for some reason includes some complicated string ops to generate the rotate/ROLE.rotate.ID.PREV filename -- this has been in the spec for over two years but seems to be completely missing here or have I misunderstood?

[jku]

this requires that key ids and roles do not contain any "."

👀

[mnm678]

I've not reviewed properly but I'll write down some early high level worries:

Thanks for taking a look!

* we can't just change the security properties that the implementation has: previously it was not possible for keyowners to change their keys, with this PR it suddenly is...

Agreed, we'll want to be intentional about adding this. It might be as easy as a major version change to the code, or we might need a new major specification version.

* we already know that TUF is a little chatty in the no-op case: even if client has all the metadata it needs, there are two requests. This PR seems to bump that minimum to 6 requests -- I don't quite understand why but that's what the test results look like

It was easier to download rotate files at the same time as other metadata, then compare that list to the rotate files listed in snapshot in the next step (similar to the root update mechanism of trying to download the next root). I'll work on a way to check snapshot first so that if no rotate files are listed, we can skip the extra requests.

* including snapshot and timestamp in the rotated roles seems not very useful, but more importantly a "layering violation": if you want to include rotate files in snapshot, then the rotation should only apply to targets

I'll want to think about this one. My current motivating example for this TAP mostly involves targets rotation (like for delegated targets in a PEP 480 style design), but there are other potential uses for rotating the snapshot/timestamp keys (so they can change more frequently). It'd be nice to have the same mechanism for both, but we'll want to make sure this doesn't mess up the security properties of snapshot.

* the TAP for some reason includes some complicated string ops to generate the `rotate/ROLE.rotate.ID.PREV` filename -- this has been in the spec for over two years but seems to be completely missing here or have I misunderstood?

I have a pr out to simplify the TAP. I based this pr on my revised design, but can update it based on any discussion in that thread.

[jku]

My current motivating example for this TAP mostly involves targets rotation (like for delegated targets in a PEP 480 style design), but there are other potential uses for rotating the snapshot/timestamp keys (so they can change more frequently).

I get that but neither of those version numbers make sense within the snapshot metadata (which won't be valid at the time you are trying to use the rotate versions):

• We use a non-versioned timestamp: requiring a versioned timestamp-rotate does not make sense to me
• snapshot versions are embedded in valid timestamp metadata. If you want to use a versioned snapshot-rotate, then you have to include that snapshot-rotate version in timestamp metadata, not snapshot metadata.

[mnm678]

The version numbers of the rotate files don't relate to the version of the metadata, but are rather used to determine the order of the rotate files in the chain and use that to find the correct signing key(s) for a role.

The snapshot validity is a bigger issue. If we skip the snapshot check for rotate files for the timestamp and snapshot roles, there's some chance that an attacker with mitm can block rotate files and convince a user to use the wrong key for these roles. I think this might be an acceptable tradeoff because (1) root retains the ability to revoke any key rotated with this mechanism, (2) all rotate files are signed, so the attacker would need mitm + a compromised key, and (3) these roles are less trusted that targets roles. I'll think about this some more and propose it over in the TAPs repo.

[jku]

The version numbers of the rotate files don't relate to the version of the metadata, but are rather used to determine the order of the rotate files in the chain and use that to find the correct signing key(s) for a role.

[EDIT: I was confused, next paragraph does not make sense]

I understand that. The point I'm trying to make is that pinning timestamp-rotate versions is not useful because an attacker with a timestamp signing key (the one defined in root metadata) can just create a new timestamp version. Because timestamp versions are not pinned, the new timestamp will always be used, and as a result the rotate file won't be used.

I do not understand what timestamp-rotate files accomplish

[mnm678]

I understand that. The point I'm trying to make is that pinning timestamp-rotate versions is not useful because an attacker with a timestamp signing key (the one defined in root metadata) can just create a new timestamp version. Because timestamp versions are not pinned, the new timestamp will always be used, and as a result the rotate file won't be used.

I do not understand what timestamp-rotate files accomplish

I'm not sure I understand the question. Here's how verification with this proposal would work (ignoring snapshot for now). The client verifies root metadata and gets the timestamp key. They then look for a 1.timestamp.rotate file, if present they verify it with the timestamp keys from root and switch to the keys listed in 1.timestamp.rotate (and so on for 2.timestamp.rotate, etc). Using the keys they end up with after checking rotations, they download the most recent timestamp and attempt to verify, following the existing procedure.

So the rotation part happens before the latest timestamp is even downloaded.

[jku]

Uh you are right, my last comment doesn't make sense, sorry.

The original point about the ordering still stands:

• We need to choose a timestamp-rotate version to decide if timestamp is valid
• timestamp-rotate versions are pinned in snapshot

Snapshot can only be loaded with a valid timestamp... but verifying timestamp validity requires looking into the snapshot content. This feels a bit weird.

[mnm678]

I opened an issue to track this that lists a couple of potential solutions. I will update this pr and the TAP based on whatever is decided there.

[jku]

I'd like to close this, would that be ok?

I understand that this is a POC for the TAP, but looking at it from python-tuf maintainer perspective this likely requires quite a bit of work still to be merged:

• Analysis/discussion that confirms this really is the solution python-tuf wants to go with -- I'm personally not fully sold on TAP8 (but could be persuaded):
  • this is not the only possible solution to handle "self-delegations"
  • we now know that this solution increases client code complexity and the common no-op update process latency (both already issues for TUF)
  • TAP is a bit vague WRT compatibility issues: should support for a feature like this be automatically enabled in existing client libraries/apps? if not, how should it be enabled?
• Design doc for python-tuf specifically would be good: I don't mean anything fancy but I think high impact PR without even an issue filed is not a reasonable approach
• PR that implements the (intended) spec/TAP: I believe this PR is behind quite a bit. It's no longer useful for evaluating the TAP

[jku]

I'll close this since no comments: please reopen when you feel like it.