From 0175ca1fd141d522eccfcfe2e1dfa7be53dca50b Mon Sep 17 00:00:00 2001 From: Effi-S Date: Mon, 10 Aug 2026 15:18:12 +0300 Subject: [PATCH] Added boolean policy to policy parser Signed-off-by: Effi-S --- .github/workflows/nightly-fuzz.yml | 3 + token/services/identity/boolpolicy/parser.go | 62 ++++++++++++++++--- .../identity/boolpolicy/parser_fuzz_test.go | 54 ++++++++++++++++ .../identity/boolpolicy/parser_test.go | 38 ++++++++++++ token/services/identity/boolpolicy/sig.go | 51 ++++++++++++--- .../services/identity/boolpolicy/sig_test.go | 56 +++++++++++++++++ 6 files changed, 244 insertions(+), 20 deletions(-) create mode 100644 token/services/identity/boolpolicy/parser_fuzz_test.go diff --git a/.github/workflows/nightly-fuzz.yml b/.github/workflows/nightly-fuzz.yml index 054a83d7cb..c6c6309e96 100644 --- a/.github/workflows/nightly-fuzz.yml +++ b/.github/workflows/nightly-fuzz.yml @@ -48,6 +48,9 @@ jobs: - name: identity-marshal-decode-identity pkg: ./token/services/identity/marshal func: FuzzDecodeIdentityNoPanic + - name: identity-boolpolicy-parse + pkg: ./token/services/identity/boolpolicy + func: FuzzParseNoPanic - name: identity-multisig-deserializer pkg: ./token/services/identity/multisig func: FuzzMultiIdentityDeserializeNoPanic diff --git a/token/services/identity/boolpolicy/parser.go b/token/services/identity/boolpolicy/parser.go index 2814683a22..ed6e8640af 100644 --- a/token/services/identity/boolpolicy/parser.go +++ b/token/services/identity/boolpolicy/parser.go @@ -18,11 +18,12 @@ SPDX-License-Identifier: Apache-2.0 package boolpolicy import ( - "errors" "fmt" "strconv" "strings" "unicode" + + "github.com/hyperledger-labs/fabric-smart-client/pkg/utils/errors" ) const ( @@ -36,6 +37,17 @@ const ( // Go call stack; capping at 64 prevents goroutine stack exhaustion from // attacker-supplied deeply-nested expressions. maxParseDepth = 64 + + // maxParseNodes is the maximum number of AST nodes (RefNode/AndNode/OrNode) + // that Parse will construct for a single expression. The depth cap bounds + // the shape of the tree but not its size: a flat, unparenthesised chain such + // as "$0 OR $1 OR ..." stays at depth 0 while producing one node per token. + // Capping the total node count bounds the memory footprint of the AST — and + // the work any later traversal performs — regardless of expression shape. + // 1024 nodes comfortably covers real policies (hundreds of component + // identities) while staying well under the ~2000-node worst case the + // maxPolicyLen byte limit alone would otherwise permit. + maxParseNodes = 1024 ) // --------------------------------------------------------------------------- @@ -140,11 +152,11 @@ func (l *lexer) next() (lexToken, error) { l.pos++ } if l.pos == start { - return lexToken{}, fmt.Errorf("expected digit after '$' at position %d", l.pos) + return lexToken{}, errors.Errorf("expected digit after '$' at position %d", l.pos) } idx, err := strconv.Atoi(string(l.runes[start:l.pos])) if err != nil { - return lexToken{}, fmt.Errorf("invalid index at position %d: %w", start, err) + return lexToken{}, errors.Wrapf(err, "invalid index at position %d", start) } return lexToken{kind: tokRef, index: idx}, nil @@ -161,11 +173,11 @@ func (l *lexer) next() (lexToken, error) { case "OR": return lexToken{kind: tokOr}, nil default: - return lexToken{}, fmt.Errorf("unknown keyword %q at position %d", word, start) + return lexToken{}, errors.Errorf("unknown keyword %q at position %d", word, start) } default: - return lexToken{}, fmt.Errorf("unexpected character %q at position %d", string(ch), l.pos) + return lexToken{}, errors.Errorf("unexpected character %q at position %d", string(ch), l.pos) } } @@ -178,19 +190,40 @@ type parser struct { lex *lexer current lexToken err error + nodes int // number of AST nodes constructed so far +} + +// countNode records the construction of one AST node and returns whether the +// parser is still within the maxParseNodes budget. On overflow it latches an +// error (if one is not already set) so the caller can bail out immediately. +func (p *parser) countNode() bool { + p.nodes++ + if p.nodes > maxParseNodes { + if p.err == nil { + p.err = errors.Errorf("policy expression exceeds maximum node count of %d", maxParseNodes) + } + + return false + } + + return true } // Parse parses a boolean expression string and returns the root AST node. // It returns an error for any lexical or syntactic problems. // -// Parse enforces two hard limits to prevent resource exhaustion: +// Parse enforces three hard limits to prevent resource exhaustion: // - input longer than maxPolicyLen bytes is rejected immediately. // - parenthesis nesting deeper than maxParseDepth levels is rejected; // this bounds the Go call-stack depth of the recursive descent and // prevents goroutine stack exhaustion from attacker-supplied input. +// - expressions producing more than maxParseNodes AST nodes are rejected; +// this bounds the size of the AST (and the cost of any later traversal) +// independently of its shape, since the depth cap alone does not limit a +// flat chain such as "$0 OR $1 OR ...". func Parse(input string) (Node, error) { if len(input) > maxPolicyLen { - return nil, fmt.Errorf("policy expression exceeds maximum length of %d bytes (got %d)", maxPolicyLen, len(input)) + return nil, errors.Errorf("policy expression exceeds maximum length of %d bytes (got %d)", maxPolicyLen, len(input)) } p := &parser{lex: newLexer(input)} @@ -224,6 +257,9 @@ func (p *parser) parseOr(depth int) Node { for p.err == nil && p.current.kind == tokOr { p.advance() right := p.parseAnd(depth) + if !p.countNode() { + return nil + } left = &OrNode{Left: left, Right: right} } @@ -237,6 +273,9 @@ func (p *parser) parseAnd(depth int) Node { for p.err == nil && p.current.kind == tokAnd { p.advance() right := p.parsePrimary(depth) + if !p.countNode() { + return nil + } left = &AndNode{Left: left, Right: right} } @@ -250,6 +289,9 @@ func (p *parser) parsePrimary(depth int) Node { } switch p.current.kind { case tokRef: + if !p.countNode() { + return nil + } node := &RefNode{Index: p.current.index} p.advance() @@ -257,7 +299,7 @@ func (p *parser) parsePrimary(depth int) Node { case tokLParen: if depth >= maxParseDepth { - p.err = fmt.Errorf("policy expression exceeds maximum nesting depth of %d", maxParseDepth) + p.err = errors.Errorf("policy expression exceeds maximum nesting depth of %d", maxParseDepth) return nil } @@ -267,7 +309,7 @@ func (p *parser) parsePrimary(depth int) Node { return nil } if p.current.kind != tokRParen { - p.err = fmt.Errorf("expected ')' but got token kind %v", p.current.kind) + p.err = errors.Errorf("expected ')' but got token kind %v", p.current.kind) return nil } @@ -276,7 +318,7 @@ func (p *parser) parsePrimary(depth int) Node { return node default: - p.err = fmt.Errorf("expected '$N' or '(' at position %d", p.lex.pos) + p.err = errors.Errorf("expected '$N' or '(' at position %d", p.lex.pos) return nil } diff --git a/token/services/identity/boolpolicy/parser_fuzz_test.go b/token/services/identity/boolpolicy/parser_fuzz_test.go new file mode 100644 index 0000000000..2fd2e51253 --- /dev/null +++ b/token/services/identity/boolpolicy/parser_fuzz_test.go @@ -0,0 +1,54 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package boolpolicy + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// FuzzParseNoPanic hunts for policy strings that panic Parse instead of +// returning a Node or an error. Parse is the entry point for +// attacker-controllable policy expressions carried inside a PolicyIdentity, so +// it must never panic and must always terminate within its resource caps +// (length, nesting depth, and node count). +func FuzzParseNoPanic(f *testing.F) { + seeds := []string{ + "", // empty + "$0", // single ref + "$42", // multi-digit ref + "$0 AND $1", // simple AND + "$0 OR $1", // simple OR + "$0 OR ($1 AND $2)", // nested + "((($0)))", // parenthesised + "$0 OR $0 OR $0 OR $0", // repeated refs (exercises memoisation callers) + "$", // dangling dollar + "$0 NOT $1", // unknown keyword + "($0 AND $1", // unmatched open paren + "$0 AND $1)", // unmatched close paren + "$0 AND", // missing operand + "$0 & $1", // unexpected character + "$99999999999999999999", // index overflows int + "(((((((((((((((((((((", // deeply nested opens + "$0 OR $1 OR $2 OR $3 OR", // trailing operator + } + for _, s := range seeds { + f.Add(s) + } + + f.Fuzz(func(t *testing.T, input string) { + require.NotPanics(t, func() { + node, err := Parse(input) + // A successful parse must yield a non-nil node; an error must yield + // a nil node. Neither may panic regardless of input. + if err == nil { + require.NotNil(t, node) + } + }) + }) +} diff --git a/token/services/identity/boolpolicy/parser_test.go b/token/services/identity/boolpolicy/parser_test.go index 87603a2f31..5634c94554 100644 --- a/token/services/identity/boolpolicy/parser_test.go +++ b/token/services/identity/boolpolicy/parser_test.go @@ -299,6 +299,44 @@ func TestErrorNestingTooDeep(t *testing.T) { assert.Contains(t, err.Error(), "exceeds maximum nesting depth") } +// orChain builds a flat, unparenthesised OR chain of `refs` references using +// the compact "$0OR$0OR..." form (no surrounding spaces), so the byte length +// stays minimal while the node count grows. The result has `refs` RefNodes and +// `refs-1` OrNodes, i.e. 2*refs-1 AST nodes, all at nesting depth 0. +func orChain(refs int) string { + var sb strings.Builder + sb.WriteString("$0") + for i := 1; i < refs; i++ { + sb.WriteString("OR$0") + } + + return sb.String() +} + +func TestErrorTooManyNodes(t *testing.T) { + // A flat OR chain stays at nesting depth 0 but produces one node per ref + // plus one per operator, so it exercises the node cap rather than the depth + // cap. With R refs the chain yields 2R-1 nodes; pick R large enough to + // exceed maxParseNodes while the string stays under maxPolicyLen. + refs := maxParseNodes // 2*maxParseNodes-1 nodes, comfortably over the cap + policy := orChain(refs) + require.LessOrEqual(t, len(policy), maxPolicyLen, "test input must stay within the length cap") + + _, err := Parse(policy) + require.Error(t, err) + assert.Contains(t, err.Error(), "exceeds maximum node count") +} + +func TestNodeCountAtLimitIsAllowed(t *testing.T) { + // Build an OR chain whose node count lands just at/under maxParseNodes: with + // R refs the node count is 2R-1, so R = (maxParseNodes+1)/2 hits the cap + // exactly when maxParseNodes is odd and lands just under it when even — + // either way it must parse without error. + refs := (maxParseNodes + 1) / 2 + _, err := Parse(orChain(refs)) + require.NoError(t, err) +} + func TestNestingAtLimitIsAllowed(t *testing.T) { // A policy nested exactly at maxParseDepth must succeed. policy := "" diff --git a/token/services/identity/boolpolicy/sig.go b/token/services/identity/boolpolicy/sig.go index feabdc6cfb..b0b04e22c7 100644 --- a/token/services/identity/boolpolicy/sig.go +++ b/token/services/identity/boolpolicy/sig.go @@ -81,31 +81,62 @@ func (v *PolicyVerifier) Verify(msg, sigBytes []byte) error { return errors.Errorf("policy signature has [%d] slots, expected [%d]", len(sig.Signatures), len(v.Verifiers)) } - if !v.evalNode(v.Policy, msg, sig.Signatures) { + // memo caches each index's verification result for the duration of this + // Verify call: a $N reference is checked against the same (msg, sigs[N]) + // no matter how many times it appears in the policy, so the potentially + // expensive Verifiers[N].Verify is invoked at most once per index. + memo := make([]refResult, len(sig.Signatures)) + if !v.evalNode(v.Policy, msg, sig.Signatures, memo) { return errors.New("policy not satisfied") } return nil } +// refResult is the memoised outcome of verifying a single component index. +type refResult int8 + +const ( + refUnknown refResult = iota // not yet verified in this Verify call + refPass // verification succeeded + refFail // verification failed (absent slot or bad signature) +) + // evalNode recursively evaluates the policy AST against the provided signatures. -func (v *PolicyVerifier) evalNode(node Node, msg []byte, sigs [][]byte) bool { +// memo is indexed by component index ($N) and caches per-index verification +// results across the whole traversal; it must have one entry per signature slot. +func (v *PolicyVerifier) evalNode(node Node, msg []byte, sigs [][]byte, memo []refResult) bool { switch n := node.(type) { case *RefNode: - i := n.Index - if i < 0 || i >= len(sigs) || len(sigs[i]) == 0 { - return false - } - - return v.Verifiers[i].Verify(msg, sigs[i]) == nil + return v.evalRef(n.Index, msg, sigs, memo) case *AndNode: - return v.evalNode(n.Left, msg, sigs) && v.evalNode(n.Right, msg, sigs) + return v.evalNode(n.Left, msg, sigs, memo) && v.evalNode(n.Right, msg, sigs, memo) case *OrNode: - return v.evalNode(n.Left, msg, sigs) || v.evalNode(n.Right, msg, sigs) + return v.evalNode(n.Left, msg, sigs, memo) || v.evalNode(n.Right, msg, sigs, memo) default: return false } } + +// evalRef verifies the component identity at index i, reusing a previously +// computed result for the same index when one is available. +func (v *PolicyVerifier) evalRef(i int, msg []byte, sigs [][]byte, memo []refResult) bool { + if i < 0 || i >= len(sigs) || len(sigs[i]) == 0 { + return false + } + if memo[i] != refUnknown { + return memo[i] == refPass + } + + ok := v.Verifiers[i].Verify(msg, sigs[i]) == nil + if ok { + memo[i] = refPass + } else { + memo[i] = refFail + } + + return ok +} diff --git a/token/services/identity/boolpolicy/sig_test.go b/token/services/identity/boolpolicy/sig_test.go index c636040de3..9ae1ff39d1 100644 --- a/token/services/identity/boolpolicy/sig_test.go +++ b/token/services/identity/boolpolicy/sig_test.go @@ -13,10 +13,24 @@ package boolpolicy import ( "testing" + tdriver "github.com/LFDT-Panurus/panurus/token/driver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// countingVerifier wraps a driver.Verifier and records how many times Verify +// was invoked, so tests can assert per-index memoisation. +type countingVerifier struct { + inner tdriver.Verifier + calls int +} + +func (c *countingVerifier) Verify(msg, sig []byte) error { + c.calls++ + + return c.inner.Verify(msg, sig) +} + // --------------------------------------------------------------------------- // AND policy // --------------------------------------------------------------------------- @@ -178,3 +192,45 @@ func TestPolicyVerify_SlotCountTooMany(t *testing.T) { sig := buildPolicySig(t, "s0", "s1", "extra") assert.Error(t, pv.Verify([]byte(testMsg), sig)) } + +// --------------------------------------------------------------------------- +// Per-index memoisation within a single Verify call +// --------------------------------------------------------------------------- + +// TestPolicyVerify_Memoisation_RepeatedRefVerifiedOnce verifies that an index +// referenced multiple times in a policy triggers the underlying verifier only +// once per Verify call. +func TestPolicyVerify_Memoisation_RepeatedRefVerifiedOnce(t *testing.T) { + stubs := makeVerifiers(testMsg, "s0", "s1") + counter := &countingVerifier{inner: stubs[0]} + + // "$0 AND ($0 OR $1)" references $0 twice. + node, err := Parse("$0 AND ($0 OR $1)") + require.NoError(t, err) + pv := &PolicyVerifier{ + Policy: node, + Verifiers: []tdriver.Verifier{counter, stubs[1]}, + } + + require.NoError(t, pv.Verify([]byte(testMsg), buildPolicySig(t, "s0", "s1"))) + assert.Equal(t, 1, counter.calls, "verifier for $0 must be invoked exactly once") +} + +// TestPolicyVerify_Memoisation_FailingRefVerifiedOnce verifies that a failing +// index is also cached: a wrong signature referenced twice is verified once and +// the cached failure is reused. +func TestPolicyVerify_Memoisation_FailingRefVerifiedOnce(t *testing.T) { + stubs := makeVerifiers(testMsg, "s0") + counter := &countingVerifier{inner: stubs[0]} + + // "$0 OR $0" references $0 twice; a wrong signature fails both times. + node, err := Parse("$0 OR $0") + require.NoError(t, err) + pv := &PolicyVerifier{ + Policy: node, + Verifiers: []tdriver.Verifier{counter}, + } + + require.Error(t, pv.Verify([]byte(testMsg), buildPolicySig(t, "wrong"))) + assert.Equal(t, 1, counter.calls, "failing verifier for $0 must be invoked exactly once") +}