Skip to content

Commit 25a1ca4

Browse files
committed
[WINA-2940] Report per-scope CSE truncation, and fix the bounds comments
The invocation cap dropped what it cut silently while the GPO cap next to it reported GPOsOmitted, so the payload annotated one cap and not the other. Add computer_cses_omitted / user_cses_omitted, omitempty and therefore free on every payload that does not overflow. The cap has less headroom than "backstop" implied: a stock Windows 11 26200 registers 57 client-side extensions under Winlogon\GPExtensions, seven below the cap of 64, and third-party extensions register into the same key. The byte-limit comment claimed one rationale for two constants that have different origins - 128 cannot be a 256-character budget under any encoding - and "worst UTF-8 case" understated it, since 512 is two bytes per character rather than the three or four a worst case implies. Each constant now carries its own derivation. Also guard seven unchecked slice indexes in the tests. Indexing before establishing the length turns a one-assertion regression into a panic that aborts the whole test binary and names an indexing expression rather than the invariant that broke. TestGPOMultiplePerCSE additionally states the ordering its [1] depends on instead of assuming it. TestSubmitEvent_WorstCasePayloadSize carries both new fields, so they sit inside the one byte budget that couples the four caps: 2,373,788 bytes of 3,000,000, up 54 bytes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1b48a23 commit 25a1ca4

3 files changed

Lines changed: 73 additions & 23 deletions

File tree

comp/logonduration/impl/grouppolicy.go

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,11 @@ import (
2727
// discarded there long after the send returned nil. A worst-case-size test derives one
2828
// byte budget from all four.
2929
const (
30-
// maxCSEInvocationsPerScope is a backstop: one invocation per extension per pass.
30+
// maxCSEInvocationsPerScope bounds the invocations one pass reports: one per
31+
// extension, and a stock Windows 11 registers 57 of them under Winlogon\GPExtensions,
32+
// which third-party extensions add to. So this is a backstop with little headroom
33+
// rather than an unreachable ceiling, and what it drops is reported per scope in
34+
// ComputerCSEsOmitted / UserCSEsOmitted.
3135
maxCSEInvocationsPerScope = 64
3236

3337
// maxGPOsPerCSE bounds the GPO references carried by one invocation. GPOs are
@@ -36,8 +40,14 @@ const (
3640
// GPOsOmitted.
3741
maxGPOsPerCSE = 32
3842

39-
// Byte limits, sized for the worst UTF-8 case: AD permits a 256-character name.
43+
// maxCSENameBytes bounds a name the provider registers rather than one anybody
44+
// chose: the longest of those 57 is 38 characters.
4045
maxCSENameBytes = 128
46+
47+
// maxGPONameBytes bounds a display name chosen in AD, where 256 characters is the
48+
// ceiling, so this carries one whole at two bytes per character - the boundary
49+
// TestGPONamesSurviveNonLatinScripts pins. A name in a three-byte script is cut on
50+
// a UTF-8 boundary instead, which is why the character figure need not be exact.
4151
maxGPONameBytes = 512
4252
)
4353

@@ -63,7 +73,12 @@ const (
6373
// and user_group_policy entries; a populated array implies its parent milestone.
6474
type GroupPolicyDetails struct {
6575
Computer []CSEInvocation `json:"computer,omitempty"`
66-
User []CSEInvocation `json:"user,omitempty"`
76+
// ComputerCSEsOmitted counts what maxCSEInvocationsPerScope cut from Computer.
77+
ComputerCSEsOmitted int `json:"computer_cses_omitted,omitempty"`
78+
79+
User []CSEInvocation `json:"user,omitempty"`
80+
// UserCSEsOmitted counts what maxCSEInvocationsPerScope cut from User.
81+
UserCSEsOmitted int `json:"user_cses_omitted,omitempty"`
6782
}
6883

6984
// CSEInvocation is one measured CSE invocation; the array it appears in sets the scope.
@@ -257,24 +272,28 @@ func (a *gpAccumulator) mergeGPONames(names map[string]string) {
257272
// Still-open records have no interval and are never closed against the trace end.
258273
func (a *gpAccumulator) finalize(tl BootTimeline) *GroupPolicyDetails {
259274
offsetOf := bootOffsetFunc(tl)
260-
details := &GroupPolicyDetails{
261-
Computer: a.buildScope(gpScopeComputer, offsetOf),
262-
User: a.buildScope(gpScopeUser, offsetOf),
263-
}
264-
if len(details.Computer) == 0 && len(details.User) == 0 {
275+
computer, computerOmitted := a.buildScope(gpScopeComputer, offsetOf)
276+
user, userOmitted := a.buildScope(gpScopeUser, offsetOf)
277+
if len(computer) == 0 && len(user) == 0 {
265278
return nil
266279
}
267-
return details
280+
return &GroupPolicyDetails{
281+
Computer: computer,
282+
ComputerCSEsOmitted: computerOmitted,
283+
User: user,
284+
UserCSEsOmitted: userOmitted,
285+
}
268286
}
269287

270-
// buildScope converts the invocations belonging to one boot pass.
271-
func (a *gpAccumulator) buildScope(scope gpScope, offsetOf func(time.Time) int64) []CSEInvocation {
288+
// buildScope converts the invocations belonging to one boot pass, reporting how many
289+
// the invocation cap cut.
290+
func (a *gpAccumulator) buildScope(scope gpScope, offsetOf func(time.Time) int64) ([]CSEInvocation, int) {
272291
if !a.passPinned[scope] {
273-
return nil
292+
return nil, 0
274293
}
275294
records := a.done[a.passActivity[scope]]
276295
if len(records) == 0 {
277-
return nil
296+
return nil, 0
278297
}
279298

280299
out := make([]CSEInvocation, 0, len(records))
@@ -291,7 +310,9 @@ func (a *gpAccumulator) buildScope(scope gpScope, offsetOf func(time.Time) int64
291310
})
292311
}
293312

313+
omitted := 0
294314
if len(out) > maxCSEInvocationsPerScope {
315+
omitted = len(out) - maxCSEInvocationsPerScope
295316
log.Warnf("Logon duration: %d Group Policy extension invocations in one pass exceeds the %d the payload carries, keeping the least healthy and the slowest",
296317
len(out), maxCSEInvocationsPerScope)
297318
out = retainMostRelevant(out)
@@ -303,7 +324,7 @@ func (a *gpAccumulator) buildScope(scope gpScope, offsetOf func(time.Time) int64
303324
}
304325
return out[i].CSEID < out[j].CSEID
305326
})
306-
return out
327+
return out, omitted
307328
}
308329

309330
// retainMostRelevant cuts an over-long list to maxCSEInvocationsPerScope: non-success

comp/logonduration/impl/grouppolicy_test.go

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,9 @@ func TestCSEZeroDurationIsReported(t *testing.T) {
185185
f.cseStart(gpTestActivity, 13*time.Second, cseRegistryGUID, "Registry", false, "")
186186
f.cseStop(gpTestActivity, 13*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
187187

188-
inv := f.details().Computer[0]
188+
invs := f.details().Computer
189+
require.Len(t, invs, 1)
190+
inv := invs[0]
189191
assert.Equal(t, int64(0), inv.DurationMs)
190192

191193
raw, err := json.Marshal(inv)
@@ -199,7 +201,9 @@ func TestCSEAsyncIsFlagged(t *testing.T) {
199201
f.cseStart(gpTestActivity, 13*time.Second, cseAuditGUID, "Audit Policy", true, "")
200202
f.cseStop(gpTestActivity, 13100*time.Millisecond, evtCSEStopSuccess, cseAuditGUID, "Audit Policy")
201203

202-
inv := f.details().Computer[0]
204+
invs := f.details().Computer
205+
require.Len(t, invs, 1)
206+
inv := invs[0]
203207
assert.True(t, inv.Async)
204208
assert.Equal(t, cseResultSuccess, inv.Result)
205209
assert.Equal(t, int64(100), inv.DurationMs)
@@ -350,6 +354,14 @@ func TestInvocationBackstopKeepsTheLeastHealthyAndTheSlowest(t *testing.T) {
350354

351355
d := f.details()
352356
require.Len(t, d.Computer, maxCSEInvocationsPerScope)
357+
assert.Equal(t, extra, d.ComputerCSEsOmitted, "the count the payload carries is what the cap cut")
358+
assert.Zero(t, d.UserCSEsOmitted, "the count belongs to the scope that overflowed, not to the block")
359+
360+
raw, err := json.Marshal(d)
361+
require.NoError(t, err)
362+
assert.Contains(t, string(raw), fmt.Sprintf(`"computer_cses_omitted":%d`, extra),
363+
"a truncated pass says so on the wire")
364+
assert.NotContains(t, string(raw), "user_cses_omitted", "the untruncated scope stays silent")
353365

354366
kept := make(map[string]CSEInvocation, len(d.Computer))
355367
for _, inv := range d.Computer {
@@ -445,7 +457,9 @@ func TestCSEOffsetsShareTheBootTimelineAxis(t *testing.T) {
445457
f.cseStop(gpUserActivity, 73*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
446458
f.send(gpUserActivity, evtUserGPEnd, 75*time.Second)
447459

448-
inv := f.details().User[0]
460+
invs := f.details().User
461+
require.Len(t, invs, 1)
462+
inv := invs[0]
449463

450464
var parent Milestone
451465
for _, m := range buildTimelineMilestones(f.coll.timeline) {
@@ -526,7 +540,9 @@ func TestGPONamesComeFromTheApplicableList(t *testing.T) {
526540
gpoFragment(gpoRichEntry(gpoDefaultDomainGUID, "Default Domain Policy")))
527541
f.cseStop(gpTestActivity, 14*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
528542

529-
gpos := f.details().Computer[0].GPOs
543+
invs := f.details().Computer
544+
require.Len(t, invs, 1)
545+
gpos := invs[0].GPOs
530546
require.Len(t, gpos, 1, "the extension GUID inside <Extensions> is not a GPO")
531547
assert.Equal(t, gpoDefaultDomainGUID, gpos[0].ID)
532548
assert.Equal(t, "Default Domain Policy", gpos[0].Name)
@@ -543,11 +559,13 @@ func TestGPONamesSurviveUnescapedAmpersand(t *testing.T) {
543559
))
544560
f.cseStop(gpTestActivity, 14*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
545561

562+
invs := f.details().Computer
563+
require.Len(t, invs, 1)
546564
assert.Equal(t, []GPORef{
547565
{ID: gpoDefaultDomainGUID, Name: "R&D Baseline"},
548566
{ID: gpoDomainCtlGUID, Name: "Sales & Marketing"},
549567
{ID: gpoThirdGUID, Name: "Plain Name"},
550-
}, f.details().Computer[0].GPOs)
568+
}, invs[0].GPOs)
551569
}
552570

553571
func TestGPONameSharedFromAnotherInvocationsList(t *testing.T) {
@@ -580,7 +598,10 @@ func TestGPOMultiplePerCSE(t *testing.T) {
580598
gpoDefaultDomainGUID+";"+gpoDomainCtlGUID+";"+gpoThirdGUID)
581599
f.cseStop(gpTestActivity, 14*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
582600

583-
gpos := f.details().Computer[1].GPOs
601+
invs := f.details().Computer
602+
require.Len(t, invs, 2)
603+
require.Equal(t, cseRegistryGUID, invs[1].CSEID, "the chronological sort puts Registry second")
604+
gpos := invs[1].GPOs
584605
require.Len(t, gpos, 3)
585606
assert.Equal(t, "Default Domain Policy", gpos[0].Name)
586607
assert.Equal(t, "Default Domain Controllers Policy", gpos[1].Name)
@@ -597,7 +618,9 @@ func TestGPODuplicateDisplayNamesStayDistinct(t *testing.T) {
597618
))
598619
f.cseStop(gpTestActivity, 14*time.Second, evtCSEStopSuccess, cseRegistryGUID, "Registry")
599620

600-
gpos := f.details().Computer[0].GPOs
621+
invs := f.details().Computer
622+
require.Len(t, invs, 1)
623+
gpos := invs[0].GPOs
601624
require.Len(t, gpos, 2)
602625
assert.NotEqual(t, gpos[0].ID, gpos[1].ID)
603626
assert.Equal(t, "Baseline", gpos[0].Name)

comp/logonduration/impl/impl_windows_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ func TestSubmitEvent_GroupPolicyReachesTheWire(t *testing.T) {
422422
assert.Equal(t, "Default Domain Policy", gpos[0].(map[string]interface{})["name"])
423423

424424
assert.NotContains(t, block, "user")
425+
assert.NotContains(t, block, "computer_cses_omitted", "a pass under the cap carries no truncation count")
425426

426427
assert.Contains(t, custom, "boot_timeline")
427428
}
@@ -459,8 +460,13 @@ func TestSubmitEvent_WorstCasePayloadSize(t *testing.T) {
459460
}
460461

461462
_, size := submitAndDecodeCustom(t, &AnalysisResult{
462-
Timeline: fullBootTimeline(boot),
463-
GroupPolicy: &GroupPolicyDetails{Computer: pass(), User: pass()},
463+
Timeline: fullBootTimeline(boot),
464+
GroupPolicy: &GroupPolicyDetails{
465+
Computer: pass(),
466+
ComputerCSEsOmitted: 4096,
467+
User: pass(),
468+
UserCSEsOmitted: 4096,
469+
},
464470
})
465471

466472
t.Logf("worst case %d bytes of %d (%d invocations x %d GPO refs at %d-byte names), %d bytes of margin",

0 commit comments

Comments
 (0)