Skip to content
This repository has been archived by the owner on Mar 7, 2024. It is now read-only.

Enable Refresh() to be called more than once #86

Closed
rdimitrov opened this issue Dec 5, 2023 · 9 comments
Closed

Enable Refresh() to be called more than once #86

rdimitrov opened this issue Dec 5, 2023 · 9 comments
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@rdimitrov
Copy link
Owner

Describe the bug

Currently a Refresh() can be done only once during the lifetime of an Updater.

This is not optimal for long-living processes so it would be better if we enable calling Refresh() more than once.

References:

Reproduction steps

...

Expected behavior

Upon calling Refresh() make sure everything is up-to-date and if that's true:

  • return nil, instead of an error (better imho)
    or
  • return a custom error type so the user can check it via errors.Is (less favourite)

If something else occurred which failed us to refresh the metadata, still return the appropriate error value.

Additional context

No response

@rdimitrov rdimitrov added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Dec 5, 2023
@rdimitrov
Copy link
Owner Author

cc: @kommendorkapten

@joshuagl
Copy link

joshuagl commented Dec 5, 2023

FWIW in python-tuf there was an explicit decision to only allow one refresh() call during an Updater's lifetime: https://github.com/theupdateframework/python-tuf/blob/f711997a08cbb558fc8ab91406a846bbe4883d1a/tuf/ngclient/updater.py#L115

Mostly sharing this so that you can dig into the rationale for the choice on the Python side and make an explicit choice to diverge from their implementation approach. I know this codebase started out implementing the same architecture.

@kommendorkapten
Copy link
Collaborator

👋 @joshuagl I'm working now on integrating this into the Sigstore TUF client, and if you remember, there are some requirements on caching, and especially to allow a client to use the locally cached metadata without performing a TUF update. We know this is a deviation from the TUF spec, but it was still a desired feature to have.

As part of adding that functionality, it would be required to first load the metadata on disk, then if expired, perform an update. This currently fails due to this.

Another example is long-lived processes, such as services that may have a lifetime far beyond the timestamp's expiration time. Such components could of course periodically recycle the client used, but that feels a bit unnecessary. Would be interesting to hear your thoughts here.

@kommendorkapten
Copy link
Collaborator

Actually I think I can solve most of this in the sigstore tuf client, let me think of that for a while.

But I would still need the deviation from the tuf spec (load metadata on disk only), I'll prepare a PR and we can discuss it more.

@kommendorkapten
Copy link
Collaborator

See #87

@joshuagl
Copy link

See theupdateframework/python-tuf#2472 for some exploration of offline mode for python-tuf

@jku
Copy link

jku commented Dec 11, 2023

See theupdateframework/python-tuf#2472 for some exploration of offline mode for python-tuf

Yes the last comment hopefully explains why we finally decided not to merge anything yet in python-tuf: At the moment sigstore-python can achieve the same security benefits (roughly none) by just using cached artifacts without verifying them.

If we had theupdateframework/python-tuf#1168 then offline verification would be a little more useful.

@kommendorkapten
Copy link
Collaborator

Ah, the bootstrapped root functionality is expected to be part of python-tuf itself? My thinking is a bit different, go-tuf requires a root.json (LocalTrustedRoot) and so it would be the the actual program's responsibility to make sure that it can be trusted (and the technique used to protect it can differ between programs).

@rdimitrov
Copy link
Owner Author

Closing in favour of theupdateframework/go-tuf#593

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants