From ef39be1c8a59f3763f370fc3832d7331630ba97b Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 23 Sep 2026 01:17:36 +1000 Subject: [PATCH 1/8] jsonutils: export ReparseJSON 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 --- internal/jsonutils/jsonext.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/jsonutils/jsonext.go b/internal/jsonutils/jsonext.go index 468d8eb..937ad4d 100644 --- a/internal/jsonutils/jsonext.go +++ b/internal/jsonutils/jsonext.go @@ -13,10 +13,10 @@ import ( // when given a top-level structure that doesn't support UnrecognizedFields. var ErrNotExtensible = errors.New("json structure not extensible") -// reParseJSON takes an arbitrary object and then re-parses as though it were +// ReparseJSON takes an arbitrary object and then re-parses as though it were // JSON for the given type parameter. This is necessary to "cast" pre-parsed // any interfaces into something strongly typed. -func reParseJSON[T any](data any) (T, error) { +func ReparseJSON[T any](data any) (T, error) { var ( encoded []byte err error @@ -39,7 +39,7 @@ func GetExtensionJSON[T any](extStruct any, field string) (*T, error) { // Get the set of structure fields as json.RawMessage so we can re-parse // them slightly more efficiently and without triggering parsing errors for // other fields. - structFields, err := reParseJSON[map[string]json.RawMessage](extStruct) + structFields, err := ReparseJSON[map[string]json.RawMessage](extStruct) if err != nil { return nil, fmt.Errorf("%w: re-parse %T as generic struct: %w", ErrNotExtensible, extStruct, err) } @@ -67,7 +67,7 @@ func SetExtensionJSON[W any](extStruct *W, field string, value any) (json.RawMes if err != nil { return nil, fmt.Errorf("marshal value %T: %w", value, err) } - structFields, err := reParseJSON[map[string]json.RawMessage](*extStruct) + structFields, err := ReparseJSON[map[string]json.RawMessage](*extStruct) if err != nil { return nil, fmt.Errorf("%w: re-parse %T as generic struct: %w", ErrNotExtensible, *new(W), err) } @@ -75,7 +75,7 @@ func SetExtensionJSON[W any](extStruct *W, field string, value any) (json.RawMes var oldExtBytes json.RawMessage oldExtBytes, structFields[field] = structFields[field], json.RawMessage(extBytes) - newExtStruct, err := reParseJSON[W](structFields) + newExtStruct, err := ReparseJSON[W](structFields) if err != nil { return nil, fmt.Errorf("%w: re-parse generic struct to %T: %w", ErrNotExtensible, *new(W), err) } @@ -83,7 +83,7 @@ func SetExtensionJSON[W any](extStruct *W, field string, value any) (json.RawMes // Make sure that round-tripping the new extension structure through // encoding still includes the same extension fields. If not, then the // struct doesn't support extensions of this form. - roundTripStructFields, err := reParseJSON[map[string]json.RawMessage](newExtStruct) + roundTripStructFields, err := ReparseJSON[map[string]json.RawMessage](newExtStruct) if err != nil { return nil, fmt.Errorf("%w: re-parse modified %T as generic struct: %w", ErrNotExtensible, *new(W), err) } From 59f7058326f7a9e006745d8d9762b8440fd9381f Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 18 Sep 2026 16:23:13 +1000 Subject: [PATCH 2/8] tufext/iter: handle terminating delegations more cleanly 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 --- internal/tufext/targets_iter.go | 38 ++++++++++------------ internal/tufext/targets_iter_test.go | 48 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 22 deletions(-) diff --git a/internal/tufext/targets_iter.go b/internal/tufext/targets_iter.go index 9654822..b97c940 100644 --- a/internal/tufext/targets_iter.go +++ b/internal/tufext/targets_iter.go @@ -102,12 +102,9 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // roleTodo indicates that we need to walk into the given role. type roleTodo struct { name, delegator string - // If non-nil, this is the delegation information for this role. - delegation *tufmetadata.DelegatedRole - // The stack of patterns which be matched for a path in this role + // The stack of patterns which must be matched for a path in this role // to be valid (this includes all ancestor patterns as well as the - // patterns for this DelegatedRole). If delegation is nil, then - // this field is ignored. + // patterns for this DelegatedRole). patternChain [][]string } @@ -183,11 +180,9 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. } // If this is a delegated role, make sure that the target path // matches one of the patterns specified by the delegator. - if delegation := thisRole.delegation; delegation != nil { - if !pathMatchesPatternChain(thisRole.patternChain, path) { - // TODO(log): Add logging. - continue targets - } + if !pathMatchesPatternChain(thisRole.patternChain, path) { + // TODO(log): Add logging. + continue targets } if !yield(TargetFileData{Path: path, TargetFiles: meta}) { return nil @@ -195,16 +190,6 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. seenTargets[path] = struct{}{} } - // If this is a terminating role, we need to make sure that any - // roles higher up on the todo stack cannot match the same paths. - // However, for descendants of this role (those about to be pushed - // onto the stack) s4.5 says that they are also permitted to match - // against the terminating patterns. So we defer this to after any - // children added below are processed. - if delegation := thisRole.delegation; delegation != nil && delegation.Terminating { - todo = append(todo, terminationTodo{chain: thisRole.patternChain}) - } - // Now append the set of delegations to the todo queue. if delegations := role.Signed.Delegations; delegations != nil { if delegations.SuccinctRoles != nil { @@ -218,11 +203,20 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. if len(delegatedRole.PathHashPrefixes) > 0 { return fmt.Errorf("role %s uses path prefixes: unsupported feature", delegatedRole.Name) } + newChain := append(slices.Clone(thisRole.patternChain), delegatedRole.Paths) + // If this is a terminating delegation then we need to + // push a termination marker beneath the role so that roles + // already on the todo stack (i.e., later siblings and + // ancestors' later siblings) cannot provide matching + // targets, while children of this role (pushed above the + // marker) still can, to match s4.5 of the TUF spec. + if delegatedRole.Terminating { + todo = append(todo, terminationTodo{chain: newChain}) + } todo = append(todo, roleTodo{ name: delegatedRole.Name, delegator: thisRole.name, - delegation: &delegatedRole, - patternChain: append(slices.Clone(thisRole.patternChain), delegatedRole.Paths), + patternChain: newChain, }) } } diff --git a/internal/tufext/targets_iter_test.go b/internal/tufext/targets_iter_test.go index 9701cde..b0beccc 100644 --- a/internal/tufext/targets_iter_test.go +++ b/internal/tufext/targets_iter_test.go @@ -452,6 +452,54 @@ func TestIterTargetFiles_MultipleTerminatingChainsTrackedIndependently(t *testin assert.Equal(t, map[string]*tufmetadata.TargetFiles{"c/yielded": yielded}, got) } +func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T) { + // go-tuf stops considering later roles the moment it *encounters* a + // matching terminating delegation in a parent's list (s5.6.7.2.1), before + // visiting the role. So the marker must be pushed when the delegation is + // seen, not when the role is processed. Here "a" delegates terminatingly + // to itself: the second visit is skipped as a cycle, and "b" must still + // be blocked. "control" and "x/in-a" are positive controls. + control, inA := tf(99), tf(1) + got := pathsFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets( + map[string]*tufmetadata.TargetFiles{"control": control}, + []tufmetadata.DelegatedRole{ + dr("a", false, "x/*"), + dr("b", false, "x/*"), + }, + ), + "a": signedTargets( + map[string]*tufmetadata.TargetFiles{"x/in-a": inA}, + []tufmetadata.DelegatedRole{dr("a", true, "x/*")}, + ), + "b": signedTargets(map[string]*tufmetadata.TargetFiles{"x/from-b": tf(2)}, nil), + }) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control, "x/in-a": inA}, got) +} + +func TestIterTargetFiles_TerminatingAppliesEvenIfRoleAlreadyVisited(t *testing.T) { + // Diamond variant of the above: "shared" is first reached via "a" + // (non-terminating) and then via "b" (terminating). A go-tuf lookup for + // x/from-c clears its stack on encountering b's terminating delegation + // and then skips the already-visited "shared", so "c" is never consulted. + control := tf(99) + got := pathsFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets( + map[string]*tufmetadata.TargetFiles{"control": control}, + []tufmetadata.DelegatedRole{ + dr("a", false, "x/*"), + dr("b", false, "x/*"), + dr("c", false, "x/*"), + }, + ), + "a": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*")}), + "b": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", true, "x/*")}), + "shared": signedTargets(nil, nil), + "c": signedTargets(map[string]*tufmetadata.TargetFiles{"x/from-c": tf(1)}, nil), + }) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) +} + func TestIterTargetFiles_CycleSelfReference(t *testing.T) { // d1 delegates to itself; the seen-set must prevent re-entry. d1Meta := tf(1) From 26a882695d79d9af3d8187888b7859bd9eec6446 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 15 Sep 2026 10:45:59 +1000 Subject: [PATCH 3/8] tufext/iter: create tufmetadata.DelegatedRole to model delegation chain 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 --- internal/tufext/targets_iter.go | 134 +++++++----- internal/tufext/targets_iter_test.go | 303 ++++++++++++++++++++++++++- 2 files changed, 377 insertions(+), 60 deletions(-) diff --git a/internal/tufext/targets_iter.go b/internal/tufext/targets_iter.go index b97c940..edd7e0a 100644 --- a/internal/tufext/targets_iter.go +++ b/internal/tufext/targets_iter.go @@ -8,9 +8,7 @@ import ( "fmt" "io/fs" "iter" - "path/filepath" "slices" - "strings" tufmetadata "github.com/theupdateframework/go-tuf/v2/metadata" @@ -27,40 +25,64 @@ type TargetFileData struct { *tufmetadata.TargetFiles } -func pathMatchesPattern(pattern, targetPath string) bool { - if targetPath == pattern { - return true // fast path for literal patterns - } - targetParts := strings.Split(targetPath, "/") - patternParts := strings.Split(pattern, "/") - if len(targetParts) != len(patternParts) { - return false +// DelegationChain represents the chain of TUF delegations that were followed +// to reach a given target role (or file). +// +// If an API returns a [DelegationChain] along with some TUF metadata, users +// must ensure that they use [DelegationChain.IsTargetPermitted] as part of +// ensuring a delegated role cannot provide a file they have no authority to +// provide. +// +// A [DelegationChain] is immutable once created (and may share storage with +// other chains), which is why [DelegationChain.Extend] creates a clone when +// extending the chain. +// +// TODO: This does not currently handle succinct delegations. +type DelegationChain struct { + links []*tufmetadata.DelegatedRole +} + +// IsEmpty returns whether the [DelegationChain] is empty (this can only be +// true for the root "targets" role). +func (chain DelegationChain) IsEmpty() bool { + return len(chain.links) == 0 +} + +// clone makes a shallow copy of a [DelegationChain]. Not exported because +// [DelegationChain]s are immutable in the public API. +func (chain DelegationChain) clone() DelegationChain { + return DelegationChain{ + links: slices.Clone(chain.links), } - for i := range targetParts { - // TODO: filepath.Match is used by go-tuf but it supports more patterns - // than the TUF specification (this is almost certainly wrong and could - // even be a security bug if someone depends on the paths not being - // matched that way). - if ok, _ := filepath.Match(patternParts[i], targetParts[i]); !ok { - return false - } +} + +// Extend returns a copy of the [DelegationChain] with the given delegation +// appended, indicating that the delegation came from the given role. The +// receiver is left untouched. +func (chain DelegationChain) Extend(delegation *tufmetadata.DelegatedRole) DelegationChain { + clone := chain.clone() + if delegation != nil { + clone.links = append(clone.links, delegation) } - return true + return clone } -func pathMatchesPatternChain(patternChain [][]string, targetPath string) bool { - unmatched := len(patternChain) -stack: - for _, patterns := range patternChain { - for _, pattern := range patterns { - if pathMatchesPattern(pattern, targetPath) { - unmatched-- - continue stack - } +// IsTargetPermitted returns whether the [DelegationChain] (of a targets role) +// is authorised to provide a target with the given path. An empty +// [DelegationChain] will match any path, as it represents the root "targets" +// role. +func (chain DelegationChain) IsTargetPermitted(targetPath string) bool { + for _, delegation := range chain.links { + // NOTE: go-tuf internally uses filepath.Match which actually accepts + // more things than the spec allows. This is really not ideal but for + // now we need to just accept that they do it that way so that fetching + // a target is consistent. + // + if ok, err := delegation.IsDelegatedPath(targetPath); !ok || err != nil { + return false } - return false // early break } - return unmatched == 0 + return true } // TargetMetadataFetchFunc is a helper function for [IterTargetFiles] that is @@ -102,22 +124,22 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // roleTodo indicates that we need to walk into the given role. type roleTodo struct { name, delegator string - // The stack of patterns which must be matched for a path in this role - // to be valid (this includes all ancestor patterns as well as the - // patterns for this DelegatedRole). - patternChain [][]string + // The delegation chain followed to reach this role. Target files + // provided by this role are only valid if every link authorises it + // (the top-level "targets" role authorises everything). + chain DelegationChain } // Once we hit a terminating delegation we need to make sure that the // paths it matches cannot be yielded afterwards. - terminatedPatternChains := make([][][]string, 0, 128) - // terminationTodo is a marker to indicate that terminatedPatternChains - // needs to be updated. This is needed because s4.5 of the TUF spec - // allows for children of a terminating pattern to match terminating - // paths, requiring deferred terminatedPatternChains updates. - type terminationTodo struct { - chain [][]string - } + terminatedDelegationChains := make([]DelegationChain, 0, 128) + // terminationTodo is a marker to indicate terminatedDelegationChains + // needs to be updated to include this DelegationChain. This needs be + // deferred this way because s4.5 of the TUF spec allows for child + // delegations of a terminating delegation to match terminating paths + // -- meaning that the DelegationChain cannot be added to the set of + // forbidden patterns until all child delegations have been processed. + type terminationTodo DelegationChain // Queue and seen-list to avoid re-iterating on a role. seen := make(map[string]struct{}, maxDelegations) @@ -133,7 +155,7 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. var thisRole roleTodo switch next := next.(type) { case terminationTodo: - terminatedPatternChains = append(terminatedPatternChains, next.chain) + terminatedDelegationChains = append(terminatedDelegationChains, DelegationChain(next)) continue roles case roleTodo: thisRole = next @@ -172,15 +194,16 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. } // Make sure we don't yield entires that were already covered // by an earlier terminating delegation. - for _, patternChain := range terminatedPatternChains { - if pathMatchesPatternChain(patternChain, path) { + for _, chain := range terminatedDelegationChains { + // A permitted target path is *bad* for terminated chains! + if chain.IsTargetPermitted(path) { // TODO(log): Add logging. continue targets } } // If this is a delegated role, make sure that the target path // matches one of the patterns specified by the delegator. - if !pathMatchesPatternChain(thisRole.patternChain, path) { + if !thisRole.chain.IsTargetPermitted(path) { // TODO(log): Add logging. continue targets } @@ -192,6 +215,9 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // Now append the set of delegations to the todo queue. if delegations := role.Signed.Delegations; delegations != nil { + // TODO: Supporting this would require more work in + // DelegationChain to properly support, and we do not support + // this in the rest of tufrepo and tufext anyway. if delegations.SuccinctRoles != nil { return fmt.Errorf("role %s uses succinct roles: unsupported feature", thisRole.name) } @@ -200,10 +226,16 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // ends up at the top of the stack (tail of todo), to match // s5.6.7 of the TUF spec. for delegatedRole := range generics.ReverseIter(delegations.Roles) { + // TODO: In principle we support path hash prefixes since + // DelegationChain.IsTargetPermitted uses go-tuf's matching + // logic, but go-tuf upstream has a bug in how they compute + // these hashes and so we are best to disallow them for + // now. + // if len(delegatedRole.PathHashPrefixes) > 0 { return fmt.Errorf("role %s uses path prefixes: unsupported feature", delegatedRole.Name) } - newChain := append(slices.Clone(thisRole.patternChain), delegatedRole.Paths) + newChain := thisRole.chain.Extend(&delegatedRole) // If this is a terminating delegation then we need to // push a termination marker beneath the role so that roles // already on the todo stack (i.e., later siblings and @@ -211,12 +243,12 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // targets, while children of this role (pushed above the // marker) still can, to match s4.5 of the TUF spec. if delegatedRole.Terminating { - todo = append(todo, terminationTodo{chain: newChain}) + todo = append(todo, terminationTodo(newChain)) } todo = append(todo, roleTodo{ - name: delegatedRole.Name, - delegator: thisRole.name, - patternChain: newChain, + name: delegatedRole.Name, + delegator: thisRole.name, + chain: newChain, }) } } diff --git a/internal/tufext/targets_iter_test.go b/internal/tufext/targets_iter_test.go index b0beccc..49933bf 100644 --- a/internal/tufext/targets_iter_test.go +++ b/internal/tufext/targets_iter_test.go @@ -5,6 +5,8 @@ package tufext_test import ( "context" + "crypto/sha256" + "encoding/base64" "errors" "fmt" "io/fs" @@ -220,6 +222,25 @@ func TestIterTargetFiles_LiteralPath(t *testing.T) { assert.Equal(t, []string{"exact/file.bin"}, keysOf(got)) } +func TestIterTargetFiles_MalformedPatternNeverMatches(t *testing.T) { + // See TestDelegationChain_MalformedPatternNeverMatches. A target whose + // delegation pattern is a malformed glob must not be yielded, even when + // the path is byte-for-byte equal to the pattern. "control" guards + // against a drop-everything regression. + control := tf(99) + got := pathsFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets( + map[string]*tufmetadata.TargetFiles{"control": control}, + []tufmetadata.DelegatedRole{dr("d1", false, "a/[b")}, + ), + "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ + "a/[b": tf(1), + "a/b": tf(2), + }, nil), + }) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) +} + func TestIterTargetFiles_MultiplePathPatterns(t *testing.T) { got := pathsFrom(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ @@ -596,15 +617,43 @@ func TestIterTargetFiles_MissingDelegatedRole(t *testing.T) { assert.Contains(t, err.Error(), "missing") } -func TestIterTargetFiles_PathHashPrefixes(t *testing.T) { - role := dr("d1", false) - role.PathHashPrefixes = []string{"abcd"} - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{role}), - "d1": signedTargets(nil, nil), - }))) - require.Error(t, err) - assert.Contains(t, err.Error(), "uses path prefixes") +func TestIterTargetFiles_PathHashPrefixes_Rejected(t *testing.T) { + // DelegationChain.IsTargetPermitted can evaluate hash-bin delegations (see + // TestDelegationChain_PathHashPrefixes_Smoke), but IterTargetFiles + // refuses to walk them while go-tuf's digest encoding is non-conformant + // (see the NOTE on hashPrefix): a conformant repository would otherwise + // be mis-binned identically by us and by the go-tuf client. The error + // must fire wherever the delegation appears, not only at the top level. + bin := dr("bin", false /* no paths */) + bin.PathHashPrefixes = []string{hashPrefix("bins/x")} + + for _, tc := range []struct { + name string + targets map[string]*tufext.SignedTargets + }{ + { + name: "top-level", + targets: map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{bin}), + "bin": signedTargets(nil, nil), + }, + }, + { + name: "nested", + targets: map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "bins/*")}), + "d1": signedTargets(nil, []tufmetadata.DelegatedRole{bin}), + "bin": signedTargets(nil, nil), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(tc.targets))) + require.Error(t, err) + assert.Contains(t, err.Error(), "uses path prefixes") + assert.Contains(t, err.Error(), "bin", "error must name the offending role") + }) + } } func TestIterTargetFiles_SuccinctRoles(t *testing.T) { @@ -896,6 +945,242 @@ func TestIterTargetFiles_FetcherCalledOncePerRole(t *testing.T) { }, counts) } +// chainOf builds a [tufext.DelegationChain] from the given delegations, +// ordered from the delegation closest to "targets" down to the leaf. +func chainOf(delegations ...tufmetadata.DelegatedRole) tufext.DelegationChain { + var chain tufext.DelegationChain + for _, delegation := range delegations { + chain = chain.Extend(&delegation) + } + return chain +} + +// hashPrefixLen is long enough that two arbitrary test paths landing in the +// same bin is not a realistic concern. +const hashPrefixLen = 8 + +// hashPrefix returns a PathHashPrefixes entry that go-tuf will treat as +// covering path: the first hashPrefixLen characters of the base64url-encoded +// SHA-256 digest of path. +// +// NOTE: base64url mirrors go-tuf's IsDelegatedPath, which deviates from the +// TUF specification (s4.5 says PATH_HASH_PREFIXES are prefixes of the +// hexadecimal digest, which is what python-tuf and go-tuf v1 implement). If +// go-tuf is fixed to use hex, this helper must change with it. +// +func hashPrefix(path string) string { + sum := sha256.Sum256([]byte(path)) + return base64.URLEncoding.EncodeToString(sum[:])[:hashPrefixLen] +} + +func TestDelegationChain_ZeroValueMatchesEverything(t *testing.T) { + // An empty chain represents the top-level "targets" role, which has + // authority over every path. + var chain tufext.DelegationChain + for _, path := range []string{"", "a", "a/b", "deep/er/path.bin"} { + assert.True(t, chain.IsTargetPermitted(path), "path %q", path) + } +} + +func TestDelegationChain_SingleLink(t *testing.T) { + chain := chainOf(dr("d1", false, "a/*")) + assert.True(t, chain.IsTargetPermitted("a/x")) + assert.False(t, chain.IsTargetPermitted("b/x"), "wrong directory") + assert.False(t, chain.IsTargetPermitted("a/x/y"), "'*' must not cross '/'") + assert.False(t, chain.IsTargetPermitted("a"), "component count must match") +} + +func TestDelegationChain_PatternsWithinLinkAreOr(t *testing.T) { + chain := chainOf(dr("d1", false, "a/*", "b/*")) + assert.True(t, chain.IsTargetPermitted("a/x")) + assert.True(t, chain.IsTargetPermitted("b/x")) + assert.False(t, chain.IsTargetPermitted("c/x")) +} + +func TestDelegationChain_LinksAreAnd(t *testing.T) { + // Every link in the chain must match: a path that satisfies the ancestor + // but not the leaf (or vice versa) is not authorised. + chain := chainOf( + dr("d1", false, "a/*/*"), + dr("d2", false, "a/foo/*"), + ) + assert.True(t, chain.IsTargetPermitted("a/foo/x")) + assert.False(t, chain.IsTargetPermitted("a/bar/x"), "matches ancestor only") + assert.False(t, chain.IsTargetPermitted("b/foo/x"), "matches neither") + + // A leaf whose patterns are wider than its parent's is clamped by the + // parent. + wide := chainOf( + dr("d1", false, "a/foo/*"), + dr("d2", false, "a/*/*"), + ) + assert.True(t, wide.IsTargetPermitted("a/foo/x")) + assert.False(t, wide.IsTargetPermitted("a/bar/x"), "matches leaf only") +} + +func TestDelegationChain_LinkWithoutPathsMatchesNothing(t *testing.T) { + // Unlike an empty chain, a chain containing a delegation with no paths + // (and no hash prefixes) can never match: that role was delegated + // nothing. + chain := chainOf(dr("d1", false /* no paths */)) + assert.False(t, chain.IsTargetPermitted("")) + assert.False(t, chain.IsTargetPermitted("a")) + assert.False(t, chain.IsTargetPermitted("a/b")) + + // This holds even when sandwiched between links that do match. + middle := chainOf( + dr("d1", false, "a/*"), + dr("d2", false /* no paths */), + dr("d3", false, "a/*"), + ) + assert.False(t, middle.IsTargetPermitted("a/x")) +} + +func TestDelegationChain_ExtendNarrowsAuthority(t *testing.T) { + // Each appended link is one more pattern the path must satisfy, so a + // longer chain never authorises more than the chain it was derived from. + // Chains are immutable: Extend derives a new one and leaves the + // receiver untouched. + var root tufext.DelegationChain + require.True(t, root.IsTargetPermitted("b/x")) + + d1 := dr("d1", false, "a/*") + one := root.Extend(&d1) + assert.True(t, one.IsTargetPermitted("a/x")) + assert.False(t, one.IsTargetPermitted("b/x")) + assert.True(t, root.IsEmpty(), "receiver must be untouched") + assert.True(t, root.IsTargetPermitted("b/x"), "receiver must be untouched") + + d2 := dr("d2", false, "a/y") + two := one.Extend(&d2) + assert.True(t, two.IsTargetPermitted("a/y")) + assert.False(t, two.IsTargetPermitted("a/x")) + assert.True(t, one.IsTargetPermitted("a/x"), "receiver must be untouched") +} + +func TestDelegationChain_IsEmpty(t *testing.T) { + // Only the root "targets" role has an empty chain, so IsEmpty is how a + // consumer tells "unrestricted" apart from "restricted to these paths". + var chain tufext.DelegationChain + assert.True(t, chain.IsEmpty(), "zero value") + + d1 := dr("d1", false, "a/*") + child := chain.Extend(&d1) + assert.False(t, child.IsEmpty(), "Extend result") + assert.True(t, chain.IsEmpty(), "Extend must not touch the receiver") + assert.False(t, child.Extend(&d1).IsEmpty(), "extending a non-empty chain") +} + +func TestDelegationChain_ExtendNilIsNoop(t *testing.T) { + // A nil delegation is dropped rather than stored, so Match never has to + // dereference it (which would panic inside go-tuf) and the chain's + // authority is unchanged. + var empty tufext.DelegationChain + stillEmpty := empty.Extend(nil) + assert.True(t, stillEmpty.IsEmpty()) + assert.True(t, stillEmpty.IsTargetPermitted("anything/at/all")) + + d1 := dr("d1", false, "a/*") + restricted := chainOf(d1) + same := restricted.Extend(nil) + assert.False(t, same.IsEmpty()) + assert.True(t, same.IsTargetPermitted("a/x")) + assert.False(t, same.IsTargetPermitted("b/x")) + + // A nil in the middle of a sequence of appends must not poison the + // links either side of it. + d2 := dr("d2", false, "a/y") + longer := restricted.Extend(nil).Extend(&d2) + assert.True(t, longer.IsTargetPermitted("a/y")) + assert.False(t, longer.IsTargetPermitted("a/x")) +} + +func TestDelegationChain_ExtendDoesNotAliasSiblings(t *testing.T) { + // Companion to TestIterTargetFiles_DeepDelegationsWithSiblings at the + // DelegationChain level: two siblings derived from the same parent must + // each get their own backing storage, and the parent must be untouched. + // Whether cap > len at a given depth is a runtime detail, so sweep a + // range of parent depths. + // This is also what exercises the unexported clone: Extend clones + // before appending, so a clone that shared the backing array would let + // one sibling clobber the other. + for _, depth := range []int{0, 1, 2, 3, 4, 5, 7, 8} { + t.Run(fmt.Sprintf("depth=%d", depth), func(t *testing.T) { + var parent tufext.DelegationChain + for i := range depth { + d := dr(fmt.Sprintf("r%d", i), false, "shared/*") + parent = parent.Extend(&d) + } + + leafA := dr("leafA", false, "shared/A") + leafB := dr("leafB", false, "shared/B") + a := parent.Extend(&leafA) + b := parent.Extend(&leafB) + + assert.True(t, a.IsTargetPermitted("shared/A")) + assert.False(t, a.IsTargetPermitted("shared/B"), "sibling B clobbered A's leaf") + assert.True(t, b.IsTargetPermitted("shared/B")) + assert.False(t, b.IsTargetPermitted("shared/A"), "sibling A clobbered B's leaf") + + // The parent has no leaf restriction and must accept both. + assert.True(t, parent.IsTargetPermitted("shared/A")) + assert.True(t, parent.IsTargetPermitted("shared/B")) + }) + } +} + +func TestDelegationChain_MalformedPatternNeverMatches(t *testing.T) { + // go-tuf treats a pattern that filepath.IsTargetPermitted rejects as matching + // nothing. The matcher that DelegationChain replaced had a + // literal-equality fast path which would have accepted "a/[b" for + // itself. We must not diverge from go-tuf here: a mismatch between what + // the TUF client resolves and what we iterate is exactly the class of + // bug this type exists to prevent. + chain := chainOf(dr("d1", false, "a/[b")) + assert.False(t, chain.IsTargetPermitted("a/[b")) + assert.False(t, chain.IsTargetPermitted("a/b")) +} + +func TestDelegationChain_PathHashPrefixes_Smoke(t *testing.T) { + // We don't use hash-bin delegations, but DelegationChain.IsTargetPermitted must at + // least honour them the way go-tuf does, so that a chain containing one + // can't silently accept everything (or nothing). + const ( + covered = "bins/covered.bin" + uncovered = "bins/uncovered.bin" + ) + prefix := hashPrefix(covered) + require.NotEqual(t, prefix, hashPrefix(uncovered), "test paths must land in different bins") + + bin := dr("bin", false /* no paths */) + bin.PathHashPrefixes = []string{prefix} + + chain := chainOf(bin) + assert.True(t, chain.IsTargetPermitted(covered)) + assert.False(t, chain.IsTargetPermitted(uncovered)) + + // Hash-bin links compose with pattern links like any other link. + nested := chainOf(dr("d1", false, "bins/*"), bin) + assert.True(t, nested.IsTargetPermitted(covered)) + assert.False(t, nested.IsTargetPermitted(uncovered)) +} + +func TestDelegationChain_BothPathsAndHashPrefixes_PathsWin(t *testing.T) { + // Spec s4.5 requires exactly one of "paths" and "path_hash_prefixes". + // go-tuf only enforces that when marshalling, so a delegation that + // arrives with both set is accepted on parse; IsDelegatedPath then + // consults Paths and ignores PathHashPrefixes entirely. Pin that so a + // change in go-tuf's precedence (or a hex fix, see hashPrefix) shows up + // here rather than as a silent change in which role is authorised. + const hashed = "b/x" + both := dr("both", false, "a/*") + both.PathHashPrefixes = []string{hashPrefix(hashed)} + + chain := chainOf(both) + assert.True(t, chain.IsTargetPermitted("a/x"), "Paths must still be honoured") + assert.False(t, chain.IsTargetPermitted(hashed), "PathHashPrefixes must be ignored when Paths is set") +} + // keysOf returns the sorted keys. The iterator's within-role order is // unspecified, so most tests compare sorted slices or sets. func keysOf[V any](m map[string]V) []string { From b2973ee9f34393c523ff3f44c21f521e0d23d054 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 16 Sep 2026 00:54:18 +1000 Subject: [PATCH 4/8] tufext/iter: implement (more) correct delegation skipping behaviour 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 --- internal/tufclient/client.go | 63 +++- internal/tufclient/client_delegations_test.go | 252 ++++++++++++++ internal/tufext/targets_iter.go | 124 +++++-- internal/tufext/targets_iter_test.go | 307 +++++++++++++----- internal/tufext/validate.go | 8 + 5 files changed, 644 insertions(+), 110 deletions(-) create mode 100644 internal/tufclient/client_delegations_test.go diff --git a/internal/tufclient/client.go b/internal/tufclient/client.go index e4fd5e7..0023fab 100644 --- a/internal/tufclient/client.go +++ b/internal/tufclient/client.go @@ -341,12 +341,69 @@ func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, met var mu sync.RWMutex // to serialise access to TrustedMetadata return func(ctx context.Context, roleName, delegatorName string) (_ *tufext.SignedTargets, Err error) { - mu.RLock() // TODO: Make this cancellable with ctx. - metaRef, ok := metadata.Snapshot.Signed.Meta[roleName+".json"] + // TODO: Make this critical section cancellable with ctx? + mu.RLock() + // Fetch the link from the snapshot. + metaRef, isKnownRole := metadata.Snapshot.Signed.Meta[roleName+".json"] + // Fetch the cached data if we have it already. + var ( + savedTarget, haveFetched = metadata.Targets[roleName] + savedDelegator tufext.TargetDelegatorRole + haveDelegator bool + ) + if delegatorName == tufmetadata.ROOT { + savedDelegator, haveDelegator = metadata.Root, true + } else { + savedDelegator, haveDelegator = metadata.Targets[delegatorName] + } mu.RUnlock() - if !ok { + + if !isKnownRole { return nil, fmt.Errorf("role %s: %w", roleName, fs.ErrNotExist) } + // IterTargetFiles can iterate over the same role more than once, so we + // can avoid unneeded fetches by returning the local data if we've + // already fetched and validated this role's hashes. + // + // It is safe to re-use the data because TrustedMetadata explicitly + // does not permit the timestamp or snapshot role data to be updated + // after targets have been fetched -- if we have the target already + // then this must be the exact same thing we would've fetched anyway. + if haveFetched { + // This really cannot happen, but add a check just in case. Sadly + // we cannot assert the hashes because those are not necessarily + // recomputable. + if savedTarget.Signed.Version != metaRef.Version { + return nil, fmt.Errorf( + "previously-fetched-and-trusted target role %s has inconsistent version (snapshot says %d but role has %d)", + roleName, metaRef.Version, savedTarget.Signed.Version, + ) + } + // This cannot happen by construction (in order to reach a + // delegatee we must have already parsed the delegator data), but + // do it anyway to avoid nil panics. + if !haveDelegator { + return nil, fmt.Errorf( + "target role %s was reached from delegator %s without delegator being fetched (should never happen)", + roleName, delegatorName, + ) + } + // Make sure that the saved target is actually signed by keys + // trusted by *this* delegator as well. + // NOTE: go-tuf does not do this because they basically implement + // + // incorrectly and never walk the same role twice. + // FIXME: This needs to be moved to IterTargetFiles so that the + // checking logic is generic -- we might even want to skip over bad + // delegations instead of erroring out...? + if err := savedDelegator.VerifyDelegate(roleName, savedTarget); err != nil { + return nil, fmt.Errorf( + "previously-fetched-and-trusted target role %s has bad signature for delegation from role %s: %w", + roleName, delegatorName, err, + ) + } + return savedTarget, nil + } metaPath := fmt.Sprintf("%d.%s.json", metaRef.Version, roleName) metaURL := repo.MetaRootURL.JoinPath(metaPath) diff --git a/internal/tufclient/client_delegations_test.go b/internal/tufclient/client_delegations_test.go new file mode 100644 index 0000000..c4c9492 --- /dev/null +++ b/internal/tufclient/client_delegations_test.go @@ -0,0 +1,252 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package tufclient_test + +import ( + "bytes" + "context" + "crypto/ed25519" + "crypto/rand" + "io" + "net/http" + "slices" + "strings" + "sync/atomic" + "testing" + + "github.com/secure-systems-lab/go-securesystemslib/cjson" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + tufmetadata "github.com/theupdateframework/go-tuf/v2/metadata" + + "go.amutable.dev/quarry/internal/testrepo" + "go.amutable.dev/quarry/internal/tufext" + "go.amutable.dev/quarry/internal/tufrepo" +) + +// roleKey is an in-memory ed25519 key for a delegated targets role. Delegated +// roles are signed directly rather than through the server's keystore so a +// test can control exactly which keys each delegator trusts. +type roleKey struct { + priv ed25519.PrivateKey + pub *tufmetadata.Key + id string +} + +func newRoleKey(t *testing.T) *roleKey { + t.Helper() + pubKey, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + pub, err := tufmetadata.KeyFromPublicKey(pubKey) + require.NoError(t, err) + id, err := pub.ID() + require.NoError(t, err) + return &roleKey{priv: priv, pub: pub, id: id} +} + +// sign adds a signature over meta.Signed, replacing any earlier signature by +// the same key. +func (key *roleKey) sign(t *testing.T, meta *tufext.SignedTargets) { + t.Helper() + payload, err := cjson.EncodeCanonical(meta.Signed) + require.NoError(t, err) + meta.Signatures = slices.DeleteFunc(meta.Signatures, func(sig tufmetadata.Signature) bool { + return sig.KeyID == key.id + }) + meta.Signatures = append(meta.Signatures, tufmetadata.Signature{ + KeyID: key.id, + Signature: ed25519.Sign(key.priv, payload), + }) +} + +// delegatedRole describes a delegated targets role to be created by +// delegationsOp: its targets and the keys that sign it. +type delegatedRole struct { + name string + targets map[string]*tufmetadata.TargetFiles + signers []*roleKey +} + +// delegation describes one edge of the delegation graph: from delegates the +// given paths to to, trusting keys (threshold 1) to sign it. +type delegation struct { + from, to string + paths []string + keys []*roleKey +} + +// delegationsOp returns a TxnOp that creates the given delegated roles and +// adds the given delegation edges. Roles named in edges but not in roles must +// already exist in the transaction (or be "targets"); they are re-read, +// extended, and written back. Every role in roles is fully assembled (targets +// and outgoing delegations) before it is signed, so its signatures are valid +// at Sign time and the transaction does not need keystore keys for it. +func delegationsOp(t *testing.T, roles []delegatedRole, edges []delegation) tufrepo.TxnOp { + return tufrepo.NewTxnOp("test: install delegations", func(ctx context.Context, tx *tufrepo.Transaction) error { + metas := make(map[string]*tufext.SignedTargets, len(roles)+1) + signers := make(map[string][]*roleKey, len(roles)) + for _, role := range roles { + meta := tufext.DefaultTargets(tx.RefTime.Add(tufrepo.DefaultTargetsExpiry)) + meta.Signed.Version = 1 + for path, target := range role.targets { + meta.Signed.Targets[path] = target + } + metas[role.name] = meta + signers[role.name] = role.signers + } + for _, edge := range edges { + delegator, ok := metas[edge.from] + if !ok { + existing, err := tx.TargetsRoleData(ctx, edge.from) + if err != nil { + return err + } + delegator, metas[edge.from] = existing, existing + } + if delegator.Signed.Delegations == nil { + delegator.Signed.Delegations = &tufmetadata.Delegations{ + Keys: make(map[string]*tufmetadata.Key), + } + } + keyIDs := make([]string, 0, len(edge.keys)) + for _, key := range edge.keys { + delegator.Signed.Delegations.Keys[key.id] = key.pub + keyIDs = append(keyIDs, key.id) + } + delegator.Signed.Delegations.Roles = append(delegator.Signed.Delegations.Roles, tufmetadata.DelegatedRole{ + Name: edge.to, + KeyIDs: keyIDs, + Threshold: 1, + Paths: edge.paths, + }) + } + for name, meta := range metas { + for _, key := range signers[name] { + key.sign(t, meta) + } + if err := tx.UpdateRoleData(name, meta); err != nil { + return err + } + } + return nil + }) +} + +// countMetaFetches serves the repository metadata itself (at the "meta" +// subdirectory, which the server must have been created with) and counts +// requests for the given role's versioned metadata file. +func countMetaFetches(srv *testrepo.Server, roleName string) *atomic.Int32 { + var count atomic.Int32 + suffix := "." + roleName + ".json" + // A single-segment wildcard is more specific than the server's "/meta/" + // catch-all, so it takes over metadata requests. + srv.Handle("/meta/{file}", http.HandlerFunc(func(wtr http.ResponseWriter, req *http.Request) { + file := req.PathValue("file") + if strings.HasSuffix(file, suffix) { + count.Add(1) + } + rdr, _, err := srv.Repo.GetBlob(req.Context(), file) + if err != nil { + http.NotFound(wtr, req) + return + } + defer func() { _ = rdr.Close() }() + _, _ = io.Copy(wtr, rdr) + })) + return &count +} + +// Diamond: targets -> a (x/*) and targets -> b (y/*) both delegate to +// "shared", which is signed by a key both delegators trust. Per-path cycle +// detection walks "shared" twice, so both x/file (reachable only via a) and +// y/file (only via b) are listed, but the fetcher must download shared.json +// only once and re-verify the cached copy against b's delegation. +func TestClient_DelegationDiamond_SharedFetchedOnce(t *testing.T) { + srv := testrepo.New(t, testrepo.WithMetaSubdir("meta")) + sharedFetches := countMetaFetches(srv, "shared") + + keyA, keyB, keyShared := newRoleKey(t), newRoleKey(t), newRoleKey(t) + xFile := srv.WriteTarget(t, "x/file", bytes.NewReader([]byte("x"))) + yFile := srv.WriteTarget(t, "y/file", bytes.NewReader([]byte("y"))) + srv.Publish(t, delegationsOp(t, + []delegatedRole{ + {name: "a", signers: []*roleKey{keyA}}, + {name: "b", signers: []*roleKey{keyB}}, + {name: "shared", signers: []*roleKey{keyShared}, targets: map[string]*tufmetadata.TargetFiles{ + "x/file": xFile, + "y/file": yFile, + }}, + }, + []delegation{ + {from: tufmetadata.TARGETS, to: "a", paths: []string{"x/*"}, keys: []*roleKey{keyA}}, + {from: tufmetadata.TARGETS, to: "b", paths: []string{"y/*"}, keys: []*roleKey{keyB}}, + {from: "a", to: "shared", paths: []string{"x/*", "y/*"}, keys: []*roleKey{keyShared}}, + {from: "b", to: "shared", paths: []string{"x/*", "y/*"}, keys: []*roleKey{keyShared}}, + }, + )) + + client := newClient(t, testrepo.Config(t, srv.ConfigBlock("diamond"))) + var paths []string + for info, err := range client.IterTargetFiles(t.Context()) { + require.NoError(t, err) + paths = append(paths, info.Path) + } + slices.Sort(paths) + assert.Equal(t, []string{"x/file", "y/file"}, paths) + assert.Equal(t, int32(1), sharedFetches.Load(), "shared.json must be downloaded once and served from the trusted set afterwards") +} + +// The same diamond, but b trusts a key that never signed "shared". A real +// go-tuf lookup of y/file goes through b and fails verification, so the walk +// must not list y/file on the strength of a's trust: the cached copy is +// re-verified against b and the failure aborts the walk. +// +// Delegation keys come from the delegatee's publisher and are recorded by the +// delegator, so a mismatch like this is a publisher error rather than a +// supported configuration. The client-side abort is the security measure; it +// is not something the repository tooling is expected to catch, which is why +// the second transaction publishes without complaint. +func TestClient_DelegationDiamond_ReverifyFailureAborts(t *testing.T) { + srv := testrepo.New(t, testrepo.WithMetaSubdir("meta")) + + keyA, keyB, keyShared, keyWrong := newRoleKey(t), newRoleKey(t), newRoleKey(t), newRoleKey(t) + xFile := srv.WriteTarget(t, "x/file", bytes.NewReader([]byte("x"))) + yFile := srv.WriteTarget(t, "y/file", bytes.NewReader([]byte("y"))) + srv.Publish(t, delegationsOp(t, + []delegatedRole{ + {name: "a", signers: []*roleKey{keyA}}, + {name: "shared", signers: []*roleKey{keyShared}, targets: map[string]*tufmetadata.TargetFiles{ + "x/file": xFile, + "y/file": yFile, + }}, + }, + []delegation{ + {from: tufmetadata.TARGETS, to: "a", paths: []string{"x/*"}, keys: []*roleKey{keyA}}, + {from: "a", to: "shared", paths: []string{"x/*", "y/*"}, keys: []*roleKey{keyShared}}, + }, + )) + srv.Publish(t, delegationsOp(t, + []delegatedRole{ + {name: "b", signers: []*roleKey{keyB}}, + }, + []delegation{ + {from: tufmetadata.TARGETS, to: "b", paths: []string{"y/*"}, keys: []*roleKey{keyB}}, + {from: "b", to: "shared", paths: []string{"x/*", "y/*"}, keys: []*roleKey{keyWrong}}, + }, + )) + + client := newClient(t, testrepo.Config(t, srv.ConfigBlock("diamond"))) + var paths []string + var walkErr error + for info, err := range client.IterTargetFiles(t.Context()) { + if err != nil { + walkErr = err + break + } + paths = append(paths, info.Path) + } + require.Error(t, walkErr) + assert.Contains(t, walkErr.Error(), "shared", "the error must name the role that failed re-verification") + assert.Equal(t, []string{"x/file"}, paths, "the a-path is walked first and is still trusted; y/file must never appear") +} diff --git a/internal/tufext/targets_iter.go b/internal/tufext/targets_iter.go index edd7e0a..a4db5ff 100644 --- a/internal/tufext/targets_iter.go +++ b/internal/tufext/targets_iter.go @@ -8,11 +8,12 @@ import ( "fmt" "io/fs" "iter" - "slices" + "maps" tufmetadata "github.com/theupdateframework/go-tuf/v2/metadata" "go.amutable.dev/quarry/internal/generics" + "go.amutable.dev/quarry/internal/third_party/assert" ) // TargetFileData is a tuple of (name, *[tufmetadata.TargetFiles]), mainly used @@ -39,30 +40,49 @@ type TargetFileData struct { // // TODO: This does not currently handle succinct delegations. type DelegationChain struct { - links []*tufmetadata.DelegatedRole + // NOTE: The order of iteration doesn't matter for IsTargetPermitted. + links map[string]*tufmetadata.DelegatedRole +} + +// Length returns the number of entries in the [DelegationChain]. +func (chain DelegationChain) Length() int { + return len(chain.links) } // IsEmpty returns whether the [DelegationChain] is empty (this can only be // true for the root "targets" role). func (chain DelegationChain) IsEmpty() bool { - return len(chain.links) == 0 + return chain.Length() == 0 } // clone makes a shallow copy of a [DelegationChain]. Not exported because // [DelegationChain]s are immutable in the public API. func (chain DelegationChain) clone() DelegationChain { return DelegationChain{ - links: slices.Clone(chain.links), + links: maps.Clone(chain.links), } } +// Contains returns true if the given role name was walked through in this +// delegation chain (these are the same role names passed to +// [DelegationChain.Extend]). +func (chain DelegationChain) Contains(roleName string) bool { + return generics.MapContains(chain.links, roleName) +} + // Extend returns a copy of the [DelegationChain] with the given delegation // appended, indicating that the delegation came from the given role. The -// receiver is left untouched. -func (chain DelegationChain) Extend(delegation *tufmetadata.DelegatedRole) DelegationChain { +// receiver is left untouched. This function will panic if several delegations +// from the same role are appended. +func (chain DelegationChain) Extend(fromRole string, delegation *tufmetadata.DelegatedRole) DelegationChain { clone := chain.clone() if delegation != nil { - clone.links = append(clone.links, delegation) + assert.Assertf(!chain.Contains(fromRole), + "DelegationChain must not have the same role %q inserted multiple times", fromRole) + if clone.links == nil { + clone.links = make(map[string]*tufmetadata.DelegatedRole, 32) + } + clone.links[fromRole] = delegation } return clone } @@ -108,7 +128,7 @@ func TargetsMapFetcher(targets map[string]*SignedTargets) TargetMetadataFetchFun // algorithm used by TUF to search for the correct target file. Note that the // order of files yielded is not stable. // -// The provided [GetTargetMetadataFunc] is called each time a target's role +// The provided [TargetMetadataFetchFunc] is called each time a target's role // metadata needs to be loaded. // // TODO(links): This will need callbacks and a lot more infrastructure once we @@ -116,8 +136,12 @@ func TargetsMapFetcher(targets map[string]*SignedTargets) TargetMetadataFetchFun // ideal because we need to pre-fetch all of the targets which the default // updater doesn't do.) func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter.Seq2[TargetFileData, error] { - const maxDelegations = 128 - + // TODO: Pass these in as configuration so that this matches tufclient's + // limits exactly and potentially works better with links? + const ( + maxDelegationDepth = 32 // go-tuf default limit + maxDelegations = 4096 + ) return generics.ErrorIter(func(yield func(TargetFileData) bool) error { seenTargets := make(map[string]struct{}, 256) @@ -141,8 +165,8 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // forbidden patterns until all child delegations have been processed. type terminationTodo DelegationChain - // Queue and seen-list to avoid re-iterating on a role. - seen := make(map[string]struct{}, maxDelegations) + var seenDelegations int + // Stack of remaining delegations to visit. todo := []any{roleTodo{ name: tufmetadata.TARGETS, delegator: tufmetadata.ROOT, @@ -163,24 +187,68 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. panic(fmt.Sprintf("unexpected type %T", next)) } - if len(seen) >= maxDelegations { - // In order to avoid a DoS by some role creating a long chain, - // we have a limit but we do not - // TODO(log): Add logging... - return nil - } - if _, ok := seen[thisRole.name]; ok { - // As per TUF specification s5.6.7.1. - // TODO: While the spec implies this is what we should do, in - // practice clients do not descend into roles that do not match - // so really we would need to collate . + // s5.6.7.1 places several restrictions on walking into delegations + // to avoid unbounded loops or DoSes: + switch { + // Skip walking into roles that were already seen in this + // delegation chain, to avoid cycles. + // + // TODO: The current text of the TUF specification (s5.6.7.1) + // implies that this needs to be a global seen list but based on + // recent upstream discussions, do it this way instead. + // See . + case thisRole.chain.Contains(thisRole.name): continue roles + + // When fetching a target file, if you hit too many delegations you + // need to stop iterating and just return *without an error* (to + // avoid a DoS). This is a global limit for each target file + // lookup. + // + // TODO(links): We must make sure that the delegation iteration + // limits here apply to links so links don't reset the delegation + // depth restriction. + // + // Unfortunately, we cannot really guarantee the exact same + // behaviour when iterating over delegations because we are + // effectively emulating a virtual lookup for *all* target files. + // The completely "correct" mechanism would be to track the degree + // of overlap between different delegation path patterns (good luck + // with hash-based delegations) and restrict iteration based on + // that, but that is not really workable. I think we just have to + // accept that we will iterate over delegations that are not + // reachable when actually fetching. This issue was briefly + // mentioned in . + // + // So, we have two heuristics to try to have somewhat reasonable + // behaviour: + case thisRole.chain.Length() > maxDelegationDepth: + // 1. Skip delegation chains longer than maxDelegationDepth, + // as they would be skipped by clients as per s5.6.7.1. + // + // TODO(log): Add logging...? + continue roles + case seenDelegations >= maxDelegations: + // 2. Add a hard upper bound on the number of total delegations + // in a repository. This might not violate s5.6.7.1 but such a + // repository is clearly "wrong". If we didn't do this, a bad + // repository could create one 32-deep delegation tree then + // delegate to it millions of times. + // + // TODO: Find a better solution. + // TODO(log): Add logging...? + break roles } role, err := fetchFn(ctx, thisRole.name, thisRole.delegator) if err != nil { return fmt.Errorf("target file walk aborted: failed to get role %s: %w", thisRole.name, err) } - seen[thisRole.name] = struct{}{} + // TODO: Validate the delegation signatures here as well (the real + // fetchFn used by clients does so implicitly but we should do it + // for everything -- though "targets" might be a bit tricky to + // validate since we don't have the root here). This might also + // allow us to skip bad delegations...? + seenDelegations++ // First, yield all of the immediate target files. targets: @@ -207,6 +275,11 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // TODO(log): Add logging. continue targets } + // TODO: We probably should check if this target file would + // actually be fetched by a tuf client by seeing how many + // previous delegation paths match against it -- if it is over + // the limit (maxDelegationDepth) then we should mask it here + // too. if !yield(TargetFileData{Path: path, TargetFiles: meta}) { return nil } @@ -221,7 +294,6 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. if delegations.SuccinctRoles != nil { return fmt.Errorf("role %s uses succinct roles: unsupported feature", thisRole.name) } - // Append the delegations in reverse order so the first entry // ends up at the top of the stack (tail of todo), to match // s5.6.7 of the TUF spec. @@ -235,7 +307,7 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. if len(delegatedRole.PathHashPrefixes) > 0 { return fmt.Errorf("role %s uses path prefixes: unsupported feature", delegatedRole.Name) } - newChain := thisRole.chain.Extend(&delegatedRole) + newChain := thisRole.chain.Extend(thisRole.name, &delegatedRole) // If this is a terminating delegation then we need to // push a termination marker beneath the role so that roles // already on the todo stack (i.e., later siblings and diff --git a/internal/tufext/targets_iter_test.go b/internal/tufext/targets_iter_test.go index 49933bf..d82cf27 100644 --- a/internal/tufext/targets_iter_test.go +++ b/internal/tufext/targets_iter_test.go @@ -12,6 +12,7 @@ import ( "io/fs" "iter" "slices" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -498,11 +499,13 @@ func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control, "x/in-a": inA}, got) } -func TestIterTargetFiles_TerminatingAppliesEvenIfRoleAlreadyVisited(t *testing.T) { - // Diamond variant of the above: "shared" is first reached via "a" - // (non-terminating) and then via "b" (terminating). A go-tuf lookup for - // x/from-c clears its stack on encountering b's terminating delegation - // and then skips the already-visited "shared", so "c" is never consulted. +func TestIterTargetFiles_TerminatingOnSecondPathBlocksLaterSiblings(t *testing.T) { + // Diamond variant of the above: "shared" is reached via "a" + // (non-terminating) and again via "b" (terminating); cycle detection is + // per path, so the second visit is walked rather than skipped. A go-tuf + // lookup for x/from-c clears its stack on encountering b's terminating + // delegation, so "c" is never consulted, and the marker must apply after + // the second visit's subtree however that visit is handled. control := tf(99) got := pathsFrom(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( @@ -521,8 +524,38 @@ func TestIterTargetFiles_TerminatingAppliesEvenIfRoleAlreadyVisited(t *testing.T assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } +func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByDepthCap(t *testing.T) { + // Companion to TerminatingAppliesEvenIfRoleSkippedByCycle for the other + // reason a role can be skipped: r31 delegates terminatingly to r32, which + // sits past the depth cap and is never walked. go-tuf clears its stack on + // encountering the terminating delegation (and gives up at its own cap + // right after), so the later top-level sibling "b" must still be blocked + // for x/*. "control" is the positive control. + const depth = 33 // r32 has a chain of length 33 > maxDelegationDepth + control := tf(99) + all := map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets( + map[string]*tufmetadata.TargetFiles{"control": control}, + []tufmetadata.DelegatedRole{ + dr("r0", false, "x/*"), + dr("b", false, "x/*"), + }, + ), + "b": signedTargets(map[string]*tufmetadata.TargetFiles{"x/from-b": tf(2)}, nil), + } + for i := 0; i < depth-1; i++ { + all[fmt.Sprintf("r%d", i)] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr(fmt.Sprintf("r%d", i+1), i == depth-2, "x/*"), // only r31 -> r32 terminates + }) + } + all[fmt.Sprintf("r%d", depth-1)] = signedTargets(map[string]*tufmetadata.TargetFiles{"x/deep": tf(3)}, nil) + + got := pathsFrom(t, all) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) +} + func TestIterTargetFiles_CycleSelfReference(t *testing.T) { - // d1 delegates to itself; the seen-set must prevent re-entry. + // d1 delegates to itself; the per-path cycle check must prevent re-entry. d1Meta := tf(1) got := pathsFrom(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ @@ -555,8 +588,8 @@ func TestIterTargetFiles_CycleTwoRoles(t *testing.T) { } func TestIterTargetFiles_DiamondGraph(t *testing.T) { - // Two parents delegate to "shared"; the seen-set ensures it's processed - // once via whichever path reaches it first. + // Two parents delegate to "shared". It is walked once per delegation + // path, but the target is yielded once: the first path to reach it wins. got := pathsFrom(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("a", false, "x/*"), @@ -575,6 +608,28 @@ func TestIterTargetFiles_DiamondGraph(t *testing.T) { assert.Equal(t, []string{"x/file"}, keysOf(got)) } +func TestIterTargetFiles_DiamondDifferentPatterns(t *testing.T) { + // "shared" is reachable via "a" (x/*) and via "b" (y/*). A real lookup + // for "y/file" never descends into "a", so it reaches "shared" through + // "b" and succeeds. A global visited set would have walked "shared" via + // "a" only, rejected "y/file" against a's chain and never come back; + // per-path cycle detection walks it again via "b". + xf, yf := tf(1), tf(2) + got := pathsFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("a", false, "x/*"), + dr("b", false, "y/*"), + }), + "a": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*", "y/*")}), + "b": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*", "y/*")}), + "shared": signedTargets(map[string]*tufmetadata.TargetFiles{ + "x/file": xf, + "y/file": yf, + }, nil), + }) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{"x/file": xf, "y/file": yf}, got) +} + func TestIterTargetFiles_PreOrderDepthFirst(t *testing.T) { // Spec s5.6.7: pre-order DFS in declared order. One target per role // keeps within-role map-iteration randomness from scrambling the order. @@ -668,27 +723,32 @@ func TestIterTargetFiles_SuccinctRoles(t *testing.T) { assert.Contains(t, err.Error(), "succinct roles") } -func TestIterTargetFiles_MaxDelegationsCap(t *testing.T) { - // Linear chain of 130 delegations exceeds the 128 cap. To match go-tuf - // and avoid DoS amplification on long-chain inputs, hitting the cap - // stops iteration cleanly without an error. Targets beyond the cap are - // silently dropped; targets within it are still yielded. +func TestIterTargetFiles_MaxDelegationDepth(t *testing.T) { + // A chain deeper than go-tuf's default MaxDelegations (32) can never be + // reached by a real lookup, so roles past that depth are skipped without + // an error. go-tuf's loop runs while visited <= 32, so it walks 33 roles + // down a pure chain: "targets" plus r0..r31. r_i has a chain of length + // i+1 and is skipped once that exceeds 32, so r31 is the last role walked + // and r32 onwards are dropped. // - // TARGETS is the 1st role processed; r_i is the (i+2)th. The cap permits - // processing while len(seen) < 128, so r126 is the last role processed - // (seen=127 on entry, seen=128 after) and r127 onwards are skipped. - const linearLen = 130 - all := make(map[string]*tufext.SignedTargets, linearLen+1) + // The depth cap only prunes the over-deep branch: a sibling declared + // after it must still be walked, unlike the total cap (see + // TestIterTargetFiles_MaxDelegationsTotal). + const linearLen = 40 + all := make(map[string]*tufext.SignedTargets, linearLen+2) all[tufmetadata.TARGETS] = signedTargets( map[string]*tufmetadata.TargetFiles{"x/start": tf(1)}, - []tufmetadata.DelegatedRole{dr("r0", false, "x/*")}, + []tufmetadata.DelegatedRole{ + dr("r0", false, "x/*"), + dr("shallow", false, "y/*"), + }, ) for i := 0; i < linearLen-1; i++ { var targets map[string]*tufmetadata.TargetFiles switch i { - case 126: + case 31: targets = map[string]*tufmetadata.TargetFiles{"x/at-cap": tf(2)} - case 127: + case 32: targets = map[string]*tufmetadata.TargetFiles{"x/over-cap": tf(3)} } all[fmt.Sprintf("r%d", i)] = signedTargets(targets, []tufmetadata.DelegatedRole{ @@ -696,9 +756,10 @@ func TestIterTargetFiles_MaxDelegationsCap(t *testing.T) { }) } all[fmt.Sprintf("r%d", linearLen-1)] = signedTargets(nil, nil) + all["shallow"] = signedTargets(map[string]*tufmetadata.TargetFiles{"y/shallow": tf(4)}, nil) got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(all))) - require.NoError(t, err, "exceeding the cap must not produce an error") + require.NoError(t, err, "exceeding the depth cap must not produce an error") paths := make(map[string]struct{}, len(got)) for _, td := range got { @@ -707,6 +768,49 @@ func TestIterTargetFiles_MaxDelegationsCap(t *testing.T) { assert.Contains(t, paths, "x/start") assert.Contains(t, paths, "x/at-cap") assert.NotContains(t, paths, "x/over-cap") + assert.Contains(t, paths, "y/shallow", "the depth cap must only prune the deep branch") +} + +func TestIterTargetFiles_MaxDelegationsTotal(t *testing.T) { + // Roles are no longer deduplicated globally, so a repository could make + // the walk revisit a modest tree an enormous number of times. A hard cap + // on fetched roles stops the walk, without an error, once reached. Unlike + // the depth cap this ends the whole walk: nothing declared after the + // cut-off is visited. The fetcher synthesises roles on demand so the + // fixture stays small. + const ( + totalCap = 4096 + fanOut = totalCap + 100 + ) + roles := make([]tufmetadata.DelegatedRole, 0, fanOut) + for i := range fanOut { + roles = append(roles, dr(fmt.Sprintf("c%d", i), false, "c/*")) + } + var fetches int + fetch := func(_ context.Context, roleName, _ string) (*tufext.SignedTargets, error) { + fetches++ + if roleName == tufmetadata.TARGETS { + return signedTargets(nil, roles), nil + } + if !strings.HasPrefix(roleName, "c") { + return nil, fmt.Errorf("unexpected fetch %q", roleName) + } + return signedTargets(map[string]*tufmetadata.TargetFiles{"c/" + roleName: tf(1)}, nil), nil + } + got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) + require.NoError(t, err, "hitting the total cap must not produce an error") + + // "targets" is the first fetch, so totalCap-1 children are walked, in + // declared order, before the cap trips. + assert.Equal(t, totalCap, fetches) + assert.Len(t, got, totalCap-1) + paths := make(map[string]struct{}, len(got)) + for _, td := range got { + paths[td.Path] = struct{}{} + } + assert.Contains(t, paths, "c/c0") + assert.Contains(t, paths, fmt.Sprintf("c/c%d", totalCap-2)) + assert.NotContains(t, paths, fmt.Sprintf("c/c%d", totalCap-1)) } func TestIterTargetFiles_Reusable(t *testing.T) { @@ -759,11 +863,11 @@ func TestIterTargetFiles_EarlyTermination(t *testing.T) { } func TestIterTargetFiles_DeepDelegationsWithSiblings(t *testing.T) { - // Regression for a patternChain aliasing bug: when a parent's slice has - // cap > len, two siblings' appends share a backing array and the second - // clobbers the first. Whether cap > len holds at a given depth is a Go - // runtime detail, so we sweep several depths to stay robust against - // growth-strategy changes (today, depths 3/5/6/7 trigger; 4/8/16 don't). + // Regression for a chain aliasing bug: siblings derived from the same + // parent shared the parent's backing storage (a slice with cap > len at + // the time), so the second sibling clobbered the first. The chain is now + // map-backed, but the depth sweep is cheap and guards the property + // regardless of representation. for _, depth := range []int{2, 3, 4, 5, 6, 7, 8, 16} { t.Run(fmt.Sprintf("depth=%d", depth), func(t *testing.T) { leafA := tf(int64(depth)*10 + 1) @@ -915,12 +1019,16 @@ func TestIterTargetFiles_FetcherSeesContext(t *testing.T) { assert.ErrorIs(t, err, context.Canceled) } -func TestIterTargetFiles_FetcherCalledOncePerRole(t *testing.T) { - // Diamond graph: "shared" is delegated by both "a" and "b". The seen-set - // must ensure the fetcher is invoked at most once per distinct role. - counts := map[string]int{} - fetch := func(_ context.Context, roleName, _ string) (*tufext.SignedTargets, error) { - counts[roleName]++ +func TestIterTargetFiles_FetcherCalledOncePerDelegationPath(t *testing.T) { + // Diamond graph: "shared" is delegated by both "a" and "b". Cycle + // detection is per delegation path (spec issue 321), not global, so + // "shared" is fetched once per path, each time naming the delegator that + // path came through, in pre-order DFS order. The target is still yielded + // once: the first path wins. + type call struct{ role, delegator string } + var calls []call + fetch := func(_ context.Context, roleName, delegatorName string) (*tufext.SignedTargets, error) { + calls = append(calls, call{roleName, delegatorName}) switch roleName { case tufmetadata.TARGETS: return signedTargets(nil, []tufmetadata.DelegatedRole{ @@ -937,20 +1045,26 @@ func TestIterTargetFiles_FetcherCalledOncePerRole(t *testing.T) { got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) require.NoError(t, err) require.Len(t, got, 1) - assert.Equal(t, map[string]int{ - tufmetadata.TARGETS: 1, - "a": 1, - "b": 1, - "shared": 1, - }, counts) + assert.Equal(t, []call{ + {tufmetadata.TARGETS, tufmetadata.ROOT}, + {"a", tufmetadata.TARGETS}, + {"shared", "a"}, + {"b", tufmetadata.TARGETS}, + {"shared", "b"}, + }, calls) } // chainOf builds a [tufext.DelegationChain] from the given delegations, -// ordered from the delegation closest to "targets" down to the leaf. +// ordered from the delegation closest to "targets" down to the leaf. Each +// link is recorded under the role that made the delegation: "targets" for the +// first, then the previous link's role name, mirroring how IterTargetFiles +// builds chains. func chainOf(delegations ...tufmetadata.DelegatedRole) tufext.DelegationChain { var chain tufext.DelegationChain + fromRole := tufmetadata.TARGETS for _, delegation := range delegations { - chain = chain.Extend(&delegation) + chain = chain.Extend(fromRole, &delegation) + fromRole = delegation.Name } return chain } @@ -1045,16 +1159,18 @@ func TestDelegationChain_ExtendNarrowsAuthority(t *testing.T) { require.True(t, root.IsTargetPermitted("b/x")) d1 := dr("d1", false, "a/*") - one := root.Extend(&d1) + one := root.Extend(tufmetadata.TARGETS, &d1) assert.True(t, one.IsTargetPermitted("a/x")) assert.False(t, one.IsTargetPermitted("b/x")) assert.True(t, root.IsEmpty(), "receiver must be untouched") assert.True(t, root.IsTargetPermitted("b/x"), "receiver must be untouched") d2 := dr("d2", false, "a/y") - two := one.Extend(&d2) + two := one.Extend("d1", &d2) assert.True(t, two.IsTargetPermitted("a/y")) assert.False(t, two.IsTargetPermitted("a/x")) + assert.Equal(t, 2, two.Length()) + assert.Equal(t, 1, one.Length(), "receiver must be untouched") assert.True(t, one.IsTargetPermitted("a/x"), "receiver must be untouched") } @@ -1065,68 +1181,97 @@ func TestDelegationChain_IsEmpty(t *testing.T) { assert.True(t, chain.IsEmpty(), "zero value") d1 := dr("d1", false, "a/*") - child := chain.Extend(&d1) + child := chain.Extend(tufmetadata.TARGETS, &d1) assert.False(t, child.IsEmpty(), "Extend result") assert.True(t, chain.IsEmpty(), "Extend must not touch the receiver") - assert.False(t, child.Extend(&d1).IsEmpty(), "extending a non-empty chain") + assert.False(t, child.Extend("d1", &d1).IsEmpty(), "extending a non-empty chain") +} + +func TestDelegationChain_ContainsAndLength(t *testing.T) { + // A chain records the roles that *delegated* along the path, keyed by + // delegator, so the leaf itself is never a member. IterTargetFiles relies + // on exactly this for cycle detection: a role is skipped only if it has + // already delegated on the current path. + var chain tufext.DelegationChain + assert.Equal(t, 0, chain.Length()) + assert.False(t, chain.Contains(tufmetadata.TARGETS)) + + chain = chainOf(dr("d1", false, "a/*"), dr("d2", false, "a/*")) // targets -> d1 -> d2 + assert.Equal(t, 2, chain.Length()) + assert.True(t, chain.Contains(tufmetadata.TARGETS)) + assert.True(t, chain.Contains("d1")) + assert.False(t, chain.Contains("d2"), "the leaf has not delegated anything on this path") + assert.False(t, chain.Contains("unrelated")) +} + +func TestDelegationChain_DuplicateDelegatorPanics(t *testing.T) { + // A path can only pass through a delegator once, so a second delegation + // under the same role is a programming error rather than a metadata + // error, and the failed append must leave the receiver untouched. + d1 := dr("d1", false, "a/*") + d2 := dr("d2", false, "a/*") + chain := chainOf(d1) // {targets: d1} + assert.Panics(t, func() { _ = chain.Extend(tufmetadata.TARGETS, &d2) }) + assert.Equal(t, 1, chain.Length()) + + // A different delegator is fine. + var longer tufext.DelegationChain + assert.NotPanics(t, func() { longer = chain.Extend("d1", &d2) }) + assert.Equal(t, 2, longer.Length()) + assert.Equal(t, 1, chain.Length()) } func TestDelegationChain_ExtendNilIsNoop(t *testing.T) { // A nil delegation is dropped rather than stored, so Match never has to // dereference it (which would panic inside go-tuf) and the chain's - // authority is unchanged. + // authority is unchanged. The nil check also precedes the duplicate + // delegator check, so a nil under an already-used delegator must not + // panic either. var empty tufext.DelegationChain - stillEmpty := empty.Extend(nil) + stillEmpty := empty.Extend(tufmetadata.TARGETS, nil) assert.True(t, stillEmpty.IsEmpty()) assert.True(t, stillEmpty.IsTargetPermitted("anything/at/all")) d1 := dr("d1", false, "a/*") - restricted := chainOf(d1) - same := restricted.Extend(nil) - assert.False(t, same.IsEmpty()) + restricted := chainOf(d1) // {targets: d1} + var same tufext.DelegationChain + assert.NotPanics(t, func() { same = restricted.Extend(tufmetadata.TARGETS, nil) }) + assert.Equal(t, 1, same.Length()) assert.True(t, same.IsTargetPermitted("a/x")) assert.False(t, same.IsTargetPermitted("b/x")) // A nil in the middle of a sequence of appends must not poison the // links either side of it. d2 := dr("d2", false, "a/y") - longer := restricted.Extend(nil).Extend(&d2) + longer := restricted.Extend("d1", nil).Extend("d1", &d2) + assert.Equal(t, 2, longer.Length()) assert.True(t, longer.IsTargetPermitted("a/y")) assert.False(t, longer.IsTargetPermitted("a/x")) } func TestDelegationChain_ExtendDoesNotAliasSiblings(t *testing.T) { // Companion to TestIterTargetFiles_DeepDelegationsWithSiblings at the - // DelegationChain level: two siblings derived from the same parent must - // each get their own backing storage, and the parent must be untouched. - // Whether cap > len at a given depth is a runtime detail, so sweep a - // range of parent depths. - // This is also what exercises the unexported clone: Extend clones - // before appending, so a clone that shared the backing array would let - // one sibling clobber the other. - for _, depth := range []int{0, 1, 2, 3, 4, 5, 7, 8} { - t.Run(fmt.Sprintf("depth=%d", depth), func(t *testing.T) { - var parent tufext.DelegationChain - for i := range depth { - d := dr(fmt.Sprintf("r%d", i), false, "shared/*") - parent = parent.Extend(&d) - } - - leafA := dr("leafA", false, "shared/A") - leafB := dr("leafB", false, "shared/B") - a := parent.Extend(&leafA) - b := parent.Extend(&leafB) - - assert.True(t, a.IsTargetPermitted("shared/A")) - assert.False(t, a.IsTargetPermitted("shared/B"), "sibling B clobbered A's leaf") - assert.True(t, b.IsTargetPermitted("shared/B")) - assert.False(t, b.IsTargetPermitted("shared/A"), "sibling A clobbered B's leaf") - - // The parent has no leaf restriction and must accept both. - assert.True(t, parent.IsTargetPermitted("shared/A")) - assert.True(t, parent.IsTargetPermitted("shared/B")) - }) - } + // DelegationChain level: two siblings derived from the same parent get + // their own storage and the parent is untouched. Both siblings are + // appended under the same delegator, so shared storage would either + // clobber one leaf or trip the duplicate-delegator check. This is also + // what exercises the unexported clone: Extend clones before + // appending, so a clone that shared the map would fail here. + parent := chainOf(dr("p", false, "shared/*")) // {targets: p} + leafA := dr("leafA", false, "shared/A") + leafB := dr("leafB", false, "shared/B") + a := parent.Extend("p", &leafA) + b := parent.Extend("p", &leafB) + + assert.True(t, a.IsTargetPermitted("shared/A")) + assert.False(t, a.IsTargetPermitted("shared/B"), "sibling B clobbered A's leaf") + assert.True(t, b.IsTargetPermitted("shared/B")) + assert.False(t, b.IsTargetPermitted("shared/A"), "sibling A clobbered B's leaf") + + // The parent has no leaf restriction and must accept both. + assert.Equal(t, 1, parent.Length()) + assert.True(t, parent.IsTargetPermitted("shared/A")) + assert.True(t, parent.IsTargetPermitted("shared/B")) } func TestDelegationChain_MalformedPatternNeverMatches(t *testing.T) { diff --git a/internal/tufext/validate.go b/internal/tufext/validate.go index c6dc9f9..c300829 100644 --- a/internal/tufext/validate.go +++ b/internal/tufext/validate.go @@ -62,3 +62,11 @@ func CheckMetadataType[T tufmetadata.Roles](roleName string, data *tufmetadata.M } return nil } + +// TargetDelegatorRole is implemented by the TUF metadata types which can +// delegate to other target roles. +type TargetDelegatorRole interface { + // VerifyDelegate verifies that delegatedMetadata is signed with the + // required threshold of keys for the delegated role delegatedRole. + VerifyDelegate(delegatedRole string, delegatedMetadata any) error +} From 9640c80ff4877e8d29fe6a81f61da876e0b1a6fa Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 18 Sep 2026 15:44:16 +1000 Subject: [PATCH 5/8] tufext/iter: hoist IterTargetRoles from IterTargetFiles 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 --- internal/tufclient/client.go | 58 +- internal/tufclient/client_delegations_test.go | 109 ++ internal/tufclient/client_repos_test.go | 28 + .../tufext/{targets_iter.go => iter_roles.go} | 149 ++- ...argets_iter_test.go => iter_roles_test.go} | 1070 ++++++++--------- 5 files changed, 730 insertions(+), 684 deletions(-) rename internal/tufext/{targets_iter.go => iter_roles.go} (71%) rename internal/tufext/{targets_iter_test.go => iter_roles_test.go} (62%) diff --git a/internal/tufclient/client.go b/internal/tufclient/client.go index 0023fab..08ca363 100644 --- a/internal/tufclient/client.go +++ b/internal/tufclient/client.go @@ -361,7 +361,7 @@ func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, met if !isKnownRole { return nil, fmt.Errorf("role %s: %w", roleName, fs.ErrNotExist) } - // IterTargetFiles can iterate over the same role more than once, so we + // IterTargetRoles can iterate over the same role more than once, so we // can avoid unneeded fetches by returning the local data if we've // already fetched and validated this role's hashes. // @@ -393,7 +393,7 @@ func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, met // NOTE: go-tuf does not do this because they basically implement // // incorrectly and never walk the same role twice. - // FIXME: This needs to be moved to IterTargetFiles so that the + // FIXME: This needs to be moved to IterTargetRoles so that the // checking logic is generic -- we might even want to skip over bad // delegations instead of erroring out...? if err := savedDelegator.VerifyDelegate(roleName, savedTarget); err != nil { @@ -446,7 +446,9 @@ func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, met } // IterTargetFiles iterates over all target files in all repositories defined -// in the [Client]. +// in the [Client]. Each target file will only be listed once even if it is +// provided by multiple repositories (first-repository-wins semantics). The +// order of target files is not consistent, however. func (client *Client) IterTargetFiles(ctx context.Context) iter.Seq2[*TargetInfo, error] { return generics.ErrorIter(func(yield func(*TargetInfo) bool) error { seen := make(map[string]struct{}, 512) // TODO: Figure out a reasonable default map size. @@ -469,29 +471,43 @@ func (client *Client) IterTargetFiles(ctx context.Context) iter.Seq2[*TargetInfo } meta = updater.GetTrustedMetadataSet() } - fetchFn := client.trustedMetadataTargetsFetcher(repo, &meta) - for target, err := range tufext.IterTargetFiles(ctx, fetchFn) { + fetchFn := client.trustedMetadataTargetsFetcher(repo, &meta) + for roleChain, err := range tufext.IterTargetRoles(ctx, fetchFn) { if err != nil { return fmt.Errorf("error while scanning repo %s: %w", repoName, err) } - if err := ctx.Err(); err != nil { - return err - } - if _, ok := seen[target.Path]; ok { - // IterRepos provides a consistent ordering based on - // order indexes so if we have already seen this entry - // then any later examples must be masked. - continue - } - seen[target.Path] = struct{}{} - info := &TargetInfo{ - TargetFiles: target.TargetFiles, - Repo: repo, - } - if !yield(info) { - return nil + for _, target := range roleChain.Role.Signed.Targets { + if err := ctx.Err(); err != nil { + return err + } + if _, ok := seen[target.Path]; ok { + // First instance of a target wins: + // 1. IterTargetRoles iterates over the roles in the + // TUF lookup order (s5.6.7); and + // 2. IterRepos provides a consistent ordering, so the + // first entry we hit masks any later entries. + continue + } + if !roleChain.IsTargetPermitted(target.Path) { + // TODO(log): Add logging? + continue + } + seen[target.Path] = struct{}{} + + // TODO: We probably should check if this target file would + // actually be fetched by a tuf client by seeing how many + // previous delegation paths match against it -- if it is + // over the limit (maxDelegationDepth in IterTargetRoles) + // then we should mask it here too. + info := &TargetInfo{ + TargetFiles: target, + Repo: repo, + } + if !yield(info) { + return nil + } } } } diff --git a/internal/tufclient/client_delegations_test.go b/internal/tufclient/client_delegations_test.go index c4c9492..e25d2c6 100644 --- a/internal/tufclient/client_delegations_test.go +++ b/internal/tufclient/client_delegations_test.go @@ -9,6 +9,7 @@ import ( "crypto/ed25519" "crypto/rand" "io" + "io/fs" "net/http" "slices" "strings" @@ -250,3 +251,111 @@ func TestClient_DelegationDiamond_ReverifyFailureAborts(t *testing.T) { assert.Contains(t, walkErr.Error(), "shared", "the error must name the role that failed re-verification") assert.Equal(t, []string{"x/file"}, paths, "the a-path is walked first and is still trusted; y/file must never appear") } + +// Within one repository a path provided by several roles is listed from the +// first role a lookup would reach: the top-level role outranks delegations, +// and delegations rank in declared order (s5.6.7). go-tuf's real lookup must +// agree with the listing. Distinct content lengths identify each role's entry. +func TestClient_DelegationPriority_FirstRoleWins(t *testing.T) { + srv := testrepo.New(t, testrepo.WithMetaSubdir("meta")) + keyD1, keyD2 := newRoleKey(t), newRoleKey(t) + write := func(path, data string) *tufmetadata.TargetFiles { + return srv.WriteTarget(t, path, bytes.NewReader([]byte(data))) + } + topShared := write("x/shared", "T") + d1Shared, d1Both := write("x/shared", "D1"), write("x/both", "D1") + d2Shared, d2Both := write("x/shared", "D22"), write("x/both", "D22") + srv.Publish(t, + testrepo.AddTargetOp("x/shared", topShared), + delegationsOp(t, + []delegatedRole{ + {name: "d1", signers: []*roleKey{keyD1}, targets: map[string]*tufmetadata.TargetFiles{ + "x/shared": d1Shared, "x/both": d1Both, "x/only-d1": write("x/only-d1", "1"), + }}, + {name: "d2", signers: []*roleKey{keyD2}, targets: map[string]*tufmetadata.TargetFiles{ + "x/shared": d2Shared, "x/both": d2Both, "x/only-d2": write("x/only-d2", "2"), + }}, + }, + []delegation{ + {from: tufmetadata.TARGETS, to: "d1", paths: []string{"x/*"}, keys: []*roleKey{keyD1}}, + {from: tufmetadata.TARGETS, to: "d2", paths: []string{"x/*"}, keys: []*roleKey{keyD2}}, + }, + ), + ) + + client := newClient(t, testrepo.Config(t, srv.ConfigBlock("priority"))) + lengths := make(map[string]int64) + for info, err := range client.IterTargetFiles(t.Context()) { + require.NoError(t, err) + _, dup := lengths[info.Path] + require.False(t, dup, "path %s listed twice", info.Path) + lengths[info.Path] = info.Length + } + assert.Equal(t, map[string]int64{ + "x/shared": topShared.Length, // targets outranks both delegations + "x/both": d1Both.Length, // d1 is declared before d2 + "x/only-d1": 1, + "x/only-d2": 1, + }, lengths) + + for path, want := range map[string]int64{"x/shared": topShared.Length, "x/both": d1Both.Length} { + info, err := client.GetTargetInfo(t.Context(), path) + require.NoError(t, err) + assert.Equal(t, want, info.Length, "lookup of %s must agree with the listing", path) + } +} + +// A terminating delegation stops later roles from providing the paths it +// covers (s4.5), even when the terminating role itself lacks the target. The +// listing must exclude such targets, and go-tuf's real lookup must fail to +// find them, while paths outside the terminating patterns are unaffected. +func TestClient_TerminatingDelegation_BlocksLaterRole(t *testing.T) { + srv := testrepo.New(t, testrepo.WithMetaSubdir("meta")) + keyTerm, keyLater := newRoleKey(t), newRoleKey(t) + write := func(path string) *tufmetadata.TargetFiles { + return srv.WriteTarget(t, path, bytes.NewReader([]byte(path))) + } + srv.Publish(t, delegationsOp(t, + []delegatedRole{ + {name: "term", signers: []*roleKey{keyTerm}, targets: map[string]*tufmetadata.TargetFiles{ + "x/in-term": write("x/in-term"), + }}, + {name: "later", signers: []*roleKey{keyLater}, targets: map[string]*tufmetadata.TargetFiles{ + "x/from-later": write("x/from-later"), // covered by term's terminating x/* + "z/free": write("z/free"), // outside it + }}, + }, + []delegation{ + {from: tufmetadata.TARGETS, to: "term", paths: []string{"x/*"}, keys: []*roleKey{keyTerm}}, + {from: tufmetadata.TARGETS, to: "later", paths: []string{"x/*", "z/*"}, keys: []*roleKey{keyLater}}, + }, + )) + // delegationsOp does not expose the terminating flag; set it on the + // published metadata in a second transaction so the fixture stays small. + srv.Publish(t, tufrepo.NewTxnOp("test: mark term terminating", func(ctx context.Context, tx *tufrepo.Transaction) error { + top, err := tx.TargetsRoleData(ctx, tufmetadata.TARGETS) + if err != nil { + return err + } + for i := range top.Signed.Delegations.Roles { + if top.Signed.Delegations.Roles[i].Name == "term" { + top.Signed.Delegations.Roles[i].Terminating = true + } + } + return tx.UpdateRoleData(tufmetadata.TARGETS, top) + })) + + client := newClient(t, testrepo.Config(t, srv.ConfigBlock("terminating"))) + var paths []string + for info, err := range client.IterTargetFiles(t.Context()) { + require.NoError(t, err) + paths = append(paths, info.Path) + } + slices.Sort(paths) + assert.Equal(t, []string{"x/in-term", "z/free"}, paths) + + _, err := client.GetTargetInfo(t.Context(), "x/from-later") + require.ErrorIs(t, err, fs.ErrNotExist, "go-tuf's lookup must also stop at the terminating delegation") + _, err = client.GetTargetInfo(t.Context(), "z/free") + require.NoError(t, err) +} diff --git a/internal/tufclient/client_repos_test.go b/internal/tufclient/client_repos_test.go index 6aed330..3a4bce8 100644 --- a/internal/tufclient/client_repos_test.go +++ b/internal/tufclient/client_repos_test.go @@ -357,6 +357,34 @@ func TestIterRepos_EarlyBreak(t *testing.T) { // Updates published through a [testrepo.Server] transaction must be visible // to (fresh) clients of the repository. +// Breaking out of IterTargetFiles must stop the walk cleanly, and the returned +// sequence must be re-rangeable afterwards: each range starts over and sees +// every target. +func TestClient_IterTargetFiles_EarlyBreak(t *testing.T) { + srv := testrepo.New(t) + srv.Publish(t, + srv.AddTarget(t, "a.txt", bytes.NewReader([]byte("a"))), + srv.AddTarget(t, "b.txt", bytes.NewReader([]byte("b"))), + ) + client := newClient(t, testrepo.Config(t, srv.ConfigBlock("test-repo"))) + seq := client.IterTargetFiles(t.Context()) + + var yielded int + for _, err := range seq { + require.NoError(t, err) + yielded++ + break + } + assert.Equal(t, 1, yielded) + + var paths []string + for info, err := range seq { + require.NoError(t, err) + paths = append(paths, info.Path) + } + assert.ElementsMatch(t, []string{"a.txt", "b.txt"}, paths, "a fresh range must start over") +} + func TestClient_TargetUpdates(t *testing.T) { targetData := []byte("hello quarry") diff --git a/internal/tufext/targets_iter.go b/internal/tufext/iter_roles.go similarity index 71% rename from internal/tufext/targets_iter.go rename to internal/tufext/iter_roles.go index a4db5ff..c06e410 100644 --- a/internal/tufext/targets_iter.go +++ b/internal/tufext/iter_roles.go @@ -9,6 +9,7 @@ import ( "io/fs" "iter" "maps" + "slices" tufmetadata "github.com/theupdateframework/go-tuf/v2/metadata" @@ -16,16 +17,6 @@ import ( "go.amutable.dev/quarry/internal/third_party/assert" ) -// TargetFileData is a tuple of (name, *[tufmetadata.TargetFiles]), mainly used -// as an iterator value for [IterTargetFiles]. -type TargetFileData struct { - // Path is the logical pathname for this target file. - Path string - - // TargetFiles is the TUF target file metadata. - *tufmetadata.TargetFiles -} - // DelegationChain represents the chain of TUF delegations that were followed // to reach a given target role (or file). // @@ -105,15 +96,23 @@ func (chain DelegationChain) IsTargetPermitted(targetPath string) bool { return true } -// TargetMetadataFetchFunc is a helper function for [IterTargetFiles] that is -// called to get a particular target. For [tuftrustedmetadata.TrustedMetadata] -// backends it provides the necessary information to be able to automatically -// verify the corresponding target file. +// TargetMetadataFetchFunc is a helper function for [IterTargetRoles] (and thus +// [IterTargetFiles]) which is called to fetch a particular target role by +// name. +// +// For [tuftrustedmetadata.TrustedMetadata] backends it provides the necessary +// information to be able to automatically verify the corresponding target +// file, and such backends are expected to validate the data before returning +// it. +// +// TODO: Maybe we should make this return generic role data so that we can +// fetch "root" in [IterTargetRoles] and do the validation there regardless of +// the [TargetMetadataFetchFunc] implementation? type TargetMetadataFetchFunc = func(ctx context.Context, roleName, delegatorName string) (*SignedTargets, error) // TargetsMapFetcher returns a [TargetMetadataFetchFunc] backed by the provided -// map, for use with [IterTargetFiles] when all of the target files have -// already been pre-loaded by a user. +// map, for use with [IterTargetRoles] or [IterTargetFiles] when all of the +// target role metadata has already been pre-loaded by a user. func TargetsMapFetcher(targets map[string]*SignedTargets) TargetMetadataFetchFunc { return func(_ context.Context, roleName, _ string) (*SignedTargets, error) { role, ok := targets[roleName] @@ -124,27 +123,75 @@ func TargetsMapFetcher(targets map[string]*SignedTargets) TargetMetadataFetchFun } } -// IterTargetFiles iterates over all target files, matching the equivalent -// algorithm used by TUF to search for the correct target file. Note that the -// order of files yielded is not stable. +// RoleDelegationChain describes the [DelegationChain] that [IterTargetRoles] +// encountered when reaching a target role as well as the target role itself. +// +// Aside from the obvious bits of information (the target role name, data, and +// [DelegationChain] used to reach the role), this structure also provides +// information about the set of terminating delegations reached up to this +// point in [IterTargetRoles]. As this information is very security-critical, +// callers that plan to yield target files from this role *MUST* use +// [RoleDelegationChain.IsTargetPermitted] to filter out forbidden target files +// so that terminating delegations are respected correctly. +// +// Note that the [DelegationChain] used to reach a target role might not be +// unique, as roles can be delegated to via several distinct chains (though +// this usage is quite rare in practice). +type RoleDelegationChain struct { + // Name is the name of the delegated target role. "targets" is the + // top-level targets role. + Name string + // Role is the [SignedTargets] data for this role, as returned by the + // corresponding [TargetMetadataFetchFunc] call. This value must be treated + // as read-only. + Role *SignedTargets + // ThisChain is the [DelegationChain] used to reach this role. + ThisChain DelegationChain + // TerminatedChains is the set of [DelegationChain] to terminating roles up + // until now in the iteration of target roles -- target files that match + // these terminated chains *MUST NOT BE YIELDED*. This is used by + // [RoleDelegationChain.IsTargetPermitted]. + TerminatedChains []DelegationChain +} + +// IsTargetPermitted returns whether the [RoleDelegationChain] reached during +// an [IterTargetRoles] walk is authorised to provide a target with the given +// path. Users that yield target files when iterating over [IterTargetRoles] +// *MUST* make use of this method. +func (chain RoleDelegationChain) IsTargetPermitted(targetPath string) bool { + if !chain.ThisChain.IsTargetPermitted(targetPath) { + return false + } + for _, badChain := range chain.TerminatedChains { + // A permitted target path is *bad* for terminated chains! + if badChain.IsTargetPermitted(targetPath) { + return false + } + } + return true +} + +// IterTargetRoles iterates over all target roles from a single repository in a +// pre-order depth-first traversal order (as defined in s5.6.7 of the TUF +// specification). // // The provided [TargetMetadataFetchFunc] is called each time a target's role // metadata needs to be loaded. // -// TODO(links): This will need callbacks and a lot more infrastructure once we -// add cross-repository links. (Arguably even today it's a little less than -// ideal because we need to pre-fetch all of the targets which the default -// updater doesn't do.) -func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter.Seq2[TargetFileData, error] { +// Users that yield target files when iterating over [IterTargetRoles] *MUST* +// make use of [RoleDelegationChain.IsTargetPermitted] to ensure they do not +// violate the security properties of terminating delegations in TUF. Note that +// it is possible for the same role to be reached via different +// [DelegationChain]s, in which case [IterTargetRoles] will yield the same role +// multiple times. +func IterTargetRoles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter.Seq2[RoleDelegationChain, error] { // TODO: Pass these in as configuration so that this matches tufclient's // limits exactly and potentially works better with links? const ( maxDelegationDepth = 32 // go-tuf default limit maxDelegations = 4096 ) - return generics.ErrorIter(func(yield func(TargetFileData) bool) error { - seenTargets := make(map[string]struct{}, 256) - + return generics.ErrorIter(func(yield func(RoleDelegationChain) bool) error { // roleTodo indicates that we need to walk into the given role. type roleTodo struct { name, delegator string @@ -241,7 +288,7 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. } role, err := fetchFn(ctx, thisRole.name, thisRole.delegator) if err != nil { - return fmt.Errorf("target file walk aborted: failed to get role %s: %w", thisRole.name, err) + return fmt.Errorf("target role walk aborted: failed to get role %s: %w", thisRole.name, err) } // TODO: Validate the delegation signatures here as well (the real // fetchFn used by clients does so implicitly but we should do it @@ -250,40 +297,15 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // allow us to skip bad delegations...? seenDelegations++ - // First, yield all of the immediate target files. - targets: - for path, meta := range role.Signed.Targets { - // If we already saw this target path before, it was provided - // by a higher-priority (i.e., earlier in the chain) target - // file. - if _, ok := seenTargets[path]; ok { - // TODO(log): Add logging? - continue targets - } - // Make sure we don't yield entires that were already covered - // by an earlier terminating delegation. - for _, chain := range terminatedDelegationChains { - // A permitted target path is *bad* for terminated chains! - if chain.IsTargetPermitted(path) { - // TODO(log): Add logging. - continue targets - } - } - // If this is a delegated role, make sure that the target path - // matches one of the patterns specified by the delegator. - if !thisRole.chain.IsTargetPermitted(path) { - // TODO(log): Add logging. - continue targets - } - // TODO: We probably should check if this target file would - // actually be fetched by a tuf client by seeing how many - // previous delegation paths match against it -- if it is over - // the limit (maxDelegationDepth) then we should mask it here - // too. - if !yield(TargetFileData{Path: path, TargetFiles: meta}) { - return nil - } - seenTargets[path] = struct{}{} + val := RoleDelegationChain{ + Name: thisRole.name, + Role: role, + ThisChain: thisRole.chain, + // terminatedDelegationChains gets mutated later so use a copy. + TerminatedChains: slices.Clone(terminatedDelegationChains), + } + if !yield(val) { + return nil } // Now append the set of delegations to the todo queue. @@ -302,8 +324,7 @@ func IterTargetFiles(ctx context.Context, fetchFn TargetMetadataFetchFunc) iter. // DelegationChain.IsTargetPermitted uses go-tuf's matching // logic, but go-tuf upstream has a bug in how they compute // these hashes and so we are best to disallow them for - // now. - // + // now. if len(delegatedRole.PathHashPrefixes) > 0 { return fmt.Errorf("role %s uses path prefixes: unsupported feature", delegatedRole.Name) } diff --git a/internal/tufext/targets_iter_test.go b/internal/tufext/iter_roles_test.go similarity index 62% rename from internal/tufext/targets_iter_test.go rename to internal/tufext/iter_roles_test.go index d82cf27..07ec08f 100644 --- a/internal/tufext/targets_iter_test.go +++ b/internal/tufext/iter_roles_test.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "io/fs" - "iter" "slices" "strings" "testing" @@ -23,6 +22,25 @@ import ( "go.amutable.dev/quarry/internal/tufext" ) +// keysOf returns the sorted keys. The iterator's within-role order is +// unspecified, so most tests compare sorted slices or sets. +func keysOf[V any](m map[string]V) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + slices.Sort(out) + return out +} + +func asSet(s []string) map[string]struct{} { + out := make(map[string]struct{}, len(s)) + for _, v := range s { + out[v] = struct{}{} + } + return out +} + // tf returns a [tufmetadata.TargetFiles] with the given Length, used as a // distinctive identity to round-trip through the iterator. func tf(size int64) *tufmetadata.TargetFiles { @@ -30,7 +48,7 @@ func tf(size int64) *tufmetadata.TargetFiles { } // dr builds a [tufmetadata.DelegatedRole]. Keys/threshold are zero-valued -- -// [tufext.IterTargetFiles] does not consult them. +// [tufext.IterTargetRoles] does not consult them. func dr(name string, terminating bool, paths ...string) tufmetadata.DelegatedRole { return tufmetadata.DelegatedRole{ Name: name, @@ -59,131 +77,324 @@ func signedTargets(targets map[string]*tufmetadata.TargetFiles, delegatedRoles [ return st } -// pathsFrom collects iterator output as a path-to-metadata map, asserting no -// duplicates. -func pathsFrom(t *testing.T, targets map[string]*tufext.SignedTargets) map[string]*tufmetadata.TargetFiles { +// permittedTargets emulates a target listing over IterTargetRoles: every +// target of every yielded role that the role's chain permits, keyed by path. +// It does not deduplicate, so a path permitted via more than one role fails +// the test; no fixture here has one. Priority between roles (first wins) is +// the consumer's concern and is tested in tufclient. +func permittedTargets(t *testing.T, targets map[string]*tufext.SignedTargets) map[string]*tufmetadata.TargetFiles { t.Helper() out := make(map[string]*tufmetadata.TargetFiles) - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(targets))) - require.NoError(t, err) - for _, td := range got { - _, dup := out[td.Path] - require.False(t, dup, "iterator yielded duplicate path %q", td.Path) - out[td.Path] = td.TargetFiles + for _, role := range rolesFrom(t, targets) { + for path, meta := range role.Role.Signed.Targets { + if !role.IsTargetPermitted(path) { + continue + } + _, dup := out[path] + require.False(t, dup, "path %q permitted via more than one role", path) + out[path] = meta + } } return out } -// orderedPathsFrom returns the paths in yield order. Cross-role order is the -// pre-order DFS of the delegation tree; within-role order is unspecified. -func orderedPathsFrom(t *testing.T, targets map[string]*tufext.SignedTargets) []string { +// rolesFrom collects the roles yielded by IterTargetRoles over pre-loaded +// metadata, failing on any error. +func rolesFrom(t *testing.T, targets map[string]*tufext.SignedTargets) []tufext.RoleDelegationChain { t.Helper() - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(targets))) + got, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(targets))) require.NoError(t, err) - out := make([]string, 0, len(got)) - for _, td := range got { - out = append(out, td.Path) + return got +} + +// roleNames returns the role names in yield order. +func roleNames(roles []tufext.RoleDelegationChain) []string { + out := make([]string, 0, len(roles)) + for _, role := range roles { + out = append(out, role.Name) } return out } -func TestIterTargetFiles_NoTargetsRole(t *testing.T) { - // The algorithm starts at the "targets" role; missing it must error. - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(nil))) - require.Error(t, err) - assert.Contains(t, err.Error(), tufmetadata.TARGETS) +// roleByName returns the role with the given name, failing unless it was +// yielded exactly once. +func roleByName(t *testing.T, roles []tufext.RoleDelegationChain, name string) tufext.RoleDelegationChain { + t.Helper() + var found []tufext.RoleDelegationChain + for _, role := range roles { + if role.Name == name { + found = append(found, role) + } + } + require.Len(t, found, 1, "role %q must be yielded exactly once", name) + return found[0] } -func TestIterTargetFiles_OnlyTargets_Empty(t *testing.T) { - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, nil), - }) - assert.Empty(t, got) +func TestIterTargetRoles_OnlyTargets(t *testing.T) { + top := signedTargets(map[string]*tufmetadata.TargetFiles{"a": tf(1)}, nil) + roles := rolesFrom(t, map[string]*tufext.SignedTargets{tufmetadata.TARGETS: top}) + require.Len(t, roles, 1) + + got := roles[0] + assert.Equal(t, tufmetadata.TARGETS, got.Name) + assert.Same(t, top, got.Role, "Role must be the fetcher's pointer, not a copy") + assert.True(t, got.ThisChain.IsEmpty(), "the top-level role has no delegations above it") + assert.Empty(t, got.TerminatedChains) + assert.True(t, got.IsTargetPermitted("anything/at/all"), "an unrestricted chain authorises every path") } -func TestIterTargetFiles_OnlyTargets_Some(t *testing.T) { - a, b := tf(1), tf(2) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets( - map[string]*tufmetadata.TargetFiles{"a": a, "b": b}, - nil, - ), +func TestIterTargetRoles_PreOrderDepthFirst(t *testing.T) { + // Pre-order DFS in declared order (s5.6.7), observed at the role level + // where map-iteration randomness cannot interfere. + roles := rolesFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("a", false, "a/*"), + dr("b", false, "b/*"), + }), + "a": signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("a-1", false, "a/1"), + dr("a-2", false, "a/2"), + }), + "a-1": signedTargets(nil, nil), + "a-2": signedTargets(nil, nil), + "b": signedTargets(nil, nil), }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a": a, "b": b}, got) + assert.Equal(t, []string{tufmetadata.TARGETS, "a", "a-1", "a-2", "b"}, roleNames(roles)) } -func TestIterTargetFiles_SimpleDelegation(t *testing.T) { - a, b := tf(1), tf(2) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets( - map[string]*tufmetadata.TargetFiles{"top": a}, - []tufmetadata.DelegatedRole{dr("d1", false, "d1/*")}, - ), - "d1": signedTargets( - map[string]*tufmetadata.TargetFiles{"d1/file": b}, - nil, - ), +func TestIterTargetRoles_ChainReflectsPathTaken(t *testing.T) { + roles := rolesFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "a/*/*")}), + "d1": signedTargets(nil, []tufmetadata.DelegatedRole{dr("d2", false, "a/foo/*")}), + "d2": signedTargets(nil, nil), }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{ - "top": a, - "d1/file": b, - }, got) + require.Equal(t, []string{tufmetadata.TARGETS, "d1", "d2"}, roleNames(roles)) + + d1 := roleByName(t, roles, "d1") + assert.Equal(t, 1, d1.ThisChain.Length()) + assert.True(t, d1.ThisChain.Contains(tufmetadata.TARGETS)) + assert.False(t, d1.ThisChain.Contains("d1"), "a chain records delegators, not the role itself") + + d2 := roleByName(t, roles, "d2") + assert.Equal(t, 2, d2.ThisChain.Length()) + assert.True(t, d2.ThisChain.Contains("d1")) + assert.True(t, d2.IsTargetPermitted("a/foo/x")) + assert.False(t, d2.IsTargetPermitted("a/bar/x"), "every link must authorise the path") +} + +func TestIterTargetRoles_YieldsUnreachableRolesButMatchRejects(t *testing.T) { + // The role walk does not know which paths a consumer will ask about, so + // it yields every role it can reach, including ones whose patterns do + // not fit inside their parent's. Match is what enforces authority, which + // is why consumers are required to call it. + roles := rolesFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "b/*")}), + "d1": signedTargets(nil, []tufmetadata.DelegatedRole{dr("e1", false, "a/x")}), + "e1": signedTargets(nil, nil), + }) + e1 := roleByName(t, roles, "e1") + assert.False(t, e1.IsTargetPermitted("a/x"), "e1's own pattern matches but d1's does not") + assert.False(t, e1.IsTargetPermitted("b/x"), "d1's pattern matches but e1's does not") } -func TestIterTargetFiles_PathOutsideOwnRoleSkipped(t *testing.T) { - // A target whose path doesn't match its own role's paths is unreachable - // via TUF lookup and must be skipped. - got := pathsFrom(t, map[string]*tufext.SignedTargets{ +func TestIterTargetRoles_TerminatedChainsApplyToLaterRolesOnly(t *testing.T) { + // Roles yielded before a terminating delegation takes effect -- the + // terminating role itself and its descendants (s4.5) -- see no terminated + // chains. Roles yielded afterwards see it, and Match rejects the paths it + // covers even where their own chain would allow them. + roles := rolesFrom(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "d1/*"), + dr("term", true, "a/*"), + dr("later", false, "a/*", "b/*"), }), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ - "d1/in": tf(1), - "elsewhere/file": tf(2), - }, nil), + "term": signedTargets(nil, []tufmetadata.DelegatedRole{dr("child", false, "a/*")}), + "child": signedTargets(nil, nil), + "later": signedTargets(nil, nil), }) - assert.Equal(t, []string{"d1/in"}, keysOf(got)) -} + require.Equal(t, []string{tufmetadata.TARGETS, "term", "child", "later"}, roleNames(roles)) -func TestIterTargetFiles_PathMatchesAncestorButNotOwnRole(t *testing.T) { - // Per-layer AND matching: a target that matches an ancestor's pattern but - // not its own role's must be skipped. A flat-OR check across the whole - // chain would wrongly accept "a/bar/y" because it matches d1's "a/*/*". - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + for _, name := range []string{tufmetadata.TARGETS, "term", "child"} { + assert.Empty(t, roleByName(t, roles, name).TerminatedChains, "%s is yielded before the termination applies", name) + } + child := roleByName(t, roles, "child") + assert.True(t, child.IsTargetPermitted("a/x"), "descendants of a terminating role may still match") + + later := roleByName(t, roles, "later") + require.Len(t, later.TerminatedChains, 1) + assert.True(t, later.ThisChain.IsTargetPermitted("a/x"), "later's own chain allows a/*") + assert.False(t, later.IsTargetPermitted("a/x"), "the terminated chain must override later's own chain") + assert.True(t, later.IsTargetPermitted("b/x"), "paths outside the terminated chain are unaffected") +} + +func TestIterTargetRoles_TerminatedChainsAreIndependentCopies(t *testing.T) { + // The iterator keeps extending its own list of terminated chains, so each + // yielded TerminatedChains slice must be a copy: a consumer that + // overwrites a slot in the slice it was handed must not affect later + // roles. (The chains themselves are immutable, so replacing a slot is the + // only mutation a consumer can perform.) + targets := map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "a/*/*"), - }), - "d1": signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d2", false, "a/foo/*"), + dr("term", true, "a/*"), + dr("first", false, "a/*"), + dr("second", false, "a/*", "b/*"), }), - "d2": signedTargets(map[string]*tufmetadata.TargetFiles{ - "a/bar/y": tf(1), - "a/foo/y": tf(2), - }, nil), + "term": signedTargets(nil, nil), + "first": signedTargets(nil, nil), + "second": signedTargets(nil, nil), + } + var second tufext.RoleDelegationChain + for role, err := range tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(targets)) { + require.NoError(t, err) + switch role.Name { + case "first": + require.Len(t, role.TerminatedChains, 1) + // Overwrite the terminated chain with an empty, match-all one. + role.TerminatedChains[0] = tufext.DelegationChain{} + case "second": + second = role + } + } + require.Len(t, second.TerminatedChains, 1) + assert.False(t, second.TerminatedChains[0].IsEmpty(), "first's scribbling leaked into second") + assert.True(t, second.IsTargetPermitted("b/x"), "a match-all terminated chain would have rejected this") + assert.False(t, second.IsTargetPermitted("a/x")) +} + +func TestIterTargetRoles_CycleSkipped(t *testing.T) { + roles := rolesFrom(t, map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}), + "d1": signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}), }) - assert.Equal(t, []string{"a/foo/y"}, keysOf(got)) + assert.Equal(t, []string{tufmetadata.TARGETS, "d1"}, roleNames(roles)) } -func TestIterTargetFiles_PathHitsAllAncestorPatterns(t *testing.T) { - // Same chain but the target matches every layer's pattern. - yielded := tf(42) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ +func TestIterTargetRoles_DiamondWalkedOncePerPath(t *testing.T) { + // "shared" is reachable via "a" and via "b": it is yielded once per path, + // each time with that path's chain, and the fetcher is told the delegator + // each time. + type call struct{ role, delegator string } + var calls []call + fetch := func(_ context.Context, roleName, delegatorName string) (*tufext.SignedTargets, error) { + calls = append(calls, call{roleName, delegatorName}) + switch roleName { + case tufmetadata.TARGETS: + return signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("a", false, "x/*"), + dr("b", false, "y/*"), + }), nil + case "a", "b": + return signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*", "y/*")}), nil + case "shared": + return signedTargets(nil, nil), nil + } + return nil, fmt.Errorf("unexpected fetch %q", roleName) + } + roles, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), fetch)) + require.NoError(t, err) + require.Equal(t, []string{tufmetadata.TARGETS, "a", "shared", "b", "shared"}, roleNames(roles)) + assert.Equal(t, []call{ + {tufmetadata.TARGETS, tufmetadata.ROOT}, + {"a", tufmetadata.TARGETS}, + {"shared", "a"}, + {"b", tufmetadata.TARGETS}, + {"shared", "b"}, + }, calls) + + viaA, viaB := roles[2], roles[4] + assert.True(t, viaA.ThisChain.Contains("a")) + assert.False(t, viaA.ThisChain.Contains("b")) + assert.True(t, viaA.IsTargetPermitted("x/f")) + assert.False(t, viaA.IsTargetPermitted("y/f"), "a only delegated x/*") + assert.True(t, viaB.ThisChain.Contains("b")) + assert.False(t, viaB.ThisChain.Contains("a")) + assert.True(t, viaB.IsTargetPermitted("y/f")) + assert.False(t, viaB.IsTargetPermitted("x/f"), "b only delegated y/*") +} + +func TestIterTargetRoles_EarlyBreakStopsWalk(t *testing.T) { + var fetches int + fetch := func(_ context.Context, roleName, _ string) (*tufext.SignedTargets, error) { + fetches++ + if roleName == tufmetadata.TARGETS { + return signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("d1", false, "x/*"), + dr("d2", false, "y/*"), + }), nil + } + return signedTargets(nil, nil), nil + } + for role, err := range tufext.IterTargetRoles(t.Context(), fetch) { + require.NoError(t, err) + assert.Equal(t, tufmetadata.TARGETS, role.Name) + break + } + assert.Equal(t, 1, fetches, "nothing may be fetched after the consumer breaks") +} + +func TestIterTargetRoles_FetchErrorAfterPartialYield(t *testing.T) { + // Roles walked before the failure are delivered; the error then names + // the role that could not be fetched and wraps the fetcher's error. + roles, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "a/*/*"), - }), - "d1": signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d2", false, "a/foo/*"), + dr("d1", false, "x/*"), + dr("missing", false, "y/*"), }), - "d2": signedTargets(map[string]*tufmetadata.TargetFiles{ - "a/foo/file": yielded, - }, nil), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a/foo/file": yielded}, got) + "d1": signedTargets(nil, nil), + }))) + require.Error(t, err) + assert.ErrorIs(t, err, fs.ErrNotExist) //nolint:testifylint // assert is fine for error path checks + assert.Contains(t, err.Error(), "missing") + assert.Equal(t, []string{tufmetadata.TARGETS, "d1"}, roleNames(roles)) +} + +func TestIterTargetRoles_RoleYieldedBeforeUnsupportedDelegationError(t *testing.T) { + // A role whose own delegations use an unsupported feature is still + // yielded (its targets are fine); the error follows when the walk tries + // to descend. A consumer listing targets therefore sees that role's own + // targets before failing. + bin := dr("bin", false /* no paths */) + bin.PathHashPrefixes = []string{hashPrefix("x/y")} + top := signedTargets(nil, []tufmetadata.DelegatedRole{bin}) + roles, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: top, + "bin": signedTargets(nil, nil), + }))) + require.Error(t, err) + assert.Contains(t, err.Error(), "uses path prefixes") + require.Len(t, roles, 1) + assert.Same(t, top, roles[0].Role) +} + +func TestIterTargetRoles_DepthCapMatchesGoTUF(t *testing.T) { + // go-tuf's default MaxDelegations of 32 lets it visit 33 roles down a + // pure chain ("targets" plus r0..r31); the depth cap must yield exactly + // those and no more. + const linearLen = 40 + all := map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("r0", false, "x/*")}), + } + for i := 0; i < linearLen-1; i++ { + all[fmt.Sprintf("r%d", i)] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr(fmt.Sprintf("r%d", i+1), false, "x/*"), + }) + } + all[fmt.Sprintf("r%d", linearLen-1)] = signedTargets(nil, nil) + + names := roleNames(rolesFrom(t, all)) + require.Len(t, names, 33) + assert.Equal(t, tufmetadata.TARGETS, names[0]) + assert.Equal(t, "r0", names[1]) + assert.Equal(t, "r31", names[32]) } -func TestIterTargetFiles_GlobPatterns(t *testing.T) { +// The tests below observe the walk through an emulated target listing (see +// permittedTargets): which paths the yielded roles are permitted to provide. +// That is the property IterTargetRoles exists to establish, and it is the +// most direct way to express terminating-delegation and path semantics. + +func TestIterTargetRoles_GlobPatterns(t *testing.T) { // Spec s4.5: '*' and '?' are wildcards, but neither matches '/'. - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d_star", false, "star/*.tgz"), dr("d_qmark", false, "qmark/foo-?.bin"), @@ -209,8 +420,8 @@ func TestIterTargetFiles_GlobPatterns(t *testing.T) { }, asSet(keysOf(got))) } -func TestIterTargetFiles_LiteralPath(t *testing.T) { - got := pathsFrom(t, map[string]*tufext.SignedTargets{ +func TestIterTargetRoles_LiteralPath(t *testing.T) { + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", false, "exact/file.bin"), }), @@ -223,94 +434,12 @@ func TestIterTargetFiles_LiteralPath(t *testing.T) { assert.Equal(t, []string{"exact/file.bin"}, keysOf(got)) } -func TestIterTargetFiles_MalformedPatternNeverMatches(t *testing.T) { - // See TestDelegationChain_MalformedPatternNeverMatches. A target whose - // delegation pattern is a malformed glob must not be yielded, even when - // the path is byte-for-byte equal to the pattern. "control" guards - // against a drop-everything regression. - control := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets( - map[string]*tufmetadata.TargetFiles{"control": control}, - []tufmetadata.DelegatedRole{dr("d1", false, "a/[b")}, - ), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ - "a/[b": tf(1), - "a/b": tf(2), - }, nil), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) -} - -func TestIterTargetFiles_MultiplePathPatterns(t *testing.T) { - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "a/*", "b/*"), - }), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ - "a/foo": tf(1), - "b/bar": tf(2), - "c/baz": tf(3), // not in either pattern - }, nil), - }) - assert.Equal(t, map[string]struct{}{ - "a/foo": {}, - "b/bar": {}, - }, asSet(keysOf(got))) -} - -func TestIterTargetFiles_EmptyDelegationPaths(t *testing.T) { - // A delegation with no paths is unreachable in TUF lookup; its targets - // must not be yielded. - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false /* no paths */), - }), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ - "anything": tf(1), - }, nil), - }) - assert.Empty(t, got) -} - -func TestIterTargetFiles_TargetsRoleShadowsDelegated(t *testing.T) { - // "targets" outranks any delegation: same path in both, top-level wins. - topMeta := tf(1) - delegMeta := tf(2) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets( - map[string]*tufmetadata.TargetFiles{"shared": topMeta}, - []tufmetadata.DelegatedRole{dr("d1", false, "shared")}, - ), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{ - "shared": delegMeta, - }, nil), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"shared": topMeta}, got) -} - -func TestIterTargetFiles_EarlierDelegationShadowsLater(t *testing.T) { - // Spec s5.6.7: delegations are processed in declared order, "which - // implicitly orders trustworthiness". - d1Meta := tf(1) - d2Meta := tf(2) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "shared"), - dr("d2", false, "shared"), - }), - "d1": signedTargets(map[string]*tufmetadata.TargetFiles{"shared": d1Meta}, nil), - "d2": signedTargets(map[string]*tufmetadata.TargetFiles{"shared": d2Meta}, nil), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"shared": d1Meta}, got) -} - -func TestIterTargetFiles_TerminatingBlocksLaterSiblings(t *testing.T) { +func TestIterTargetRoles_TerminatingBlocksLaterSiblings(t *testing.T) { // After a terminating delegation's subtree, no later role may claim a // target matching its paths. "control" is a positive control: it must // still appear, so a drop-everything regression can't pass this test. control := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( map[string]*tufmetadata.TargetFiles{"control": control}, []tufmetadata.DelegatedRole{ @@ -326,9 +455,9 @@ func TestIterTargetFiles_TerminatingBlocksLaterSiblings(t *testing.T) { assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } -func TestIterTargetFiles_TerminatingDoesNotBlockUnrelatedPaths(t *testing.T) { +func TestIterTargetRoles_TerminatingDoesNotBlockUnrelatedPaths(t *testing.T) { bx := tf(7) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", true, "a/*"), dr("d2", false, "b/*"), @@ -339,11 +468,11 @@ func TestIterTargetFiles_TerminatingDoesNotBlockUnrelatedPaths(t *testing.T) { assert.Equal(t, map[string]*tufmetadata.TargetFiles{"b/x": bx}, got) } -func TestIterTargetFiles_TerminatingAllowsDescendants(t *testing.T) { +func TestIterTargetRoles_TerminatingAllowsDescendants(t *testing.T) { // Spec s4.5: a terminating role's own descendants are still processed and // may match paths the terminating role claimed. leaf := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", true, "a/*/*"), }), @@ -355,12 +484,12 @@ func TestIterTargetFiles_TerminatingAllowsDescendants(t *testing.T) { assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a/foo/leaf": leaf}, got) } -func TestIterTargetFiles_TerminatingPropagatesAcrossSubtree(t *testing.T) { +func TestIterTargetRoles_TerminatingPropagatesAcrossSubtree(t *testing.T) { // A terminating role nested inside a non-terminating parent still blocks // later roles outside its parent's subtree. "control" is the positive // control to detect a drop-everything regression. control := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( map[string]*tufmetadata.TargetFiles{"control": control}, []tufmetadata.DelegatedRole{ @@ -379,13 +508,13 @@ func TestIterTargetFiles_TerminatingPropagatesAcrossSubtree(t *testing.T) { assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } -func TestIterTargetFiles_TerminatingDoesNotApplyWhenChainMismatchedAtRoot(t *testing.T) { +func TestIterTargetRoles_TerminatingDoesNotApplyWhenChainMismatchedAtRoot(t *testing.T) { // e1's paths don't subset its parent d1's, so a TUF lookup for "a/x" // would never reach e1. The terminating effect must be scoped by the // full ancestor chain, not just e1's own Paths -- otherwise d2's "a/x" // would be wrongly suppressed. yielded := tf(42) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", false, "b/*"), dr("d2", false, "a/*"), @@ -399,12 +528,12 @@ func TestIterTargetFiles_TerminatingDoesNotApplyWhenChainMismatchedAtRoot(t *tes assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a/x": yielded}, got) } -func TestIterTargetFiles_TerminatingDoesNotApplyWhenChainMismatchedMidway(t *testing.T) { +func TestIterTargetRoles_TerminatingDoesNotApplyWhenChainMismatchedMidway(t *testing.T) { // Like the at-root variant, but the chain breaks at an intermediate // ancestor (d2's "b/*"), proving the chain check spans every layer, not // only the root. yielded := tf(42) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", false, "a/*/*"), dr("d3", false, "a/*/*"), @@ -421,12 +550,12 @@ func TestIterTargetFiles_TerminatingDoesNotApplyWhenChainMismatchedMidway(t *tes assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a/foo/x": yielded}, got) } -func TestIterTargetFiles_TerminatingChainBlocksWhenFullyMatched(t *testing.T) { +func TestIterTargetRoles_TerminatingChainBlocksWhenFullyMatched(t *testing.T) { // Companion to the chain-mismatch tests: in a well-formed chain, the // terminating effect still fires. "control" guards against a regression // that drops everything. control := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( map[string]*tufmetadata.TargetFiles{"control": control}, []tufmetadata.DelegatedRole{ @@ -445,11 +574,11 @@ func TestIterTargetFiles_TerminatingChainBlocksWhenFullyMatched(t *testing.T) { assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } -func TestIterTargetFiles_MultipleTerminatingChainsTrackedIndependently(t *testing.T) { +func TestIterTargetRoles_MultipleTerminatingChainsTrackedIndependently(t *testing.T) { // Two terminating delegations in disjoint subtrees: each suppresses only // targets matching its own chain. yielded := tf(7) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ // First subtree: terminating on "a/*". dr("ta", true, "a/*"), @@ -474,7 +603,7 @@ func TestIterTargetFiles_MultipleTerminatingChainsTrackedIndependently(t *testin assert.Equal(t, map[string]*tufmetadata.TargetFiles{"c/yielded": yielded}, got) } -func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T) { +func TestIterTargetRoles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T) { // go-tuf stops considering later roles the moment it *encounters* a // matching terminating delegation in a parent's list (s5.6.7.2.1), before // visiting the role. So the marker must be pushed when the delegation is @@ -482,7 +611,7 @@ func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T // to itself: the second visit is skipped as a cycle, and "b" must still // be blocked. "control" and "x/in-a" are positive controls. control, inA := tf(99), tf(1) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( map[string]*tufmetadata.TargetFiles{"control": control}, []tufmetadata.DelegatedRole{ @@ -499,7 +628,7 @@ func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByCycle(t *testing.T assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control, "x/in-a": inA}, got) } -func TestIterTargetFiles_TerminatingOnSecondPathBlocksLaterSiblings(t *testing.T) { +func TestIterTargetRoles_TerminatingOnSecondPathBlocksLaterSiblings(t *testing.T) { // Diamond variant of the above: "shared" is reached via "a" // (non-terminating) and again via "b" (terminating); cycle detection is // per path, so the second visit is walked rather than skipped. A go-tuf @@ -507,7 +636,7 @@ func TestIterTargetFiles_TerminatingOnSecondPathBlocksLaterSiblings(t *testing.T // delegation, so "c" is never consulted, and the marker must apply after // the second visit's subtree however that visit is handled. control := tf(99) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets( map[string]*tufmetadata.TargetFiles{"control": control}, []tufmetadata.DelegatedRole{ @@ -524,7 +653,7 @@ func TestIterTargetFiles_TerminatingOnSecondPathBlocksLaterSiblings(t *testing.T assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } -func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByDepthCap(t *testing.T) { +func TestIterTargetRoles_TerminatingAppliesEvenIfRoleSkippedByDepthCap(t *testing.T) { // Companion to TerminatingAppliesEvenIfRoleSkippedByCycle for the other // reason a role can be skipped: r31 delegates terminatingly to r32, which // sits past the depth cap and is never walked. go-tuf clears its stack on @@ -550,27 +679,12 @@ func TestIterTargetFiles_TerminatingAppliesEvenIfRoleSkippedByDepthCap(t *testin } all[fmt.Sprintf("r%d", depth-1)] = signedTargets(map[string]*tufmetadata.TargetFiles{"x/deep": tf(3)}, nil) - got := pathsFrom(t, all) + got := permittedTargets(t, all) assert.Equal(t, map[string]*tufmetadata.TargetFiles{"control": control}, got) } -func TestIterTargetFiles_CycleSelfReference(t *testing.T) { - // d1 delegates to itself; the per-path cycle check must prevent re-entry. - d1Meta := tf(1) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("d1", false, "x/*"), - }), - "d1": signedTargets( - map[string]*tufmetadata.TargetFiles{"x/file": d1Meta}, - []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}, - ), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"x/file": d1Meta}, got) -} - -func TestIterTargetFiles_CycleTwoRoles(t *testing.T) { - got := pathsFrom(t, map[string]*tufext.SignedTargets{ +func TestIterTargetRoles_CycleTwoRoles(t *testing.T) { + got := permittedTargets(t, map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ dr("d1", false, "x/*"), }), @@ -587,94 +701,53 @@ func TestIterTargetFiles_CycleTwoRoles(t *testing.T) { }, asSet(keysOf(got))) } -func TestIterTargetFiles_DiamondGraph(t *testing.T) { - // Two parents delegate to "shared". It is walked once per delegation - // path, but the target is yielded once: the first path to reach it wins. - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("a", false, "x/*"), - dr("b", false, "x/*"), - }), - "a": signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("shared", false, "x/*"), - }), - "b": signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("shared", false, "x/*"), - }), - "shared": signedTargets(map[string]*tufmetadata.TargetFiles{ - "x/file": tf(1), - }, nil), - }) - assert.Equal(t, []string{"x/file"}, keysOf(got)) -} +func TestIterTargetRoles_DeepDelegationsWithSiblings(t *testing.T) { + // Regression for a chain aliasing bug: siblings derived from the same + // parent shared the parent's backing storage (a slice with cap > len at + // the time), so the second sibling clobbered the first. The chain is now + // map-backed, but the depth sweep is cheap and guards the property + // regardless of representation. + for _, depth := range []int{2, 3, 4, 5, 6, 7, 8, 16} { + t.Run(fmt.Sprintf("depth=%d", depth), func(t *testing.T) { + leafA := tf(int64(depth)*10 + 1) + leafB := tf(int64(depth)*10 + 2) + all := map[string]*tufext.SignedTargets{} -func TestIterTargetFiles_DiamondDifferentPatterns(t *testing.T) { - // "shared" is reachable via "a" (x/*) and via "b" (y/*). A real lookup - // for "y/file" never descends into "a", so it reaches "shared" through - // "b" and succeeds. A global visited set would have walked "shared" via - // "a" only, rejected "y/file" against a's chain and never come back; - // per-path cycle detection walks it again via "b". - xf, yf := tf(1), tf(2) - got := pathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("a", false, "x/*"), - dr("b", false, "y/*"), - }), - "a": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*", "y/*")}), - "b": signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*", "y/*")}), - "shared": signedTargets(map[string]*tufmetadata.TargetFiles{ - "x/file": xf, - "y/file": yf, - }, nil), - }) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"x/file": xf, "y/file": yf}, got) -} + // targets -> r0 -> r1 -> ... -> r(depth-1) -> {leafA, leafB}. + all[tufmetadata.TARGETS] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("r0", false, "shared/*"), + }) + for i := 0; i < depth-1; i++ { + all[fmt.Sprintf("r%d", i)] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr(fmt.Sprintf("r%d", i+1), false, "shared/*"), + }) + } + all[fmt.Sprintf("r%d", depth-1)] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("leafA", false, "shared/A"), + dr("leafB", false, "shared/B"), + }) + all["leafA"] = signedTargets(map[string]*tufmetadata.TargetFiles{"shared/A": leafA}, nil) + all["leafB"] = signedTargets(map[string]*tufmetadata.TargetFiles{"shared/B": leafB}, nil) -func TestIterTargetFiles_PreOrderDepthFirst(t *testing.T) { - // Spec s5.6.7: pre-order DFS in declared order. One target per role - // keeps within-role map-iteration randomness from scrambling the order. - got := orderedPathsFrom(t, map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets( - map[string]*tufmetadata.TargetFiles{"top": tf(0)}, - []tufmetadata.DelegatedRole{ - dr("a", false, "a/*"), - dr("b", false, "b/*"), - }, - ), - "a": signedTargets( - map[string]*tufmetadata.TargetFiles{"a/self": tf(0)}, - []tufmetadata.DelegatedRole{ - dr("a-1", false, "a/1"), - dr("a-2", false, "a/2"), - }, - ), - "a-1": signedTargets(map[string]*tufmetadata.TargetFiles{"a/1": tf(0)}, nil), - "a-2": signedTargets(map[string]*tufmetadata.TargetFiles{"a/2": tf(0)}, nil), - "b": signedTargets(map[string]*tufmetadata.TargetFiles{"b/self": tf(0)}, nil), - }) - assert.Equal(t, []string{ - "top", - "a/self", - "a/1", - "a/2", - "b/self", - }, got) + got := permittedTargets(t, all) + assert.Equal(t, map[string]*tufmetadata.TargetFiles{ + "shared/A": leafA, + "shared/B": leafB, + }, got) + }) + } } -func TestIterTargetFiles_MissingDelegatedRole(t *testing.T) { - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("missing", false, "x/*"), - }), - }))) +func TestIterTargetRoles_NoTargetsRole(t *testing.T) { + // The algorithm starts at the "targets" role; missing it must error. + _, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(nil))) require.Error(t, err) - assert.ErrorIs(t, err, fs.ErrNotExist) //nolint:testifylint // assert is fine for error path checks - assert.Contains(t, err.Error(), "missing") + assert.Contains(t, err.Error(), tufmetadata.TARGETS) } -func TestIterTargetFiles_PathHashPrefixes_Rejected(t *testing.T) { +func TestIterTargetRoles_PathHashPrefixes_Rejected(t *testing.T) { // DelegationChain.IsTargetPermitted can evaluate hash-bin delegations (see - // TestDelegationChain_PathHashPrefixes_Smoke), but IterTargetFiles + // TestDelegationChain_PathHashPrefixes_Smoke), but IterTargetRoles // refuses to walk them while go-tuf's digest encoding is non-conformant // (see the NOTE on hashPrefix): a conformant repository would otherwise // be mis-binned identically by us and by the go-tuf client. The error @@ -703,7 +776,7 @@ func TestIterTargetFiles_PathHashPrefixes_Rejected(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(tc.targets))) + _, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(tc.targets))) require.Error(t, err) assert.Contains(t, err.Error(), "uses path prefixes") assert.Contains(t, err.Error(), "bin", "error must name the offending role") @@ -711,70 +784,77 @@ func TestIterTargetFiles_PathHashPrefixes_Rejected(t *testing.T) { } } -func TestIterTargetFiles_SuccinctRoles(t *testing.T) { +func TestIterTargetRoles_SuccinctRoles(t *testing.T) { st := signedTargets(nil, nil) st.Signed.Delegations = &tufmetadata.Delegations{ SuccinctRoles: &tufmetadata.SuccinctRoles{BitLength: 4, NamePrefix: "bin"}, } - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ + _, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ tufmetadata.TARGETS: st, }))) require.Error(t, err) assert.Contains(t, err.Error(), "succinct roles") } -func TestIterTargetFiles_MaxDelegationDepth(t *testing.T) { - // A chain deeper than go-tuf's default MaxDelegations (32) can never be - // reached by a real lookup, so roles past that depth are skipped without - // an error. go-tuf's loop runs while visited <= 32, so it walks 33 roles - // down a pure chain: "targets" plus r0..r31. r_i has a chain of length - // i+1 and is skipped once that exceeds 32, so r31 is the last role walked - // and r32 onwards are dropped. - // - // The depth cap only prunes the over-deep branch: a sibling declared - // after it must still be walked, unlike the total cap (see - // TestIterTargetFiles_MaxDelegationsTotal). +func TestIterTargetRoles_FetcherErrorSurfaces(t *testing.T) { + // A fetcher error must be returned to the caller, wrapped with the role + // name for context. + sentinel := errors.New("fetcher boom") + fetch := func(_ context.Context, roleName, _ string) (*tufext.SignedTargets, error) { + if roleName == tufmetadata.TARGETS { + return signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}), nil + } + return nil, sentinel + } + _, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), fetch)) + require.Error(t, err) + assert.ErrorIs(t, err, sentinel) //nolint:testifylint // assert is fine for error path checks + assert.Contains(t, err.Error(), "d1", "wrapped error must name the failing role") +} + +func TestIterTargetRoles_FetcherSeesContext(t *testing.T) { + // The context passed to IterTargetRoles is plumbed through to the + // fetcher. Cancellation surfaced by the fetcher propagates as the + // iterator's error. + ctx, cancel := context.WithCancel(t.Context()) + cancel() + fetch := func(ctx context.Context, _, _ string) (*tufext.SignedTargets, error) { + return nil, ctx.Err() + } + _, err := generics.CollectErrorSeq(tufext.IterTargetRoles(ctx, fetch)) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) +} + +func TestIterTargetRoles_DepthCapOnlyPrunesDeepBranch(t *testing.T) { + // Companion to DepthCapMatchesGoTUF: the depth cap skips only the + // over-deep role and its subtree. A sibling declared after the deep + // branch is still walked, unlike the total cap (see MaxDelegationsTotal), + // which ends the whole walk. const linearLen = 40 all := make(map[string]*tufext.SignedTargets, linearLen+2) - all[tufmetadata.TARGETS] = signedTargets( - map[string]*tufmetadata.TargetFiles{"x/start": tf(1)}, - []tufmetadata.DelegatedRole{ - dr("r0", false, "x/*"), - dr("shallow", false, "y/*"), - }, - ) + all[tufmetadata.TARGETS] = signedTargets(nil, []tufmetadata.DelegatedRole{ + dr("r0", false, "x/*"), + dr("shallow", false, "y/*"), + }) for i := 0; i < linearLen-1; i++ { - var targets map[string]*tufmetadata.TargetFiles - switch i { - case 31: - targets = map[string]*tufmetadata.TargetFiles{"x/at-cap": tf(2)} - case 32: - targets = map[string]*tufmetadata.TargetFiles{"x/over-cap": tf(3)} - } - all[fmt.Sprintf("r%d", i)] = signedTargets(targets, []tufmetadata.DelegatedRole{ + all[fmt.Sprintf("r%d", i)] = signedTargets(nil, []tufmetadata.DelegatedRole{ dr(fmt.Sprintf("r%d", i+1), false, "x/*"), }) } all[fmt.Sprintf("r%d", linearLen-1)] = signedTargets(nil, nil) - all["shallow"] = signedTargets(map[string]*tufmetadata.TargetFiles{"y/shallow": tf(4)}, nil) - - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(all))) - require.NoError(t, err, "exceeding the depth cap must not produce an error") + all["shallow"] = signedTargets(nil, nil) - paths := make(map[string]struct{}, len(got)) - for _, td := range got { - paths[td.Path] = struct{}{} - } - assert.Contains(t, paths, "x/start") - assert.Contains(t, paths, "x/at-cap") - assert.NotContains(t, paths, "x/over-cap") - assert.Contains(t, paths, "y/shallow", "the depth cap must only prune the deep branch") + names := roleNames(rolesFrom(t, all)) + assert.Contains(t, names, "r31") + assert.NotContains(t, names, "r32") + assert.Equal(t, "shallow", names[len(names)-1], "the sibling after the deep branch must still be walked, last") } -func TestIterTargetFiles_MaxDelegationsTotal(t *testing.T) { - // Roles are no longer deduplicated globally, so a repository could make - // the walk revisit a modest tree an enormous number of times. A hard cap - // on fetched roles stops the walk, without an error, once reached. Unlike +func TestIterTargetRoles_MaxDelegationsTotal(t *testing.T) { + // Roles are not deduplicated globally, so a repository could make the + // walk revisit a modest tree an enormous number of times. A hard cap on + // fetched roles stops the walk, without an error, once reached. Unlike // the depth cap this ends the whole walk: nothing declared after the // cut-off is visited. The fetcher synthesises roles on demand so the // fixture stays small. @@ -795,269 +875,80 @@ func TestIterTargetFiles_MaxDelegationsTotal(t *testing.T) { if !strings.HasPrefix(roleName, "c") { return nil, fmt.Errorf("unexpected fetch %q", roleName) } - return signedTargets(map[string]*tufmetadata.TargetFiles{"c/" + roleName: tf(1)}, nil), nil + return signedTargets(nil, nil), nil } - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) + got, err := generics.CollectErrorSeq(tufext.IterTargetRoles(t.Context(), fetch)) require.NoError(t, err, "hitting the total cap must not produce an error") // "targets" is the first fetch, so totalCap-1 children are walked, in // declared order, before the cap trips. assert.Equal(t, totalCap, fetches) - assert.Len(t, got, totalCap-1) - paths := make(map[string]struct{}, len(got)) - for _, td := range got { - paths[td.Path] = struct{}{} - } - assert.Contains(t, paths, "c/c0") - assert.Contains(t, paths, fmt.Sprintf("c/c%d", totalCap-2)) - assert.NotContains(t, paths, fmt.Sprintf("c/c%d", totalCap-1)) -} - -func TestIterTargetFiles_Reusable(t *testing.T) { - // The returned iter.Seq2 must be safe to range over more than once. - a, b := tf(1), tf(2) - seq := tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(map[string]*tufmetadata.TargetFiles{ - "a": a, - "b": b, - }, nil), + names := roleNames(got) + assert.Len(t, names, totalCap) + assert.Equal(t, tufmetadata.TARGETS, names[0]) + assert.Equal(t, "c0", names[1]) + assert.Equal(t, fmt.Sprintf("c%d", totalCap-2), names[len(names)-1]) + assert.NotContains(t, names, fmt.Sprintf("c%d", totalCap-1)) +} + +func TestIterTargetRoles_Reusable(t *testing.T) { + // The returned iter.Seq2 must be safe to range over more than once: each + // range re-runs the walk from scratch. + seq := tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}), + "d1": signedTargets(nil, nil), })) for range 2 { got, err := generics.CollectErrorSeq(seq) require.NoError(t, err) - gotMap := make(map[string]*tufmetadata.TargetFiles, len(got)) - for _, td := range got { - gotMap[td.Path] = td.TargetFiles - } - assert.Equal(t, map[string]*tufmetadata.TargetFiles{"a": a, "b": b}, gotMap) - } -} - -func TestIterTargetFiles_EarlyTermination(t *testing.T) { - // Wrapper counts producer yields. If the producer ignores the stop - // signal after the consumer breaks, the counter would exceed 1; the - // 5-target input gives it plenty of values to overrun on. - in := map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(map[string]*tufmetadata.TargetFiles{ - "a": tf(1), "b": tf(2), "c": tf(3), "d": tf(4), "e": tf(5), - }, nil), - } - - underlying := tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(in)) - var producerYields int - counted := iter.Seq2[tufext.TargetFileData, error](func(yield func(tufext.TargetFileData, error) bool) { - for td, err := range underlying { - producerYields++ - if !yield(td, err) { - return - } - } - }) - - for td, err := range counted { - _ = td - require.NoError(t, err) - break - } - assert.Equal(t, 1, producerYields, "producer must stop after the consumer breaks") -} - -func TestIterTargetFiles_DeepDelegationsWithSiblings(t *testing.T) { - // Regression for a chain aliasing bug: siblings derived from the same - // parent shared the parent's backing storage (a slice with cap > len at - // the time), so the second sibling clobbered the first. The chain is now - // map-backed, but the depth sweep is cheap and guards the property - // regardless of representation. - for _, depth := range []int{2, 3, 4, 5, 6, 7, 8, 16} { - t.Run(fmt.Sprintf("depth=%d", depth), func(t *testing.T) { - leafA := tf(int64(depth)*10 + 1) - leafB := tf(int64(depth)*10 + 2) - all := map[string]*tufext.SignedTargets{} - - // targets -> r0 -> r1 -> ... -> r(depth-1) -> {leafA, leafB}. - all[tufmetadata.TARGETS] = signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("r0", false, "shared/*"), - }) - for i := 0; i < depth-1; i++ { - all[fmt.Sprintf("r%d", i)] = signedTargets(nil, []tufmetadata.DelegatedRole{ - dr(fmt.Sprintf("r%d", i+1), false, "shared/*"), - }) - } - all[fmt.Sprintf("r%d", depth-1)] = signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("leafA", false, "shared/A"), - dr("leafB", false, "shared/B"), - }) - all["leafA"] = signedTargets(map[string]*tufmetadata.TargetFiles{"shared/A": leafA}, nil) - all["leafB"] = signedTargets(map[string]*tufmetadata.TargetFiles{"shared/B": leafB}, nil) - - got := pathsFrom(t, all) - assert.Equal(t, map[string]*tufmetadata.TargetFiles{ - "shared/A": leafA, - "shared/B": leafB, - }, got) - }) + assert.Equal(t, []string{tufmetadata.TARGETS, "d1"}, roleNames(got)) } } -func TestIterTargetFiles_BreakAfterError(t *testing.T) { +func TestIterTargetRoles_BreakAfterError(t *testing.T) { // Breaking after an error yield must be safe, and the seq must remain - // reusable on a fresh range loop. ErrorIter yields the error once and - // returns, so observed == 1 in both passes. - in := map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("missing", false, "x/*"), - }), - } - seq := tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(in)) - - // First pass: break immediately after the error. - var sawError bool - var observed int - for _, err := range seq { - observed++ - if err != nil { - sawError = true - break - } - } - require.True(t, sawError) - assert.Equal(t, 1, observed, "ErrorIter delivers the error in a single yield") - - // Second pass: do not break, confirm no further values appear. - sawError = false - observed = 0 - for _, err := range seq { - observed++ - if err != nil { - sawError = true - } - } - require.True(t, sawError) - assert.Equal(t, 1, observed) -} - -func TestIterTargetFiles_PathFieldMatchesMapKey(t *testing.T) { - // Without going through pathsFrom: yielded Path equals the input map - // key and the embedded pointer is the same *TargetFiles the caller put in. - a, b := tf(1), tf(2) - in := map[string]*tufext.SignedTargets{ - tufmetadata.TARGETS: signedTargets(map[string]*tufmetadata.TargetFiles{ - "alpha": a, - "b/eta": b, - }, nil), - } - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), tufext.TargetsMapFetcher(in))) - require.NoError(t, err) - require.Len(t, got, 2) - - for _, td := range got { - switch td.Path { - case "alpha": - assert.Same(t, a, td.TargetFiles) - case "b/eta": - assert.Same(t, b, td.TargetFiles) - default: - t.Errorf("yielded unexpected path %q", td.Path) - } - } -} - -func TestIterTargetFiles_FetcherReceivesDelegatorContext(t *testing.T) { - // The fetcher receives (roleName, delegatorName) pairs. Per s5.6.7, the - // top-level "targets" role is delegated by "root"; each subsequent role - // names its parent delegator. The order also follows pre-order DFS. - type call struct{ role, delegator string } - var calls []call - fetch := func(_ context.Context, roleName, delegatorName string) (*tufext.SignedTargets, error) { - calls = append(calls, call{roleName, delegatorName}) - switch roleName { - case tufmetadata.TARGETS: - return signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/y")}), nil - case "d1": - return signedTargets(nil, []tufmetadata.DelegatedRole{dr("d2", false, "x/y")}), nil - case "d2": - return signedTargets(map[string]*tufmetadata.TargetFiles{"x/y": tf(1)}, nil), nil - } - return nil, fmt.Errorf("unexpected fetch %q", roleName) - } - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) - require.NoError(t, err) - assert.Equal(t, []call{ - {tufmetadata.TARGETS, tufmetadata.ROOT}, - {"d1", tufmetadata.TARGETS}, - {"d2", "d1"}, - }, calls) -} - -func TestIterTargetFiles_FetcherErrorSurfaces(t *testing.T) { - // A fetcher error must be returned to the caller, wrapped with the role - // name for context. - sentinel := errors.New("fetcher boom") - fetch := func(_ context.Context, roleName, _ string) (*tufext.SignedTargets, error) { - if roleName == tufmetadata.TARGETS { - return signedTargets(nil, []tufmetadata.DelegatedRole{dr("d1", false, "x/*")}), nil + // reusable on a fresh range loop. "targets" is yielded first, then the + // missing role produces exactly one error yield and nothing after it. + seq := tufext.IterTargetRoles(t.Context(), tufext.TargetsMapFetcher(map[string]*tufext.SignedTargets{ + tufmetadata.TARGETS: signedTargets(nil, []tufmetadata.DelegatedRole{dr("missing", false, "x/*")}), + })) + for pass, breakOnError := range []bool{true, false} { + var values, errs int + for _, err := range seq { + if err != nil { + errs++ + if breakOnError { + break + } + continue + } + values++ } - return nil, sentinel + assert.Equal(t, 1, values, "pass %d: only targets is yielded before the error", pass) + assert.Equal(t, 1, errs, "pass %d: the error is delivered exactly once", pass) } - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) - require.Error(t, err) - assert.ErrorIs(t, err, sentinel) //nolint:testifylint // assert is fine for error path checks - assert.Contains(t, err.Error(), "d1", "wrapped error must name the failing role") } -func TestIterTargetFiles_FetcherSeesContext(t *testing.T) { - // The context passed to IterTargetFiles is plumbed through to the - // fetcher. Cancellation surfaced by the fetcher propagates as the - // iterator's error. - ctx, cancel := context.WithCancel(t.Context()) - cancel() - fetch := func(ctx context.Context, _, _ string) (*tufext.SignedTargets, error) { - return nil, ctx.Err() - } - _, err := generics.CollectErrorSeq(tufext.IterTargetFiles(ctx, fetch)) - require.Error(t, err) - assert.ErrorIs(t, err, context.Canceled) -} +func TestRoleDelegationChain_Match(t *testing.T) { + // Match is "no terminated chain covers the path" AND "this chain + // authorises the path". The zero value authorises everything, matching + // what the top-level role is yielded with. + var zero tufext.RoleDelegationChain + assert.True(t, zero.IsTargetPermitted("anything")) -func TestIterTargetFiles_FetcherCalledOncePerDelegationPath(t *testing.T) { - // Diamond graph: "shared" is delegated by both "a" and "b". Cycle - // detection is per delegation path (spec issue 321), not global, so - // "shared" is fetched once per path, each time naming the delegator that - // path came through, in pre-order DFS order. The target is still yielded - // once: the first path wins. - type call struct{ role, delegator string } - var calls []call - fetch := func(_ context.Context, roleName, delegatorName string) (*tufext.SignedTargets, error) { - calls = append(calls, call{roleName, delegatorName}) - switch roleName { - case tufmetadata.TARGETS: - return signedTargets(nil, []tufmetadata.DelegatedRole{ - dr("a", false, "x/*"), - dr("b", false, "x/*"), - }), nil - case "a", "b": - return signedTargets(nil, []tufmetadata.DelegatedRole{dr("shared", false, "x/*")}), nil - case "shared": - return signedTargets(map[string]*tufmetadata.TargetFiles{"x/file": tf(1)}, nil), nil - } - return nil, fmt.Errorf("unexpected fetch %q", roleName) + rc := tufext.RoleDelegationChain{ + ThisChain: chainOf(dr("d1", false, "a/*", "b/*")), + TerminatedChains: []tufext.DelegationChain{chainOf(dr("t", true, "a/*"))}, } - got, err := generics.CollectErrorSeq(tufext.IterTargetFiles(t.Context(), fetch)) - require.NoError(t, err) - require.Len(t, got, 1) - assert.Equal(t, []call{ - {tufmetadata.TARGETS, tufmetadata.ROOT}, - {"a", tufmetadata.TARGETS}, - {"shared", "a"}, - {"b", tufmetadata.TARGETS}, - {"shared", "b"}, - }, calls) + assert.False(t, rc.IsTargetPermitted("a/x"), "covered by a terminated chain") + assert.True(t, rc.IsTargetPermitted("b/x")) + assert.False(t, rc.IsTargetPermitted("c/x"), "not authorised by this chain") } // chainOf builds a [tufext.DelegationChain] from the given delegations, // ordered from the delegation closest to "targets" down to the leaf. Each // link is recorded under the role that made the delegation: "targets" for the -// first, then the previous link's role name, mirroring how IterTargetFiles +// first, then the previous link's role name, mirroring how IterTargetRoles // builds chains. func chainOf(delegations ...tufmetadata.DelegatedRole) tufext.DelegationChain { var chain tufext.DelegationChain @@ -1081,7 +972,7 @@ const hashPrefixLen = 8 // TUF specification (s4.5 says PATH_HASH_PREFIXES are prefixes of the // hexadecimal digest, which is what python-tuf and go-tuf v1 implement). If // go-tuf is fixed to use hex, this helper must change with it. -// +// func hashPrefix(path string) string { sum := sha256.Sum256([]byte(path)) return base64.URLEncoding.EncodeToString(sum[:])[:hashPrefixLen] @@ -1189,7 +1080,7 @@ func TestDelegationChain_IsEmpty(t *testing.T) { func TestDelegationChain_ContainsAndLength(t *testing.T) { // A chain records the roles that *delegated* along the path, keyed by - // delegator, so the leaf itself is never a member. IterTargetFiles relies + // delegator, so the leaf itself is never a member. IterTargetRoles relies // on exactly this for cycle detection: a role is skipped only if it has // already delegated on the current path. var chain tufext.DelegationChain @@ -1250,7 +1141,7 @@ func TestDelegationChain_ExtendNilIsNoop(t *testing.T) { } func TestDelegationChain_ExtendDoesNotAliasSiblings(t *testing.T) { - // Companion to TestIterTargetFiles_DeepDelegationsWithSiblings at the + // Companion to TestIterTargetRoles_DeepDelegationsWithSiblings at the // DelegationChain level: two siblings derived from the same parent get // their own storage and the parent is untouched. Both siblings are // appended under the same delegator, so shared storage would either @@ -1325,22 +1216,3 @@ func TestDelegationChain_BothPathsAndHashPrefixes_PathsWin(t *testing.T) { assert.True(t, chain.IsTargetPermitted("a/x"), "Paths must still be honoured") assert.False(t, chain.IsTargetPermitted(hashed), "PathHashPrefixes must be ignored when Paths is set") } - -// keysOf returns the sorted keys. The iterator's within-role order is -// unspecified, so most tests compare sorted slices or sets. -func keysOf[V any](m map[string]V) []string { - out := make([]string, 0, len(m)) - for k := range m { - out = append(out, k) - } - slices.Sort(out) - return out -} - -func asSet(s []string) map[string]struct{} { - out := make(map[string]struct{}, len(s)) - for _, v := range s { - out[v] = struct{}{} - } - return out -} From 5e93f4135485c711124828a738d7f9c0f55e6d99 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 23 Sep 2026 01:19:53 +1000 Subject: [PATCH 6/8] tufext: split some of tufclient/config.Repository to tufext 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 --- cmd/quarry-client/http.go | 6 +- cmd/quarry-client/list.go | 6 +- cmd/quarry-client/list_test.go | 7 +- cmd/quarry-client/utils_tuf.go | 23 ++- internal/generics/error.go | 10 + internal/serde/doc.go | 6 + internal/serde/utils.go | 28 +++ internal/tufclient/client.go | 44 +++-- internal/tufclient/client_test.go | 9 +- internal/tufclient/config/config.go | 239 ++++++++--------------- internal/tufclient/config/config_test.go | 130 ++++++++---- internal/tufext/repo.go | 77 ++++++++ internal/tufext/repo_test.go | 86 ++++++++ internal/tufext/root_trust.go | 163 ++++++++++++++++ internal/xsysupdate/transfer_test.go | 10 +- 15 files changed, 605 insertions(+), 239 deletions(-) create mode 100644 internal/generics/error.go create mode 100644 internal/serde/doc.go create mode 100644 internal/serde/utils.go create mode 100644 internal/tufext/repo.go create mode 100644 internal/tufext/repo_test.go create mode 100644 internal/tufext/root_trust.go diff --git a/cmd/quarry-client/http.go b/cmd/quarry-client/http.go index d2fa542..b892e06 100644 --- a/cmd/quarry-client/http.go +++ b/cmd/quarry-client/http.go @@ -224,7 +224,11 @@ func proxyTargetFile(rw http.ResponseWriter, req *http.Request) (Err error) { // TODO(uapi16): Once we get UAPI.16 support into systemd, we can serve a // manifest that lists every URL candidate as an alternative contents source // (as "quarry-client list --uapi-16" does) and drop this shim entirely. - for url, err := range infoExt.FetchURLs(&info.Repo.DataRootURL.URL) { + dataRootURL, err := info.Repo.DataURL() + if err != nil { + return fmt.Errorf("bad repo %s definition: cannot compute target url: %w", info.Repo.Name, err) + } + for url, err := range infoExt.FetchURLs(dataRootURL) { if err != nil { return fmt.Errorf("bad target data in repo %s for target %s: cannot compute target url: %w", info.Repo.Name, targetPath, err) } diff --git a/cmd/quarry-client/list.go b/cmd/quarry-client/list.go index d6a74c4..c296392 100644 --- a/cmd/quarry-client/list.go +++ b/cmd/quarry-client/list.go @@ -118,7 +118,11 @@ func (o *uapi16ListFormatter) Begin(ctx context.Context, client *tufclient.Clien } func (o *uapi16ListFormatter) Output(_ context.Context, target *tufclient.TargetInfo) error { - file, err := uapi16ext.FromTargetFile(target.TargetFiles, &target.Repo.DataRootURL.URL) + dataRootURL, err := target.Repo.DataURL() + if err != nil { + return err + } + file, err := uapi16ext.FromTargetFile(target.TargetFiles, dataRootURL) if err != nil { return err } diff --git a/cmd/quarry-client/list_test.go b/cmd/quarry-client/list_test.go index be12511..91524ab 100644 --- a/cmd/quarry-client/list_test.go +++ b/cmd/quarry-client/list_test.go @@ -33,9 +33,8 @@ const ( emptyHash = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" ) -// testRepo returns a [config.Repository] for a dummy repository. The config -// has to be parsed because the URL fields are not exported types. -func testRepo(t *testing.T) *config.Repository { +// testRepo returns the [tufext.Repository] for a dummy repository config. +func testRepo(t *testing.T) *tufext.Repository { t.Helper() cfg, err := config.Parse(strings.NewReader(` config_version = 1 @@ -49,7 +48,7 @@ data_root_url = "https://example.com/data" require.NoError(t, err) repo, ok := cfg.Repos["test-repo"] require.True(t, ok, "test-repo should be in the parsed config") - return repo + return repo.AsRepository() } // testClient returns a client with no repositories configured. That is enough diff --git a/cmd/quarry-client/utils_tuf.go b/cmd/quarry-client/utils_tuf.go index 40fce1c..2491538 100644 --- a/cmd/quarry-client/utils_tuf.go +++ b/cmd/quarry-client/utils_tuf.go @@ -20,7 +20,6 @@ import ( "go.amutable.dev/quarry/internal/expand" "go.amutable.dev/quarry/internal/third_party/funchelpers" "go.amutable.dev/quarry/internal/tufclient" - "go.amutable.dev/quarry/internal/tufclient/config" "go.amutable.dev/quarry/internal/tufext" ) @@ -86,7 +85,7 @@ func pprintHashes(wtr io.Writer, prefix string, hashes tufmetadata.Hashes) { } } -func pprintTargetFile(wtr io.Writer, prefix string, repo *config.Repository, target *tufmetadata.TargetFiles) { +func pprintTargetFile(wtr io.Writer, prefix string, repo *tufext.Repository, target *tufmetadata.TargetFiles) { targetExt := tufext.TargetFilesExt(target) mustFprintf(wtr, "%s%s:\n", prefix, target.Path) @@ -98,11 +97,15 @@ func pprintTargetFile(wtr io.Writer, prefix string, repo *config.Repository, tar mustFprintf(wtr, "%sInline data: %d bytes\n", prefix, len(data)) } mustFprintf(wtr, "%sURL(s):\n", prefix) - for url, err := range targetExt.FetchURLs(&repo.DataRootURL.URL) { - if err != nil { - mustFprintf(wtr, "%s - \n", prefix, err) + if dataRootURL, err := repo.DataURL(); err != nil { + mustFprintf(wtr, "%s - \n", prefix, err) + } else { + for url, err := range targetExt.FetchURLs(dataRootURL) { + if err != nil { + mustFprintf(wtr, "%s - \n", prefix, err) + } + mustFprintf(wtr, "%s - %s\n", prefix, url) } - mustFprintf(wtr, "%s - %s\n", prefix, url) } mustFprintf(wtr, "%sSize: %d\n", prefix, target.Length) pprintHashes(wtr, prefix, target.Hashes) @@ -113,7 +116,7 @@ func pprintTargetFile(wtr io.Writer, prefix string, repo *config.Repository, tar // TODO(ext): UnrecognisedFields } -func expandTargetFile(wtr io.Writer, fmtStr string, repo *config.Repository, target *tufmetadata.TargetFiles) error { +func expandTargetFile(wtr io.Writer, fmtStr string, repo *tufext.Repository, target *tufmetadata.TargetFiles) error { targetExt := tufext.TargetFilesExt(target) expander := expand.NewExpansions(). @@ -131,7 +134,11 @@ func expandTargetFile(wtr io.Writer, fmtStr string, repo *config.Repository, tar } else if data != nil { return "data:;base64," + base64.StdEncoding.EncodeToString(data), nil } - for url, err := range targetExt.FetchURLs(&repo.DataRootURL.URL) { + dataRootURL, err := repo.DataURL() + if err != nil { + return "", err + } + for url, err := range targetExt.FetchURLs(dataRootURL) { var urlStr string if url != nil { urlStr = url.String() diff --git a/internal/generics/error.go b/internal/generics/error.go new file mode 100644 index 0000000..813e39a --- /dev/null +++ b/internal/generics/error.go @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package generics + +// TakeError can be used to wrap a function that returns (T, erorr) to extract +// just the error in one line. +func TakeError[T any](_ T, err error) error { + return err +} diff --git a/internal/serde/doc.go b/internal/serde/doc.go new file mode 100644 index 0000000..8d2aab7 --- /dev/null +++ b/internal/serde/doc.go @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +// Package serde provides a poor-mans scheme for serialising and deserialising +// data in different formats. +package serde diff --git a/internal/serde/utils.go b/internal/serde/utils.go new file mode 100644 index 0000000..fe3d4b8 --- /dev/null +++ b/internal/serde/utils.go @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package serde + +import ( + "errors" + "fmt" +) + +// ErrMissingField is returned by [parseMapKey] if the key is missing entirely. +var ErrMissingField = errors.New("missing required field") + +// ParseMapKey takes the value from the map with the given key, parses it into +// the given slot, and drops it from the original map. This is quite handy for +// detecting unsupported fields in an ergonomic way when parsing maps from +// encodings like JSON or TOML. +func ParseMapKey[T any](data map[string]any, key string, slot *T) error { + if valAny, ok := data[key]; !ok { + return fmt.Errorf("%w %q", ErrMissingField, key) + } else if val, ok := valAny.(T); !ok { + return fmt.Errorf("field %q has incorrect value type: %v (%T) is not a %T", key, valAny, valAny, *new(T)) + } else { //nolint:revive // variable chaining makes this uglier vis-a-vis indent-error-flow + *slot = val + delete(data, key) + return nil + } +} diff --git a/internal/tufclient/client.go b/internal/tufclient/client.go index 08ca363..cdf68b4 100644 --- a/internal/tufclient/client.go +++ b/internal/tufclient/client.go @@ -47,8 +47,19 @@ import ( var ErrSkippableRepo = errors.New("skippable repository error") // RepoClient constructs a [tufupdater.Updater] for a single TUF repository, -// defined by a [config.Repository] configuration. -func RepoClient(ctx context.Context, cacheDir *pathrs.Root, repo *config.Repository) (_ *tufupdater.Updater, Err error) { +// defined by a [tufext.Repository] configuration. +func RepoClient(ctx context.Context, cacheDir *pathrs.Root, repoLike tufext.RepositoryLike) (_ *tufupdater.Updater, Err error) { + repo := repoLike.AsRepository() + + metaRootURL, err := repo.RootURL() + if err != nil { + return nil, fmt.Errorf("invalid repository definition: %w", err) + } + dataRootURL, err := repo.DataURL() + if err != nil { + return nil, fmt.Errorf("invalid repository definition: %w", err) + } + repoCacheDirHandle, err := cacheDir.MkdirAll(repo.Name, 0o755) if err != nil { return nil, fmt.Errorf("open repo cache dir: %w", err) @@ -121,14 +132,14 @@ func RepoClient(ctx context.Context, cacheDir *pathrs.Root, repo *config.Reposit } // Use the go-tuf defaults and adjust the arguments. - tufConfig, err := tufconfig.New(repo.MetaRootURL.String(), rootData) + tufConfig, err := tufconfig.New(metaRootURL.String(), rootData) if err != nil { return nil, fmt.Errorf("initialise tuf-client config: %w", err) } - tufConfig.RootMaxLength = config.MaxRootBytes + tufConfig.RootMaxLength = tufext.MaxRootBytes // Custom URLs. - tufConfig.RemoteMetadataURL = repo.MetaRootURL.String() - tufConfig.RemoteTargetsURL = repo.DataRootURL.String() + tufConfig.RemoteMetadataURL = metaRootURL.String() + tufConfig.RemoteTargetsURL = dataRootURL.String() // Use our own cache dir. tufConfig.LocalMetadataDir = repoCacheDir.IntoFile().Name() // NOTE: Ideally we wouldn't have this (there is little point to this kind @@ -247,13 +258,13 @@ func (client *Client) SetRefTime(ctx context.Context, refTime time.Time) { } } -// TargetInfo is a tuple of [*tufmetadata.TargetFiles] and [*config.Repository] +// TargetInfo is a tuple of [*tufmetadata.TargetFiles] and [*tufext.Repository] // which is returned by most [Client] methods. This is necessary to help with // identifying which repository a target file comes from, as well as doing some // other operations. type TargetInfo struct { *tufmetadata.TargetFiles - Repo *config.Repository + Repo *tufext.Repository } // Fetch retreives the target file referenced by this [TargetInfo] and returns @@ -276,7 +287,11 @@ func (info *TargetInfo) Fetch(ctx context.Context) (io.ReadCloser, error) { // Rather than using the go-tuf DownloadTarget (which requires the data be // stored in-memory) we fetch it directly. - for url, err := range infoExt.FetchURLs(&info.Repo.DataRootURL.URL) { + dataRootURL, err := info.Repo.DataURL() + if err != nil { + return nil, fmt.Errorf("check target candidate urls: %w", err) + } + for url, err := range infoExt.FetchURLs(dataRootURL) { if err != nil { return nil, fmt.Errorf("check target candidate urls: %w", err) } @@ -315,7 +330,7 @@ func (client *Client) GetTargetInfo(ctx context.Context, targetPath string) (*Ta // be masked anyway. return &TargetInfo{ TargetFiles: info, - Repo: client.Config.Repos[repoName], + Repo: client.Config.Repos[repoName].AsRepository(), }, nil } return nil, fmt.Errorf("target %s not found: %w", targetPath, fs.ErrNotExist) @@ -337,7 +352,7 @@ func (client *Client) FetchTargetFile(ctx context.Context, targetPath string) (i // trustedMetadataTargetsFetcher returns a [tufext.TargetMetadataFetchFunc] for // the given repository in the client. -func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, metadata *tuftrustedmetadata.TrustedMetadata) tufext.TargetMetadataFetchFunc { +func (client *Client) trustedMetadataTargetsFetcher(repo *tufext.Repository, metadata *tuftrustedmetadata.TrustedMetadata) tufext.TargetMetadataFetchFunc { var mu sync.RWMutex // to serialise access to TrustedMetadata return func(ctx context.Context, roleName, delegatorName string) (_ *tufext.SignedTargets, Err error) { @@ -405,7 +420,10 @@ func (client *Client) trustedMetadataTargetsFetcher(repo *config.Repository, met return savedTarget, nil } metaPath := fmt.Sprintf("%d.%s.json", metaRef.Version, roleName) - metaURL := repo.MetaRootURL.JoinPath(metaPath) + metaURL, err := repo.RootURL(metaPath) + if err != nil { + return nil, err + } rdr, _, err := httputils.VerifiedHTTPGet(ctx, metaURL, metaRef.Length, metaRef.Hashes) if err != nil { @@ -457,7 +475,7 @@ func (client *Client) IterTargetFiles(ctx context.Context) iter.Seq2[*TargetInfo return err } - repo := client.Config.Repos[repoName] + repo := client.Config.Repos[repoName].AsRepository() meta := updater.GetTrustedMetadataSet() if meta.Timestamp == nil { // FIXME: The local client TrustedMetadata state does not get diff --git a/internal/tufclient/client_test.go b/internal/tufclient/client_test.go index a7bc87b..fc65aa5 100644 --- a/internal/tufclient/client_test.go +++ b/internal/tufclient/client_test.go @@ -21,19 +21,18 @@ import ( "go.amutable.dev/quarry/internal/testrepo" "go.amutable.dev/quarry/internal/tufclient" - "go.amutable.dev/quarry/internal/tufclient/config" "go.amutable.dev/quarry/internal/tufext" ) const targetPath = "foo/target.txt" -// serverRepo returns the [config.Repository] for the given server. -func serverRepo(t *testing.T, srv *testrepo.Server) *config.Repository { +// serverRepo returns the [tufext.Repository] for the given server. +func serverRepo(t *testing.T, srv *testrepo.Server) *tufext.Repository { t.Helper() cfg := testrepo.Config(t, srv.ConfigBlock("testrepo")) repo, ok := cfg.Repos["testrepo"] require.True(t, ok, "config did not yield repository testrepo") - return repo + return repo.AsRepository() } // targetURLPath returns the URL path the test target is fetched from. @@ -64,7 +63,7 @@ func serveStatus(srv *testrepo.Server, pattern string, status int) { // data. Any extensions (inline data, override URLs) must be applied in ext // rather than afterwards -- SetExtensionJSON round-trips the struct through // JSON, which drops non-JSON fields like Path. -func newTargetInfo(repo *config.Repository, data []byte, ext func(*tufmetadata.TargetFiles)) *tufclient.TargetInfo { +func newTargetInfo(repo *tufext.Repository, data []byte, ext func(*tufmetadata.TargetFiles)) *tufclient.TargetInfo { sum := sha256.Sum256(data) target := &tufmetadata.TargetFiles{ Length: int64(len(data)), diff --git a/internal/tufclient/config/config.go b/internal/tufclient/config/config.go index 2ba3109..a45a76c 100644 --- a/internal/tufclient/config/config.go +++ b/internal/tufclient/config/config.go @@ -10,9 +10,7 @@ import ( "errors" "fmt" "io" - "io/fs" "maps" - "net/http" "net/url" "os" "path/filepath" @@ -21,68 +19,22 @@ import ( "github.com/BurntSushi/toml" "go.amutable.dev/quarry/internal/expand" - "go.amutable.dev/quarry/internal/third_party/funchelpers" + "go.amutable.dev/quarry/internal/generics" + "go.amutable.dev/quarry/internal/serde" + "go.amutable.dev/quarry/internal/tufext" ) -// TODO: Make these more configurable. -const ( - MaxRootBytes = 512_000 // 512k -) - -// RootTrustSource represents a source of trust for the initial state of a -// client's locally cached root.json. -type RootTrustSource interface { - Type() string - fmt.Stringer - - // This is a helper method called from [parseTomlRootTrust] to fill the - // structure based on the pre-parsed TOML table. Unfortunately, we cannot - // do this generically (i.e., there doesn't appear to be a way to have a - // generic requirement to operate on a type whose pointer implements an - // interface) so we need to return a [RootTrustSource] (which is a copy of - // the object itself). - // - // We do not implement [encoding.TextUnmarshaler] or [toml.Unmarshaler] - // here because there are multiple types of root trust and you need to - // unmarshal [tomlRootTrust] for this to work generically. - fromTomlMap(table map[string]any) (RootTrustSource, error) - - // IsRemote indicates whether the root source is to be fetched from a - // remote resource. Callers can use this as a hint for whether some errors - // from [FetchRoot] should be skipped. - IsRemote() bool - - // FetchRoot fetches the initial root.json for the given [Repository], - // based on the internal policy of this [RootTrustSource]. - FetchRoot(ctx context.Context, repo *Repository) ([]byte, error) -} - -// tomlRootTrust is a wrapper around [RootTrustSource] that allows for a -// generic parsing of [RootTrustSource] implementations. +// tomlRootTrust is a wrapper around [tufext.RootTrustSource] that allows for a +// generic parsing of [tufext.RootTrustSource] implementations. type tomlRootTrust struct { - RootTrustSource -} - -// parseTomlKey takes the value from the map with the given key, parses it into -// the given slot, and drops it from the original map. This is quite handy for -// detecting unsupported fields in an ergonomic way when parsing TOML maps. -func parseTomlKey[T any](data map[string]any, key string, slot *T) error { - if valAny, ok := data[key]; !ok { - return fmt.Errorf("missing required field %q", key) - } else if val, ok := valAny.(T); !ok { - return fmt.Errorf("field %q has incorrect value type: %v (%T) is not a %T", key, valAny, valAny, *new(T)) - } else { //nolint:revive // variable chaining makes this uglier vis-a-vis indent-error-flow - *slot = val - delete(data, key) - return nil - } + tufext.RootTrustSource } // errWrongType is a sentinel error returned from [parseTomlRootTrust] if the // generic type does not match the type of the TOML object. var errWrongType = errors.New("[internal error] wrong type") -func parseTomlRootTrust[T RootTrustSource](data any) (RootTrustSource, error) { +func parseTomlRootTrust[T tufext.RootTrustSource](data any) (tufext.RootTrustSource, error) { rootTrust := *new(T) trustType := rootTrust.Type() @@ -95,7 +47,7 @@ func parseTomlRootTrust[T RootTrustSource](data any) (RootTrustSource, error) { // Name-based specifications are only valid for types which also accept // empty TOML tables (usually empty structs but also structs with no // required fields). - rootTrust, err := rootTrust.fromTomlMap(map[string]any{}) + rootTrust, err := rootTrust.FromMap(map[string]any{}) if err != nil { return nil, fmt.Errorf("root trust %q cannot be instantiated using a plain string: %w", trustType, err) } @@ -110,7 +62,7 @@ func parseTomlRootTrust[T RootTrustSource](data any) (RootTrustSource, error) { table = maps.Clone(table) var gotType string - if err := parseTomlKey(table, "type", &gotType); err != nil { + if err := serde.ParseMapKey(table, "type", &gotType); err != nil { return nil, err } if gotType != trustType { @@ -119,7 +71,7 @@ func parseTomlRootTrust[T RootTrustSource](data any) (RootTrustSource, error) { } // Let the RootTrustSource parse the rest of the options. - rootTrust, err := rootTrust.fromTomlMap(table) + rootTrust, err := rootTrust.FromMap(table) if err != nil { return nil, fmt.Errorf("root trust %q could not be parsed: %w", trustType, err) } @@ -130,10 +82,10 @@ func parseTomlRootTrust[T RootTrustSource](data any) (RootTrustSource, error) { } func (t *tomlRootTrust) UnmarshalTOML(data any) error { - for _, parser := range []func(any) (RootTrustSource, error){ - parseTomlRootTrust[tofuRootTrust], - parseTomlRootTrust[bundledRootTrust], - parseTomlRootTrust[inlineRootTrust], + for _, parser := range []func(any) (tufext.RootTrustSource, error){ + parseTomlRootTrust[tufext.TofuRootTrust], + parseTomlRootTrust[tomlBundledRootTrust], + parseTomlRootTrust[tomlInlineRootTrust], } { rootTrust, err := parser(data) if errors.Is(err, errWrongType) { @@ -152,14 +104,14 @@ func (t *tomlRootTrust) UnmarshalTOML(data any) error { } // Expand applies the given [expand.Expansions] to the underlying -// [RootTrustSource]. +// [tufext.RootTrustSource]. func (t *tomlRootTrust) Expand(exp *expand.Expansions) error { - var newRootTrust RootTrustSource + var newRootTrust tufext.RootTrustSource switch rootTrust := t.RootTrustSource.(type) { - case tofuRootTrust, inlineRootTrust: + case tufext.TofuRootTrust, tomlInlineRootTrust: // nothing to expand newRootTrust = rootTrust - case bundledRootTrust: + case tomlBundledRootTrust: path, err := exp.ExpandString(rootTrust.Path) if err != nil { return fmt.Errorf("cannot %%-expand path %q: %w", rootTrust.Path, err) @@ -176,73 +128,26 @@ func (t *tomlRootTrust) Expand(exp *expand.Expansions) error { return nil } -// tofuRootTrust indicates that makeUpdater should fetch the root.json -// directly from the repository with a trust-on-first-use policy. -// *This is inherently insecure*. -type tofuRootTrust struct{} - -var _ RootTrustSource = &tofuRootTrust{} - -func (tofuRootTrust) Type() string { return "insecure-tofu" } - -func (t tofuRootTrust) String() string { return t.Type() } - -func (t tofuRootTrust) fromTomlMap(data map[string]any) (RootTrustSource, error) { - if len(data) > 0 { - return nil, fmt.Errorf("unsupported fields: %v", slices.Collect(maps.Keys(data))) - } - return t, nil -} - -func (tofuRootTrust) IsRemote() bool { return true } - -func (tofuRootTrust) FetchRoot(ctx context.Context, repo *Repository) (_ []byte, Err error) { - // The updater will bump the root.json to the latest version afterwards. - rootURL := repo.MetaRootURL.JoinPath("1.root.json") - - req, err := http.NewRequestWithContext(ctx, "GET", rootURL.String(), nil) - if err != nil { - return nil, fmt.Errorf("create http request: %w", err) - } - req.Header.Set("Accept", "application/json") - - client := http.DefaultClient - res, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("fetch %s: %w", rootURL, err) - } - if res.StatusCode >= 300 { - if res.Body != nil { - _ = res.Body.Close() - } - err := fmt.Errorf("fetch %s failed with status code %.3d", rootURL, res.StatusCode) - if res.StatusCode == http.StatusNotFound { - // Emulate ENOENT for 404. - err = fmt.Errorf("%w: %w", err, fs.ErrNotExist) - } - return nil, err - } - rdr := http.MaxBytesReader(nil, res.Body, MaxRootBytes) // use same max as client - defer funchelpers.VerifyClose(&Err, rdr) - - return io.ReadAll(rdr) -} - -// bundledRootTrust indicates that the root.json for this makeUpdater should be -// sourced from a particular on-disk file (usually distributed as part of the -// base OS image). -type bundledRootTrust struct { +// tomlBundledRootTrust indicates that the root.json for this repository should +// be sourced from a particular on-disk file (usually distributed as part of +// the base OS image). *This only makes sense for repositories defined via +// config files*. +// +// TODO: Does this really belong here and not in [tufclient/config]? It is +// quite a local-config concept. +type tomlBundledRootTrust struct { Path string `toml:"path"` + // TODO: UnrecognizedFields? } -var _ RootTrustSource = bundledRootTrust{} +var _ tufext.RootTrustSource = tomlBundledRootTrust{} -func (t bundledRootTrust) Type() string { return "bundled" } +func (t tomlBundledRootTrust) Type() string { return "bundled" } -func (t bundledRootTrust) String() string { return t.Type() + ":" + t.Path } +func (t tomlBundledRootTrust) String() string { return t.Type() + ":" + t.Path } -func (t bundledRootTrust) fromTomlMap(data map[string]any) (RootTrustSource, error) { - if err := parseTomlKey(data, "path", &t.Path); err != nil { +func (t tomlBundledRootTrust) FromMap(data map[string]any) (tufext.RootTrustSource, error) { + if err := serde.ParseMapKey(data, "path", &t.Path); err != nil { return nil, err } if len(data) > 0 { @@ -251,29 +156,27 @@ func (t bundledRootTrust) fromTomlMap(data map[string]any) (RootTrustSource, err return t, nil } -func (bundledRootTrust) IsRemote() bool { return false } +func (tomlBundledRootTrust) IsRemote() bool { return false } -func (t bundledRootTrust) FetchRoot(_ context.Context, _ *Repository) ([]byte, error) { +func (t tomlBundledRootTrust) FetchRoot(_ context.Context, _ *tufext.Repository) ([]byte, error) { return os.ReadFile(t.Path) //nolint:forbidigo // user-controlled host path } -// inlineRootTrust is like [bundledRootTrust] except the root.json is embedded -// directly into the configuration file as a string, which is much easier to -// manage than [bundledRootTrust] when dealing with drop-in files. -type inlineRootTrust struct { +// tomlInlineRootTrust is the TOML version of [tufext.InlineRootTrust]. +type tomlInlineRootTrust struct { RootJSON string `toml:"root.json"` } -var _ RootTrustSource = inlineRootTrust{} +var _ tufext.RootTrustSource = tomlInlineRootTrust{} -func (t inlineRootTrust) Type() string { return "inline" } +func (t tomlInlineRootTrust) Type() string { return "inline" } -func (t inlineRootTrust) String() string { return fmt.Sprintf("%s:%q", t.Type(), t.RootJSON) } +func (t tomlInlineRootTrust) String() string { return fmt.Sprintf("%s:%q", t.Type(), t.RootJSON) } -func (inlineRootTrust) IsRemote() bool { return false } +func (tomlInlineRootTrust) IsRemote() bool { return false } -func (t inlineRootTrust) fromTomlMap(data map[string]any) (RootTrustSource, error) { - if err := parseTomlKey[string](data, "root.json", &t.RootJSON); err != nil { +func (t tomlInlineRootTrust) FromMap(data map[string]any) (tufext.RootTrustSource, error) { + if err := serde.ParseMapKey[string](data, "root.json", &t.RootJSON); err != nil { return nil, err } if len(data) > 0 { @@ -282,7 +185,7 @@ func (t inlineRootTrust) fromTomlMap(data map[string]any) (RootTrustSource, erro return t, nil } -func (t inlineRootTrust) FetchRoot(_ context.Context, _ *Repository) ([]byte, error) { +func (t tomlInlineRootTrust) FetchRoot(_ context.Context, _ *tufext.Repository) ([]byte, error) { return []byte(t.RootJSON), nil } @@ -337,10 +240,11 @@ type Repository struct { RootTrust *tomlRootTrust `toml:"root_trust"` // MetaRootURL is the base URL for the directory containing TUF metadata. + // If unset, the default is derived by [tufext.Repository.RootURL]. MetaRootURL *tomlURL `toml:"meta_root_url"` // DataRootURL is the base URL for the directory containing target data - // files. + // files. If unset, the default is derived by [tufext.Repository.DataURL]. DataRootURL *tomlURL `toml:"data_root_url"` } @@ -356,6 +260,24 @@ func (repo Repository) OrderIndex() int64 { return defaultOrderIndex } +var _ tufext.RepositoryLike = Repository{} + +// AsRepository maps this configured repository to the more generic +// [tufext.Repository] representation. +func (repo Repository) AsRepository() *tufext.Repository { + extRepo := &tufext.Repository{ + Name: repo.Name, + RootTrust: repo.RootTrust.RootTrustSource, + } + if u := repo.MetaRootURL; u != nil { + extRepo.MetaRootURL = generics.Ptr(u.URL) + } + if u := repo.DataRootURL; u != nil { + extRepo.DataRootURL = generics.Ptr(u.URL) + } + return extRepo +} + // ConfigVersion is the current version of the configuration file format. const ConfigVersion = 1 @@ -451,8 +373,8 @@ func parseToml(rdr io.Reader) (*Config, error) { // DefaultCacheDir is the default value of [Config.CacheDir] if unspecified. const DefaultCacheDir = "/var/lib/quarry-client/latest-metadata" -// expandAndValidate applies the %-expansions, fills in the URL fields derived -// from the repository name, and validates the merged configuration. +// expandAndValidate applies the %-expansions and validates the merged +// configuration. func (cfg *Config) expandAndValidate() error { var err error @@ -482,24 +404,23 @@ func (cfg *Config) expandAndValidate() error { return fmt.Errorf("repository %s has invalid root_trust value: %w", repo.Name, err) } - if repo.MetaRootURL == nil { - // If unspecified, assume that the metadata URL is the same as the - // repository name. - repo.MetaRootURL = &tomlURL{rawString: "https://" + repo.Name} + // Unset URLs are defaulted by [tufext.Repository]. + if repo.MetaRootURL != nil { + if err := repo.MetaRootURL.Expand(subExpander); err != nil { + return fmt.Errorf("repository %s has invalid meta_root_url value: %w", repo.Name, err) + } } - if err := repo.MetaRootURL.Expand(subExpander); err != nil { - return fmt.Errorf("repository %s has invalid meta_root_url value: %w", repo.Name, err) - } - - if repo.DataRootURL == nil { - // If unspecified, assume that the targets URL is a subdirectory of - // the metadata URL (this matches the stock go-tuf client - // behaviour). - rootURL := repo.MetaRootURL.JoinPath("targets") - repo.DataRootURL = &tomlURL{rawString: rootURL.String()} + if repo.DataRootURL != nil { + if err := repo.DataRootURL.Expand(subExpander); err != nil { + return fmt.Errorf("repository %s has invalid data_root_url value: %w", repo.Name, err) + } } - if err := repo.DataRootURL.Expand(subExpander); err != nil { - return fmt.Errorf("repository %s has invalid data_root_url value: %w", repo.Name, err) + // Verify the [tufext.Repository]-derived URLs are actually valid URLs. + if err := errors.Join( + generics.TakeError(repo.AsRepository().RootURL()), + generics.TakeError(repo.AsRepository().DataURL()), + ); err != nil { + return fmt.Errorf("repository %s has invalid default url: %w", repo.Name, err) } } return nil diff --git a/internal/tufclient/config/config_test.go b/internal/tufclient/config/config_test.go index 12099b5..9b490a6 100644 --- a/internal/tufclient/config/config_test.go +++ b/internal/tufclient/config/config_test.go @@ -12,12 +12,26 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "go.amutable.dev/quarry/internal/tufext" ) const uuidPat = `[0-9a-f]{32}` var uuidRe = regexp.MustCompile(`^` + uuidPat + `$`) +// repoURLs returns the effective metadata and target data root URLs of repo, +// including any defaults derived by [tufext.Repository]. +func repoURLs(t *testing.T, repo *Repository) (metaRootURL, dataRootURL string) { + t.Helper() + extRepo := repo.AsRepository() + meta, err := extRepo.RootURL() + require.NoError(t, err) + data, err := extRepo.DataURL() + require.NoError(t, err) + return meta.String(), data.String() +} + func repoBlock(rootTrust string) string { return ` config_version = 1 @@ -32,44 +46,44 @@ func TestParseConfig_RootTrust_Valid(t *testing.T) { for _, tc := range []struct { name string rootTrust string - want RootTrustSource + want tufext.RootTrustSource }{ { name: "TofuPlainString", rootTrust: `root_trust = "insecure-tofu"`, - want: tofuRootTrust{}, + want: tufext.TofuRootTrust{}, }, { name: "TofuInlineTable", rootTrust: `root_trust = { type = "insecure-tofu" }`, - want: tofuRootTrust{}, + want: tufext.TofuRootTrust{}, }, { name: "BundledInlineTable", rootTrust: `root_trust = { type = "bundled", path = "/etc/root.json" }`, - want: bundledRootTrust{Path: "/etc/root.json"}, + want: tomlBundledRootTrust{Path: "/etc/root.json"}, }, { name: "BundledEmptyPath", rootTrust: `root_trust = { type = "bundled", path = "" }`, - want: bundledRootTrust{Path: ""}, + want: tomlBundledRootTrust{Path: ""}, }, { name: "InlineInlineTable", rootTrust: `root_trust = { type = "inline", "root.json" = '{"signed": {}}' }`, - want: inlineRootTrust{RootJSON: `{"signed": {}}`}, + want: tomlInlineRootTrust{RootJSON: `{"signed": {}}`}, }, { name: "InlineEmptyRootJSON", rootTrust: `root_trust = { type = "inline", "root.json" = "" }`, - want: inlineRootTrust{RootJSON: ""}, + want: tomlInlineRootTrust{RootJSON: ""}, }, { // Unlike bundled paths, inline root.json data is exempt from // %-expansion, so % sequences must be preserved verbatim. name: "InlinePercentNotExpanded", rootTrust: `root_trust = { type = "inline", "root.json" = '{"pct": "100%Z"}' }`, - want: inlineRootTrust{RootJSON: `{"pct": "100%Z"}`}, + want: tomlInlineRootTrust{RootJSON: `{"pct": "100%Z"}`}, }, } { t.Run(tc.name, func(t *testing.T) { @@ -202,7 +216,7 @@ type = "inline" require.Contains(t, conf.Repos, "example") require.NotNil(t, conf.Repos["example"].RootTrust) assert.Equal(t, - inlineRootTrust{RootJSON: "{\"signed\": {\"_type\": \"root\", \"version\": 1}}\n"}, + tomlInlineRootTrust{RootJSON: "{\"signed\": {\"_type\": \"root\", \"version\": 1}}\n"}, conf.Repos["example"].RootTrust.RootTrustSource) } @@ -320,10 +334,32 @@ root_trust = "insecure-tofu" `)) require.NoError(t, err) repo := conf.Repos["updates.example.com/alpha"] - require.NotNil(t, repo.MetaRootURL) - assert.Equal(t, "https://updates.example.com/alpha", repo.MetaRootURL.String()) - require.NotNil(t, repo.DataRootURL) - assert.Equal(t, "https://updates.example.com/alpha/targets", repo.DataRootURL.String()) + assert.Nil(t, repo.MetaRootURL) // not specified + assert.Nil(t, repo.DataRootURL) // not specified + metaRootURL, dataRootURL := repoURLs(t, repo) + assert.Equal(t, "https://updates.example.com/alpha", metaRootURL) + assert.Equal(t, "https://updates.example.com/alpha/targets", dataRootURL) +} + +// A repository name that is not a valid URL is only an error if the metadata +// URL has to be derived from it. +func TestParseConfig_DefaultMetaRootURL_InvalidName(t *testing.T) { + _, err := Parse(strings.NewReader(` +config_version = 1 +[repo."bad name"] +root_trust = "insecure-tofu" +`)) + require.ErrorContains(t, err, "invalid default url") + + conf, err := Parse(strings.NewReader(` +config_version = 1 +[repo."bad name"] +root_trust = "insecure-tofu" +meta_root_url = "https://example.com/meta" +`)) + require.NoError(t, err) + _, dataRootURL := repoURLs(t, conf.Repos["bad name"]) + assert.Equal(t, "https://example.com/meta/targets", dataRootURL) } func TestParseConfig_DefaultDataRootURL(t *testing.T) { @@ -336,8 +372,9 @@ meta_root_url = "https://meta.example.com/sub" require.NoError(t, err) repo := conf.Repos["example"] assert.Equal(t, "https://meta.example.com/sub", repo.MetaRootURL.String()) - require.NotNil(t, repo.DataRootURL) - assert.Equal(t, "https://meta.example.com/sub/targets", repo.DataRootURL.String()) + assert.Nil(t, repo.DataRootURL) // not specified + _, dataRootURL := repoURLs(t, repo) + assert.Equal(t, "https://meta.example.com/sub/targets", dataRootURL) } func TestParseConfig_CustomURLs(t *testing.T) { @@ -426,12 +463,13 @@ data_root_url = "https://beta.example.com/data" require.Contains(t, conf.Repos, "alpha") assert.Equal(t, "alpha", conf.Repos["alpha"].Name) - assert.Equal(t, tofuRootTrust{}, conf.Repos["alpha"].RootTrust.RootTrustSource) - assert.Equal(t, "https://alpha.example.com/targets", conf.Repos["alpha"].DataRootURL.String()) + assert.Equal(t, tufext.TofuRootTrust{}, conf.Repos["alpha"].RootTrust.RootTrustSource) + _, alphaDataRootURL := repoURLs(t, conf.Repos["alpha"]) + assert.Equal(t, "https://alpha.example.com/targets", alphaDataRootURL) require.Contains(t, conf.Repos, "beta") assert.Equal(t, "beta", conf.Repos["beta"].Name) - assert.Equal(t, bundledRootTrust{Path: "/etc/beta-root.json"}, conf.Repos["beta"].RootTrust.RootTrustSource) + assert.Equal(t, tomlBundledRootTrust{Path: "/etc/beta-root.json"}, conf.Repos["beta"].RootTrust.RootTrustSource) assert.Equal(t, "https://beta.example.com/data", conf.Repos["beta"].DataRootURL.String()) } @@ -495,7 +533,7 @@ func TestParseConfig_ExampleFile(t *testing.T) { assert.Nil(t, repo.RawOrderIndex) // not specified assert.Equal(t, int64(100), repo.OrderIndex()) assert.Equal(t, - bundledRootTrust{Path: `/usr/share/amutable/quarry/trusted/updates.example.com-base\x2dos-nightly-root.json`}, + tomlBundledRootTrust{Path: `/usr/share/amutable/quarry/trusted/updates.example.com-base\x2dos-nightly-root.json`}, repo.RootTrust.RootTrustSource) assert.Equal(t, "https://updates.example.com/update", repo.MetaRootURL.String()) assert.Equal(t, "https://updates.example.com/update", repo.DataRootURL.String()) @@ -503,21 +541,21 @@ func TestParseConfig_ExampleFile(t *testing.T) { // [tomlRootTrust.UnmarshalTOML]'s parser-dispatch loop relies on // [errWrongType] propagating from [parseTomlRootTrust] when the TOML data is -// tagged for a different [RootTrustSource] type. Pin that contract here. +// tagged for a different [tufext.RootTrustSource] type. Pin that contract here. func TestParseTomlRootTrust_WrongType(t *testing.T) { for _, tc := range []struct { name string - fn func(any) (RootTrustSource, error) + fn func(any) (tufext.RootTrustSource, error) data any }{ - {"TofuParser_BundledString", parseTomlRootTrust[tofuRootTrust], "bundled"}, - {"TofuParser_BundledTable", parseTomlRootTrust[tofuRootTrust], map[string]any{"type": "bundled", "path": "/x"}}, - {"BundledParser_TofuString", parseTomlRootTrust[bundledRootTrust], "insecure-tofu"}, - {"BundledParser_TofuTable", parseTomlRootTrust[bundledRootTrust], map[string]any{"type": "insecure-tofu"}}, - {"TofuParser_InlineTable", parseTomlRootTrust[tofuRootTrust], map[string]any{"type": "inline", "root.json": "{}"}}, - {"BundledParser_InlineTable", parseTomlRootTrust[bundledRootTrust], map[string]any{"type": "inline", "root.json": "{}"}}, - {"InlineParser_TofuString", parseTomlRootTrust[inlineRootTrust], "insecure-tofu"}, - {"InlineParser_BundledTable", parseTomlRootTrust[inlineRootTrust], map[string]any{"type": "bundled", "path": "/x"}}, + {"TofuParser_BundledString", parseTomlRootTrust[tufext.TofuRootTrust], "bundled"}, + {"TofuParser_BundledTable", parseTomlRootTrust[tufext.TofuRootTrust], map[string]any{"type": "bundled", "path": "/x"}}, + {"BundledParser_TofuString", parseTomlRootTrust[tomlBundledRootTrust], "insecure-tofu"}, + {"BundledParser_TofuTable", parseTomlRootTrust[tomlBundledRootTrust], map[string]any{"type": "insecure-tofu"}}, + {"TofuParser_InlineTable", parseTomlRootTrust[tufext.TofuRootTrust], map[string]any{"type": "inline", "root.json": "{}"}}, + {"BundledParser_InlineTable", parseTomlRootTrust[tomlBundledRootTrust], map[string]any{"type": "inline", "root.json": "{}"}}, + {"InlineParser_TofuString", parseTomlRootTrust[tomlInlineRootTrust], "insecure-tofu"}, + {"InlineParser_BundledTable", parseTomlRootTrust[tomlInlineRootTrust], map[string]any{"type": "bundled", "path": "/x"}}, } { t.Run(tc.name, func(t *testing.T) { _, err := tc.fn(tc.data) @@ -528,7 +566,7 @@ func TestParseTomlRootTrust_WrongType(t *testing.T) { func TestInlineRootTrust_FetchRoot(t *testing.T) { const rootJSON = `{"signed": {"_type": "root"}}` - data, err := inlineRootTrust{RootJSON: rootJSON}.FetchRoot(t.Context(), nil) + data, err := tomlInlineRootTrust{RootJSON: rootJSON}.FetchRoot(t.Context(), nil) require.NoError(t, err) assert.Equal(t, []byte(rootJSON), data) //nolint:testifylint // we are doing a direct byte-for-byte comparison here } @@ -613,8 +651,11 @@ data_root_url = "" repo := cfg.Repos["example.com/base-os"] require.NotNil(t, repo) - assert.Equal(t, "https://example.com/base-os", repo.MetaRootURL.String()) - assert.Equal(t, "https://example.com/base-os/targets", repo.DataRootURL.String()) + assert.Nil(t, repo.MetaRootURL) // reset to default + assert.Nil(t, repo.DataRootURL) // reset to default + metaRootURL, dataRootURL := repoURLs(t, repo) + assert.Equal(t, "https://example.com/base-os", metaRootURL) + assert.Equal(t, "https://example.com/base-os/targets", dataRootURL) } func TestMerge_EmptyURLResetsOnFirstDefinition(t *testing.T) { @@ -627,7 +668,9 @@ meta_root_url = "" repo := cfg.Repos["example.com/base-os"] require.NotNil(t, repo) - assert.Equal(t, "https://example.com/base-os", repo.MetaRootURL.String()) + assert.Nil(t, repo.MetaRootURL) // reset to default + metaRootURL, _ := repoURLs(t, repo) + assert.Equal(t, "https://example.com/base-os", metaRootURL) } func TestMerge_UnsetURLKeepsOverride(t *testing.T) { @@ -835,9 +878,10 @@ meta_root_url = "https://example.com/100%25-uptime/%2A/%aF" repo := conf.Repos["example"] require.NotNil(t, repo) assert.Equal(t, "https://example.com/100%25-uptime/%2A/%aF", repo.MetaRootURL.String()) - // data_root_url is defaulted from the expanded meta URL, so the %XX - // sequences pass through a second Expand pass. - assert.Equal(t, "https://example.com/100%25-uptime/%2A/%aF/targets", repo.DataRootURL.String()) + // data_root_url is derived from the expanded meta URL, so the %XX + // sequences must survive the URL join. + _, dataRootURL := repoURLs(t, repo) + assert.Equal(t, "https://example.com/100%25-uptime/%2A/%aF/targets", dataRootURL) } func TestParseConfig_Expand_URL_PerRepoR(t *testing.T) { @@ -895,7 +939,7 @@ meta_root_url = "https://example.com" require.NoError(t, err) repo := conf.Repos["example.com/foo"] require.NotNil(t, repo) - bundled, ok := repo.RootTrust.RootTrustSource.(bundledRootTrust) + bundled, ok := repo.RootTrust.RootTrustSource.(tomlBundledRootTrust) require.True(t, ok) if tc.wantPathRe != "" { assert.Regexp(t, tc.wantPathRe, bundled.Path) @@ -1095,14 +1139,14 @@ cache_dir = "%m"`, func TestRootTrustSource_String(t *testing.T) { for _, tc := range []struct { name string - src RootTrustSource + src tufext.RootTrustSource want string }{ - {"Tofu", tofuRootTrust{}, "insecure-tofu"}, - {"Bundled", bundledRootTrust{Path: "/etc/root.json"}, "bundled:/etc/root.json"}, - {"BundledEmptyPath", bundledRootTrust{}, "bundled:"}, - {"Inline", inlineRootTrust{RootJSON: `{"a": 1}`}, `inline:"{\"a\": 1}"`}, - {"InlineEmpty", inlineRootTrust{}, `inline:""`}, + {"Tofu", tufext.TofuRootTrust{}, "insecure-tofu"}, + {"Bundled", tomlBundledRootTrust{Path: "/etc/root.json"}, "bundled:/etc/root.json"}, + {"BundledEmptyPath", tomlBundledRootTrust{}, "bundled:"}, + {"Inline", tomlInlineRootTrust{RootJSON: `{"a": 1}`}, `inline:"{\"a\": 1}"`}, + {"InlineEmpty", tomlInlineRootTrust{}, `inline:""`}, } { t.Run(tc.name, func(t *testing.T) { assert.Equal(t, tc.want, tc.src.String()) diff --git a/internal/tufext/repo.go b/internal/tufext/repo.go new file mode 100644 index 0000000..a4b76cd --- /dev/null +++ b/internal/tufext/repo.go @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package tufext + +import ( + "fmt" + "net/url" +) + +// Repository represents a TUF repository that [tufclient] fetches. +type Repository struct { + // Name is the "logical" name of the repository, which uniquely identifies + // the repository and thus must remain unchanged for the life of the + // repository. In [tufclient/config]'s TOML configuration, this is + // generated based on the repos."..." table keys. + Name string + + // RootTrust indicates the source of trust for the initial root.json of + // this repository (if the local cache already has a root.json, this source + // is ignored). + RootTrust RootTrustSource + + // MetaRootURL is the base URL for the directory containing TUF metadata. + // Users should prefer [Repository.RootURL], which handles the default when + // this is nil. + // + // TODO: This should be more flexible than a static string, it should be + // possible for this to be auto-updated based on tags or other + // configuration. + MetaRootURL *url.URL + + // DataRootURL is the base URL for the directory containing target data + // files. Users should prefer [Repository.DataURL], which handles the + // default when this is nil. + // + // TODO: This should be more flexible than a static string, it should be + // possible for this to be auto-updated based on tags or other + // configuration. + DataRootURL *url.URL +} + +// RootURL returns [Repository.MetaRootURL] joined with elem (as with +// [url.URL.JoinPath]). If unset, the metadata URL defaults to "https://" + +// [Repository.Name]. +func (repo *Repository) RootURL(elems ...string) (*url.URL, error) { + rootURL := repo.MetaRootURL + if rootURL == nil { + u, err := url.Parse("https://" + repo.Name) + if err != nil { + return nil, fmt.Errorf("derive meta_root_url from repository name %q: %w", repo.Name, err) + } + rootURL = u + } + return rootURL.JoinPath(elems...), nil +} + +// DataURL returns [Repository.DataRootURL] joined with elem (as with +// [url.URL.JoinPath]). If unset, the targets URL defaults to the "targets" +// subdirectory of [Repository.RootURL]. +func (repo *Repository) DataURL(elems ...string) (*url.URL, error) { + rootURL := repo.DataRootURL + if rootURL == nil { + u, err := repo.RootURL("targets") + if err != nil { + return nil, fmt.Errorf("derive data_root_url: %w", err) + } + rootURL = u + } + return rootURL.JoinPath(elems...), nil +} + +// RepositoryLike is implemented by types that are a more specialised form of +// [Repository] but can be represented as the generic [Repository]. +type RepositoryLike interface { + AsRepository() *Repository +} diff --git a/internal/tufext/repo_test.go b/internal/tufext/repo_test.go new file mode 100644 index 0000000..728fbd7 --- /dev/null +++ b/internal/tufext/repo_test.go @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package tufext_test + +import ( + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.amutable.dev/quarry/internal/tufext" +) + +func mustParseURL(t *testing.T, s string) *url.URL { + t.Helper() + u, err := url.Parse(s) + require.NoError(t, err) + return u +} + +func TestRepository_URLs(t *testing.T) { + for _, tc := range []struct { + name string + repo tufext.Repository + wantMetaRoot string + wantDataRoot string + }{ + { + name: "Defaults", + repo: tufext.Repository{Name: "updates.example.com/alpha"}, + wantMetaRoot: "https://updates.example.com/alpha", + wantDataRoot: "https://updates.example.com/alpha/targets", + }, + { + name: "MetaRootURL", + repo: tufext.Repository{ + Name: "alpha", + MetaRootURL: mustParseURL(t, "https://meta.example.com/sub"), + }, + wantMetaRoot: "https://meta.example.com/sub", + wantDataRoot: "https://meta.example.com/sub/targets", + }, + { + name: "BothURLs", + repo: tufext.Repository{ + Name: "alpha", + MetaRootURL: mustParseURL(t, "https://meta.example.com/sub"), + DataRootURL: mustParseURL(t, "https://data.example.com/blobs"), + }, + wantMetaRoot: "https://meta.example.com/sub", + wantDataRoot: "https://data.example.com/blobs", + }, + } { + t.Run(tc.name, func(t *testing.T) { + metaRootURL, err := tc.repo.RootURL() + require.NoError(t, err) + assert.Equal(t, tc.wantMetaRoot, metaRootURL.String()) + dataRootURL, err := tc.repo.DataURL() + require.NoError(t, err) + assert.Equal(t, tc.wantDataRoot, dataRootURL.String()) + + rootURL, err := tc.repo.RootURL("1.root.json") + require.NoError(t, err) + assert.Equal(t, tc.wantMetaRoot+"/1.root.json", rootURL.String()) + targetURL, err := tc.repo.DataURL("foo", "bar.txt") + require.NoError(t, err) + assert.Equal(t, tc.wantDataRoot+"/foo/bar.txt", targetURL.String()) + }) + } +} + +func TestRepository_URLs_InvalidName(t *testing.T) { + repo := tufext.Repository{Name: "bad name"} + _, err := repo.RootURL() + require.Error(t, err) + _, err = repo.DataURL() + require.Error(t, err) + + // The name is only parsed when the metadata URL has to be derived from it. + repo.MetaRootURL = mustParseURL(t, "https://meta.example.com/sub") + dataRootURL, err := repo.DataURL() + require.NoError(t, err) + assert.Equal(t, "https://meta.example.com/sub/targets", dataRootURL.String()) +} diff --git a/internal/tufext/root_trust.go b/internal/tufext/root_trust.go new file mode 100644 index 0000000..9fc7ae4 --- /dev/null +++ b/internal/tufext/root_trust.go @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package tufext + +import ( + "context" + "encoding/json" + "fmt" + "io" + "io/fs" + "maps" + "net/http" + "slices" + + "go.amutable.dev/quarry/internal/serde" + "go.amutable.dev/quarry/internal/third_party/funchelpers" +) + +// TODO: Make these more configurable. +const ( + MaxRootBytes = 512_000 // 512k +) + +// RootTrustSource represents a source of trust for the initial state of a +// client's locally cached root.json. +type RootTrustSource interface { + // Type returns the self-defining string used when generically parsing + // [RootTrustSource] from formats like JSON and TOML. + Type() string + fmt.Stringer + + // FromMap is a helper method called from parsers to fill [RootTrustSource] + // based on a pre-parsed map[string]any. + // + // We do not implement generic unmarshallers because when parsing a + // [RootTrustSource] you invariably need a wrapper structure to select the + // right underlying implementation. Types that wish to permit + // name-only definitions in configuration files need to accept empty maps + // to [FromMap]. + // + // Unfortunately, we cannot do this generically (i.e., there doesn't appear + // to be a way to have a generic requirement to operate on a type whose + // pointer implements an interface) so we need to return a + // [RootTrustSource] (which is a copy of the object itself). + // + // TODO: Move this to internal/serde and make it more generic. + FromMap(data map[string]any) (RootTrustSource, error) + + // TODO: We should have a ToMap that can be used to do somewhat generic + // serialisation in internal/serde? + + // IsRemote indicates whether the root source is to be fetched from a + // remote resource. Callers can use this as a hint for whether some errors + // from [FetchRoot] should be skipped. + IsRemote() bool + + // FetchRoot fetches the initial root.json for the given [Repository], + // based on the internal policy of this [RootTrustSource]. + FetchRoot(ctx context.Context, repo *Repository) ([]byte, error) +} + +// TofuRootTrust fetches the root.json directly from the repository with a +// trust-on-first-use policy. *This is inherently insecure*. +type TofuRootTrust struct { + // TODO: UnrecognizedFields? +} + +var _ RootTrustSource = &TofuRootTrust{} + +// Type returns the self-defining string used when generically parsing +// [RootTrustSource] from formats like JSON and TOML. +func (TofuRootTrust) Type() string { return "insecure-tofu" } + +func (t TofuRootTrust) String() string { return t.Type() } + +// FromMap is a helper method called from parsers to fill [RootTrustSource] +// based on a pre-parsed map[string]any. +func (t TofuRootTrust) FromMap(data map[string]any) (RootTrustSource, error) { + if len(data) > 0 { + return nil, fmt.Errorf("unsupported fields: %v", slices.Collect(maps.Keys(data))) + } + return t, nil +} + +// IsRemote indicates whether the root source is to be fetched from a remote +// resource. It is always true for [TofuRootTrust]. +func (TofuRootTrust) IsRemote() bool { return true } + +// FetchRoot for [TofuRootTrust] fetches the initial root.json directly from +// the given repository without any validation. +func (TofuRootTrust) FetchRoot(ctx context.Context, repo *Repository) (_ []byte, Err error) { + // The updater will bump the root.json to the latest version afterwards. + rootURL, err := repo.RootURL("1.root.json") + if err != nil { + return nil, err + } + + req, err := http.NewRequestWithContext(ctx, "GET", rootURL.String(), nil) + if err != nil { + return nil, fmt.Errorf("create http request: %w", err) + } + req.Header.Set("Accept", "application/json") + + client := http.DefaultClient + res, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("fetch %s: %w", rootURL, err) + } + if res.StatusCode >= 300 { + if res.Body != nil { + _ = res.Body.Close() + } + err := fmt.Errorf("fetch %s failed with status code %.3d", rootURL, res.StatusCode) + if res.StatusCode == http.StatusNotFound { + // Emulate ENOENT for 404. + err = fmt.Errorf("%w: %w", err, fs.ErrNotExist) + } + return nil, err + } + rdr := http.MaxBytesReader(nil, res.Body, MaxRootBytes) // use same max as client + defer funchelpers.VerifyClose(&Err, rdr) + + return io.ReadAll(rdr) +} + +// InlineRootTrust is used for cases where it makes sense to embed the +// root.json directly inside some structure or configuration rather than +// referencing some external value. +type InlineRootTrust struct { + RootJSON json.RawMessage `json:"root.json"` + // TODO: UnrecognizedFields? +} + +var _ RootTrustSource = InlineRootTrust{} + +// Type returns the self-defining string used when generically parsing +// [RootTrustSource] from formats like JSON and TOML. +func (t InlineRootTrust) Type() string { return "inline" } + +func (t InlineRootTrust) String() string { return fmt.Sprintf("%s:%q", t.Type(), t.RootJSON) } + +// IsRemote indicates whether the root source is to be fetched from a remote +// resource. It is always false for [InlineRootTrust]. +func (InlineRootTrust) IsRemote() bool { return false } + +// FromMap is a helper method called from parsers to fill [RootTrustSource] +// based on a pre-parsed map[string]any. +func (t InlineRootTrust) FromMap(data map[string]any) (RootTrustSource, error) { + if err := serde.ParseMapKey(data, "root.json", &t.RootJSON); err != nil { + return nil, err + } + if len(data) > 0 { + return nil, fmt.Errorf("unsupported fields: %v", slices.Collect(maps.Keys(data))) + } + return t, nil +} + +// FetchRoot for [InlineRootTrust] returns the inlined root.json data and +// cannot return an error. +func (t InlineRootTrust) FetchRoot(_ context.Context, _ *Repository) ([]byte, error) { + return []byte(t.RootJSON), nil +} diff --git a/internal/xsysupdate/transfer_test.go b/internal/xsysupdate/transfer_test.go index d68f856..f6e670c 100644 --- a/internal/xsysupdate/transfer_test.go +++ b/internal/xsysupdate/transfer_test.go @@ -26,7 +26,7 @@ import ( "go.amutable.dev/quarry/internal/ctxext" "go.amutable.dev/quarry/internal/testrepo" "go.amutable.dev/quarry/internal/tufclient" - "go.amutable.dev/quarry/internal/tufclient/config" + "go.amutable.dev/quarry/internal/tufext" ) var fixedRefTime = time.Date(2026, 5, 24, 12, 0, 0, 0, time.UTC) @@ -99,14 +99,14 @@ func initExt(ctx context.Context, t *testing.T) *TransferFileExtension { return ext } -// makeRepo returns the [config.Repository] backed by the given test -// repository server. -func makeRepo(t *testing.T, srv *testrepo.Server, name string) *config.Repository { +// makeRepo returns the [tufext.Repository] backed by the given test repository +// server. +func makeRepo(t *testing.T, srv *testrepo.Server, name string) *tufext.Repository { t.Helper() cfg := testrepo.Config(t, srv.ConfigBlock(name)) repo, ok := cfg.Repos[name] require.True(t, ok, "testrepo config did not yield repository %q", name) - return repo + return repo.AsRepository() } func TestPatchTransferFile_OverrideSourcePath(t *testing.T) { From fa74d5cb29b3aebe4c25b0529ea744e0b0f13b4c Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 9 Apr 2026 19:13:21 +1000 Subject: [PATCH 7/8] [wip] tufext: add basic cross-repo link structure Signed-off-by: Aleksa Sarai --- internal/tufext/links.go | 161 ++++++++++++++++++++++++++++++++++ internal/tufext/repo.go | 4 +- internal/tufext/root_trust.go | 3 +- 3 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 internal/tufext/links.go diff --git a/internal/tufext/links.go b/internal/tufext/links.go new file mode 100644 index 0000000..6ace3bd --- /dev/null +++ b/internal/tufext/links.go @@ -0,0 +1,161 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright (C) 2026 Amutable GmbH + +package tufext + +import ( + "bytes" + "encoding/json" + "fmt" + "slices" + + "go.amutable.dev/quarry/internal/jsonutils" +) + +/* + { + "name": "updates.example.com/foobar", + "root.json": {...}, + "snapshot-pin": "insecure-latest", + "snapshot-pin": {"type": "insecure-latest"}, + "snapshot-pin": {"type": "version", "want": "exact", version: ...}, + "snapshot-pin": {"type": "version", "want": "at-least", version: ...}, + "snapshot-pin": {"type": "hash", "version": ..., "hashes": {}, "length": ...}, + "snapshot-pin": {"type": "inline", "snapshot.json": {...}}, + "urls": [ + {}, + ], + } +*/ + +// SnapshotPin represents a type of version pinning in [RepoLink] for +// publishers to alleviate mix-and-match attacks against links to varying +// degrees. +type SnapshotPin struct { + // TODO: Create a properly type-driven generic impl like RootTrustSource. +} + +const insecurePin = `"insecure-latest"` + +// UnmarshalJSON implements [json.Unmarshaler]. +func (*SnapshotPin) UnmarshalJSON(data []byte) error { + if !bytes.Equal(data, []byte(insecurePin)) { + return fmt.Errorf("snapshot-pin currently only supports %s", insecurePin) + } + return nil +} + +// MarshalJSON implements [json.Marshaler]. +func (SnapshotPin) MarshalJSON() ([]byte, error) { + return []byte(insecurePin), nil +} + +// RepoLink is a representation of a cross-repository link, a TUF extension +// used by Quarry to allow for linking disparate repositories together. +type RepoLink struct { + // Name is the logical name of the repository. For public repositories, it + // is the base URL where the repository metafiles can be located. This is + // primarily used by clients to identify their local cached copy of the + // repository metadata to avoid rollback attacks when repository links are + // added or removed. + Name string `json:"repo"` + + // SnapshotPin specifies the mechanism and strictness of pinning the link + // based on the target repo's snapshot. + // + // Clients that have a local cache of this repository and have seen a newer + // snapshot version must still reject downgrades, despite the existence of + // this link. + SnapshotPin SnapshotPin `json:"snapshot-pin"` + + // Root is an embedded copy of the latest root role data for the repository + // at the time the link was created. + // + // Clients that have a local cache of this repository should still follow + // the TUF specification's algorithm for updating their local root state. + // This field is only intended for bootstrapping trust for clients that + // have never seen this repository before (and do not otherwise have some + // more authoritative source for the repo's root trust data), though + // clients should verify that the root role data embedded here matches + // their local copy if they upgrade to it. + RootJSON json.RawMessage `json:"root.json"` + + // UnrecognizedFields contains any extension fields that this + // implementation does not know about. They are stored as raw JSON so that + // they survive a decode-encode round-trip untouched. + // + // An entry whose name collides with one of the fields above is dropped + // when encoding -- the typed field always wins, even when it is unset and + // thus not emitted at all. Names are compared case-insensitively, as that + // is how [encoding/json] matches them when decoding. + UnrecognizedFields map[string]json.RawMessage `json:"-"` +} + +// AsRepository maps a [RepoLink] to a [Repository] so it can be used for other +// purposes. +func (link RepoLink) AsRepository() *Repository { + return &Repository{ + Name: link.Name, + RootTrust: InlineRootTrust{RootJSON: link.RootJSON}, + } +} + +// MarshalJSON implements [json.Marshaler], emitting [RepoLink.UnrecognizedFields] +// alongside the fields this implementation knows about. +func (link RepoLink) MarshalJSON() ([]byte, error) { + type knownFields RepoLink // shed the methods to avoid recursing forever + return jsonutils.MarshalExtensible(knownFields(link), link.UnrecognizedFields) +} + +// UnmarshalJSON implements [json.Unmarshaler], collecting every field this +// implementation does not know about into [RepoLink.UnrecognizedFields]. +func (link *RepoLink) UnmarshalJSON(data []byte) error { + type knownFields RepoLink // shed the methods to avoid recursing forever + known, extensions, err := jsonutils.UnmarshalExtensible[knownFields](data) + if err != nil { + return err + } + *link = RepoLink(known) + link.UnrecognizedFields = extensions + return nil +} + +type targetsExt struct { + *SignedTargets +} + +// RepoLinks is the extension data type for [RepoLinkField], stored in +// [tufmetadata.TargetsType] (see [targetsExt.RepoLinks]). +type RepoLinks []RepoLink + +// RepoLinkField is the cross-repository link extension field that holds the +// [RepoLinks] information in [tufmetadata.TargetsType] role data (see +// [targetsExt.RepoLinks]). +const RepoLinkField = "x-quarry-links" + +// WithRepoLinks replaces the list of RepoLinks for the target role with the +// given slice. +func (t targetsExt) WithRepoLinks(links RepoLinks) targetsExt { + _, err := jsonutils.SetExtensionJSON(&t.SignedTargets.Signed, RepoLinkField, links) + if err != nil { + panic(err) // programmer error + } + return t +} + +// RepoLinks returns the list of [RepoLink]s for this target role, or nil if +// none was set. +func (t targetsExt) RepoLinks() (*RepoLinks, error) { + // TODO: Cache this....? + return jsonutils.GetExtensionJSON[RepoLinks](t.SignedTargets.Signed, RepoLinkField) +} + +// WithRepoLink appends a [RepoLink] to the target role, and is shorthand for +// [targetsExt.WithRepoLinks]. +func (t targetsExt) WithRepoLink(link RepoLink) targetsExt { + var links RepoLinks + if oldLinks, err := t.RepoLinks(); err == nil && oldLinks != nil { + links = slices.Clone(*oldLinks) + } + return t.WithRepoLinks(append(links, link)) +} diff --git a/internal/tufext/repo.go b/internal/tufext/repo.go index a4b76cd..3d13bd0 100644 --- a/internal/tufext/repo.go +++ b/internal/tufext/repo.go @@ -8,7 +8,9 @@ import ( "net/url" ) -// Repository represents a TUF repository that [tufclient] fetches. +// Repository represents a TUF repository that [tufclient] fetches. It may have +// been explicitly configured with [tufclient/config] or may have been derived +// from a [RepoLink]. type Repository struct { // Name is the "logical" name of the repository, which uniquely identifies // the repository and thus must remain unchanged for the life of the diff --git a/internal/tufext/root_trust.go b/internal/tufext/root_trust.go index 9fc7ae4..84741cf 100644 --- a/internal/tufext/root_trust.go +++ b/internal/tufext/root_trust.go @@ -126,7 +126,8 @@ func (TofuRootTrust) FetchRoot(ctx context.Context, repo *Repository) (_ []byte, // InlineRootTrust is used for cases where it makes sense to embed the // root.json directly inside some structure or configuration rather than -// referencing some external value. +// referencing some external value (the most obvious examples being [RepoLink] +// and drop-in config fragments). type InlineRootTrust struct { RootJSON json.RawMessage `json:"root.json"` // TODO: UnrecognizedFields? From 36761b7f8fcab7fd727338802a55c459aceed528 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 24 Sep 2026 19:19:28 +1000 Subject: [PATCH 8/8] wip Signed-off-by: Aleksa Sarai --- internal/tufclient/client.go | 28 +++++++++++++++++++++++++--- internal/tufext/iter_roles.go | 7 +++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/internal/tufclient/client.go b/internal/tufclient/client.go index cdf68b4..15f1d8c 100644 --- a/internal/tufclient/client.go +++ b/internal/tufclient/client.go @@ -215,11 +215,32 @@ func (client *Client) WithRepos(repoNames ...string) error { return nil } +// Repository is the [Client] representation of a [tufext.Repository]. +type Repository struct { + *tufupdater.Updater + chains []tufext.RoleDelegationChain +} + +// IsTargetPermitted returns whether the delegation chain taken to reach this +// [Repository] (this can only return "false" if it was reached via +// [tufext.RepoLink]). +func (repo Repository) IsTargetPermitted(targetPath string) bool { + for _, chain := range repo.chains { + if !chain.IsTargetPermitted(targetPath) { + return false + } + } + return true +} + // IterRepos returns an iterator over the set of repositories in the [Client], // the order is always consistent for a given configuration and is based on the -// repository order index (and name as a tie-breaker). Note that use of this -// operation directly is very rarely necessary, most of the time -// [GetTargetInfo] and [FetchTargetFile] are more ergonomic. +// repository order index (and name as a tie-breaker). If repository contains a +// [tufext.RepoLink] extension, [IterRepos] will iterate over those +// repositories too. +// +// Note that use of this operation directly is very rarely necessary, most of +// the time [GetTargetInfo] and [FetchTargetFile] are more ergonomic. // // TODO: Return some custom type? func (client *Client) IterRepos(_ context.Context) iter.Seq2[string, *tufupdater.Updater] { @@ -233,6 +254,7 @@ func (client *Client) IterRepos(_ context.Context) iter.Seq2[string, *tufupdater cmp.Compare(repoA, repoB), // name is for tie-breaks ) }) + // The active repos set is the starting point of our repo iteration. for _, name := range order { if _, ok := client.activeRepos[name]; !ok { continue diff --git a/internal/tufext/iter_roles.go b/internal/tufext/iter_roles.go index c06e410..f763957 100644 --- a/internal/tufext/iter_roles.go +++ b/internal/tufext/iter_roles.go @@ -65,9 +65,12 @@ func (chain DelegationChain) Contains(roleName string) bool { // appended, indicating that the delegation came from the given role. The // receiver is left untouched. This function will panic if several delegations // from the same role are appended. -func (chain DelegationChain) Extend(fromRole string, delegation *tufmetadata.DelegatedRole) DelegationChain { +func (chain DelegationChain) Extend(fromRole string, delegations ...*tufmetadata.DelegatedRole) DelegationChain { clone := chain.clone() - if delegation != nil { + for _, delegation := range delegations { + if delegation == nil { + continue + } assert.Assertf(!chain.Contains(fromRole), "DelegationChain must not have the same role %q inserted multiple times", fromRole) if clone.links == nil {