Repository navigation
Conversation
We will need this for some of the unsightly "reparse this struct as a map[string]any" needed for some variants of generic JSON round-tripping. Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
It makes more sense for us to push the terminating marker while in the parent delegation rather than doing it when we reach the child role. This is better both in terms of semantics and it also avoids the risk of us not pushing the marker if the role with the marker got skipped for some other reason. Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
This lets us expose DelegationChain as a type (needed for later patches related to iterating over delegations). It also means that we now use the upstream path matching logic for both iteration and matching which will avoid any security issues due to behaviour differences. On paper this means that path hash prefix delegations are supported, but it turns out go-tuf/v2 has a serious bug in their matching logic that means we need to keep it disabled for now. Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
The current text of s5.6.7.1 of the TUF spec implies that roles should
only ever be walked once when resolving delegations (which is how we
implemented things before).
However, after speaking to the spec maintainers it seems that the
wording will need some adjustment to better describe the intended
behaviour -- roles should only be skipped if they would form a loop
(i.e., only skipped if we reached the role in the middle of the current
delegation chain, not at any previous point in the walk).
On the whole, changing this primarily requires just checking if
DelegationChain (which now stores the walked roles) contains the role we
are about to walk into, but there are some other problematic semantic
issues that we needed to work around -- TUF clients have a limit on the
number of roles walked during lookup but that limit only applies to
delegations *that match a particular path being looked up* (for generic
targets listing it is somewhat unfeasible to compute which target
strings are still okay to resolve at that point and if no target string
could possibly be allowed after that point).
As a practical solution we just have a hard limit on the number of
delegations walked, which hopefully will be enough in practice.
It is also important to note that (by design) the same role can be
reached more than once now, which means that two important things need
to be done in tufclient's TargetMetadataFetchFunc implementation that
were not necessary before:
1. Before returning the role data, it is critical that we do an
explicit VerifyDelegate check against the delegator role -- this is
because a role's signatures being acceptable by one delegator does
not mean a future delegator should accept it implicitly.
Unfortunately, go-tuf's TrustedMetadata does not handle this
correctly (it will "accept" the role without re-checking its
signatures), hence the need for the explicit VerifyDelegate call.
Arguably this is a bug in go-tuf but they also don't implement
s5.6.7.1 properly so you can't hit this in the problematic bit of
their delegation walk, making it a bit of a wash?
2. In order to avoid unnecessary round-trips (which may be very
numerous in the case of a pathological repository), we should use
the cached data in TrustedMetadata when re-visiting a role.
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
The ability to iterate over delegated target roles directly will be quite important in a later patch (when implementing cross-repository links) so it makes more sense to split the logic. This does necessitate a new RoleDelegationChain that includes both the targets role data and the DelegationChain used to reach the targets role, but it is also necessary to include the set of terminating DelegationChains reached at that point of the iteration so that terminating delegations are properly respected. However, after splitting it became clear that tufext.IterTargetFiles doesn't really make much sense -- it would end up as a very small wrapper around IterTargetRoles which couldn't be used by tufclient once we add cross-repository links. It is much simpler to just open-code the targets iteration in tufclient's IterTargetFiles, which is where it kind of belongs. Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
The TOML cruft mostly stays in tufclient/config and is somewhat abstracted so that it can mostly work with JSON (this is primarily important for RootTrustSource). This is needed so that cross-repo links can construct their own repository definitions during tufclient.IterRepos from tufext directly without all of the TOML cruft. Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Aleksa Sarai aleksa@amutable.com