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>
The purpose of this config was to ensure blocking
inside the download loop and releasing CPU resources.
To our best knowledge the network stack currently used
in RequestsFetcher will always block inside the loop
and the issue cannot be reproduced.
'chunk_size' and 'socket_timeout' are currently the
settings provided by RequestsFetcher to tweak
CPU usage and download granularity.
Signed-off-by: Teodora Sechkova <tsechkova@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>
This may have been required by a linter at some point, but isn't
anymore: Not annotating makes the documentation look better.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Also add a summary to the page -- unfortunately getting a standard
TOC would require creating a rst page for each class.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
This makes the individual pages easier to read.
Use some autodoc configuration so we can have less config
in the automodule/autoclass declarations.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Situation before
* constructor args are not documented
* object attributes are documented
* sphinx cannot show object attribute type annotations
* attribute docs take a lot of vertical space
Now:
* constructor args are documented
* sphinx can show annotated types of constructor args
* class docstring now explains the attributes are the same as
constructor args (and attributes are not explicitly documented)
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>
Root class has the functionality to add and remove keys for delegated
metadata (add_key()/remove_key()) but the other delegator Targets does
not.
It should provide the same/similar functionality.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Move the Delegation class serialization tests from "test_api.py"
to test_metadata_serialization.py module focused on serialization
testing.
Additionally, a test for empty keys and roles will be added in my
upcomming pr #1511.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
The spec does not say anything about role name uniqueness in a
delegations object, but I believe we cannot safely allow multiple roles
with the same role name in the roles array of a delegations object.
If we did then the roles could have different keyids, and then we would
end up in a situation where metadata may be both a valid delegation
and an invalid delegation at the same time, depending on how the role
gets chosen and that does not seem like the intention of the design.
There is an issue open in the specification with number 167 about
that issue.
Regardless of the Metadata API, I think we should enforce role name
uniqueness.
I chose to change the data structure containing roles to
OrderedDict, where keys are role names and values are DelegatedRole
instances.
This made sense to me as role names are the unique identifier of a role
and their order is important to the way they are traversed afterward.
Note: we can't use OrderedDict as type annotation until we drop support
for Python 3.6:
https://docs.python.org/3/library/typing.html#typing.OrderedDict
That's why I used quotes around "OrderedDict" annotation, because I
can't import it.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
In Timestamp, the only valid "meta" value is the dictionary representing
meta information for the snapshot file. This makes the API unnecessarily
complicated and requires validation that only information about snapshot
is available inside "meta".
Together with the python-tuf maintainers, we decided that snapshot meta
information will not be represented by a "meta" dictionary but instead
by a MetaFile instance and with this it will diverge from the
specification.
Additionally, to prevent confusion, I will rename the "meta" attribute
to "snapshot_meta" as this attribute will be related only to meta
information about snapshot.
This decision is coherent with ADR9 and the rationale
behind it is to provide easier, safer, and direct access to the
snapshot meta information.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
We already do not require individual build uploads to succeed: let's
also not require the final step to succeed.
The immediate context for this is that coveralls has been down for
three days now.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
pylint 2.7 supports Python 3.9. This issue might reappear with next
Python release but let's deal with that if it happens.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Currently the github UI dropdown for checks looks useless since
checks are named "Run TUF tests and...".
Tweak the workflow and job names to hopefully fit the actual
step name in the UI.
Signed-off-by: Jussi Kukkonen <jkukkonen@vmware.com>
Simplify the code by removing some of the uses of "global" by using
alternativies to the assignements.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
New pylint warnings appeared with code "W0602: Using global for X but
no assignment is done (global-variable-not-assigned)" where X is a
global variable.
In the lines pointed by the warnings, "global" was used inside a
function scope in order to enable assignment of the global variable to
a new value in the function.
The reason for these warnings was that pylint noticed we are using
"global" for the variable X inside a function scope without actually
assigning it to a new value.
Overall that was true for the reported cases and I removed the use of
"global", but there were only two cases where pylint reported a
false-positive. Then we were using "global", but pylint didn't
understand there were assignments.
In those cases, I disabled the warnings.
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
