Rollback attacks and fast forward recovery #150

mnm678 · 2021-02-25T17:33:11Z

This is my attempted takeover of #86. Based on discussion in that pr, I decided to explicitly recommend that fast forward attack recovery for delegated targets roles requires replacement of snapshot keys. Specifically this pr:

Removes redundant rollback protection by leaving Remove problematic targets rollback attack check #65 in place and removes the redundant rollback check for delegated targets.
Adds a note about fast forward attack recovery for delegated targets.
Includes related clarifications from Clarify how delegated roles are downloaded #72, Clarify rotation in 1.9 #74, and Fast-forward recovery and rollback prevention #85 rebased for the new specification format.

It might be worth adding an ADR/other documentation about the possible methods for fast forward attack recovery and why this one was chosen.

cc @lukpueh @joshuagl @trishankatdatadog

joshuagl · 2021-03-03T09:26:10Z

Thank you for working on this Marina! FYI I'm planning to review this early next week.

tuf-spec.md

trishankatdatadog · 2021-03-04T16:18:46Z

tuf-spec.md

+    1. **Targets recovery** If a threshold of targets keys have been
+    removed in the new trusted root metadata compared to the previous trusted
+    root metadata, delete the old top-level targets and snapshot metadata
+    files. Note that delegated targets are susceptible to fast forward attacks,


I have to admit these two sentences are not immediately clear to me. Would you please rephrase and clarify?

trishankatdatadog · 2021-03-04T16:19:35Z

tuf-spec.md

-  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
+5. **Check for a rollback attack**. The version number of all targets metadata files in the


Nit: looks like line overflow

trishankatdatadog · 2021-03-04T16:21:01Z

tuf-spec.md

@@ -1470,18 +1486,53 @@ it in the next step.
  2. Otherwise, recursively search the list of delegations in
     order of appearance.

-    1. If the current delegation is a multi-role delegation,
+    1. Let DELEGATEE denote the current target role TARGETS is


Could we please say DELEGATOR instead of TARGETS? Can be conflated at first glance with the top-level targets role.

This is using the same TARGETS defined at the beginning of step 7 (line 1475 in this version). I'll see if I can clarify this.

Sorry, just to be sparkling clear, I think we should start off by setting the DELEGATOR role to the top-level targets role, and go from there. Otherwise, it could confuse a few readers.

I think it could be clearer. Please see my latest suggestions?

That makes sense. I think yours maps more closely to what the code would look like.

mnm678 · 2021-05-13T18:06:56Z

ping @trishankatdatadog @joshuagl

tuf-spec.md

mnm678 · 2021-05-13T20:32:46Z

(I'll squash these last few commits once we finalize)

joshuagl · 2021-05-14T11:24:33Z

(I'll squash these last few commits once we finalize)

Possibly better to do it now, as squash & force push will discard any approvals.

mnm678 · 2021-05-14T15:28:31Z

Possibly better to do it now, as squash & force push will discard any approvals.

Fair enough, it's either discard the approvals or discard the comment history :). I'll go ahead and fix it now

trishankatdatadog · 2021-05-14T22:04:30Z

The whole preorder DFS is really not defined accurately. I can help to edit the text, but this code might help in the meantime.

mnm678 · 2021-05-19T18:27:12Z

The whole preorder DFS is really not defined accurately. I can help to edit the text, but this code might help in the meantime.

Which part? This pr just adds the snapshot and hash checks for delegated targets

trishankatdatadog · 2021-05-19T19:41:01Z

Which part? This pr just adds the snapshot and hash checks for delegated targets

Yeah, maybe best done in another PR, but if you read the whole thing, you can see it has some work to do.

mnm678 · 2021-05-20T19:06:31Z

Yeah, maybe best done in another PR, but if you read the whole thing, you can see it has some work to do.

I think I see what you mean. I'll move the check added in this pr to after the section about multi-role delegations, because these checks should be done for each role in the multi-role delegation. There might be some better clarifications to the section as a whole, but I'll leave that to a separate pr to avoid too much scope creep here.

joshuagl

This looks good. Some minor nits and the version & date will need to be bumped following the recent merge.

joshuagl · 2021-05-24T19:54:53Z

tuf-spec.md

+  timestamp, snapshot, targets, or delegated targets metadata. The attacker's goal
+  is to cause clients to refuse to update the metadata later because the attacker's
+  listed metadata version number (possibly MAX_INT) is greater than the new valid
+  version.  To recover from a fast-forward attacks after the repository has been


"recover from fast-forward attacks" dropping a
Or
"Recover from a fast forward attack" dropping plural s attacks

joshuagl · 2021-05-24T19:58:30Z

tuf-spec.md

+       number of bytes. The value for Z is set by the authors of the application
+       using TUF. For example, Z may be tens of kilobytes. IF DELEGATEE cannot be
+       found, end the search and report the target cannot be found.  If
+       consistent snapshots are not used (see Section 7), then the filename used


Let's cross-reference the consistent snapshots section here and not refer to it by explicit number, i.e. "[[#consistent-snapshots]]"

joshuagl · 2021-05-24T20:02:18Z

tuf-spec.md

+    6. **Check for a freeze attack.** The latest known time
+       should be lower than the expiration timestamp in the new DELEGATEE
+       metadata file. If so, the new DELEGATEE file becomes the trusted DELEGATEE


We changed the way we describe a freeze attack check, could you make this consistent with the other checks in the spec?

"The expiration timestamp in new DELEGATEE metadata file MUST be higher than the fixed update start time."

mnm678 · 2021-05-24T21:31:38Z

Thanks @joshuagl, I updated and rebased over the recent merge.

Section 7 is a little vague how on delegated targets are fetched and validated. This updates that section to use the same logic and verification process as downloading the top-level targets role to be explicit. One thing to point out though is that the old section 7 suggests that we don't report verification errors to the user. I've preserved this in my explicit version. I imagine users would still be notified if their delegated roles may be undergoing an attack. Is this intentional, or should I switch to the "abort the update cycle, and report the potential rollback attack"-style phrasing used elsewhere in the spec?

A recent commit added a detailed verification workflow for delegated targets, including check against snapshot, and signature and version check. This commit adds the missing freeze attack (i.e. timestamp) check.

Recent commits added a detailed verification workflow for delegated targets, including check against snapshot, and signature, version and timestamp checks. This commit adopts the failure reporting phrasing used elsewhere in the client workflow.

Clarify what files to delete or untrust in various situations to recover from a fast-forward attack on top-level metadata.

To recover from a fast-forward attack on the top-level targets metadata, only targets and snapshot (not timestamp) metadata must be untrusted/deleted. Targets, because it was attacker controlled, and snapshot, because it unwittingly might have unwittingly recorded the attacker controlled targets version.

Akin to the recovery from fast-forward attacks on the top-level targets role, if a delegated targets role has been compromised, the previously trusted delegated targets metadata and the previously trusted snapshot metadata must be deleted. This must happen so that the rollback attack check (*), which makes sure that the version number of the new delegated targets is higher (or equal) than that of the old does not prevent updates after an ffwd attack. For the top-level targets metadata ffwd recovery logic is performed based on key removals in the root metadata and thus can happen before downloading the snapshot metadata. For delegated targets, on the other hand, where the keys are defined by delegating targets role(s) and not in the root metadata, ffwd recovery logic can only be performed after the delegating targets have been downloaded. (*) Note that there are two targets role rollback checks. One is based on the snapshot metadata, to fail early, i.e. before a potentially compromised (delegated) targets metadata is downloaded, and the other is based directly on the (delegated) targets metadata, so that an attacker needs to compromise snapshot and (delegated) targets keys, to successfully perform a rollback attack. This commit updates the client workflow according to above observations.

Co-Authored-By: Justin Cappos <justincappos@gmail.com> Co-Authored-By: Joshua Lock <jlock@vmware.com>

trishankatdatadog · 2021-09-01T15:55:17Z

What I mean is: I believe we only care whether metadata is signed by the current threshold of keys. It it is not, it should not be used as trusted metadata.
What do you mean by current?

Whenever we load a delegator like root we get current keys/thresholds for the delegated metadata. If delegated metadata is not signed by these keys, it should not be trusted after this point. So in practice: if a new root is loaded with new timestamp keys, the local timestamp we might have on disk (or even loaded in memory) is not signed with those keys and cannot be trusted from now on.

So IIUC you mean if no m_t of k_t exists in m_{t+n} of k_{t+n}, then delete previous timestamp/snapshots/targets metadata.

This was also my interpretation, but Hossein pointed out that it's too strong. Look at his example here.

Just because a threshold of old keys have colluded and caused fast-forward attacks, doesn't mean you have the luxury to rotate all the rest that have not provably colluded.

So, the requirement should be less strict: if any (not all) old threshold of old keys have been rotated out, then take that as a signal of possible fast-forward attack, and delete the previous metadata.

jku · 2021-09-01T16:39:19Z

So IIUC you mean if no m_t of k_t exists in m_{t+n} of k_{t+n}, then delete previous timestamp/snapshots/targets metadata.

I have to admit I don't understand what that means... but I'll look at the examples:

(1) If the target role keys are [a, b, c, d] and threshold is 2 in v1 and the new keys are [b, c, d, e] then no need to remove targets.json because only one key is being revoked (for being compromised) and an attacker could not have run the fast-forward attack.

If we have loaded root v2 and current targets keys are [b,c,d,e], we absolutely cannot trust targets.json if it was signed by only two keys and one of them was a, because at that point it is not signed by threshold of keys.

On the other hand if targets.json was signed by more than two keys (or one of them was not a), things are just fine.

(2) If the target role keys are [a, b, c, d] and threshold is 2 in v1 and the new keys are [c, d, e] then we have to remove targets.json

If targets.json was signed by at least c & d, then we can trust it and do not have to remove it 🤷

From the context I finally think I understand what you are getting at: you think you can use the number of removed keys in a specific delegator update in comparison to the threshold of keys as a signal... I don't think you can as the keys might have been removed one by one in multiple metadata versions -- the possibility of a ffwd attack is still there even though only one key was removed at a time.

I don't quite understand why you want to remove targets.json if you suspect a ffwd attack may have happened? the real issue is in snapshot (where the targets.json version might be too big) and the issue may have been there for multiple snapshot versions already (so you can't just go back to a previous snapshot or something). I don't think there are other solutions to this problem than snapshot key rotation, and frankly it seems like a really good solution to me...

trishankatdatadog · 2021-09-01T18:40:18Z

we absolutely cannot trust targets.json if it was signed by only two keys and one of them was a, because at that point it is not signed by threshold of keys.

The key problem is that we are looking at only the previous and current roots now to decide whether to delete previous timestamp/snapshot/targets metadata without looking at the current version of the latter, you see? So you cannot check based on what the actual signatures are. You can only decide based on comparing previous/current threshold of timestamp/snapshot/targets keys.

mnm678 · 2021-09-01T19:39:51Z

As I see it there are 2 things being discussed (and possibly confused) here:

Do we need explicit fast forward attack workflows in the client for non-snapshot metadata? If targets or timestamp are fast-forwarded, this can be solved by replacing the snapshot keys on the repo to trigger the snapshot metadata workflow.
When doing the client fast forward attack recovery, when should this recovery be triggered?

For 1, this logic is similar to the reason for not providing explicit fast-forward attack recovery for delegated targets. I'm starting to agree with @jku that snapshot might be the only place that fast forward attack recovery is needed.

2 is a bit trickier. If you have a previous trusted snapshot metadata file A and a new one B (both signed by a valid threshold of keys from the root metadata at the time they were downloaded), how do you know if you should do rollback checks between B and A, or skip these to recover from a possible fast forward attack. I would argue that the rollback checks should be skipped whenever a threshold_A of keys from A is not present in B (which I think is what @trishankatdatadog is arguing).

Am I missing anything?

jku · 2021-09-01T20:14:08Z

we absolutely cannot trust targets.json if it was signed by only two keys and one of them was a, because at that point it is not signed by threshold of keys.

The key problem is that we are looking at only the previous and current roots now to decide whether to delete previous timestamp/snapshot/targets metadata without looking at the current version of the latter, you see?

This is what I'm suggesting, no? If the new root rotated targets keys in a way that the local targets.json is no longer signed by correct threshold of keys, we should not trust that targets.json for anything. I'm not waiting for some next version of targets before making that decision, I'm deciding that immediately as the keys are rotated.

You can only decide based on comparing previous/current threshold of timestamp/snapshot/targets keys.

I agree that we should only look at the keys/threshold, but I'm arguing we can only consider the currently valid keys/threshold. The idea that we can somehow compare current and previous keys is just not sound. Just as one example, imagine this scenario:

client updates root from remote, gets a new version N+1 that includes targets key rotations
update process gets somehow canceled before other metadata is updated. N+1 is now the current root version
client starts the update again, updates root to N+2 (it has no additional key rotations)
when targets is being updated, root N+1 is already the "previous" version: previous and current versions have exact same keys and the ffwd detection does not trigger

jku · 2021-09-01T20:29:30Z

2 is a bit trickier. If you have a previous trusted snapshot metadata file A and a new one B (both signed by a valid threshold of keys from the root metadata at the time they were downloaded), how do you know if you should do rollback checks between B and A, or skip these to recover from a possible fast forward attack. I would argue that the rollback checks should be skipped whenever a thresholdA of keys from A is not present in B (which I think is what @trishankatdatadog is arguing).

I still don't understand why the repository can't handle this. If there is a fast forward attack I think it's completely reasonable to expect the repository to update root and rotate the keys of the snapshot role: key rotation logically leads to client not using the old snapshot for the rollback checks (as at that point the old snapshot is not signed by current threshold of snapshot keys). is this for some reason a bad solution?

mnm678 · 2021-09-01T20:45:10Z

I still don't understand why the repository can't handle this. If there is a fast forward attack I think it's completely reasonable to expect the repository to update root and rotate the keys of the snapshot role: key rotation logically leads to client not using the old snapshot for the rollback checks (as at that point the old snapshot is not signed by current threshold of snapshot keys). is this for some reason a bad solution?

Oh, I think I understand what you're saying now. The client would use the keys in the current root file to verify both the previous and the new snapshot, then if the verification for the previous failed with the current keys, the rollback check would be skipped. Is that what you're talking about? That does seem simpler than the previous/current comparison

jku · 2021-09-01T20:52:08Z

I still don't understand why the repository can't handle this. If there is a fast forward attack I think it's completely reasonable to expect the repository to update root and rotate the keys of the snapshot role: key rotation logically leads to client not using the old snapshot for the rollback checks (as at that point the old snapshot is not signed by current threshold of snapshot keys). is this for some reason a bad solution?

Oh, I think I understand what you're saying now. The client would use the keys in the current root file to verify both the previous and the new snapshot, then if the verification for the previous failed with the current keys, the rollback check would be skipped. Is that what you're talking about? That does seem simpler than the previous/current comparison

Yes! I think it's a fundamental idea that's already encoded into every rule in the client workflow but for some reason not spelled out: do not trust or use metadata that is not signed by current keys for that role. This should apply at all times: If the local ("old") snapshot metadata is not signed by current snapshot keys after a root update, the "old" snapshot should not be used for rollback checks (or anything at all, because it's not trusted!).

trishankatdatadog · 2021-09-01T22:05:26Z

Yes! I think it's a fundamental idea that's already encoded into every rule in the client workflow but for some reason not spelled out: do not trust or use metadata that is not signed by current keys for that role. This should apply at all times: If the local ("old") snapshot metadata is not signed by current snapshot keys after a root update, the "old" snapshot should not be used for rollback checks (or anything at all, because it's not trusted!).

We still disagree if I understand you correctly.

Take Hossein's example of m_t = 2, k_t = [a,b,c,d], m_{t+n} = 2, k_{t+n} = [c,d,e].

You can still use m_{t+n} and k_{t+n} (specifically [c,d]) to verify A_t, but that doesn't mean [a,b] wasn't used to fast-forward snapshot (fuhgedabout targets, no need to get distracted by that red herring). Hence Hossein's condition: if he sees any m_t of k_t removed in k_{t+n}, he reasonably and automatically assumes a fast-forward.

trishankatdatadog · 2021-09-01T23:19:51Z

Really, the best resolution is to use an explicit instead of implicit, ambiguous signal to delete timestamp/snapshot/targets metadata from one root to another, but that requires a spec change. Might be worth it than trying to get consensus by abusing an existing, orthogonal mechanism.

jku · 2021-09-02T06:48:16Z

Really, the best resolution is to use an explicit instead of implicit, ambiguous signal to delete timestamp/snapshot/targets metadata from one root to another, but that requires a spec change. Might be worth it than trying to get consensus by abusing an existing, orthogonal mechanism.

rotating keys is an explicit unambiguous signal that the old metadata (now no longer signed by current keys) should not be trusted and should not be used for anything, including rollback checks. I don't see how it's an orthogonal mechanism: this property is the signatures sole reason for existence. Do we really not have consensus on that?

Or did I misread what you mean by the "abusing an existing, orthogonal mechanism"?

mnm678 · 2021-09-02T13:43:19Z

rotating keys is an explicit unambiguous signal that the old metadata (now no longer signed by current keys) should not be trusted and should not be used for anything, including rollback checks. I don't see how it's an orthogonal mechanism: this property is the signatures sole reason for existence. Do we really not have consensus on that?

I agree, but I don't think 'key rotation' is currently well defined in the spec. Is it when one key is removed? A threshold of keys? I think that @jku's definition will make the code much simpler, and it relies on trusting the existing root metadata (which is already more trusted than the previous snapshot). However, @trishankatdatadog's definition is a bit more robust in the event that the threshold is lowered in the new root metadata. Although because an attacker that can lower the threshold in root metadata can do much stronger attacks, like just replacing all the keys, I think that we should go with 'trust existing root metadata' as a way to detect a rotation.

cc @JustinCappos

jku · 2021-09-02T17:14:50Z

I agree, but I don't think 'key rotation' is currently well defined in the spec.

yes, apologies: I meant any change to the roles keys/threshold that leads to local/old metadata to be not trusted.

Require that the snapshot keys are replaced for fast-forward attack recovery. This commit also simplifies the key rotation check by deleting the snapshot metadata if a threshold of new keys cannot verify the old snapshot metadata. Signed-off-by: Marina Moore <mnm678@gmail.com>

mnm678 · 2021-09-10T21:23:30Z

@trishankatdatadog @jku I updated the fast-forward attack recovery section. Does this look good to both of you?

trishankatdatadog · 2021-09-13T16:43:41Z

tuf-spec.md

-    2. **Snapshot recovery** If a threshold of snapshot keys have
-    been removed in the new trusted root metadata compared to the previous
-    trusted root metadata, delete the old snapshot and timestamp metadata
+    1. **Snapshot recovery** If the trusted snapshot metadata cannot be


@hosseinsia thoughts?

This is clever. I like it! So at least a combination of non-revoked keys should have signed the metadata?
And an attacker who uses the compromised keys to sign metadata won't succeed.

tuf-spec.md

hosseinsia · 2021-09-13T18:57:33Z

tuf-spec.md

-    2. **Snapshot recovery** If a threshold of snapshot keys have
-    been removed in the new trusted root metadata compared to the previous
-    trusted root metadata, delete the old snapshot and timestamp metadata
+    1. **Snapshot recovery** If the trusted snapshot metadata cannot be


This is clever. I like it! So at least a combination of non-revoked keys should have signed the metadata?
And an attacker who uses the compromised keys to sign metadata won't succeed.

Co-authored-by: Hossein Siadati <87046468+hosseinsia@users.noreply.github.com>

jku

I think we're moving closer to an agreement but making timestamp untrusted because of something in snapshot changes (even if it's the snapshot keys) still sounds fundamentally wrong.

Left a longer comment inline but that's the core issue I have I think.

jku · 2021-09-17T07:53:10Z

tuf-spec.md

+    1. **Snapshot recovery** If the trusted snapshot metadata cannot be
+    validated using a threshold of snapshot keys from the new trusted root
+    metadata, delete the trusted snapshot and timestamp metadata
+    files.


I think the core idea here is correct but I'm still not sure it makes sense to put the heuristic part into the client (that client must revoke trust on a correctly signed, valid timestamp just because snapshot is not signed). If snapshot key has been compromised and used to create snapshot version 1billion and timestamp process has accepted that into a timestamp, what prevents us from saying that the timestamp keys must then be rotated to enable rollback?

The client workflow is already complicated and quite tricky to implement: much more difficult than reading the spec text implies. Every addition like this makes it more complex, and it feels like things are added to client workflow partly because the repository workflows are not defined so things can't be added there.

I think the well defined ffwd recoveries are based on the repository rotating keys, and client taking care to not use untrusted metadata to do rollback checks:

if snapshot version must be rolled back, timestamp keys must be rotated (meaning timestamp keys should be modified so that all timestamps with "compromised snapshot versions" are no longer correctly signed, and will not be used for rollback checks by the client

if any targets versions must be rolled back (or removed from snapshot), snapshot keys must be rotated (meaning snapshot keys should be modified so that all snapshots with "compromised targets data" are no longer correctly signed, and will not be used for rollback checks by the client

For the client this creates basically no complexity: it just follows the basic rule, only believe trusted, correctly signed metadata. I don't think it puts an unfair burden on the repository either:

Some of its keys have been compromised

the online processes failed to prevent the ffwd (especially targets ffwd should be completely preventable)

we're only asking for online key rotation

The second issue is a nitpick and I don't expect you to change this as it uses language already in the spec but I'll mention it: my opinion is that we should stop talking about "deleting files", or at least make that a secondary aspect: what is important is that the client should not consider the metadata trusted anymore (in practice, we might want to say out loud why exactly we do this: the existing, now untrusted, metadata should not be used to do rollback checks on the new metadata)

In fact, how would this heuristic even work in practice:

client has a timestamp with snapshot version==1billion

client starts a refresh, downloads root, then downloads new timestamp with snapshot version==1

client sees a rollback attack, stops the update and never even downloads the snapshot that would include the heuristic that would make the valid timestamp now invalid

I agree that we should separate back out the timestamp vs snapshot recovery. Looking at it again, I actually don't think this check is in the right place in the workflow. I think the timestamp recovery should happen when the timestamp is downloaded, not as part of the root metadata workflow, and the same for snapshot. Especially as it is no longer tied to anything in the previous root metadata file.

It would be great to have separate documentation for the repository workflow. Right now all the advise for managing a fast forward attack (and a lot of other pieces) is buried here in the client workflow.

Friends, but this is where and what we recommend in the Mercury paper. See "5.3 Recovering from a repository compromise" on page 7.

This is getting contentious enough (for good reasons) that I'm not sure that endless cycles of reviews and comments would do sufficient justice. A meeting hasn't solved the problem either. Perhaps it's time for the BDFL to step in and make a decision one way or another? @JustinCappos

It would be great to have separate documentation for the repository workflow. Right now all the advise for managing a fast forward attack (and a lot of other pieces) is buried here in the client workflow.

💯 I've filed #186 to track this

The second issue is a nitpick and I don't expect you to change this as it uses language already in the spec but I'll mention it: my opinion is that we should stop talking about "deleting files", or at least make that a secondary aspect: what is important is that the client should not consider the metadata trusted anymore (in practice, we might want to say out loud why exactly we do this: the existing, now untrusted, metadata should not be used to do rollback checks on the new metadata)

Added this notion of changing from "delete file" to "stop trusting" to #103

Friends, but this is where and what we recommend in the Mercury paper. See "5.3 Recovering from a repository compromise" on page 7.

Mercury tells us "the online keys used to sign snapshot metadata can be revoked and replaced with new keys." and "The root-of-trust metadata indicates which keys can be trusted for verifying metadata files, including snapshot metadata files. This leads to a seamless and automatic recovery from fast-forward attacks after a repository compromise."

This seems to state that root metadata indicates which keys can be used for verifying trusted metadata. That is, that key rotation is the correct way to recover from fast-forward attacks and that rollback protection is only provided for top-level metadata which is trusted by the current root metadata.

JustinCappos · 2021-09-18T16:45:16Z

I've re-read the thread and feel like we could perhaps do with another meeting about this. I can unblock us if we can't resolve it after another round of discussion. I did spend a fair amount of time thinking about a way to explicitly indicate that the metadata should be deleted, but could not think of a good way to do so when considering clients that may miss updates over an extended period.

trishankatdatadog · 2021-09-18T18:54:26Z

I've re-read the thread and feel like we could perhaps do with another meeting about this. I can unblock us if we can't resolve it after another round of discussion.

Sure, I am willing to do one more meeting.

I did spend a fair amount of time thinking about a way to explicitly indicate that the metadata should be deleted, but could not think of a good way to do so when considering clients that may miss updates over an extended period.

Why not just set an explicit signal in the root metadata? You will always pick it up when you rotate roots.

JustinCappos · 2021-09-19T00:50:16Z

If you set the signal, do you remove it when you next rotate roots normally? What if a client misses the intermediate update? What if there is also a spec version upgrade? Does a client always get all root files?

…

On Sun, Sep 19, 2021 at 2:54 AM Trishank Karthik Kuppusamy < ***@***.***> wrote: I've re-read the thread and feel like we could perhaps do with another meeting about this. I can unblock us if we can't resolve it after another round of discussion. Sure, I am willing to do one more meeting. I did spend a fair amount of time thinking about a way to explicitly indicate that the metadata should be deleted, but could not think of a good way to do so when considering clients that may miss updates over an extended period. Why not just set an explicit signal in the root metadata? You will always pick it up when you rotate roots. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#150 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGROD2GZEQIBTO3B2XJALTUCTN63ANCNFSM4YG5OYRQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

joshuagl

Thanks for all the great discussion here, apologies for missing most of it.

I'd be willing to do have another discussion, but I think that might be more productive if we capture the options being proposed as a prerequisite?

joshuagl · 2021-09-21T09:15:54Z

tuf-spec.md

+  compromised and recovered, certain metadata files need to be deleted as
+  specified in this section. If a targets file is subjected to a
+  fast-forward attack, the snapshot role's keys should be replaced. Please see
+  [the Mercury paper](https://ssl.engineering.nyu.edu/papers/kuppusamy-mercury-usenix-2017.pdf)


Suggested change

[the Mercury paper](https://ssl.engineering.nyu.edu/papers/kuppusamy-mercury-usenix-2017.pdf)

[the Mercury paper](https://theupdateframework.io/papers/prevention-rollback-attacks-atc2017.pdf)

Let's link to our own copies.

joshuagl · 2021-09-21T09:20:40Z

tuf-spec.md

+  [the Mercury paper](https://ssl.engineering.nyu.edu/papers/kuppusamy-mercury-usenix-2017.pdf)
+  for more details on fast-forward attacks.
+
+    1. **Snapshot recovery** If the trusted snapshot metadata cannot be


Note: we don't clearly define what a trusted metadata file is, see #179. Given the implications via this PR and the importance of that term for rollback attack protection, we should address that.

joshuagl · 2021-09-21T10:15:25Z

tuf-spec.md

+    1. **Snapshot recovery** If the trusted snapshot metadata cannot be
+    validated using a threshold of snapshot keys from the new trusted root
+    metadata, delete the trusted snapshot and timestamp metadata
+    files.


It would be great to have separate documentation for the repository workflow. Right now all the advise for managing a fast forward attack (and a lot of other pieces) is buried here in the client workflow.

💯 I've filed #186 to track this

The second issue is a nitpick and I don't expect you to change this as it uses language already in the spec but I'll mention it: my opinion is that we should stop talking about "deleting files", or at least make that a secondary aspect: what is important is that the client should not consider the metadata trusted anymore (in practice, we might want to say out loud why exactly we do this: the existing, now untrusted, metadata should not be used to do rollback checks on the new metadata)

Added this notion of changing from "delete file" to "stop trusting" to #103

Friends, but this is where and what we recommend in the Mercury paper. See "5.3 Recovering from a repository compromise" on page 7.

Mercury tells us "the online keys used to sign snapshot metadata can be revoked and replaced with new keys." and "The root-of-trust metadata indicates which keys can be trusted for verifying metadata files, including snapshot metadata files. This leads to a seamless and automatic recovery from fast-forward attacks after a repository compromise."

This seems to state that root metadata indicates which keys can be used for verifying trusted metadata. That is, that key rotation is the correct way to recover from fast-forward attacks and that rollback protection is only provided for top-level metadata which is trusted by the current root metadata.

joshuagl · 2021-09-21T10:21:16Z

tuf-spec.md

+  desired target.** Let DELEGATOR refer to the current
+  top-level targets metadata role.


Suggested change

desired target.** Let DELEGATOR refer to the current

top-level targets metadata role.

desired target.** Let DELEGATOR refer to the

top-level targets metadata role.

At first I thought we were re-defining top-level targets metadata for each cycle, but then I realised by current you just mean the current trusted. I think that's implicit but either a) let's drop current or b) let's make it explicit

jku · 2022-01-07T10:01:46Z

Just a related note here: we now have tests for all rollback attacks and all fast-forward recovery cases in python-tuf ngclient thanks to @kairoaraujo (search for "rollback" in https://github.com/theupdateframework/python-tuf/blob/develop/tests/test_updater_top_level_update.py)

No ff-recovery specific rules are needed in the client. The only rule is:
only ever use metadata that is signed with current threshold of current keys, do not trust any other metadata

This directly leads to following matrix because of the rollback checks:

| to rollback | these keys must be rotated |
|-------------+----------------------------|
| timestamp   | timestamp                  |
| snapshot    | timestamp, snapshot        |
| targets     | snapshot                   |

(where "rotate" means that no previous, possibly fast forwarded, metadata is valid according to the new keys).

I believe that is in line with this PR and I think the PR is an improvement... but I think long term the spec would be more useful to implementers if it was built around this idea (only ever use metadata that is signed with current threshold of current keys) and not specific individual rules (like if specific situation A then delete files B and C): The same advice can still be given but it would be easier to understand cause-and-effect and design good software.

trishankatdatadog reviewed Mar 4, 2021

View reviewed changes

tuf-spec.md Outdated Show resolved Hide resolved

trishankatdatadog reviewed Mar 4, 2021

View reviewed changes

trishankatdatadog reviewed May 13, 2021

View reviewed changes

tuf-spec.md Outdated Show resolved Hide resolved

tuf-spec.md Outdated Show resolved Hide resolved

tuf-spec.md Outdated Show resolved Hide resolved

trishankatdatadog reviewed May 13, 2021

View reviewed changes

tuf-spec.md Outdated Show resolved Hide resolved

mnm678 force-pushed the client-verification branch from cd54ef6 to 7c534bc Compare May 14, 2021 15:32

joshuagl requested changes May 24, 2021

View reviewed changes

mnm678 force-pushed the client-verification branch from c0ec26d to f4f162d Compare May 24, 2021 21:30

This was referenced May 27, 2021

Remove multi-role vestigial #151

Merged

Clarify the preorder DFS #164

Open

mnm678 mentioned this pull request Jul 15, 2021

ngclient: improve rollback protection theupdateframework/python-tuf#1498

Closed

erickt and others added 8 commits August 12, 2021 15:05

Fix typos in client application workflow

8a20ecf

Add freeze attack check for delegated targets

33c221b

A recent commit added a detailed verification workflow for delegated targets, including check against snapshot, and signature and version check. This commit adds the missing freeze attack (i.e. timestamp) check.

Clarify recovery from ffwd on top-level roles

869129c

Clarify what files to delete or untrust in various situations to recover from a fast-forward attack on top-level metadata.

Add details about ffwd attacker goals

1706edf

Co-Authored-By: Justin Cappos <justincappos@gmail.com> Co-Authored-By: Joshua Lock <jlock@vmware.com>

trishankatdatadog mentioned this pull request Sep 2, 2021

Rotate roots per spec theupdateframework/go-tuf#143

Merged

mnm678 mentioned this pull request Sep 7, 2021

Clarify rollback attack prevention and fast-forward attack recovery #86

Closed

trishankatdatadog reviewed Sep 13, 2021

View reviewed changes

hosseinsia reviewed Sep 13, 2021

View reviewed changes

mnm678 and others added 2 commits September 13, 2021 15:10

Update tuf-spec.md

e5c0729

Co-authored-by: Hossein Siadati <87046468+hosseinsia@users.noreply.github.com>

Merge branch 'master' into client-verification

5441368

jku reviewed Sep 17, 2021

View reviewed changes

This was referenced Sep 21, 2021

Repository workflows: document how a repository recovers from compromise #186

Open

Remove file system requirements #103

Open

joshuagl reviewed Sep 21, 2021

View reviewed changes

mnm678 mentioned this pull request Aug 9, 2022

root version comparison still unclear #240

Open

	[the Mercury paper](https://ssl.engineering.nyu.edu/papers/kuppusamy-mercury-usenix-2017.pdf)
	[the Mercury paper](https://theupdateframework.io/papers/prevention-rollback-attacks-atc2017.pdf)

		desired target.** Let DELEGATOR refer to the current
		top-level targets metadata role.

Rollback attacks and fast forward recovery #150

Are you sure you want to change the base?

Rollback attacks and fast forward recovery #150

Conversation

mnm678 commented Feb 25, 2021

joshuagl commented Mar 3, 2021

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

mnm678 commented May 13, 2021

mnm678 commented May 13, 2021

joshuagl commented May 14, 2021

mnm678 commented May 14, 2021

trishankatdatadog commented May 14, 2021

mnm678 commented May 19, 2021

trishankatdatadog commented May 19, 2021

mnm678 commented May 20, 2021

joshuagl 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

mnm678 commented May 24, 2021

trishankatdatadog commented Sep 1, 2021

jku commented Sep 1, 2021

trishankatdatadog commented Sep 1, 2021 • edited Loading

mnm678 commented Sep 1, 2021

jku commented Sep 1, 2021 • edited Loading

jku commented Sep 1, 2021 • edited Loading

mnm678 commented Sep 1, 2021

jku commented Sep 1, 2021

trishankatdatadog commented Sep 1, 2021

trishankatdatadog commented Sep 1, 2021

jku commented Sep 2, 2021 • edited Loading

mnm678 commented Sep 2, 2021

jku commented Sep 2, 2021

mnm678 commented Sep 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jku left a comment • 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

JustinCappos commented Sep 18, 2021 via email

trishankatdatadog commented Sep 18, 2021

JustinCappos commented Sep 19, 2021 via email

joshuagl 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

jku commented Jan 7, 2022 • edited Loading

trishankatdatadog commented Sep 1, 2021 •

edited

Loading

jku commented Sep 1, 2021 •

edited

Loading

jku commented Sep 1, 2021 •

edited

Loading

jku commented Sep 2, 2021 •

edited

Loading

jku left a comment •

edited

Loading

jku commented Jan 7, 2022 •

edited

Loading