Skip to content

Commit d2aa9aa

Browse files
committed
Added boolean policy to policy parser
Signed-off-by: Effi-S <effi.szt@gmail.com>
1 parent 53fd8f1 commit d2aa9aa

6 files changed

Lines changed: 234 additions & 11 deletions

File tree

.github/workflows/nightly-fuzz.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ jobs:
4848
- name: identity-marshal-decode-identity
4949
pkg: ./token/services/identity/marshal
5050
func: FuzzDecodeIdentityNoPanic
51+
- name: identity-boolpolicy-parse
52+
pkg: ./token/services/identity/boolpolicy
53+
func: FuzzParseNoPanic
5154
- name: identity-multisig-deserializer
5255
pkg: ./token/services/identity/multisig
5356
func: FuzzMultiIdentityDeserializeNoPanic

token/services/identity/boolpolicy/parser.go

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,17 @@ const (
3636
// Go call stack; capping at 64 prevents goroutine stack exhaustion from
3737
// attacker-supplied deeply-nested expressions.
3838
maxParseDepth = 64
39+
40+
// maxParseNodes is the maximum number of AST nodes (RefNode/AndNode/OrNode)
41+
// that Parse will construct for a single expression. The depth cap bounds
42+
// the shape of the tree but not its size: a flat, unparenthesised chain such
43+
// as "$0 OR $1 OR ..." stays at depth 0 while producing one node per token.
44+
// Capping the total node count bounds the memory footprint of the AST — and
45+
// the work any later traversal performs — regardless of expression shape.
46+
// 1024 nodes comfortably covers real policies (hundreds of component
47+
// identities) while staying well under the ~2000-node worst case the
48+
// maxPolicyLen byte limit alone would otherwise permit.
49+
maxParseNodes = 1024
3950
)
4051

4152
// ---------------------------------------------------------------------------
@@ -178,16 +189,37 @@ type parser struct {
178189
lex *lexer
179190
current lexToken
180191
err error
192+
nodes int // number of AST nodes constructed so far
193+
}
194+
195+
// countNode records the construction of one AST node and returns whether the
196+
// parser is still within the maxParseNodes budget. On overflow it latches an
197+
// error (if one is not already set) so the caller can bail out immediately.
198+
func (p *parser) countNode() bool {
199+
p.nodes++
200+
if p.nodes > maxParseNodes {
201+
if p.err == nil {
202+
p.err = fmt.Errorf("policy expression exceeds maximum node count of %d", maxParseNodes)
203+
}
204+
205+
return false
206+
}
207+
208+
return true
181209
}
182210

183211
// Parse parses a boolean expression string and returns the root AST node.
184212
// It returns an error for any lexical or syntactic problems.
185213
//
186-
// Parse enforces two hard limits to prevent resource exhaustion:
214+
// Parse enforces three hard limits to prevent resource exhaustion:
187215
// - input longer than maxPolicyLen bytes is rejected immediately.
188216
// - parenthesis nesting deeper than maxParseDepth levels is rejected;
189217
// this bounds the Go call-stack depth of the recursive descent and
190218
// prevents goroutine stack exhaustion from attacker-supplied input.
219+
// - expressions producing more than maxParseNodes AST nodes are rejected;
220+
// this bounds the size of the AST (and the cost of any later traversal)
221+
// independently of its shape, since the depth cap alone does not limit a
222+
// flat chain such as "$0 OR $1 OR ...".
191223
func Parse(input string) (Node, error) {
192224
if len(input) > maxPolicyLen {
193225
return nil, fmt.Errorf("policy expression exceeds maximum length of %d bytes (got %d)", maxPolicyLen, len(input))
@@ -224,6 +256,9 @@ func (p *parser) parseOr(depth int) Node {
224256
for p.err == nil && p.current.kind == tokOr {
225257
p.advance()
226258
right := p.parseAnd(depth)
259+
if !p.countNode() {
260+
return nil
261+
}
227262
left = &OrNode{Left: left, Right: right}
228263
}
229264

@@ -237,6 +272,9 @@ func (p *parser) parseAnd(depth int) Node {
237272
for p.err == nil && p.current.kind == tokAnd {
238273
p.advance()
239274
right := p.parsePrimary(depth)
275+
if !p.countNode() {
276+
return nil
277+
}
240278
left = &AndNode{Left: left, Right: right}
241279
}
242280

@@ -250,6 +288,9 @@ func (p *parser) parsePrimary(depth int) Node {
250288
}
251289
switch p.current.kind {
252290
case tokRef:
291+
if !p.countNode() {
292+
return nil
293+
}
253294
node := &RefNode{Index: p.current.index}
254295
p.advance()
255296

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package boolpolicy
8+
9+
import (
10+
"testing"
11+
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// FuzzParseNoPanic hunts for policy strings that panic Parse instead of
16+
// returning a Node or an error. Parse is the entry point for
17+
// attacker-controllable policy expressions carried inside a PolicyIdentity, so
18+
// it must never panic and must always terminate within its resource caps
19+
// (length, nesting depth, and node count).
20+
func FuzzParseNoPanic(f *testing.F) {
21+
seeds := []string{
22+
"", // empty
23+
"$0", // single ref
24+
"$42", // multi-digit ref
25+
"$0 AND $1", // simple AND
26+
"$0 OR $1", // simple OR
27+
"$0 OR ($1 AND $2)", // nested
28+
"((($0)))", // parenthesised
29+
"$0 OR $0 OR $0 OR $0", // repeated refs (exercises memoisation callers)
30+
"$", // dangling dollar
31+
"$0 NOT $1", // unknown keyword
32+
"($0 AND $1", // unmatched open paren
33+
"$0 AND $1)", // unmatched close paren
34+
"$0 AND", // missing operand
35+
"$0 & $1", // unexpected character
36+
"$99999999999999999999", // index overflows int
37+
"(((((((((((((((((((((", // deeply nested opens
38+
"$0 OR $1 OR $2 OR $3 OR", // trailing operator
39+
}
40+
for _, s := range seeds {
41+
f.Add(s)
42+
}
43+
44+
f.Fuzz(func(t *testing.T, input string) {
45+
require.NotPanics(t, func() {
46+
node, err := Parse(input)
47+
// A successful parse must yield a non-nil node; an error must yield
48+
// a nil node. Neither may panic regardless of input.
49+
if err == nil {
50+
require.NotNil(t, node)
51+
}
52+
})
53+
})
54+
}

token/services/identity/boolpolicy/parser_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,44 @@ func TestErrorNestingTooDeep(t *testing.T) {
299299
assert.Contains(t, err.Error(), "exceeds maximum nesting depth")
300300
}
301301

302+
// orChain builds a flat, unparenthesised OR chain of `refs` references using
303+
// the compact "$0OR$0OR..." form (no surrounding spaces), so the byte length
304+
// stays minimal while the node count grows. The result has `refs` RefNodes and
305+
// `refs-1` OrNodes, i.e. 2*refs-1 AST nodes, all at nesting depth 0.
306+
func orChain(refs int) string {
307+
var sb strings.Builder
308+
sb.WriteString("$0")
309+
for i := 1; i < refs; i++ {
310+
sb.WriteString("OR$0")
311+
}
312+
313+
return sb.String()
314+
}
315+
316+
func TestErrorTooManyNodes(t *testing.T) {
317+
// A flat OR chain stays at nesting depth 0 but produces one node per ref
318+
// plus one per operator, so it exercises the node cap rather than the depth
319+
// cap. With R refs the chain yields 2R-1 nodes; pick R large enough to
320+
// exceed maxParseNodes while the string stays under maxPolicyLen.
321+
refs := maxParseNodes // 2*maxParseNodes-1 nodes, comfortably over the cap
322+
policy := orChain(refs)
323+
require.LessOrEqual(t, len(policy), maxPolicyLen, "test input must stay within the length cap")
324+
325+
_, err := Parse(policy)
326+
require.Error(t, err)
327+
assert.Contains(t, err.Error(), "exceeds maximum node count")
328+
}
329+
330+
func TestNodeCountAtLimitIsAllowed(t *testing.T) {
331+
// Build an OR chain whose node count lands just at/under maxParseNodes: with
332+
// R refs the node count is 2R-1, so R = (maxParseNodes+1)/2 hits the cap
333+
// exactly when maxParseNodes is odd and lands just under it when even —
334+
// either way it must parse without error.
335+
refs := (maxParseNodes + 1) / 2
336+
_, err := Parse(orChain(refs))
337+
require.NoError(t, err)
338+
}
339+
302340
func TestNestingAtLimitIsAllowed(t *testing.T) {
303341
// A policy nested exactly at maxParseDepth must succeed.
304342
policy := ""

token/services/identity/boolpolicy/sig.go

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,31 +81,62 @@ func (v *PolicyVerifier) Verify(msg, sigBytes []byte) error {
8181
return errors.Errorf("policy signature has [%d] slots, expected [%d]",
8282
len(sig.Signatures), len(v.Verifiers))
8383
}
84-
if !v.evalNode(v.Policy, msg, sig.Signatures) {
84+
// memo caches each index's verification result for the duration of this
85+
// Verify call: a $N reference is checked against the same (msg, sigs[N])
86+
// no matter how many times it appears in the policy, so the potentially
87+
// expensive Verifiers[N].Verify is invoked at most once per index.
88+
memo := make([]refResult, len(sig.Signatures))
89+
if !v.evalNode(v.Policy, msg, sig.Signatures, memo) {
8590
return errors.New("policy not satisfied")
8691
}
8792

8893
return nil
8994
}
9095

96+
// refResult is the memoised outcome of verifying a single component index.
97+
type refResult int8
98+
99+
const (
100+
refUnknown refResult = iota // not yet verified in this Verify call
101+
refPass // verification succeeded
102+
refFail // verification failed (absent slot or bad signature)
103+
)
104+
91105
// evalNode recursively evaluates the policy AST against the provided signatures.
92-
func (v *PolicyVerifier) evalNode(node Node, msg []byte, sigs [][]byte) bool {
106+
// memo is indexed by component index ($N) and caches per-index verification
107+
// results across the whole traversal; it must have one entry per signature slot.
108+
func (v *PolicyVerifier) evalNode(node Node, msg []byte, sigs [][]byte, memo []refResult) bool {
93109
switch n := node.(type) {
94110
case *RefNode:
95-
i := n.Index
96-
if i < 0 || i >= len(sigs) || len(sigs[i]) == 0 {
97-
return false
98-
}
99-
100-
return v.Verifiers[i].Verify(msg, sigs[i]) == nil
111+
return v.evalRef(n.Index, msg, sigs, memo)
101112

102113
case *AndNode:
103-
return v.evalNode(n.Left, msg, sigs) && v.evalNode(n.Right, msg, sigs)
114+
return v.evalNode(n.Left, msg, sigs, memo) && v.evalNode(n.Right, msg, sigs, memo)
104115

105116
case *OrNode:
106-
return v.evalNode(n.Left, msg, sigs) || v.evalNode(n.Right, msg, sigs)
117+
return v.evalNode(n.Left, msg, sigs, memo) || v.evalNode(n.Right, msg, sigs, memo)
107118

108119
default:
109120
return false
110121
}
111122
}
123+
124+
// evalRef verifies the component identity at index i, reusing a previously
125+
// computed result for the same index when one is available.
126+
func (v *PolicyVerifier) evalRef(i int, msg []byte, sigs [][]byte, memo []refResult) bool {
127+
if i < 0 || i >= len(sigs) || len(sigs[i]) == 0 {
128+
return false
129+
}
130+
if memo[i] != refUnknown {
131+
return memo[i] == refPass
132+
}
133+
134+
ok := v.Verifiers[i].Verify(msg, sigs[i]) == nil
135+
if ok {
136+
memo[i] = refPass
137+
} else {
138+
memo[i] = refFail
139+
}
140+
141+
return ok
142+
}

token/services/identity/boolpolicy/sig_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,24 @@ package boolpolicy
1313
import (
1414
"testing"
1515

16+
tdriver "github.com/LFDT-Panurus/panurus/token/driver"
1617
"github.com/stretchr/testify/assert"
1718
"github.com/stretchr/testify/require"
1819
)
1920

21+
// countingVerifier wraps a driver.Verifier and records how many times Verify
22+
// was invoked, so tests can assert per-index memoisation.
23+
type countingVerifier struct {
24+
inner tdriver.Verifier
25+
calls int
26+
}
27+
28+
func (c *countingVerifier) Verify(msg, sig []byte) error {
29+
c.calls++
30+
31+
return c.inner.Verify(msg, sig)
32+
}
33+
2034
// ---------------------------------------------------------------------------
2135
// AND policy
2236
// ---------------------------------------------------------------------------
@@ -178,3 +192,45 @@ func TestPolicyVerify_SlotCountTooMany(t *testing.T) {
178192
sig := buildPolicySig(t, "s0", "s1", "extra")
179193
assert.Error(t, pv.Verify([]byte(testMsg), sig))
180194
}
195+
196+
// ---------------------------------------------------------------------------
197+
// Per-index memoisation within a single Verify call
198+
// ---------------------------------------------------------------------------
199+
200+
// TestPolicyVerify_Memoisation_RepeatedRefVerifiedOnce verifies that an index
201+
// referenced multiple times in a policy triggers the underlying verifier only
202+
// once per Verify call.
203+
func TestPolicyVerify_Memoisation_RepeatedRefVerifiedOnce(t *testing.T) {
204+
stubs := makeVerifiers(testMsg, "s0", "s1")
205+
counter := &countingVerifier{inner: stubs[0]}
206+
207+
// "$0 AND ($0 OR $1)" references $0 twice.
208+
node, err := Parse("$0 AND ($0 OR $1)")
209+
require.NoError(t, err)
210+
pv := &PolicyVerifier{
211+
Policy: node,
212+
Verifiers: []tdriver.Verifier{counter, stubs[1]},
213+
}
214+
215+
require.NoError(t, pv.Verify([]byte(testMsg), buildPolicySig(t, "s0", "s1")))
216+
assert.Equal(t, 1, counter.calls, "verifier for $0 must be invoked exactly once")
217+
}
218+
219+
// TestPolicyVerify_Memoisation_FailingRefVerifiedOnce verifies that a failing
220+
// index is also cached: a wrong signature referenced twice is verified once and
221+
// the cached failure is reused.
222+
func TestPolicyVerify_Memoisation_FailingRefVerifiedOnce(t *testing.T) {
223+
stubs := makeVerifiers(testMsg, "s0")
224+
counter := &countingVerifier{inner: stubs[0]}
225+
226+
// "$0 OR $0" references $0 twice; a wrong signature fails both times.
227+
node, err := Parse("$0 OR $0")
228+
require.NoError(t, err)
229+
pv := &PolicyVerifier{
230+
Policy: node,
231+
Verifiers: []tdriver.Verifier{counter},
232+
}
233+
234+
require.Error(t, pv.Verify([]byte(testMsg), buildPolicySig(t, "wrong")))
235+
assert.Equal(t, 1, counter.calls, "failing verifier for $0 must be invoked exactly once")
236+
}

0 commit comments

Comments
 (0)