From 361ed840e9a2eb171c058de03615aa04b4720559 Mon Sep 17 00:00:00 2001 From: rabbitstack Date: Wed, 19 Feb 2025 01:12:27 +0100 Subject: [PATCH 1/2] chore(telemetry): Remove system registry provider After exhaustive triaging, it was spotted that the system registry provider can miss stackwalk samples. This can have disastrous consequences such as wrongly attributing stack return addresses or accumulated events. To elegantly mitigate the problem, we enable the registry flags on the system logger and receive all events within the same session. --- internal/etw/source.go | 21 --------- internal/etw/source_test.go | 24 ++++------ internal/etw/trace.go | 56 +++--------------------- pkg/kevent/ktypes/ktypes_windows.go | 2 +- pkg/kevent/ktypes/ktypes_windows_test.go | 2 +- pkg/sys/etw/types.go | 36 --------------- 6 files changed, 15 insertions(+), 126 deletions(-) diff --git a/internal/etw/source.go b/internal/etw/source.go index 0a9e513bd..2031f948c 100644 --- a/internal/etw/source.go +++ b/internal/etw/source.go @@ -71,16 +71,6 @@ var ( buffersRead = expvar.NewInt("kstream.kbuffers.read") ) -// SupportsSystemProviders determines if the support for granular -// system providers in present. -func SupportsSystemProviders() bool { - maj, _, patch := windows.RtlGetNtVersionNumbers() - if maj > 10 { - return true - } - return maj >= 10 && patch >= 20348 -} - // EventSource is the core component responsible for // starting ETW tracing sessions and setting up event // consumers. @@ -177,13 +167,6 @@ func (e *EventSource) Open(config *config.Config) error { e.addTrace(etw.KernelLoggerSession, etw.KernelTraceControlGUID) - if SupportsSystemProviders() && !config.IsCaptureSet() { - log.Info("system providers support detected") - if config.Kstream.EnableRegistryKevents { - e.addTraceKeywords(etw.SystemRegistrySession, etw.SystemRegistryProviderID, etw.RegistryKeywordGeneral) - } - } - if config.Kstream.EnableDNSEvents { e.addTrace(etw.DNSClientSession, etw.DNSClientGUID) } @@ -321,7 +304,3 @@ func (e *EventSource) RegisterEventListener(lis kevent.Listener) { func (e *EventSource) addTrace(name string, guid windows.GUID) { e.traces = append(e.traces, NewTrace(name, guid, 0x0, e.config)) } - -func (e *EventSource) addTraceKeywords(name string, guid windows.GUID, keywords uint64) { - e.traces = append(e.traces, NewTrace(name, guid, keywords, e.config)) -} diff --git a/internal/etw/source_test.go b/internal/etw/source_test.go index 09e3eb3ba..326431198 100644 --- a/internal/etw/source_test.go +++ b/internal/etw/source_test.go @@ -127,21 +127,17 @@ func TestEventSourceStartTraces(t *testing.T) { evs := NewEventSource(psnap, hsnap, tt.cfg, nil) require.NoError(t, evs.Open(tt.cfg)) defer evs.Close() - if !SupportsSystemProviders() { - assert.Equal(t, tt.wantSessions, len(evs.(*EventSource).traces)) - } + assert.Equal(t, tt.wantSessions, len(evs.(*EventSource).traces)) for _, trace := range evs.(*EventSource).traces { require.True(t, trace.Handle().IsValid()) require.NoError(t, etw.ControlTrace(0, trace.Name, trace.GUID, etw.Query)) - if !SupportsSystemProviders() { - if tt.wantFlags != nil && trace.IsKernelTrace() { - flags, err := etw.GetTraceSystemFlags(trace.Handle()) - require.NoError(t, err) - // check enabled system event flags - require.Equal(t, tt.wantFlags[0], flags[0]) - require.Equal(t, tt.wantFlags[1], flags[4]) - } + if tt.wantFlags != nil && trace.IsKernelTrace() { + flags, err := etw.GetTraceSystemFlags(trace.Handle()) + require.NoError(t, err) + // check enabled system event flags + require.Equal(t, tt.wantFlags[0], flags[0]) + require.Equal(t, tt.wantFlags[1], flags[4]) } } }) @@ -204,11 +200,7 @@ func TestEventSourceEnableFlagsDynamically(t *testing.T) { flags := evs.(*EventSource).traces[0].enableFlagsDynamically(cfg.Kstream) - if SupportsSystemProviders() { - require.Len(t, evs.(*EventSource).traces, 3) - } else { - require.Len(t, evs.(*EventSource).traces, 2) - } + require.Len(t, evs.(*EventSource).traces, 2) require.True(t, flags&etw.FileIO != 0) require.True(t, flags&etw.Process != 0) diff --git a/internal/etw/trace.go b/internal/etw/trace.go index f60610807..1f6fce354 100644 --- a/internal/etw/trace.go +++ b/internal/etw/trace.go @@ -57,9 +57,7 @@ func initEventTraceProps(c config.KstreamConfig) etw.EventTraceProperties { flushTimer = time.Second } mode := uint32(etw.ProcessTraceModeRealtime) - if SupportsSystemProviders() { - mode |= etw.EventTraceSystemLoggerMode - } + return etw.EventTraceProperties{ Wnode: etw.WnodeHeader{ BufferSize: uint32(unsafe.Sizeof(etw.EventTraceProperties{})) + maxTracePropsSize, @@ -133,24 +131,13 @@ func (t *Trace) enableCallstacks() { if t.IsKernelTrace() { t.stackExtensions.EnableProcessCallstack() - // Enabling stack tracing for kernel trace - // with granular system providers support. - // In this situation, registry event callstacks - // are enabled if system provider support is not - // detected - if !SupportsSystemProviders() { - t.stackExtensions.EnableRegistryCallstack() - } + t.stackExtensions.EnableRegistryCallstack() t.stackExtensions.EnableFileCallstack() t.stackExtensions.EnableMemoryCallstack() } - if t.IsSystemRegistryTrace() { - t.stackExtensions.EnableRegistryCallstack() - } - if t.IsThreadpoolTrace() { t.stackExtensions.EnableThreadpoolCallstack() } @@ -231,7 +218,7 @@ func (t *Trace) Start() error { // enrichment is enabled, it is necessary to instruct the provider // to emit stack addresses in the extended data item section when // writing events to the session buffers - if cfg.StackEnrichment && !t.IsSystemProvider() && !t.IsThreadpoolTrace() { + if cfg.StackEnrichment && !t.IsThreadpoolTrace() { return etw.EnableTraceWithOpts(t.GUID, t.startHandle, t.Keywords, etw.EnableTraceOpts{WithStacktrace: true}) } else if cfg.StackEnrichment && len(t.stackExtensions.EventIds()) > 0 { if err := etw.EnableStackTracing(t.startHandle, t.stackExtensions.EventIds()); err != nil { @@ -239,24 +226,6 @@ func (t *Trace) Start() error { } } - if t.IsSystemRegistryTrace() { - if err := etw.EnableTrace(t.GUID, t.startHandle, t.Keywords); err != nil { - return err - } - // when the registry provider is started - // in a separate session, we can trigger - // KCB rundown events by using a similar - // approach described previously - handle := t.startHandle - sysTraceFlags := make([]etw.EventTraceFlags, 8) - if err := etw.SetTraceSystemFlags(handle, sysTraceFlags); err != nil { - log.Warnf("unable to set empty system flags on registry provider: %v", err) - return nil - } - sysTraceFlags[0] = etw.Registry - return etw.SetTraceSystemFlags(handle, sysTraceFlags) - } - return etw.EnableTrace(t.GUID, t.startHandle, t.Keywords) } @@ -348,17 +317,9 @@ func (t *Trace) Close() error { // IsKernelTrace determines if this is the system logger trace. func (t *Trace) IsKernelTrace() bool { return t.GUID == etw.KernelTraceControlGUID } -// IsSystemRegistryTrace determines if this is the system registry logger trace. -func (t *Trace) IsSystemRegistryTrace() bool { return t.GUID == etw.SystemRegistryProviderID } - // IsThreadpoolTrace determines if this is the thread pool logger trace. func (t *Trace) IsThreadpoolTrace() bool { return t.GUID == etw.ThreadpoolGUID } -// IsSystemProvider determines if this is one of the granular system provider traces. -func (t *Trace) IsSystemProvider() bool { - return t.GUID == etw.SystemIOProviderID || t.GUID == etw.SystemRegistryProviderID || t.GUID == etw.SystemProcessProviderID || t.GUID == etw.SystemMemoryProviderID -} - // enableFlagsDynamically crafts the system logger event mask // depending on the compiled rules result or the config state. // System logger flags is a bitmask that indicates which kernel events @@ -398,15 +359,8 @@ func (t *Trace) enableFlagsDynamically(config config.KstreamConfig) etw.EventTra if config.EnableMemKevents { flags |= etw.VirtualAlloc } - - if !SupportsSystemProviders() || t.config.IsCaptureSet() { - // Registry is the only provider started - // in its own tracing session because it - // generates an overwhelming amount of - // events - if config.EnableRegistryKevents { - flags |= etw.Registry - } + if config.EnableRegistryKevents { + flags |= etw.Registry } return flags diff --git a/pkg/kevent/ktypes/ktypes_windows.go b/pkg/kevent/ktypes/ktypes_windows.go index 20ac8ab01..1eb62e2b3 100644 --- a/pkg/kevent/ktypes/ktypes_windows.go +++ b/pkg/kevent/ktypes/ktypes_windows.go @@ -595,7 +595,7 @@ func (k Ktype) Source() EventSource { // events, but it appears first on the consumer callback // before other events published before it. func (k Ktype) CanArriveOutOfOrder() bool { - return k.Category() == Registry || k.Category() == Threadpool || k.Subcategory() == DNS || + return k.Category() == Threadpool || k.Subcategory() == DNS || k == OpenProcess || k == OpenThread || k == SetThreadContext || k == CreateSymbolicLinkObject } diff --git a/pkg/kevent/ktypes/ktypes_windows_test.go b/pkg/kevent/ktypes/ktypes_windows_test.go index 5087e3898..2f6b1da56 100644 --- a/pkg/kevent/ktypes/ktypes_windows_test.go +++ b/pkg/kevent/ktypes/ktypes_windows_test.go @@ -130,7 +130,7 @@ func TestGUIDAndHookIDFromKtype(t *testing.T) { } func TestCanArriveOutOfOrder(t *testing.T) { - assert.True(t, RegSetValue.CanArriveOutOfOrder()) + assert.False(t, RegSetValue.CanArriveOutOfOrder()) assert.False(t, VirtualAlloc.CanArriveOutOfOrder()) assert.True(t, OpenProcess.CanArriveOutOfOrder()) } diff --git a/pkg/sys/etw/types.go b/pkg/sys/etw/types.go index 04d7863eb..c1a5b92c3 100644 --- a/pkg/sys/etw/types.go +++ b/pkg/sys/etw/types.go @@ -66,48 +66,12 @@ const ( // ThreadpoolSession represents the session name for the thread pool logger ThreadpoolSession = "Threadpool Logger" - // SystemRegistrySession represents system registry logger - SystemRegistrySession = "System Registry Logger" - // WnodeTraceFlagGUID indicates that the structure contains event tracing information WnodeTraceFlagGUID = 0x00020000 // ProcessTraceModeRealtime denotes that there will be a real-time consumers for events forwarded from the providers ProcessTraceModeRealtime = 0x00000100 // ProcessTraceModeEventRecord is the mode that enables the "event record" format for kernel events ProcessTraceModeEventRecord = 0x10000000 - // EventTraceSystemLoggerMode enables events from system loggers - EventTraceSystemLoggerMode = 0x02000000 -) - -var ( - // SystemProcessProviderID provides events related to the process, including lifetime information, image load events, and thread related events. - SystemProcessProviderID = windows.GUID{Data1: 0x151f55dc, Data2: 0x467d, Data3: 0x471f, Data4: [8]byte{0x83, 0xb5, 0x5f, 0x88, 0x9d, 0x46, 0xff, 0x66}} - // SystemIOProviderID provides events related to multiple kinds of IO including disk, cache, and network. - SystemIOProviderID = windows.GUID{Data1: 0x3d5c43e3, Data2: 0x0f1c, Data3: 0x4202, Data4: [8]byte{0xb8, 0x17, 0x17, 0x4c, 0x00, 0x70, 0xdc, 0x79}} - // SystemRegistryProviderID provides events related to the registry. - SystemRegistryProviderID = windows.GUID{Data1: 0x16156bd9, Data2: 0xfab4, Data3: 0x4cfa, Data4: [8]byte{0xa2, 0x32, 0x89, 0xd1, 0x09, 0x90, 0x58, 0xe3}} - // SystemMemoryProviderID provides events related to the memory manager. - SystemMemoryProviderID = windows.GUID{Data1: 0x82958ca9, Data2: 0xb6cd, Data3: 0x47f8, Data4: [8]byte{0xa3, 0xa8, 0x03, 0xae, 0x85, 0xa4, 0xbc, 0x24}} -) - -const ( - // ProcessKeywordGeneral enables process events - ProcessKeywordGeneral = 0x1 - // ProcessKeywordThread enables thread events - ProcessKeywordThread = 0x800 - // ProcessKeywordLoader enables image load/unload events - ProcessKeywordLoader = 0x1000 - - // RegistryKeywordGeneral enables registry events - RegistryKeywordGeneral = 0x1 - - // IOKeywordNetwork enables network events - IOKeywordNetwork = 0x200 - - // MemoryVirtualAlloc enables memory alloc events - MemoryVirtualAlloc = 0x400 - // MemoryVAMap enables section mapping/unmapping events - MemoryVAMap = 0x4000 ) const ( From 818fa7bb913d07204b355c5219bc5e6c23597d1f Mon Sep 17 00:00:00 2001 From: rabbitstack Date: Wed, 19 Feb 2025 01:12:59 +0100 Subject: [PATCH 2/2] fix(processors): Overwrite KCB handle --- internal/etw/processors/registry_windows.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/etw/processors/registry_windows.go b/internal/etw/processors/registry_windows.go index 759150258..50441be54 100644 --- a/internal/etw/processors/registry_windows.go +++ b/internal/etw/processors/registry_windows.go @@ -87,9 +87,7 @@ func (r *registryProcessor) processEvent(e *kevent.Kevent) (*kevent.Kevent, erro switch e.Type { case ktypes.RegKCBRundown, ktypes.RegCreateKCB: khandle := e.Kparams.MustGetUint64(kparams.RegKeyHandle) - if _, ok := r.keys[khandle]; !ok { - r.keys[khandle], _ = e.Kparams.GetString(kparams.RegPath) - } + r.keys[khandle] = e.Kparams.MustGetString(kparams.RegPath) kcbCount.Add(1) case ktypes.RegDeleteKCB: khandle := e.Kparams.MustGetUint64(kparams.RegKeyHandle)