-
Notifications
You must be signed in to change notification settings - Fork 58
Rollback attacks and fast forward recovery #150
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.
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
0e76a17
8a20ecf
33c221b
4bceaf4
869129c
0ca0b7e
6446585
1706edf
bac0c55
7726374
3de7f6e
159c0e8
c2e206f
495370d
d9e0596
eed4388
be71c07
8652ff5
180e9db
9698015
40e3e7c
e50151d
07a46db
e5c0729
5441368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1334,26 +1334,16 @@ it in the next step. | |
| listed metadata version number (possibly MAX_INT) is greater than the new valid | ||
| version. To recover from a fast-forward attack after the repository has been | ||
| compromised and recovered, certain metadata files need to be deleted as | ||
| specified in this section. If a delegated targets file is subjected to a | ||
| 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) | ||
| for more details on fast-forward attacks. | ||
|
|
||
| 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 this only applies to top-level targets metadata whose | ||
| keys are listed in root metadata. | ||
|
|
||
| 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 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hosseinsia thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is clever. I like it! So at least a combination of non-revoked keys should have signed the metadata?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| validating using a threshold of snapshot keys from the new trusted root | ||
mnm678 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| metadata, delete the trusted snapshot and timestamp metadata | ||
| files. | ||
|
Comment on lines
+1349
to
+1352
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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:
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)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, how would this heuristic even work in practice:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
💯 I've filed #186 to track this
Added this notion of changing from "delete file" to "stop trusting" to #103
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. |
||
|
|
||
| 3. **Timestamp recovery** If a threshold of timestamp keys have | ||
| been removed from the new trusted root metadata compared to the previous | ||
| trusted root metadata, delete the old timestamp metadata file. | ||
|
|
||
| 12. **Set whether consistent snapshots are used as per the trusted** | ||
| root metadata file (see [[#file-formats-root]]). | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's link to our own copies.