Clarify rollback attack prevention and fast-forward attack recovery

[lukpueh]

This PR combines and updates #72, which clarifies the verification routines for delegated targets, and #74 and #85, which fix #71, i.e. resolve ambiguity around rotating keys and deleting metadata, in order to recover from fast-forward attacks, by temporarily disabling rollback attack checks.

Pre-requisite

• 0ecf980 reverts Remove problematic targets rollback attack check #65 (see commit message for rationale)

Takeover of #72

• 75ac163 is a verbatim copy of the sole commit from Clarify how delegated roles are downloaded #72 courtesy of @erickt
• 329361a, c36e0c0 and e4a6f9e address Clarify how delegated roles are downloaded #72-reviews (by @lukpueh and @mnm678)

Takeover of #85

• Please see commit messages of b677f6a, fa2d263 and most notably 65e042d for details

[mnm678]

LGTM

[joshuagl]

Reviewed in isolation and without deep familiarity of the spec this seems like a solid set of clarifications.

[JustinCappos]

You talk about top-level targets below, but not here. Do you mean to differentiate them? Does any delegated role's key loss result in the top-level targets file being deleted?

[lukpueh]

I do mean top-level targets keys here too. I thought that might be clear, because root metadata only lists top-level targets keys.

Regarding your other question, why would we delete the top-level targets file if any delegated role's key is lost? There is a detailed explanation about ffwd-recovery when delegated targets keys have been compromised in the commit message of 65e042d. Does that answer your question?

[JustinCappos]

@mnm678 How does this relate to TAP 8?

[mnm678]

TAP 8 adds a new way to rotate keys (rather than revoking and replacing), so if a threshold of keys are rotated under TAP 8 the same fast-forward recovery should happen.

[JustinCappos]

Are you saying that the snapshot metadata file should be deleted whenever someone rotates a key for a targets role? This is what I understand is being proposed.

[lukpueh]

Thanks for your reviews @JustinCappos, @joshuagl and @mnm678. I committed your suggestions and tried to answer the questions. When preparing this PR, I was a bit torn about how much background info and explanations should be part of the instructions. In order to not bloat the spec, I kept it to a minimum and instead wrote very detailed commit messages. Let me know if you think we should move some of that info from the commit messages directly into the spec.

[trishankatdatadog]

Hold on, why are we not checking that every version number for every targets metadata file (including delegations) in the previous snapshot, if any, is <= the current metadata file?

[lukpueh]

It's described in detail in the corresponding commit message (65e042d).

The TLDR is, at this point in the process we don't know if any delegated targets keys were removed and we thus need to skip the rollback check.

[trishankatdatadog]

Hmmm, this basically breaks the workflow described in the Mercury paper, unless we're doing it elsewhere.

[mnm678]

The check in 4.5.2.7 looks for rollback attacks on delegated targets. Does that cover the workflow from the Mercury paper?

[trishankatdatadog]

Doesn't look like it, because Mercury prescribes checking vernum of every targets metadata file.

[trishankatdatadog]

All targets files. Isn't this the Mercury workflow for snapshot?

[mnm678]

@trishankatdatadog If a fast forward attack occurs in a delegated targets file, does root still rotate timestamp and snapshot? This may require coordination between an independent developer who owns the delegated target and the repository manager which could slow the recovery time. If the root still rotates, then the check in 4.5.2.7 could be moved back up here.

[trishankatdatadog]

I see. Hmm, this requires careful thinking and modeling. I want to make sure we cover all of our bases. Right now, it doesn't look like it, but maybe I'm wrong.

[lukpueh]

Thanks for helping to clarify, @mnm678!

I see. Hmm, this requires careful thinking and modeling. I want to make sure we cover all of our bases. Right now, it doesn't look like it, but maybe I'm wrong.

Can you elaborate, @trishankatdatadog? Why does it not look like we cover all of our bases? Maybe we should discuss this in person?

[trishankatdatadog]

Yes, I think we need a meeting.

[trishankatdatadog]

Could we please say DELEGATEE instead of DELEGATE? Maybe ask @jhdalek55?

[jhdalek55]

I'm inclined to agree with Trishank here. While grammatically you might make a case that delegate is correct here, a delegate, when used as a noun, generally implies a person. "Delegatee" is defined as "someone or something to which something is delegated." In this case, when the responsibility for signing is being granted to a different entity, I feel like "delegatee" more clearly conveys this concept.

[lukpueh]

Done in e96f1e8

[trishankatdatadog]

@lukpueh Oh, right, this was the big PR that needed a meeting. Yes, could someone make this happen, please? But it can wait after TAP 5 @mnm678

[lukpueh]

What stops us from landing this PR? cc @mnm678, @trishankatdatadog, @joshuagl, @JustinCappos.

[mnm678]

What stops us from landing this PR? cc @mnm678, @trishankatdatadog, @joshuagl, @JustinCappos.

I believe @trishankatdatadog wanted to meet about how this affects the Mercury workflow. I can schedule something for next week if that sounds alright.

[trishankatdatadog]

I believe @trishankatdatadog wanted to meet about how this affects the Mercury workflow. I can schedule something for next week if that sounds alright.

Yes please

[mnm678]

For anyone interested in attending, please indicate your availability here: https://doodle.com/poll/hkfze8hwnmhvaq2p

[mnm678]

The meeting is scheduled for Tuesday at 11am eastern time. Let me know if you would like an invite.

[joshuagl]

Would it perhaps be a little clearer if 4.5.2.10 said something like

If the current delegation returns a result, or is a terminating delegation, stop the search, and jump to step 5.

and remove the "Stop the search, ..." sentence from 4.5.2.11 ?

Is there a logical difference in having this part here rather than 4.5.2.10 that I'm missing?

[lukpueh]

Here's a recap of the meeting earlier this week, where we discussed this PR and the related issues with and without it.
Thanks for your time to discuss this, @trishankatdatadog, @mnm678, @joshuagl!

Rollback prevention and fast-forward recovery prior to this PR

In order to prevent rollback attacks the client checks that the version number of all targets metadata files (including delegated targets) in the trusted snapshot metadata file, if any, is less than or equal to their version number in the new snapshot metadata file.

In order to recover from a fast-forward attack (ffwd), the client temporarily disables that check after a targets role key rotation, by removing the trusted snapshot metadata file (see "if any" above).

The issue

For delegated targets, a client can only learn about a key rotation after having downloaded the delegating metadata, which happens after the snapshot versions check in the client workflow. As a consequence there is no ffwd recovery.

The solution proposed in this PR

Postpone the snapshot versions check for delegated targets until after their delegating targets were downloaded, so that the trusted snapshot can be removed upon a key rotation in time.

The problem with this solution

Rollback attacks go undetected for delegated targets that are not downloaded at the time when the rollback attack is visible in snapshot. And at a later time when the delegated targets is downloaded and the check would be performed, it is too late, because the rolled-back version in snapshot has already become the trusted version.

Usually, a second rollback check that compares the version of the delegated targets role with a prior trusted version of itself would solve this, but this does not work if it is downloaded for the first time.

Solutions (discussed in the meeting)

• Don't re-use a delegation role (name) if it is compromised, create a new role instead.

  Con: This is not ideal for hashed bin delegation, where role names are created according to a convention (e.g. '00-07', '08-0f', etc.). While we could use a different prefix (see related python-tuf discussion) for a new role, it seems a bit cumbersome for a delegator to keep track of different prefixes. Furthermore, the newly proposed succinct hash bin delegation TAP only envisions one prefix for all bin roles.

• Require snapshot key rotation when a delegation is compromised.

  Con: This defeats the purpose of separating delegated role keys from top-level role keys.

Another solution + variants (not yet discussed)

Perform snapshot versions check for all targets roles in snapshot before downloading any targets metadata (as prior to this PR), and again after the delegating role was downloaded (as per this PR). Then, if rollbacks are detected for delegated targets roles in the first check, don't abort the update, instead continue and:

(a) immediately resolve the delegation tree until all delegating roles of rolled-back roles have been downloaded, or
(b) keep a separate record of the rollbacks, until the corresponding delegating roles have been downloaded (possibly in later updates),

both to see if a key rotation for the rolled-back role took place, and the rollback might have been an attempt to recover from an ffwd.

Con
(a): requires additional state keeping across updates making the client more complex.
(b): might require to resolve the entire delegation tree.

[lukpueh]

There's another problem with rollback check vs. FFWD recovery for delegated roles.

Even if we wait with the rollback check until the delegating role of a rolled-back role has been downloaded, we might be downloading the delegating role for the first time too and thus wouldn't detect a rotation. :(

[trishankatdatadog]

Thanks for documenting our discussion, @lukpueh! I really do think choosing the simplest option and documenting the reasons why and its implications if the way to go. What do others think?

[JustinCappos]

I feel like I'm missing a little context here (apologies about this). Is this worried about primarily when the key changes but the role name stays the same? I think the assumption when designing the ffwd defenses in Mercury was that these attacks will be rare (assuming we have a defense), so using the snapshot key is fine even if it is to recover from ffwd for a delegated role.

…

On Fri, Sep 4, 2020 at 11:46 PM Trishank Karthik Kuppusamy < ***@***.***> wrote: Thanks for documenting our discussion, @lukpueh <https://github.com/lukpueh>! I really do think choosing the simplest option and documenting the reasons why and its implications if the way to go. What do others think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#86 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGROD63Z53MTJG4RNPDK63SEEDUXANCNFSM4KNWXWQQ> .

[lukpueh]

@JustinCappos and I just had a chat about this. He agrees that both solutions from above make sense despite their drawbacks. The solutions are:

• don't reuse a compromised delegated targets role name, and/or
• rotate snapshot too when a delegated targets key is compromised

@JustinCappos also pointed out that the second rollback check, as removed in #65 and re-added in this PR (0ecf980), is indeed not necessary. The argument for the second rollback check was redundancy, so that an attacker needs to compromise snapshot and targets for a rollback attack, but in reality with the targets metadata >= version check in snapshot, even a compromised snapshot can't roll back.

@mnm678, @trishankatdatadog, what do you think? ... I'm starting to lose track here... :/

[joshuagl]

@JustinCappos and I just had a chat about this. He agrees that both solutions from above make sense despite their drawbacks. The solutions are:

• don't reuse a compromised delegated targets role name, and/or

Would the onus be on the repository administrator to not reuse names? Or would we expect implementations to add some state-tracking for compromised names that can't be reused?

• rotate snapshot too when a delegated targets key is compromised

Given that the above either requires diligent repository administrators or additional state tracking in implementations, I'd favour this simpler solution.

[trishankatdatadog]

• don't reuse a compromised delegated targets role name, and/or

This looks the simplest to me, no need to bother repo admins, just the original delegator to the revoked delegatee, which may be a less onerous requirement. Bothering admins to reset snapshot should be last resort (as it can potentially affect everyone, and has bandwidth costs).

[mnm678]

• rotate snapshot too when a delegated targets key is compromised

Given that the above either requires diligent repository administrators or additional state tracking in implementations, I'd favour this simpler solution.

I agree. This does require communicating with the repository maintainer, but it looks like the other viable solutions also require some additional communication. Also, as @lukpueh pointed out, this preserves rollback protection for the first time a delegated target is downloaded.

[JustinCappos]

Would the onus be on the repository administrator to not reuse names? Or would we expect implementations to add some state-tracking for compromised names that can't be reused?

It would be up to whomever is creating the new role. Either the repo admin or the user whose account was compromised. Note also, that this only really matters if the key was not compromised but somehow a fast forward account was done on the account. (Maybe a runaway script?)

[lukpueh]

@JustinCappos:

It would be up to whomever is creating the new role. Either the repo admin
or the user whose account was compromised.

I agree. Plus coordination with the delegator is also required, because they need to add the new role to the delegating metadata, right?

Note also, that this only really matters if the key was not compromised but
somehow a fast forward account was done on the account. (Maybe a runaway
script?)

Why does it only really matter if the key was not compromised?

[lukpueh]

@JustinCappos also pointed out that the second rollback check, as removed in #65 and re-added in this PR (0ecf980), is indeed not necessary. The argument for the second rollback check was redundancy, so that an attacker needs to compromise snapshot and targets for a rollback attack, but in reality with the targets metadata >= version check in snapshot, even a compromised snapshot can't roll back.

@mnm678, @trishankatdatadog, @joshuagl, any thoughts? I submitted #65, but then let myself convince to revert it (see discussion #71 (comment) pp.). Looking at it now I think that I maybe misunderstood @mnm678's there. Rollback protection via snapshot seems to be effective even if snapshot is compromised.

I'd appreciate your opinions!

[JustinCappos]

On Tue, Sep 8, 2020 at 4:44 PM lukpueh ***@***.***> wrote: @JustinCappos <https://github.com/JustinCappos>: It would be up to whomever is creating the new role. Either the repo admin or the user whose account was compromised. I agree. Plus coordination with the delegator is also required, because they need to add the new role to the delegating metadata, right?

Yes, for sure!

Note also, that this only really matters if the key was not compromised but somehow a fast forward account was done on the account. (Maybe a runaway script?) Why does it only really matter if the key was not compromised?

If the key is compromised, you must change the key. There isn't an option to decide what to do and possibly just use the snapshot role to recover from the fast forward attack.

…

[lukpueh]

If the key is compromised, you must change the key. There isn't an option
to decide what to do and possibly just use the snapshot role to recover
from the fast forward attack.

Yes, you must change the key in the delegating role. But that alone won't allow a client to recover from a potential ffwd attack. So you do have to choose one of the above discussed options.

[mnm678]

Rollback protection via snapshot seems to be effective even if snapshot is compromised.

This is true if the rollback is to a version before the client last updated. If the client goes a week without updating, during which time there are 5 different releases, the client is only protected against a rollback to a version that is more than a week old. That being said, I think that this is an inherent tradeoff in the way snapshot handles rollback protection: the more often a client updates the better the rollback protection.

Looking back on the discussion in #71, I think my main concern was that rollback protection is not left to the timestamp role. The rollback protection using snapshot should be sufficient. Revoking the snapshot key for fast-forward attack recovery requires the support of root, and so would not introduce a rollback attack unless root is compromised.

[trishankatdatadog]

What about the option of rotating the delegatee rolename? It requires cleaning up the revoked delegatee rolename later in the snapshot, but doesn't require immediate action from repo admins.

[mnm678]

The main downside there would be hashed bin delegations or other places repositories generate rolenames.

[trishankatdatadog]

The main downside there would be hashed bin delegations or other places repositories generate rolenames.

I see. Yes, we should clarify which solution fits between in which case (if you control timestamp, snapshot, and the bins, then rotate snapshot; otherwise, just rotate delegatees for now, add revocation to snapshot, and wait for it). WDYT?

[lukpueh]

@mnm678, do you have spare cycles to revive this work? I currently can't make it a priority, but it would be quite nice if we could land this PR in one way or another.

[mnm678]

@mnm678, do you have spare cycles to revive this work? I currently can't make it a priority, but it would be quite nice if we could land this PR in one way or another.

Sure!

[mnm678]

Superseded by #150