Skip to content

Commit 60e9381

Browse files
committed
fix(identity): bound fan-out on the audit-info collection path
GetAuditInfo in multisig and boolpolicy accounted for one level of composite-identity nesting but never ran validateComponentIdentities, so the component-count bound - and with it the duplicate and none-component checks - applied to verifier deserialization and matcher construction but not to audit-info collection. The depth bound does not cover that case: a single level fanning out to thousands of components is one recursive step that resolves one audit-info lookup per component, and descends again for any composite component. A 20 KB multisig identity with 10,000 components drove 10,001 provider lookups with the component bound configured at 4. The path is reachable from remote input during recipient registration. Route both through the same choke point the other chains use, and cover the chain in the nesting tests for depth and for fan-out at limit and limit+1. Signed-off-by: AkramBitar <akram@il.ibm.com>
1 parent e9bc78c commit 60e9381

5 files changed

Lines changed: 176 additions & 2 deletions

File tree

docs/drivers/validation-resource-limits.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,12 @@ recipient, an auditor inspecting a request, tests — and those **still get the
223223
running unbounded, so a seeding site added later and forgotten weakens the bound to the default
224224
instead of disabling it.
225225

226+
The fan-out bound is applied on every chain above that walks a component list — verifier
227+
deserialization, matcher construction and audit-info collection — through the same
228+
`validateComponentIdentities` choke point, which also rejects an empty or duplicated component. The
229+
matcher-evaluation chain needs no separate check: its component count is fixed by the matcher tree
230+
built during construction, which was already bounded.
231+
226232
The fan-out bound is also applied on the honest-caller path, in `multisig.WrapIdentities` and
227233
`boolpolicy.WrapPolicyIdentity`, so an identity constructed in-process cannot exceed what a validator
228234
will later accept.
@@ -269,6 +275,8 @@ of the defaults. If a deployment needs a different limit:
269275
panic and the expected typed error at every boundary. `fabtoken driver.FuzzOwnerVerifierNoPanic`
270276
additionally fuzzes the owner-identity deserialization path itself — the one reached from the
271277
transfer validator once per input token — seeded with identities nested from 1 to 600 levels deep.
272-
Each target has a persisted seed corpus under its package's `testdata/fuzz/<TargetName>/` covering
273-
every default's boundary, and runs nightly via
278+
The three resource-dimension targets each have a persisted seed corpus under their package's
279+
`testdata/fuzz/<TargetName>/` covering every default's boundary; `FuzzOwnerVerifierNoPanic` seeds
280+
from code only, since its interesting shapes are generated (nesting depth) rather than enumerated.
281+
All of them run nightly via
274282
[`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).

token/services/identity/boolpolicy/deserializer.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ func (d *TypedIdentityDeserializer) GetAuditInfo(ctx context.Context, id driver.
7474
if err = pi.Deserialize(rawIdentity); err != nil {
7575
return nil, errors.Wrapf(err, "failed to unmarshal policy identity")
7676
}
77+
// the fan-out bound has to hold here too: this path resolves the audit info of every component
78+
// in turn, which is one provider lookup each and, for a nested composite component, a further
79+
// descent. The depth bound above does not cover a single level that fans out without limit.
80+
if err = validateComponentIdentities(pi.Identities, driver.MaxIdentityComponentsFrom(ctx)); err != nil {
81+
return nil, errors.Wrap(err, "invalid policy identity")
82+
}
7783
ai := &AuditInfo{IdentityAuditInfos: make([]IdentityAuditInfo, len(pi.Identities))}
7884
for k, compID := range pi.Identities {
7985
ai.IdentityAuditInfos[k].AuditInfo, err = p.GetAuditInfo(ctx, compID)

token/services/identity/boolpolicy/nesting_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,85 @@ func TestWrapPolicyIdentity_RejectsExcessiveFanOut(t *testing.T) {
235235
_, err = WrapPolicyIdentity("$0", toIdentities(distinctIdentities(max+1))...)
236236
require.ErrorIs(t, err, tdriver.ErrTooManyIdentityComponents)
237237
}
238+
239+
// stubAuditInfoProvider is a driver.AuditInfoProvider that records its lookups and, optionally,
240+
// resolves a composite identity by re-entering the deserializer the way the production provider does.
241+
type stubAuditInfoProvider struct {
242+
calls int
243+
fn func(context.Context, tdriver.Identity) ([]byte, error)
244+
}
245+
246+
func (s *stubAuditInfoProvider) GetAuditInfo(ctx context.Context, id tdriver.Identity) ([]byte, error) {
247+
s.calls++
248+
if s.fn != nil {
249+
return s.fn(ctx, id)
250+
}
251+
252+
return nil, nil
253+
}
254+
255+
// The fan-out bound must be enforced on the audit-info collection path too. It resolves one provider
256+
// lookup per component, and a composite component descends further, so a single level that fans out
257+
// without limit is unbounded work that the depth bound does not cover.
258+
func TestGetAuditInfo_RejectsExcessiveFanOut(t *testing.T) {
259+
const maxComponents = 4
260+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), 5, maxComponents)
261+
262+
p := &stubAuditInfoProvider{}
263+
d := NewTypedIdentityDeserializer(nil, nil)
264+
265+
raw, err := (&PolicyIdentity{Policy: "$0", Identities: distinctIdentities(maxComponents + 1)}).Bytes()
266+
require.NoError(t, err)
267+
268+
_, err = d.GetAuditInfo(ctx, token.Identity("owner"), Policy, raw, p)
269+
require.ErrorIs(t, err, tdriver.ErrTooManyIdentityComponents)
270+
assert.Equal(t, 1, p.calls,
271+
"only the cache probe for the composite identity itself may run before the bound is enforced")
272+
}
273+
274+
func TestGetAuditInfo_AllowsFanOutAtTheLimit(t *testing.T) {
275+
const maxComponents = 4
276+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), 5, maxComponents)
277+
278+
p := &stubAuditInfoProvider{}
279+
d := NewTypedIdentityDeserializer(nil, nil)
280+
281+
raw, err := (&PolicyIdentity{Policy: "$0", Identities: distinctIdentities(maxComponents)}).Bytes()
282+
require.NoError(t, err)
283+
284+
_, err = d.GetAuditInfo(ctx, token.Identity("owner"), Policy, raw, p)
285+
require.NoError(t, err)
286+
assert.Equal(t, 1+maxComponents, p.calls)
287+
}
288+
289+
// The depth bound holds on the audit-info collection path as well: the provider resolves an unstored
290+
// composite component by re-entering this deserializer, which is the recursion the bound covers.
291+
func TestGetAuditInfo_RejectsExcessiveDepth(t *testing.T) {
292+
const maxDepth = 3
293+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), maxDepth, 16)
294+
295+
d := NewTypedIdentityDeserializer(nil, nil)
296+
p := &stubAuditInfoProvider{}
297+
// the guard keeps the cache probe for an identity already being resolved from looping back into
298+
// it; production has the same shape, the provider only descends into components
299+
resolving := map[string]bool{}
300+
p.fn = func(ctx context.Context, id tdriver.Identity) ([]byte, error) {
301+
if resolving[string(id)] {
302+
return nil, nil
303+
}
304+
ti, err := identity.UnmarshalTypedIdentity(id)
305+
if err != nil || ti.Type != Policy {
306+
return nil, nil //nolint:nilerr // a non-policy component is the leaf, nothing stored
307+
}
308+
resolving[string(id)] = true
309+
310+
return d.GetAuditInfo(ctx, id, Policy, ti.Identity, p)
311+
}
312+
313+
deep := nestedPolicy(t, maxDepth+2, []byte("leaf"))
314+
ti, err := identity.UnmarshalTypedIdentity(deep)
315+
require.NoError(t, err)
316+
317+
_, err = d.GetAuditInfo(ctx, deep, Policy, ti.Identity, p)
318+
require.ErrorIs(t, err, tdriver.ErrIdentityNestingTooDeep)
319+
}

token/services/identity/multisig/deserializer.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,12 @@ func (d *TypedIdentityDeserializer) GetAuditInfo(ctx context.Context, id driver.
7474
if err != nil {
7575
return nil, errors.Wrapf(err, "failed to unmarshal mid")
7676
}
77+
// the fan-out bound has to hold here too: this path resolves the audit info of every component
78+
// in turn, which is one provider lookup each and, for a nested composite component, a further
79+
// descent. The depth bound above does not cover a single level that fans out without limit.
80+
if err := validateComponentIdentities(mid.Identities, driver.MaxIdentityComponentsFrom(ctx)); err != nil {
81+
return nil, errors.Wrap(err, "invalid multisig identity")
82+
}
7783
auditInfo := &AuditInfo{}
7884
auditInfo.IdentityAuditInfos = make([]IdentityAuditInfo, len(mid.Identities))
7985
for k, identity := range mid.Identities {

token/services/identity/multisig/nesting_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,3 +230,75 @@ func TestWrapIdentities_RejectsExcessiveFanOut(t *testing.T) {
230230
_, err = WrapIdentities(distinctIdentities(max + 1)...)
231231
require.ErrorIs(t, err, tdriver.ErrTooManyIdentityComponents)
232232
}
233+
234+
// The fan-out bound must be enforced on the audit-info collection path too. It resolves one provider
235+
// lookup per component, and a composite component descends further, so a single level that fans out
236+
// without limit is unbounded work that the depth bound does not cover.
237+
func TestGetAuditInfo_RejectsExcessiveFanOut(t *testing.T) {
238+
const maxComponents = 4
239+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), 5, maxComponents)
240+
241+
p := &mock.AuditInfoProvider{}
242+
// nothing stored for the composite identity itself, so it is assembled from its components
243+
p.GetAuditInfoReturns([]byte("audit-info"), nil)
244+
p.GetAuditInfoReturnsOnCall(0, nil, nil)
245+
246+
d := NewTypedIdentityDeserializer(nil, nil)
247+
raw, err := (&MultiIdentity{Identities: distinctIdentities(maxComponents + 1)}).Bytes()
248+
require.NoError(t, err)
249+
250+
_, err = d.GetAuditInfo(ctx, token.Identity("owner"), Multisig, raw, p)
251+
require.ErrorIs(t, err, tdriver.ErrTooManyIdentityComponents)
252+
assert.Equal(t, 1, p.GetAuditInfoCallCount(),
253+
"only the cache probe for the composite identity itself may run before the bound is enforced")
254+
}
255+
256+
func TestGetAuditInfo_AllowsFanOutAtTheLimit(t *testing.T) {
257+
const maxComponents = 4
258+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), 5, maxComponents)
259+
260+
p := &mock.AuditInfoProvider{}
261+
p.GetAuditInfoReturns([]byte("audit-info"), nil)
262+
p.GetAuditInfoReturnsOnCall(0, nil, nil)
263+
264+
d := NewTypedIdentityDeserializer(nil, nil)
265+
raw, err := (&MultiIdentity{Identities: distinctIdentities(maxComponents)}).Bytes()
266+
require.NoError(t, err)
267+
268+
_, err = d.GetAuditInfo(ctx, token.Identity("owner"), Multisig, raw, p)
269+
require.NoError(t, err)
270+
assert.Equal(t, 1+maxComponents, p.GetAuditInfoCallCount())
271+
}
272+
273+
// The depth bound holds on the audit-info collection path as well: the provider resolves an unstored
274+
// composite component by re-entering this deserializer, which is the recursion the bound covers.
275+
func TestGetAuditInfo_RejectsExcessiveDepth(t *testing.T) {
276+
const maxDepth = 3
277+
ctx := tdriver.WithIdentityNestingLimits(context.Background(), maxDepth, 16)
278+
279+
p := &mock.AuditInfoProvider{}
280+
d := NewTypedIdentityDeserializer(nil, nil)
281+
// a provider that has nothing stored and resolves a composite identity through the deserializer.
282+
// The guard keeps the cache probe for an identity already being resolved from looping back into
283+
// it; production has the same shape, the provider only descends into components.
284+
resolving := map[string]bool{}
285+
p.GetAuditInfoCalls(func(ctx context.Context, id tdriver.Identity) ([]byte, error) {
286+
if resolving[string(id)] {
287+
return nil, nil
288+
}
289+
ti, err := identity.UnmarshalTypedIdentity(id)
290+
if err != nil || ti.Type != Multisig {
291+
return nil, nil //nolint:nilerr // a non-multisig component is the leaf, nothing stored
292+
}
293+
resolving[string(id)] = true
294+
295+
return d.GetAuditInfo(ctx, id, Multisig, ti.Identity, p)
296+
})
297+
298+
deep := nestedMultisig(t, maxDepth+2, token.Identity("leaf"))
299+
ti, err := identity.UnmarshalTypedIdentity(deep)
300+
require.NoError(t, err)
301+
302+
_, err = d.GetAuditInfo(ctx, deep, Multisig, ti.Identity, p)
303+
require.ErrorIs(t, err, tdriver.ErrIdentityNestingTooDeep)
304+
}

0 commit comments

Comments
 (0)