Skip to content
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

Repository update races with clients #223

Open
znewman01 opened this issue Apr 25, 2022 · 3 comments
Open

Repository update races with clients #223

znewman01 opened this issue Apr 25, 2022 · 3 comments

Comments

@znewman01
Copy link

znewman01 commented Apr 25, 2022

Two related issues:

  1. If you don't publish the repository careful (i.e., in the same order that the spec tells you to update), you can run into lots of race conditions.
  2. Even if you do publish the repository in-order and have consistent snapshots turned on, there's still a possible race if this happens halfway through the client flow.

There are a number of race conditions which can happen if the repository update is not atomic. For instance, if you publish a repository over HTTP and scp -R the files to the repository host, they might get copied in a weird order. I think this is pretty common in real deployments: the repository manipulation is done one one machine, then gets pushed to mirrors. The point of consistent snapshots, as I understand them, was to prevent this from happening, but they require that the push be done in the correct order (i.e., files are deployed in the order described in the spec).

But even if you can deploy atomically, there's still a race when timestamp keys are updated:

  1. Client initiates update, grabs all the root metadata
  2. Repo publishes (atomically) an updating rotating the timestamp keys, and including a timestamp with the new key.
  3. Client continues with update, grabs the timestamp file, and verifying signature fails.

In general, there's pretty limited guidance in the specification here. I don't know that it's worth trying to eliminate this one race. But maybe some client guidance about retries would be helpful.

EDIT: fix a typo

@JustinCappos
Copy link
Member

Better guidance for clients makes sense to me. One would expect a single retry would resolve this. If a more pathological case is happening, it feels more like a repository trying to prevent an update than an actual race condition...

@lukpueh
Copy link
Member

lukpueh commented May 2, 2022

I agree with @JustinCappos that a single retry after a short wait should mitigate the problem with changing keys in the delegating metadata (root or targets) in the middle of an update, because it is unlikely that this happens multiple times in rapid succession.

For the timestamp-snapshot-targets relationship, on the other hand, this could be a real problem. In the worst case, a client could always see timestamp metadata, which references snapshot metadata that is not yet available (or snapshot, which references targets that are not yet available).

IMO it's a good idea to add a recommendation to publish a given consistent snapshot either in a transaction, or in bottom-up order (1. targets, 2. snapshot, 3. timestamp). #91 is probably a good fit for this.

@joshuagl
Copy link
Member

joshuagl commented May 12, 2022

Good discussion here, thank you all. I think we should indeed address both:
a) adding guidance about retries to the client workflow
b) capturing repository publishing recommendations (atomic/single transaction OR bottom-up)

Let's address b) in #91 as @lukpueh suggested and a) in this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants