Instability when updating repo and using repo_gpgcheck

[sdherr]

Describe the bug

There currently exists a timing window that can lead to client errors if you are using signed repo metadata with repo_gpgcheck=1. When you are using that option of course the server provides both the normal repomd.xml file and also a detached signature of it in repomd.xml.asc. Even if the server swaps out both files atomically on a repo update, an unlucky client can have received the old version of repomd.xml and the new version of repomd.xml.asc, if its requests happened to come on either side the update instant. The signature will then fail to verify and the client will return an error. Furthermore if the server is using any type of caching system or CDN, this mis-match can get persisted in the cache up to the TTL of the files if you are unlucky, causing many client errors and making the repo essentially unavailable for a period of time.

Unlike the other metadata files, there is no hash information in the name of repomd.xml.asc, or any other indication of what version of the file you want. This is likely because there is a chicken-and-egg problem of writing this info into repomd.xml: the file must exist first before you can sign it or calculate its hash, and the act of adding that information back in to repomd.xml invalidates the signature or changes the hash.

I furthermore assume that this is a detached signature in the first place instead of simply clearsigning repomd.xml to keep it one atomic file because there were worries about backwards-incompatibility with old clients.

Reproduction steps

See description.

Expected behavior

Any change which would indicate which version of repomd.xml.asc that you need to the server is a potential fix, but I think there are three main possibilities:

1. Changing the name of the file like you do with other metadata files, with the name calculated client-side somehow.
2. Adding a header to the request.
3. Adding a query parameter to the request.

In particular for backwards compatibility and ease-of-update, I suggest we do number 3, simply take the Last-Modified header value on the repomd.xml response, and add it as a query parameter on the repomd.xml.asc request: .../repomd.xml.asc?repomd_last_modified=<header value>

1. Almost every webserver simply ignores extra unexpected query params, so this should not break anything. On the off chance that it does and a server responds with a 4xx HTTP code, tdnf could simply revert back to today's behavior and request the file without the query param.
2. By contrast, caches / CDNs typically do include the query params as part of the cache key by default.

By including the information on which repomd.xml file you have in the cache key for repomd.xml.asc we are creating different cached versions of the file, and the client will automatically receive the correct one and proceed successfully if these cache keys are already populated. If there is no cache involved and a server is responding to every request directly, at least they are in no worse position than they were before, and they can teach their server to respond appropriately to the query param if they wish.

There is still a possible timing window here where bad pairs can get cached if both cache TTLs time out and a client re-fills the cache with an old version of repomd.xml and a new version of repomd.xml.asc?repomd_last_modified=123 from a server that is ignoring the query string. However this gives servers the ability to close this gap permanently and respond correctly to every request, by doing either:

1. Teach the webserver to respond correctly to the query param, or
2. Increase the cache TTL on */repomd.xml.asc?repomd_last_modified=* to more than double the TTL of repomd.xml, eliminating the overlapping timing window where it's possible to cache incompatible versions of the files.

Additional context

I have filed a corresponding bug against dnf here:
rpm-software-management/libdnf#1664