From 52ed52c52fc513d7f8e30a2b5e37a27bfc73d97d Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Wed, 13 May 2026 17:50:42 +0100 Subject: [PATCH 1/7] Add disable_group_name_selectors option to Windows workload attestor Signed-off-by: Janani Arunachalam --- doc/plugin_agent_workloadattestor_windows.md | 9 ++++--- .../windows/windows_windows.go | 27 ++++++++++++------- .../windows/windows_windows_test.go | 20 ++++++++++++++ 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/doc/plugin_agent_workloadattestor_windows.md b/doc/plugin_agent_workloadattestor_windows.md index 83e884ec87..bb609b0029 100644 --- a/doc/plugin_agent_workloadattestor_windows.md +++ b/doc/plugin_agent_workloadattestor_windows.md @@ -3,10 +3,11 @@ The `windows` plugin generates Windows-based selectors for workloads calling the agent. It does so by opening an access token associated with the workload process. The system is then interrogated to retrieve user and group account information from that access token. -| Configuration | Description | Default | -|--------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------|---------| -| `discover_workload_path` | If true, the workload path will be discovered by the plugin and used to provide additional selectors | false | -| `workload_size_limit` | The limit of workload binary sizes when calculating certain selectors (e.g. sha256). If zero, no limit is enforced. If negative, never calculate the hash. | 0 | +| Configuration | Description | Default | +|--------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------| +| `discover_workload_path` | If true, the workload path will be discovered by the plugin and used to provide additional selectors | false | +| `workload_size_limit` | The limit of workload binary sizes when calculating certain selectors (e.g. sha256). If zero, no limit is enforced. If negative, never calculate the hash. | 0 | +| `disable_group_name_selectors` | If true, skips resolving group SIDs to human-readable names, avoiding expensive Domain Controller lookups. Group SID selectors (`group_sid`) are always collected regardless. Only `group_name` selectors are affected. | false | ## Workload Selectors diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go index 97128f07ab..3386da8882 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "sync" + "time" "github.com/hashicorp/go-hclog" "github.com/hashicorp/hcl" @@ -33,8 +34,9 @@ func New() *Plugin { } type Configuration struct { - DiscoverWorkloadPath bool `hcl:"discover_workload_path"` - WorkloadSizeLimit int64 `hcl:"workload_size_limit"` + DiscoverWorkloadPath bool `hcl:"discover_workload_path"` + WorkloadSizeLimit int64 `hcl:"workload_size_limit"` + DisableGroupNameSelectors bool `hcl:"disable_group_name_selectors"` } func buildConfig(coreConfig catalog.CoreConfig, hclText string, status *pluginconf.Status) *Configuration { @@ -77,7 +79,7 @@ func (p *Plugin) Attest(_ context.Context, req *workloadattestorv1.AttestRequest return nil, err } - process, err := p.newProcessInfo(req.Pid, config.DiscoverWorkloadPath) + process, err := p.newProcessInfo(req.Pid, config.DiscoverWorkloadPath, config.DisableGroupNameSelectors) if err != nil { return nil, status.Errorf(codes.Internal, "failed to get process information: %v", err) } @@ -119,7 +121,7 @@ func (p *Plugin) AttestReference(_ context.Context, _ *workloadattestorv1.Attest return nil, status.Error(codes.Unimplemented, "AttestReference not implemented") } -func (p *Plugin) newProcessInfo(pid int32, queryPath bool) (*processInfo, error) { +func (p *Plugin) newProcessInfo(pid int32, queryPath bool, disableGroupNames bool) (*processInfo, error) { p.log = p.log.With(telemetry.PID, pid) h, err := p.q.OpenProcess(pid) @@ -174,13 +176,18 @@ func (p *Plugin) newProcessInfo(pid int32, queryPath bool) (*processInfo, error) // https://docs.microsoft.com/en-us/windows/win32/secauthz/sid-attributes-in-an-access-token enabledSelector := getGroupEnabledSelector(group.Attributes) processInfo.groupsSIDs = append(processInfo.groupsSIDs, enabledSelector+":"+group.Sid.String()) - groupAccount, groupDomain, err := p.q.LookupAccount(group.Sid) - if err != nil { - p.log.Warn("failed to lookup account from group SID", "sid", group.Sid, "error", err) - continue + if !disableGroupNames { + start := time.Now() + groupAccount, groupDomain, err := p.q.LookupAccount(group.Sid) + elapsed := time.Since(start).Milliseconds() + if err != nil { + p.log.Warn("failed to lookup account from group SID", "sid", group.Sid, "error", err) + continue + } + p.log.Debug("lookupAccount (group) completed", "sid", group.Sid, "elapsed_ms", elapsed) + // If the LookupAccount call succeeded, we know that groupAccount is not empty + processInfo.groups = append(processInfo.groups, enabledSelector+":"+parseAccount(groupAccount, groupDomain)) } - // If the LookupAccount call succeeded, we know that groupAccount is not empty - processInfo.groups = append(processInfo.groups, enabledSelector+":"+parseAccount(groupAccount, groupDomain)) } if queryPath { diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go index a37b29482b..8eb4d577bb 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go @@ -115,6 +115,26 @@ func TestAttest(t *testing.T) { }, expectCode: codes.OK, }, + { + name: "successful with groups, group name selectors disabled", + trustDomain: "example.org", + config: "disable_group_name_selectors = true", + pq: &fakeProcessQuery{ + handle: windows.InvalidHandle, + tokenUser: &windows.Tokenuser{User: windows.SIDAndAttributes{Sid: sidUser}}, + tokenGroups: &windows.Tokengroups{Groups: [1]windows.SIDAndAttributes{sidAndAttrGroup1}}, + account: "user1", + domain: "domain1", + sidAndAttributes: []windows.SIDAndAttributes{sidAndAttrGroup1, sidAndAttrGroup3}, + }, + expectSelectors: []string{ + "windows:user_name:domain1\\user1", + "windows:user_sid:" + sidUser.String(), + "windows:group_sid:se_group_enabled:true:" + sidGroup1.String(), + "windows:group_sid:se_group_enabled:true:" + sidGroup3.String(), + }, + expectCode: codes.OK, + }, { name: "successful getting path and hashing process binary", trustDomain: "example.org", From 28332aaa68b9b63ebcc264c9ecdc8bdedc9cecef Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Fri, 15 May 2026 12:10:01 +0100 Subject: [PATCH 2/7] Update what gets logged Signed-off-by: Janani Arunachalam --- .../workloadattestor/windows/windows_windows.go | 11 ++++++++--- .../workloadattestor/windows/windows_windows_test.go | 6 ++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go index 3386da8882..a65e48f662 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go @@ -169,6 +169,7 @@ func (p *Plugin) newProcessInfo(pid int32, queryPath bool, disableGroupNames boo } groups := p.q.AllGroups(tokenGroups) + start := time.Now() for _, group := range groups { // Each group has a set of attributes that control how // the system uses the SID in an access check. @@ -177,18 +178,18 @@ func (p *Plugin) newProcessInfo(pid int32, queryPath bool, disableGroupNames boo enabledSelector := getGroupEnabledSelector(group.Attributes) processInfo.groupsSIDs = append(processInfo.groupsSIDs, enabledSelector+":"+group.Sid.String()) if !disableGroupNames { - start := time.Now() groupAccount, groupDomain, err := p.q.LookupAccount(group.Sid) - elapsed := time.Since(start).Milliseconds() if err != nil { p.log.Warn("failed to lookup account from group SID", "sid", group.Sid, "error", err) continue } - p.log.Debug("lookupAccount (group) completed", "sid", group.Sid, "elapsed_ms", elapsed) // If the LookupAccount call succeeded, we know that groupAccount is not empty processInfo.groups = append(processInfo.groups, enabledSelector+":"+parseAccount(groupAccount, groupDomain)) } } + if !disableGroupNames { + p.log.Debug("lookupAccount (groups) completed", "count", len(groups), "elapsed_ms", time.Since(start).Milliseconds()) + } if queryPath { if processInfo.path, err = p.q.GetProcessExe(h); err != nil { @@ -209,6 +210,10 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (* defer p.mu.Unlock() p.config = newConfig + if newConfig.DisableGroupNameSelectors { + p.log.Info("group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced") + } + return &configv1.ConfigureResponse{}, nil } diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go index 8eb4d577bb..cc9e28c472 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go @@ -134,6 +134,12 @@ func TestAttest(t *testing.T) { "windows:group_sid:se_group_enabled:true:" + sidGroup3.String(), }, expectCode: codes.OK, + expectLogs: []spiretest.LogEntry{ + { + Level: logrus.InfoLevel, + Message: "group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced", + }, + }, }, { name: "successful getting path and hashing process binary", From c6cd8101e76a25d9d225d74d696b7378b6ba0c26 Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Mon, 8 Jun 2026 16:16:09 +0100 Subject: [PATCH 3/7] address PR comments Signed-off-by: Janani Arunachalam --- doc/plugin_agent_workloadattestor_windows.md | 4 +++- pkg/agent/plugin/workloadattestor/windows/windows_windows.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/plugin_agent_workloadattestor_windows.md b/doc/plugin_agent_workloadattestor_windows.md index bb609b0029..e002239150 100644 --- a/doc/plugin_agent_workloadattestor_windows.md +++ b/doc/plugin_agent_workloadattestor_windows.md @@ -7,7 +7,7 @@ It does so by opening an access token associated with the workload process. The |--------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------| | `discover_workload_path` | If true, the workload path will be discovered by the plugin and used to provide additional selectors | false | | `workload_size_limit` | The limit of workload binary sizes when calculating certain selectors (e.g. sha256). If zero, no limit is enforced. If negative, never calculate the hash. | 0 | -| `disable_group_name_selectors` | If true, skips resolving group SIDs to human-readable names, avoiding expensive Domain Controller lookups. Group SID selectors (`group_sid`) are always collected regardless. Only `group_name` selectors are affected. | false | +| `disable_group_name_selectors` | If true, skips resolving group SIDs to human-readable names, avoiding potentially expensive account name resolution (e.g. against a Domain Controller). Group SID selectors (`group_sid`) are always collected regardless. Only `group_name` selectors are affected. | false | ## Workload Selectors @@ -48,6 +48,8 @@ Defenses against this are: - User and group account names are expressed using the [down-level logon name format](https://docs.microsoft.com/en-us/windows/win32/secauthn/user-name-formats#down-level-logon-name). +- Enabling `disable_group_name_selectors` will cause existing workload registration entries that use `group_name` selectors to stop matching. Operators should audit those entries and switch them to `group_sid` selectors before enabling this flag. + ## Configuration This plugin does not require any configuration setting. It can be added in the following way in the agent configuration file: diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go index a65e48f662..634c880a07 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go @@ -188,7 +188,7 @@ func (p *Plugin) newProcessInfo(pid int32, queryPath bool, disableGroupNames boo } } if !disableGroupNames { - p.log.Debug("lookupAccount (groups) completed", "count", len(groups), "elapsed_ms", time.Since(start).Milliseconds()) + p.log.Debug("Group account name lookups completed", "count", len(groups), "duration_ms", time.Since(start).Milliseconds()) } if queryPath { From 880acd88d8fabd1044ef1a8bac9ef137bf6d1503 Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Mon, 8 Jun 2026 16:41:16 +0100 Subject: [PATCH 4/7] fix log line casing Signed-off-by: Janani Arunachalam --- pkg/agent/plugin/workloadattestor/windows/windows_windows.go | 2 +- .../plugin/workloadattestor/windows/windows_windows_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go index 634c880a07..d90501fa14 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go @@ -211,7 +211,7 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (* p.config = newConfig if newConfig.DisableGroupNameSelectors { - p.log.Info("group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced") + p.log.Info("Group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced") } return &configv1.ConfigureResponse{}, nil diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go index cc9e28c472..c4281cbf09 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go @@ -137,7 +137,7 @@ func TestAttest(t *testing.T) { expectLogs: []spiretest.LogEntry{ { Level: logrus.InfoLevel, - Message: "group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced", + Message: "Group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced", }, }, }, From 4e48a76400a35d91c5679148a2ba6608b2184fa2 Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Mon, 8 Jun 2026 17:14:30 +0100 Subject: [PATCH 5/7] md linting Signed-off-by: Janani Arunachalam --- doc/plugin_agent_workloadattestor_windows.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/plugin_agent_workloadattestor_windows.md b/doc/plugin_agent_workloadattestor_windows.md index e002239150..2d60445283 100644 --- a/doc/plugin_agent_workloadattestor_windows.md +++ b/doc/plugin_agent_workloadattestor_windows.md @@ -3,10 +3,10 @@ The `windows` plugin generates Windows-based selectors for workloads calling the agent. It does so by opening an access token associated with the workload process. The system is then interrogated to retrieve user and group account information from that access token. -| Configuration | Description | Default | -|--------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------| -| `discover_workload_path` | If true, the workload path will be discovered by the plugin and used to provide additional selectors | false | -| `workload_size_limit` | The limit of workload binary sizes when calculating certain selectors (e.g. sha256). If zero, no limit is enforced. If negative, never calculate the hash. | 0 | +| Configuration | Description | Default | +|--------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------| +| `discover_workload_path` | If true, the workload path will be discovered by the plugin and used to provide additional selectors | false | +| `workload_size_limit` | The limit of workload binary sizes when calculating certain selectors (e.g. sha256). If zero, no limit is enforced. If negative, never calculate the hash. | 0 | | `disable_group_name_selectors` | If true, skips resolving group SIDs to human-readable names, avoiding potentially expensive account name resolution (e.g. against a Domain Controller). Group SID selectors (`group_sid`) are always collected regardless. Only `group_name` selectors are affected. | false | ## Workload Selectors From 31a4ac0efdb08bef391c81c2918b25286572495f Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Thu, 2 Jul 2026 14:14:48 +0100 Subject: [PATCH 6/7] update log line as per pr comment Signed-off-by: Janani Arunachalam --- pkg/agent/plugin/workloadattestor/windows/windows_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go index d90501fa14..b0a7fcf839 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows.go @@ -211,7 +211,7 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (* p.config = newConfig if newConfig.DisableGroupNameSelectors { - p.log.Info("Group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced") + p.log.Info("Group name selectors disabled; only group_sid selectors will be produced for groups") } return &configv1.ConfigureResponse{}, nil From e7ebb722edde491a19498e8f04bff51d4c12d19f Mon Sep 17 00:00:00 2001 From: Janani Arunachalam Date: Thu, 2 Jul 2026 15:19:02 +0100 Subject: [PATCH 7/7] update unit test log line to match Signed-off-by: Janani Arunachalam --- .../plugin/workloadattestor/windows/windows_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go index c4281cbf09..7aa43b1f18 100644 --- a/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go +++ b/pkg/agent/plugin/workloadattestor/windows/windows_windows_test.go @@ -137,7 +137,7 @@ func TestAttest(t *testing.T) { expectLogs: []spiretest.LogEntry{ { Level: logrus.InfoLevel, - Message: "Group name selectors disabled; skipping LookupAccount for groups, only group_sid selectors will be produced", + Message: "Group name selectors disabled; only group_sid selectors will be produced for groups", }, }, },