Skip to content

Commit cd1e2b9

Browse files
tstirrat15miparnisari
tstirrat15
and
miparnisari
authored
fix: avoid hash collision in caveat context (#3065)
## Description Someone pointed out to us that if you use a nested list of lists in a caveat context, it was possible under our current serialization and hashing logic to have a serialization collision and therefore a dispatch cache hash collision. This fixes that by using deterministic proto marshalling of the caveat context Struct object. ## Changes * Update `context_hash.go` to use deterministic marshalling * Propagate the error signature change through the stack * change `hashableContext` to `hashableContextString` to satisfy the interface ## Testing Review --------- Co-authored-by: Maria Ines Parnisari <maria.ines.parnisari@authzed.com>
1 parent 312216f commit cd1e2b9

10 files changed

Lines changed: 245 additions & 166 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
2222
- Upgraded the spanner client, which changed the internal implementation to not use a session pool. This means that the `--datastore-spanner-max-sessions` and `--datastore-spanner-min-sessions` flags are now deprecated and no-op. We also strongly recommend using [Application Default Credentials](https://docs.cloud.google.com/docs/authentication/client-libraries#adc) in favor of a credentials file. (https://github.com/authzed/spicedb/pull/3038)
2323
- Query Planner: error `"ERROR: index \"pk_relation_tuple\" cannot be used for this query (SQLSTATE 42809)"` returned when using wildcards (https://github.com/authzed/spicedb/pull/3039)
2424
- Providing one of (`--grpc-tls-cert-path`, `--grpc-tls-key-path`) but not the other is now considered an error state, as both are necessary if you want to use TLS.
25+
- In a caveat context that uses nested lists of lists, the hashes generated for cache keys could collide because of an issue with the serialization logic. The serialization now uses deterministic protobuf serialization which avoids this issue (https://github.com/authzed/spicedb/pull/3065)
2526

2627
## [1.51.1] - 2026-04-14
2728
### Fixed

‎internal/dispatch/keys/computed.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,29 +72,37 @@ func expandRequestToKey(req *v1.DispatchExpandRequest, option dispatchCacheKeyHa
7272
}
7373

7474
// lookupResourcesRequest2ToKey converts a lookup request into a cache key
75-
func lookupResourcesRequest2ToKey(req *v1.DispatchLookupResources2Request, option dispatchCacheKeyHashComputeOption) DispatchCacheKey {
75+
func lookupResourcesRequest2ToKey(req *v1.DispatchLookupResources2Request, option dispatchCacheKeyHashComputeOption) (DispatchCacheKey, error) {
76+
stableContextString, err := caveats.StableContextStringForHashing(req.Context)
77+
if err != nil {
78+
return DispatchCacheKey{}, err
79+
}
7680
return dispatchCacheKeyHash(lookupPrefix, req.Metadata.AtRevision, option,
7781
hashableRelationReference{req.ResourceRelation},
7882
hashableRelationReference{req.SubjectRelation},
7983
hashableIds(req.SubjectIds),
8084
hashableOnr{req.TerminalSubject},
81-
hashableContext{HashableContext: caveats.HashableContext{Struct: req.Context}}, // NOTE: context is included here because lookup does a single dispatch
85+
hashableContextString(stableContextString),
8286
hashableCursor{req.OptionalCursor},
8387
hashableLimit(req.OptionalLimit),
84-
)
88+
), nil
8589
}
8690

8791
// lookupResourcesRequest3ToKey converts a lookup request into a cache key
88-
func lookupResourcesRequest3ToKey(req *v1.DispatchLookupResources3Request, option dispatchCacheKeyHashComputeOption) DispatchCacheKey {
92+
func lookupResourcesRequest3ToKey(req *v1.DispatchLookupResources3Request, option dispatchCacheKeyHashComputeOption) (DispatchCacheKey, error) {
93+
stableContextString, err := caveats.StableContextStringForHashing(req.Context)
94+
if err != nil {
95+
return DispatchCacheKey{}, err
96+
}
8997
return dispatchCacheKeyHash(lookupPrefix, req.Metadata.AtRevision, option,
9098
hashableRelationReference{req.ResourceRelation},
9199
hashableRelationReference{req.SubjectRelation},
92100
hashableIds(req.SubjectIds),
93101
hashableOnr{req.TerminalSubject},
94-
hashableContext{HashableContext: caveats.HashableContext{Struct: req.Context}}, // NOTE: context is included here because lookup does a single dispatch
102+
hashableContextString(stableContextString), // NOTE: context is included here because lookup does a single dispatch
95103
hashableCursorSections{req.OptionalCursor},
96104
hashableLimit(req.OptionalLimit),
97-
)
105+
), nil
98106
}
99107

100108
// lookupSubjectsRequestToKey converts a lookup subjects request into a cache key

‎internal/dispatch/keys/computed_test.go‎

Lines changed: 59 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func TestStableCacheKeys(t *testing.T) {
145145
{
146146
"lookup resources 2",
147147
func() DispatchCacheKey {
148-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
148+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
149149
ResourceRelation: RR("document", "view"),
150150
SubjectRelation: RR("user", "..."),
151151
SubjectIds: []string{"mariah"},
@@ -154,13 +154,14 @@ func TestStableCacheKeys(t *testing.T) {
154154
AtRevision: "1234",
155155
},
156156
}, computeBothHashes)
157+
return key
157158
},
158-
"f49fbafef9c489abe601",
159+
"9884bbb3acd3b3ca1a",
159160
},
160161
{
161162
"lookup resources 2 with zero limit",
162163
func() DispatchCacheKey {
163-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
164+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
164165
ResourceRelation: RR("document", "view"),
165166
SubjectRelation: RR("user", "..."),
166167
SubjectIds: []string{"mariah"},
@@ -170,13 +171,14 @@ func TestStableCacheKeys(t *testing.T) {
170171
},
171172
OptionalLimit: 0,
172173
}, computeBothHashes)
174+
return key
173175
},
174-
"f49fbafef9c489abe601",
176+
"9884bbb3acd3b3ca1a",
175177
},
176178
{
177179
"lookup resources 2 with non-zero limit",
178180
func() DispatchCacheKey {
179-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
181+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
180182
ResourceRelation: RR("document", "view"),
181183
SubjectRelation: RR("user", "..."),
182184
SubjectIds: []string{"mariah"},
@@ -186,13 +188,14 @@ func TestStableCacheKeys(t *testing.T) {
186188
},
187189
OptionalLimit: 42,
188190
}, computeBothHashes)
191+
return key
189192
},
190-
"ea88adb1c1dfa6ebab01",
193+
"dba285cdd9caeef36e",
191194
},
192195
{
193196
"lookup resources 2 with nil context",
194197
func() DispatchCacheKey {
195-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
198+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
196199
ResourceRelation: RR("document", "view"),
197200
SubjectRelation: RR("user", "..."),
198201
SubjectIds: []string{"mariah"},
@@ -202,13 +205,14 @@ func TestStableCacheKeys(t *testing.T) {
202205
},
203206
Context: nil,
204207
}, computeBothHashes)
208+
return key
205209
},
206-
"f49fbafef9c489abe601",
210+
"9884bbb3acd3b3ca1a",
207211
},
208212
{
209213
"lookup resources 2 with empty context",
210214
func() DispatchCacheKey {
211-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
215+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
212216
ResourceRelation: RR("document", "view"),
213217
SubjectRelation: RR("user", "..."),
214218
SubjectIds: []string{"mariah"},
@@ -221,13 +225,14 @@ func TestStableCacheKeys(t *testing.T) {
221225
return v
222226
}(),
223227
}, computeBothHashes)
228+
return key
224229
},
225-
"f49fbafef9c489abe601",
230+
"9884bbb3acd3b3ca1a",
226231
},
227232
{
228233
"lookup resources 2 with context",
229234
func() DispatchCacheKey {
230-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
235+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
231236
ResourceRelation: RR("document", "view"),
232237
SubjectRelation: RR("user", "..."),
233238
SubjectIds: []string{"mariah"},
@@ -243,13 +248,14 @@ func TestStableCacheKeys(t *testing.T) {
243248
return v
244249
}(),
245250
}, computeBothHashes)
251+
return key
246252
},
247-
"bbd78884eff9edbebd01",
253+
"a3dad09ce9d690b78401",
248254
},
249255
{
250256
"lookup resources 2 with different context",
251257
func() DispatchCacheKey {
252-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
258+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
253259
ResourceRelation: RR("document", "view"),
254260
SubjectRelation: RR("user", "..."),
255261
SubjectIds: []string{"mariah"},
@@ -265,13 +271,14 @@ func TestStableCacheKeys(t *testing.T) {
265271
return v
266272
}(),
267273
}, computeBothHashes)
274+
return key
268275
},
269-
"94d0faa5feaaa4dc17",
276+
"f6d4bc92bae9e9d64b",
270277
},
271278
{
272279
"lookup resources 2 with escaped string",
273280
func() DispatchCacheKey {
274-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
281+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
275282
ResourceRelation: RR("document", "view"),
276283
SubjectRelation: RR("user", "..."),
277284
SubjectIds: []string{"mariah"},
@@ -286,13 +293,14 @@ func TestStableCacheKeys(t *testing.T) {
286293
return v
287294
}(),
288295
}, computeBothHashes)
296+
return key
289297
},
290-
"cdffc895cb9299b9da01",
298+
"a0aebfb9a8abd1b802",
291299
},
292300
{
293301
"lookup resources 2 with nested context",
294302
func() DispatchCacheKey {
295-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
303+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
296304
ResourceRelation: RR("document", "view"),
297305
SubjectRelation: RR("user", "..."),
298306
SubjectIds: []string{"mariah"},
@@ -311,13 +319,14 @@ func TestStableCacheKeys(t *testing.T) {
311319
return v
312320
}(),
313321
}, computeBothHashes)
322+
return key
314323
},
315-
"84d8d38ff9fadbb36d",
324+
"8e8ddfd8affeecc918",
316325
},
317326
{
318327
"lookup resources 2 with empty cursor",
319328
func() DispatchCacheKey {
320-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
329+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
321330
ResourceRelation: RR("document", "view"),
322331
SubjectRelation: RR("user", "..."),
323332
SubjectIds: []string{"mariah"},
@@ -327,13 +336,14 @@ func TestStableCacheKeys(t *testing.T) {
327336
},
328337
OptionalCursor: &v1.Cursor{},
329338
}, computeBothHashes)
339+
return key
330340
},
331-
"f49fbafef9c489abe601",
341+
"9884bbb3acd3b3ca1a",
332342
},
333343
{
334344
"lookup resources 2 with non-empty cursor",
335345
func() DispatchCacheKey {
336-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
346+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
337347
ResourceRelation: RR("document", "view"),
338348
SubjectRelation: RR("user", "..."),
339349
SubjectIds: []string{"mariah"},
@@ -345,13 +355,14 @@ func TestStableCacheKeys(t *testing.T) {
345355
Sections: []string{"foo"},
346356
},
347357
}, computeBothHashes)
358+
return key
348359
},
349-
"f09894c0d9a0b8ff7f",
360+
"9e82ddefb6ccbfd6aa01",
350361
},
351362
{
352363
"lookup resources 2 with different cursor",
353364
func() DispatchCacheKey {
354-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
365+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
355366
ResourceRelation: RR("document", "view"),
356367
SubjectRelation: RR("user", "..."),
357368
SubjectIds: []string{"mariah"},
@@ -363,13 +374,14 @@ func TestStableCacheKeys(t *testing.T) {
363374
Sections: []string{"foo", "bar"},
364375
},
365376
}, computeBothHashes)
377+
return key
366378
},
367-
"badfd7c59cdbcef671",
379+
"e593e789a89a9acd13",
368380
},
369381
{
370382
"lookup resources 2 with different terminal subject",
371383
func() DispatchCacheKey {
372-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
384+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
373385
ResourceRelation: RR("document", "view"),
374386
SubjectRelation: RR("user", "..."),
375387
SubjectIds: []string{"mariah"},
@@ -381,13 +393,14 @@ func TestStableCacheKeys(t *testing.T) {
381393
Sections: []string{"foo", "bar"},
382394
},
383395
}, computeBothHashes)
396+
return key
384397
},
385-
"8787b685e9cea993a901",
398+
"f6cf8df7bdc7959520",
386399
},
387400
{
388401
"lookup resources 2 with different subject IDs",
389402
func() DispatchCacheKey {
390-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
403+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
391404
ResourceRelation: RR("document", "view"),
392405
SubjectRelation: RR("user", "..."),
393406
SubjectIds: []string{"mariah", "tom"},
@@ -399,8 +412,9 @@ func TestStableCacheKeys(t *testing.T) {
399412
Sections: []string{"foo", "bar"},
400413
},
401414
}, computeBothHashes)
415+
return key
402416
},
403-
"f4d6d884eaa5e4b4ed01",
417+
"de839ec8eea2f7bf19",
404418
},
405419
}
406420

@@ -474,19 +488,20 @@ var generatorFuncs = map[string]generatorFunc{
474488
subjectRelation *core.RelationReference,
475489
metadata *v1.ResolverMeta,
476490
) (DispatchCacheKey, []string) {
477-
return lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
478-
ResourceRelation: resourceRelation,
479-
SubjectRelation: subjectRelation,
480-
SubjectIds: subjectIds,
481-
TerminalSubject: ONR(subjectRelation.Namespace, subjectIds[0], subjectRelation.Relation),
482-
Metadata: metadata,
483-
}, computeBothHashes), []string{
484-
resourceRelation.Namespace,
485-
resourceRelation.Relation,
486-
subjectRelation.Namespace,
487-
subjectIds[0],
488-
subjectRelation.Relation,
489-
}
491+
key, _ := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
492+
ResourceRelation: resourceRelation,
493+
SubjectRelation: subjectRelation,
494+
SubjectIds: subjectIds,
495+
TerminalSubject: ONR(subjectRelation.Namespace, subjectIds[0], subjectRelation.Relation),
496+
Metadata: metadata,
497+
}, computeBothHashes)
498+
return key, []string{
499+
resourceRelation.Namespace,
500+
resourceRelation.Relation,
501+
subjectRelation.Namespace,
502+
subjectIds[0],
503+
subjectRelation.Relation,
504+
}
490505
},
491506

492507
// Expand.
@@ -616,7 +631,7 @@ func TestComputeOnlyStableHash(t *testing.T) {
616631
}
617632

618633
func TestComputeContextHash(t *testing.T) {
619-
result := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
634+
result, err := lookupResourcesRequest2ToKey(&v1.DispatchLookupResources2Request{
620635
ResourceRelation: RR("document", "view"),
621636
SubjectRelation: RR("user", "..."),
622637
SubjectIds: []string{"mariah"},
@@ -640,5 +655,6 @@ func TestComputeContextHash(t *testing.T) {
640655
}(),
641656
}, computeBothHashes)
642657

643-
require.Equal(t, "81aab1c790f0be947d", hex.EncodeToString(result.StableSumAsBytes()))
658+
require.NoError(t, err)
659+
require.Equal(t, "e49efdc8e1d99daca601", hex.EncodeToString(result.StableSumAsBytes()))
644660
}

‎internal/dispatch/keys/hasher_common.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"sort"
55
"strconv"
66

7-
"github.com/authzed/spicedb/pkg/caveats"
87
core "github.com/authzed/spicedb/pkg/proto/core/v1"
98
v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
109
"github.com/authzed/spicedb/pkg/tuple"
@@ -95,8 +94,8 @@ func (hc hashableCursorSections) AppendToHash(hasher hasherInterface) {
9594
}
9695
}
9796

98-
type hashableContext struct{ caveats.HashableContext }
97+
type hashableContextString string
9998

100-
func (hc hashableContext) AppendToHash(hasher hasherInterface) {
101-
hc.HashableContext.AppendToHash(hasher)
99+
func (hc hashableContextString) AppendToHash(hasher hasherInterface) {
100+
hasher.WriteString(string(hc))
102101
}

‎internal/dispatch/keys/keys.go‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,11 @@ type Handler interface {
4444
type baseKeyHandler struct{}
4545

4646
func (b baseKeyHandler) LookupResources2CacheKey(_ context.Context, req *v1.DispatchLookupResources2Request) (DispatchCacheKey, error) {
47-
return lookupResourcesRequest2ToKey(req, computeBothHashes), nil
47+
return lookupResourcesRequest2ToKey(req, computeBothHashes)
4848
}
4949

5050
func (b baseKeyHandler) LookupResources3CacheKey(_ context.Context, req *v1.DispatchLookupResources3Request) (DispatchCacheKey, error) {
51-
return lookupResourcesRequest3ToKey(req, computeBothHashes), nil
51+
return lookupResourcesRequest3ToKey(req, computeBothHashes)
5252
}
5353

5454
func (b baseKeyHandler) LookupSubjectsCacheKey(_ context.Context, req *v1.DispatchLookupSubjectsRequest) (DispatchCacheKey, error) {
@@ -64,11 +64,19 @@ func (b baseKeyHandler) CheckDispatchKey(_ context.Context, req *v1.DispatchChec
6464
}
6565

6666
func (b baseKeyHandler) LookupResources2DispatchKey(_ context.Context, req *v1.DispatchLookupResources2Request) ([]byte, error) {
67-
return lookupResourcesRequest2ToKey(req, computeOnlyStableHash).StableSumAsBytes(), nil
67+
hash, err := lookupResourcesRequest2ToKey(req, computeOnlyStableHash)
68+
if err != nil {
69+
return nil, err
70+
}
71+
return hash.StableSumAsBytes(), nil
6872
}
6973

7074
func (b baseKeyHandler) LookupResources3DispatchKey(_ context.Context, req *v1.DispatchLookupResources3Request) ([]byte, error) {
71-
return lookupResourcesRequest3ToKey(req, computeOnlyStableHash).StableSumAsBytes(), nil
75+
hash, err := lookupResourcesRequest3ToKey(req, computeOnlyStableHash)
76+
if err != nil {
77+
return nil, err
78+
}
79+
return hash.StableSumAsBytes(), nil
7280
}
7381

7482
func (b baseKeyHandler) LookupSubjectsDispatchKey(_ context.Context, req *v1.DispatchLookupSubjectsRequest) ([]byte, error) {

0 commit comments

Comments
 (0)
⚡