Skip to content

ngclient: Implicitly call refresh() #1654

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.

Sign up for GitHub

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

Merged
sechkova merged 2 commits into from
Nov 17, 2021
Merged

ngclient: Implicitly call refresh() #1654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec74499
ngclient: Implicitly call refresh()
Sep 15, 2021
26213ae
tests: Add test for implicit refresh()
Nov 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions tests/test_updater_ng.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,20 @@ def test_refresh_with_only_local_root(self):
expected_files = ["role1", "root", "snapshot", "targets", "timestamp"]
self._assert_files(expected_files)

def test_implicit_refresh_with_only_local_root(self):
os.remove(os.path.join(self.client_directory, "timestamp.json"))
os.remove(os.path.join(self.client_directory, "snapshot.json"))
os.remove(os.path.join(self.client_directory, "targets.json"))
os.remove(os.path.join(self.client_directory, "role1.json"))
os.remove(os.path.join(self.client_directory, "role2.json"))
os.remove(os.path.join(self.client_directory, "1.root.json"))
self._assert_files(["root"])

# Get targetinfo for 'file3.txt' listed in the delegated role1
targetinfo3 = self.updater.get_targetinfo("file3.txt")
expected_files = ["role1", "root", "snapshot", "targets", "timestamp"]
self._assert_files(expected_files)

def test_both_target_urls_not_set(self):
# target_base_url = None and Updater._target_base_url = None
updater = ngclient.Updater(self.client_directory, self.metadata_url, self.dl_dir)
Expand Down
72 changes: 36 additions & 36 deletions tuf/ngclient/updater.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,23 @@
secure manner: All downloaded files are verified by signed metadata.

High-level description of Updater functionality:
* Initializing an :class:`~tuf.ngclient.updater.Updater` loads and validates
the trusted local root metadata: This root metadata is used as the source
of trust for all other metadata.
* Calling :func:`~tuf.ngclient.updater.Updater.refresh()` will update root
metadata and load all other top-level metadata as described in the
specification, using both locally cached metadata and metadata downloaded
from the remote repository.
* When metadata is up-to-date, targets can be dowloaded. The repository
snapshot is consistent so multiple targets can be downloaded without
fear of repository content changing. For each target:

* :func:`~tuf.ngclient.updater.Updater.get_targetinfo()` is
used to find information about a specific target. This will load new
targets metadata as needed (from local cache or remote repository).
* :func:`~tuf.ngclient.updater.Updater.find_cached_target()` can be used
to check if a target file is already locally cached.
* :func:`~tuf.ngclient.updater.Updater.download_target()` downloads a
target file and ensures it is verified correct by the metadata.
* Initializing an ``Updater`` loads and validates the trusted local root
metadata: This root metadata is used as the source of trust for all other
metadata.
* ``refresh()`` can optionally be called to update and load all top-level
metadata as described in the specification, using both locally cached
metadata and metadata downloaded from the remote repository. If refresh is
not done explicitly, it will happen automatically during the first target
info lookup.
* Updater can be used to download targets. For each target:

* ``Updater.get_targetinfo()`` is first used to find information about a
specific target. This will load new targets metadata as needed (from
local cache or remote repository).
* ``Updater.find_cached_target()`` can optionally be used to check if a
target file is already locally cached.
* ``Updater.download_target()`` downloads a target file and ensures it is
verified correct by the metadata.

Below is a simple example of using the Updater to download and verify
"file.txt" from a remote repository. The required environment for this example
Expand All @@ -52,9 +51,6 @@
target_base_url="http://localhost:8000/targets/",
)

# Update top-level metadata from remote
updater.refresh()

# Update metadata, then download target if needed
info = updater.get_targetinfo("file.txt")
path = updater.find_cached_target(info)
Expand Down Expand Up @@ -130,11 +126,16 @@ def refresh(self) -> None:
specified order (root -> timestamp -> snapshot -> targets) implementing
all the checks required in the TUF client workflow.

The metadata for delegated roles are not refreshed by this method as
that happens on demand during get_targetinfo().
A ``refresh()`` can be done only once during the lifetime of an Updater.
Copy link
Collaborator

@MVrachev MVrachev Nov 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that I have forgotten, but why can we call refresh only once during the lifetime of an Updater?

Copy link
Member Author

@jku jku Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue (#1650) explains it -- with current design allowing a second refresh would be essentially the same as creating a new updater

If ``refresh()`` has not been explicitly called before the first
``get_targetinfo()`` call, it will be done implicitly at that time.

The refresh() method should be called by the client before any other
method calls.
The metadata for delegated roles is not updated by ``refresh()``:
that happens on demand during ``get_targetinfo()``. However, if the
repository uses `consistent_snapshot
<https://theupdateframework.github.io/specification/latest/#consistent-snapshots>`_,
then all metadata downloaded downloaded by the Updater will use the same
consistent repository state.

Raises:
OSError: New metadata could not be written to disk
Expand All @@ -159,22 +160,18 @@ def get_targetinfo(self, target_path: str) -> Optional[TargetFile]:
"""Returns TargetFile instance with information for 'target_path'.

The return value can be used as an argument to
:func:`download_target()` and :func:`find_cached_target()`.
:func:`refresh()` must be called before calling
`get_targetinfo()`. Subsequent calls to
`get_targetinfo()` will use the same consistent repository
state: Changes that happen in the repository between calling
:func:`refresh()` and `get_targetinfo()` will not be
seen by the updater.
``download_target()`` and ``find_cached_target()``.

If ``refresh()`` has not been called before calling
``get_targetinfo()``, the refresh will be done implicitly.

As a side-effect this method downloads all the additional (delegated
targets) metadata it needs to return the target information.

Args:
target_path: A target identifier that is a path-relative-URL string
(https://url.spec.whatwg.org/#path-relative-url-string).
Typically this is also the unix file path of the eventually
downloaded file.
target_path: A `path-relative-URL string
<https://url.spec.whatwg.org/#path-relative-url-string>`_
that uniquely identifies the target within the repository.

Raises:
OSError: New metadata could not be written to disk
Expand All @@ -184,6 +181,9 @@ def get_targetinfo(self, target_path: str) -> Optional[TargetFile]:
Returns:
A TargetFile instance or None.
"""

if self._trusted_set.targets is None:
self.refresh()
Comment on lines +185 to +186
Copy link
Member Author

@jku jku Nov 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here get_targetinfo() decides that not having "targets" means it should run refresh itself -- I think this is appropriate as Updater really has only two states it can be in (assuming it has not raised errors):

  • trusted set contains root but nothing else
  • trusted set contains all top-level metadata (and potentially other metadata)

return self._preorder_depth_first_walk(target_path)

def find_cached_target(
Expand Down