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

Cache all root metadata versions #2767

Draft
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

jku
Copy link
Member

@jku jku commented Jan 14, 2025

I suppose this is ready to be a draft -- at least I won't lose the work this way.

Problem

Currently python-tuf clients typically initialize once from the "bootstrap" root (the root metadata shipped with the client software), subsequent startups will use the cached root as starting point. This is mostly good but there is a downside: if the bootstrap root is more secure than the cache, then we lose some security (as cache could get poisoned between the first cache initialization and the actual use). This situation can arise if the client software (and bootstrap root) is shipped in a Operating System update, container image or e.g. a Debian package: in this case the client cache is writable by user but the bootstrap root may not be.

Solution

  • Cache all root versions
  • Always initialize the client starting with the secure bootstrap root
  • When loading subsequent root versions, load from local cache and from remote repository

This should give us best of both worlds:

  • client always initializes from most securely stored root
  • client does not download any more data than now
  • client still uses local cache for roots so repository cannot serve obsolete data

The only downside is that client will now verify every root version on startup: I think this perfectly acceptable as client can and should periodically update the bootstrap root as well -- the added computation is not an issue in practice.

Details

The ngclient cache directory looked like this before:

cachedir/
    root.json
    timestamp.json
    snapshot.json
    targets.json
    somedelegatedrole.json

Now it looks like this:

cachedir/
    root.json        #  This is a symlink to "root_history/3.root.json"
    timestamp.json
    snapshot.json
    targets.json
    somedelegatedrole.json
    root_history/
        1.root.json
        2.root.json
        3.root.json
  • non-versioned root.json is preserved as symlink so that older python-tuf versions can coexist (and so it's easier to check current state for humans and tests)
  • root_history directory is used to make name clashes impossible (imagine a delegated role named "1.root") : as long as the client is not susceptible to path traversal issues, this should be safe
  • unused test data was removed in this PR, some tests were cleaned up to make them work with new cache structure
  • tests have been added to verify that roots are cached, cached roots are used and that poisoned cache is handled correctly (remote metadata is used if cache is not valid)

There is a cleanup PR that should be handled before this one: #2768

@jku
Copy link
Member Author

jku commented Jan 17, 2025

I think I'll refactor the large test cleanups to a different PR so this doesn't look so big (it isn't)

@kairoaraujo
Copy link
Collaborator

I think I'll refactor the large test cleanups to a different PR so this doesn't look so big (it isn't)

I'm ok with reviewing it if you want to add to this PR (separate commit).

@jku
Copy link
Member Author

jku commented Jan 17, 2025

not planning to add anything: I just meant the test file removals that are already in the PR: "+268 −1,253" looks like a big change but the actual PR is really manageable

@jku jku force-pushed the bootstrap-root-metadata branch from 392623d to 4534bbf Compare January 17, 2025 09:06
jku added 3 commits January 17, 2025 11:11
test_updater_ng.py is a little archaic (as it uses the static test
repository content from ye olden days). This commit does not change that
but removes an extra file in client cache dir: it is now quite confusing
as it looks a bit like intermediate root caching but is just an unused
file.

This has the nice side effect that tests now longer need to workaround
this extra file.

Signed-off-by: Jussi Kukkonen <[email protected]>
Signed-off-by: Jussi Kukkonen <[email protected]>
This is still copy-paste in three different files but now at least
the function is the same in every location and not directly copied.

We really should have generic TestCase class...

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku mentioned this pull request Jan 17, 2025
jku added 9 commits January 17, 2025 11:38
Application may have a "more secure" data store than the metadata cache
is: Allow application to bootstrap the Updater with this more secure
root. This means the Updater must also cache the subsequent root versions
(and not just the last one).

* Store versioned root metadata in local cache
* maintain a non versioned symlink to last known good root
* When loading root metadata, look in local cache too
* Add a 'bootstrap' argument to Updater: this allows
  initializing the Updater with known good root metadata
  instead of trusting the root.json in cache

Additional changes to current functionality:
* when using bootstrap argument, the initial root is written to cache.
  This write happens every time Updater is initialized with bootstrap
* The "root.json" symlink is recreated at the end of every refresh()

Signed-off-by: Jussi Kukkonen <[email protected]>
Expect (failing) call to open for "root_history/2.root.json" now that
the client stores versioned roots.

Signed-off-by: Jussi Kukkonen <[email protected]>
Even if last root version from remote is not accepted (leading to an
exception in load_root()) we should update the symlink "root.json" in
local cache to point to last good version.

Signed-off-by: Jussi Kukkonen <[email protected]>
Update test_updater_toplevel_update to use bootstrap argument by
default.

This still does not include tests for bootstrap feature specifically
but it should prove nothing has broken when the feature was added.

Signed-off-by: Jussi Kukkonen <[email protected]>
When application initializes an Updater with bootstrap, it should be
considered the trusted version from that point onwards: Update the
symlink "root.json" already here (even if refresh is never called).
n that Updater instance).

Signed-off-by: Jussi Kukkonen <[email protected]>
Signed-off-by: Jussi Kukkonen <[email protected]>
This includes some minor example improvements

Signed-off-by: Jussi Kukkonen <[email protected]>
@jku jku force-pushed the bootstrap-root-metadata branch from 4534bbf to 10703ac Compare January 17, 2025 09:39
@jku
Copy link
Member Author

jku commented Jan 17, 2025

OK:
there is now #2768 that should be handled before this one: this changes this PR into a fairly small one: 235 insertions(+), 89 deletions(-)

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

Successfully merging this pull request may close these issues.

Updater feature request: verify chain of trust from bootstrapped root metadata
2 participants