After the recent changes there are a couple of new pylint warnings that
appeared.
They are caused by the new test file that was added
test_updater_top_level_update.py and the limit of public functions was
reached in the TestMetadata class in test_api.py
The warnings should be addressed before enabling all of the linters
on the tests files.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Address or disable pylint warnings raised on all test files inside
the "tests/" directory testing the code of the new implementation.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
In order to make the `RootVersion` test cases understandable it's
better if we use keywords and don't split the different RootVersions
into multiple lines.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
test_sign_verify() is testing too many cases and after the recent
pylint warning about the usage of too many local variables it became
clear it's time to split this test function.
I decided to split it logically as half of the function was about
failures connected with verify.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Except for 'root' role, RepositorySimulator does not
keep previous metadata versions, it always serves the latest
one. The metadata version check during fetch serves mostly
for informative purposes and removing it allows generating test
metadata with mismatching version.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Extend the TestRefresh cases with additional checks
for expected metadata files and their content written
on the file system.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
Add ngclient/updater tests following the top-level-roles metadata
update from the specification (Detailed client workflow)
using RepositorySimulator.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
This is a minimal test case to show the basic functionality:
I'd like to update most of the tests to use the implicit method
but would like to do that when there's less churn in these tests.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Add one test with 1 subtests for various root key rotation situations.
The test data definition format is a bit tricky but I tried to document
that in the test function docstring.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Store signers with their keyids so they are easier to remove.
The signers structure now looks like:
{
"role1": {
"keyidA": SSlibSigner,
"keyidB": SSlibSigner,
}
}
Add convenience method for adding a signer.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
This is slightly cosmetic but rename get_one_valid_targetinfo to
get_targetinfo:
* The function name is long without any reason: "one" and "valid" are
always implicit
* shortening makes code (incl. our examples and tests) easier to read
* We're also already changing updater API (compared to legacy) so this
alone does not break things -- it's also not a difficult "port".
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Remove updated_targets() as it doesn't fit the rest of the API.
In its stead add find_cached_target() which has a similar signature
as download_target(): both accept an optional local filepath as
argument and return full local filepath. In the
find_cached_target() case None is returned if the local file is not the
correct target file.
Updater constructor gets a new optional target_dir argument: This means
client can avoid giving a local filepath as an argument to
find_cached_target()/download_target() -- Updater will then generate a
filename within targets_dir.
A reasonable use pattern (when targets_dir is set in constructor):
info = updater.get_one_valid_targetinfo("targetname")
path = updater.find_cached_target(info)
if path is None:
path = updater.download_target(info)
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Validate that root role names are 4 and that they are exactly
"root", "snapshot", "targets" and "timestamp" as described in
the spec:
https://theupdateframework.github.io/specification/latest/#root-role
Additionally, fix the valid_roots dataset, so each of the cases contains
the top metadata role names inside the roles dictionary.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Our newly added metadata files in the
tests/repository_data/fishy_rolenames/metadata directory have an expiry
date until "2021-10-22T11:21:56Z" and today while running the tests on
develop branch I recived this error:
ExpiredMetadataError("Metadata X expired on Fri Oct 22 11:21:56 2021")
when running the tests in tests/test_updater.py file and more precisly
the TestUpdaterRolenames.test_unusual_rolenames() test.
That's why I decided to bump the expiration date to a random time in
the future (October 22-nd 2050) and I had to resign all of the metadata
files.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
When I tried adding support for Python3.10 we had CI errors due to
test failures: https://github.com/theupdateframework/python-tuf/pull/1610/checks?check_run_id=3861875325
The problem comes from the fact that we start a subprocess
executing simple_https_server.py, but then we fail to communicate the
message we expect from the server process to the main process actually
running the test. We expect our custom message to be the first line
printed from the server process, but instead, a deprecation warning is
printed first about the usage of ssl.wrap_socket(). Our custom message
is printed second.
As of Python 3.7 this function has been deprecated:
https://docs.python.org/3/library/ssl.html#ssl.wrap_socket and for
whatever the reason we didn't get a warning when using it before.
My fix does what is suggested in the warning and replaces the usage of
ssl.wrap_socket() by instantiating a ssl.SSLContext object and then
calling SSLContext.wrap_socket().
This removes the warning.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
_fileinfo_has_changed() and _update_fileinfo() have been unused internal
methods since 2016. Remove them.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
The original commit 051b8229 handled the loading and saving metadata
cases but the legacy client actually checks for the files existence
in various other places:
* _update_versioninfo() never reads the file but operates differently
depending on whether the file exists or not
* _move_current_to_previous() that copies files around
* MultiRepoUpdater initialization: this only handle root.json so
is still correct
* _update_fileinfo() which is dead code
Fix the first two of these cases.
Make sure rolenames like "../a" won't trick ngclient into creating the
metadata file outside the metadata cache.
The test data was semi-manually created with RepositorySimulator:
this test code could use RepositorySimulator directly instead (like the
ngclient tests do) but that would require some more infrastructural
work.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Add support for adding delegated targets into RepositorySimulator.
Make the metadata URL parsing in RepositorySimulator more robust.
Add a test to make sure "../a" won't trick ngclient into creating the
metadata file outside the metadata cache.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Modify RepositorySimulator function delegates() to all_targets(), so
that all targets can be traversed and updated with one cycle when
calling update_snapshot() (which is the only use case for now for
delegates()).
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Add an option to calculate the hashes and length for timestamp/snapshot
meta.
This will help to cover more use cases with the repository simulator.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
If you do the following steps:
1. call Updater.refresh() and load, verify and cache all metadata files
2. modify timestamp snapshot meta information:
(One or more of hashes or length for snapshot changes here)
3. call Updater.refresh() again
4. root and timestamp will be updated to their latest versions
5. local snapshot will be loaded, but hashes/length will be different
than the ones in timestamp.snapshot_meta and that will prevent loading
6. remote snapshot is loaded and verification starts
then when executing step 6 the rollback checks will not be done because
the old snapshot was not loaded on step 5.
In order to resolve this issue, we are introducing the idea of trusted and
untrusted snapshot.
Trusted snapshot is the locally available cached version. This version has
been verified at least once meaning hashes and length were already checked
against timestamp.snapshot_meta hashes and length.
That's why we can allow loading a trusted snapshot version even if there is a
mismatch between the current timestamp.snapshot_meta hashes/length and
hashes/length inside the trusted snapshot.
Untrusted snapshot is the one downloaded from the web. It hasn't been verified
before and that's why we mandate that timestamp.snapshot_meta hashes and length
should match the hashes and legth calculated on this untrusted version of
snapshot.
As the TrustedMetadataSet doesn't have information which snapshot is trusted or
not, so possibly the best solution is to add a new argument "trusted"
to update_snapshot.
Even though this is ugly as the rest of the update functions doesn't
have such an argument, it seems the best solution as it seems to work
in all cases:
- when loading a local snapshot, we know the data has at some point been
trusted (signatures have been checked): it doesn't need to match hashes
now
- if there is no local snapshot and we're updating from remote, the
remote data must match meta hashes in timestamp
- if there is a local snapshot and we're updating from remote, the remote
data must match meta hashes in timestamp
Lastly, I want to point out that hash checks for metadata files are not
essential to TUF security guarantees: they are just an additional layer of
security that allows us to avoid even parsing json that could be malicious -
we already know the malicious metadata would be stopped at metadata
verification after the parsing.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Reuse the decorator defined in tests/utils.py in order
to receive more helpful messages when an assertion
fails in test_tragets().
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Generalize the decorator used in test_metadata_serialization.py and
move it inside tests/utils.py, so it can be reused in other similar
situations.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
The definition of consistent targets in the spec is ambiguous:
"consistent target files should be written to non-volatile storage
as digest.filename.ext"
Additionally, the specification describes consistent targets when the
client builds the download URL as follows:
"The filename is of the form HASH.FILENAME.EXT".
The issue is about how we interpreted those quotes.
The legacy updater has decided this means a target path "a/b" will
translate to a download url path "a/{HASH}.b".
The ngclient however translates the target path "a/b" to a download url
path "{HASH}.a/b".
We decided we want to follow the same approach taken from the legacy
updater and thus change how we construct the consistent targets.
Additionally, we want to make sure we test for cases when the TARGETPATH
is an empty string or points to a directory.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Doing so is not always safe and has various other issues
(like target paths "a/../b" and "b" ending up as the same
local path).
Instead URL-encode the target path to make it a plain filename. This
removes any opportunity for path trickery and removes the need to create
the required sub directories (which we were not doing currently, leading
to failed downloads). URL-encoding encodes much more than we really need
but doing so should not hurt: the important thing is that it encodes
all path separators.
Return the actual filepath as return value. I would like to modify the
arguments so caller could decide the filename if they want to. But I
won't do it now because updated_targets() (the caching mechanism)
relies on filenames being chosen by TUF. The plan is to make it
possible for caller to choose the filename though.
This is clearly a "filesystem API break" for anyone depending on the
actual target file names, and does not make sense if we do not plan to
go forward with other updated_targets()/download_target() changes
listed in #1580.
This is part of bigger plan in #1580Fixes#1571
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
The handling of consistent snapshot was not very clear: try to make
it more obvious what is supported and what is not.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
* Add very simple targets support into simulator
* Add documentation for the simulator
* Add an example targets test
This might need to be tweaked and/or extended as we add tests but the
implementation should give a good indication of how to extend it.
As an example, non-consistent targets are not yet supported, but
making fetch() check for the consistent_snapshot state and respond
accordingly should be easy.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Instead of starting a dedicated slow_retrieval_server
to test for read timeout in RequestsFetcher, use
unittest.mock to mock the response.raw.read call.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
RequestsFetcher now handles connect/read timeout
errors on session.get() as it does when reading data.
Adding a test which uses unittest.mock to patch the
session.get method instead of simulating the timeout
with a server.
Signed-off-by: Teodora Sechkova <tsechkova@vmware.com>
This is an API change to the exceptions thrown in Root.add_key()
and Root.remove_key().
The reason for that change is that in my opinion the correct exceptions
in these cases should be "ValueError" instead of "KeyError" as
the problems are in the given values - role doesn't exist or
key is not used by a particular role.
Additionally, document the thrown exceptions in "Root.add_key" and
add a test which invokes that exception.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
