Skip to content

Commit 91b0c18

Browse files
committed
fix(identity): bound nesting depth and fan-out when deserializing composite identities
Signed-off-by: AkramBitar <akram@il.ibm.com>
1 parent e63e6ea commit 91b0c18

20 files changed

Lines changed: 1195 additions & 47 deletions

File tree

.github/workflows/nightly-fuzz.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ jobs:
9696
- name: fabricx-endorser-for-threshold-rule
9797
pkg: ./token/services/network/fabricx/endorsement
9898
func: FuzzEndorserForThresholdRuleNoPanic
99+
- name: fabtoken-owner-verifier
100+
pkg: ./token/core/fabtoken/v1/driver
101+
func: FuzzOwnerVerifierNoPanic
99102

100103
steps:
101104
- name: Checkout code

docs/configuration.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ token:
129129
maxMetadataKeyBytes: 256
130130
maxMetadataValueBytes: 4096
131131
maxProofBytes: 131072
132+
maxIdentityDepth: 5
133+
maxIdentityComponents: 16
132134

133135
# optional global SQL table name overrides (applied to all TMS instances).
134136
# The value replaces the short code; the FSC-generated prefix and params still wrap it.
@@ -495,6 +497,8 @@ token:
495497
maxMetadataKeyBytes: 256
496498
maxMetadataValueBytes: 4096
497499
maxProofBytes: 131072
500+
maxIdentityDepth: 5
501+
maxIdentityComponents: 16
498502
```
499503

500504
Default values:
@@ -510,6 +514,10 @@ Default values:
510514
- maxMetadataKeyBytes: 256
511515
- maxMetadataValueBytes: 4096 (4 KiB)
512516
- maxProofBytes: 131072 (128 KiB) — ignored by drivers without a zero-knowledge proof (fabtoken)
517+
- maxIdentityDepth: 5 — how deeply composite owner identities (multisig, policy, HTLC script) may
518+
nest inside one another. Real deployments nest 2–3 levels, e.g. a policy over a multisig over x509
519+
- maxIdentityComponents: 16 — how many component identities a single composite identity may carry.
520+
Bounds fan-out, which maxIdentityDepth does not
513521

514522
Every field is optional; any field omitted (or the whole `token.validation.limits` key omitted)
515523
resolves to its default. Read via the config service, so this key applies only to the FSC/DI

docs/drivers/validation-resource-limits.md

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ The token request validators (`token/core/common`, and the fabtoken/zkatdlog dri
1010
of it) accept raw, attacker-controlled bytes over the network. Aside from the signing anchor
1111
(`driver.MaxAnchorSize`), nothing else bounds the size of the raw request, the number of actions or
1212
signatures, the size of an individual action or signature, the number of inputs/outputs/metadata
13-
entries in an action, or the length of a zero-knowledge proof — unless these limits are enforced.
14-
Without them, an attacker could force unbounded allocations
15-
(`make([]..., len(attackerControlledCount))`) and expensive cryptographic work (proof
16-
deserialization, ZK verification) purely by shaping the wire bytes, without needing any valid
17-
signature.
13+
entries in an action, the length of a zero-knowledge proof, or how deeply a composite owner identity
14+
nests — unless these limits are enforced. Without them, an attacker could force unbounded allocations
15+
(`make([]..., len(attackerControlledCount))`), unbounded recursion (a composite identity nested
16+
inside itself), and expensive cryptographic work (proof deserialization, ZK verification) purely by
17+
shaping the wire bytes, without needing any valid signature.
1818

1919
## Configuration mechanism
2020

@@ -37,6 +37,8 @@ Two sources resolve a `driver.ResourceLimits` value at composition-root time, bo
3737
limits:
3838
maxActions: 128
3939
maxProofBytes: 65536
40+
maxIdentityDepth: 5
41+
maxIdentityComponents: 16
4042
```
4143
4244
Every field is optional; an entirely absent `token.validation.limits` key resolves to
@@ -156,6 +158,75 @@ Auditor-side deserializers (`.../audit/auditor.go` in both drivers) are not the
156158
consensus-endorsement boundary and are unaffected by this configuration mechanism — they always
157159
run with `DefaultResourceLimits()`.
158160

161+
### 3. Composite identity nesting
162+
163+
A token's owner is an identity, and three identity types are *composite* — their components are
164+
themselves identities:
165+
166+
| Type | Registered as | Components |
167+
| --- | --- | --- |
168+
| `multisig` (`token/services/identity/multisig`) | `driver.MultiSigIdentityType` | N component identities, all of which must sign |
169+
| `boolpolicy` (`token/services/identity/boolpolicy`) | `driver.PolicyIdentityType` | N component identities, combined by a boolean expression |
170+
| `htlc` (`token/services/identity/interop/htlc`) | `htlc.ScriptType` | the script's sender and recipient |
171+
172+
Each driver's `NewDeserializer` (`token/core/fabtoken/v1/driver/deserializer.go`,
173+
`token/core/zkatdlog/nogh/v1/driver/deserializer.go`) registers the verifier multiplex as the
174+
*component* deserializer for all three, including for itself:
175+
176+
```go
177+
des := deserializer.NewTypedVerifierDeserializerMultiplex()
178+
...
179+
des.AddTypedVerifierDeserializer(htlc2.ScriptType, htlc.NewTypedIdentityDeserializer(des))
180+
des.AddTypedVerifierDeserializer(multisig.Multisig, multisig.NewTypedIdentityDeserializer(des, des))
181+
des.AddTypedVerifierDeserializer(boolpolicy.Policy, boolpolicy.NewTypedIdentityDeserializer(des, des))
182+
```
183+
184+
That self-registration is what makes composite identities compose — a policy over a multisig over an
185+
x509 identity resolves correctly — and it is also what makes the recursion unbounded without an
186+
explicit budget. `GetOwnerVerifier` is called once per input token from the transfer validator, so an
187+
attacker-shaped owner identity drives that recursion **before any signature is verified**.
188+
189+
Two bounds close it:
190+
191+
| Field | Default | Bounds |
192+
| --- | --- | --- |
193+
| `MaxIdentityDepth` | 5 | How deeply composite identities may nest inside one another |
194+
| `MaxIdentityComponents` | 16 | How many components a single composite identity may carry |
195+
196+
Both are needed. Depth alone does not bound fan-out (one level with thousands of components is a
197+
single recursive step doing unbounded work), and fan-out alone does not bound depth. Real deployments
198+
nest 2–3 levels — a policy over a multisig over x509 — comfortably inside the default.
199+
200+
The depth budget is carried in `context.Context` (`token/driver/identity_nesting.go`), which is
201+
already the first parameter of every method in the recursion. `driver.EnterCompositeIdentity(ctx)`
202+
accounts for one level and returns the context to pass to the components; it returns an error
203+
wrapping `driver.ErrIdentityNestingTooDeep` past the limit. Exceeding `MaxIdentityComponents` returns
204+
`driver.ErrTooManyIdentityComponents`. Because the count rides in the context, it is **per-path**:
205+
sibling components each descend from their parent's depth rather than sharing a running total, which
206+
is the correct semantics for a depth bound and the reason the fan-out bound is required alongside it.
207+
208+
There are four independent recursion chains, each of which accounts for its own depth — the matcher
209+
paths recurse separately from the deserialization that produced them, so a budget spent during
210+
construction must not be inherited at match time:
211+
212+
| Chain | Entry point |
213+
| --- | --- |
214+
| Verifier deserialization | `TypedIdentityDeserializer.DeserializeVerifier` in all three packages |
215+
| Matcher construction | `TypedIdentityDeserializer.GetAuditInfoMatcher` (`multisig`, `boolpolicy`) |
216+
| Matcher evaluation | `InfoMatcher.Match` (`multisig`, `boolpolicy`), `AuditInfoMatcher.Match` (`htlc`) |
217+
| Audit-info collection | `TypedIdentityDeserializer.GetAuditInfo` in all three packages |
218+
219+
Validators seed the configured limits into the context at each public entry point
220+
(`(*Validator).withIdentityNestingLimits`, `token/core/common/limits.go`). Composite identity
221+
deserialization is also reachable from paths that carry no `ResourceLimits` — a wallet resolving a
222+
recipient, an auditor inspecting a request, tests — and those **still get the defaults** rather than
223+
running unbounded, so a seeding site added later and forgotten weakens the bound to the default
224+
instead of disabling it.
225+
226+
The fan-out bound is also applied on the honest-caller path, in `multisig.WrapIdentities` and
227+
`boolpolicy.WrapPolicyIdentity`, so an identity constructed in-process cannot exceed what a validator
228+
will later accept.
229+
159230
## Choosing and changing these values
160231

161232
The default values are conservative but comfortably above real usage observed across the unit,
@@ -186,9 +257,18 @@ of the defaults. If a deployment needs a different limit:
186257
oversized proof is rejected in well under 50ms — i.e. before any verifier is constructed. This is
187258
a timing property, verified as a plain (non-fuzzed) unit test so it isn't subject to fuzz-worker
188259
CPU contention.
260+
- **Identity-nesting tests**: `nesting_test.go` in `multisig` and `boolpolicy` covers each of the
261+
four recursion chains at `limit` and `limit+1`, that the depth counter is per-path rather than
262+
global, and that an unseeded context is still bounded by the defaults.
263+
`deserializer_nesting_test.go` (`token/core/fabtoken/v1/driver`) proves the same against the real
264+
assembled multiplex, including composite types alternating with each other so that no type gets a
265+
fresh budget, and asserts a realistic three-level identity is *not* rejected as over-nested.
189266
- **Fuzzing**: `common.FuzzRequestResourceLimits`, `zkatdlog validator.FuzzActionResourceLimits`,
190267
and `fabtoken validator.FuzzActionResourceLimits` fuzz requests/actions shaped directly by their
191268
resource dimensions (counts and byte lengths) against `DefaultResourceLimits()`, asserting no
192-
panic and the expected typed error at every boundary. Each target has a persisted seed corpus
193-
under its package's `testdata/fuzz/<TargetName>/` covering every default's boundary, and runs
194-
nightly via [`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).
269+
panic and the expected typed error at every boundary. `fabtoken driver.FuzzOwnerVerifierNoPanic`
270+
additionally fuzzes the owner-identity deserialization path itself — the one reached from the
271+
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
274+
[`.github/workflows/nightly-fuzz.yml`](../../.github/workflows/nightly-fuzz.yml).

token/core/common/limits.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ SPDX-License-Identifier: Apache-2.0
77
package common
88

99
import (
10+
"context"
11+
1012
"github.com/LFDT-Panurus/panurus/token/driver"
1113
"github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors"
1214
)
@@ -33,6 +35,19 @@ var (
3335
ErrActionTooLarge = errors.New("action exceeds maximum allowed size")
3436
)
3537

38+
// withIdentityNestingLimits returns a context carrying this validator's bounds on composite
39+
// identity nesting, so that the deserializers that turn an untrusted owner identity into a
40+
// verifier enforce the configured limits rather than the package defaults.
41+
//
42+
// It is called at each public entry point of the validator rather than deeper down because the
43+
// owner identity is reached from several of them - transfer input verification, auditing, the
44+
// matcher path - and seeding once at the top covers all of them. Deserialization reached from a
45+
// context that was never seeded still gets the defaults, so a missed entry point weakens the bound
46+
// to the default rather than removing it (see driver.EnterCompositeIdentity).
47+
func (v *Validator[P, T, TA, IA, DS]) withIdentityNestingLimits(ctx context.Context) context.Context {
48+
return driver.WithIdentityNestingLimits(ctx, v.Limits.MaxIdentityDepth, v.Limits.MaxIdentityComponents)
49+
}
50+
3651
// CheckRawRequestSize rejects raw token request bytes that exceed v.Limits.MaxRequestBytes.
3752
// It must be called before unmarshalling so oversized payloads are rejected before any
3853
// allocation proportional to their content.

token/core/common/validator.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ func (v *Validator[P, T, TA, IA, DS]) VerifyTokenRequestFromRaw(ctx context.Cont
134134
if len(raw) == 0 {
135135
return nil, nil, errors.New("empty token request")
136136
}
137+
ctx = v.withIdentityNestingLimits(ctx)
137138
if err := v.CheckRawRequestSize(raw); err != nil {
138139
return nil, nil, err
139140
}
@@ -232,6 +233,7 @@ func (v *Validator[P, T, TA, IA, DS]) VerifyTokenRequest(
232233
if utils.IsNil(v.ActionDeserializer) {
233234
return nil, nil, ErrNilActionDeserializer
234235
}
236+
ctx = v.withIdentityNestingLimits(ctx)
235237
if err := v.VerifyAuditing(ctx, anchor, tr, ledger, signatureProvider, attributes); err != nil {
236238
return nil, nil, errors.Wrapf(err, "failed to verify auditor signatures [%s]", anchor)
237239
}

0 commit comments

Comments
 (0)