From 82c1608abbb2058f43b608d255ecc75da37e30dc Mon Sep 17 00:00:00 2001 From: Amit Schendel Date: Thu, 21 Nov 2024 11:22:01 +0000 Subject: [PATCH] Improving rule r0006 Signed-off-by: Amit Schendel --- ...unexpected_service_account_token_access.go | 143 +++++++++---- ...ected_service_account_token_access_test.go | 189 ++++++++++++++---- 2 files changed, 254 insertions(+), 78 deletions(-) diff --git a/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access.go b/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access.go index 7a1c7f0a..4d8fd511 100644 --- a/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access.go +++ b/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access.go @@ -2,6 +2,7 @@ package ruleengine import ( "fmt" + "path/filepath" "strings" "github.com/kubescape/node-agent/pkg/objectcache" @@ -10,7 +11,6 @@ import ( apitypes "github.com/armosec/armoapi-go/armotypes" traceropentype "github.com/inspektor-gadget/inspektor-gadget/pkg/gadgets/trace/open/types" - "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" ) @@ -19,8 +19,7 @@ const ( R0006Name = "Unexpected Service Account Token Access" ) -// ServiceAccountTokenPathsPrefixs is a list because of symlinks. -var serviceAccountTokenPathsPrefix = []string{ +var serviceAccountTokenPathsPrefixes = []string{ "/run/secrets/kubernetes.io/serviceaccount", "/var/run/secrets/kubernetes.io/serviceaccount", "/run/secrets/eks.amazonaws.com/serviceaccount", @@ -31,7 +30,7 @@ var R0006UnexpectedServiceAccountTokenAccessRuleDescriptor = ruleengine.RuleDesc ID: R0006ID, Name: R0006Name, Description: "Detecting unexpected access to service account token.", - Tags: []string{"token", "malicious", "whitelisted"}, + Tags: []string{"token", "malicious", "security", "kubernetes"}, Priority: RulePriorityHigh, Requirements: &RuleRequirements{ EventTypes: []utils.EventType{ @@ -42,15 +41,68 @@ var R0006UnexpectedServiceAccountTokenAccessRuleDescriptor = ruleengine.RuleDesc return CreateRuleR0006UnexpectedServiceAccountTokenAccess() }, } -var _ ruleengine.RuleEvaluator = (*R0006UnexpectedServiceAccountTokenAccess)(nil) type R0006UnexpectedServiceAccountTokenAccess struct { BaseRule } +// getTokenBasePath returns the base service account token path if the path is a token path, +// otherwise returns an empty string. Using a single iteration through prefixes. +func getTokenBasePath(path string) string { + for _, prefix := range serviceAccountTokenPathsPrefixes { + if strings.HasPrefix(path, prefix) { + return prefix + } + } + return "" +} + +// normalizeTokenPath removes timestamp directories from the path while maintaining +// the essential structure. Optimized for minimal allocations. +func normalizeTokenPath(path string) string { + // Get the base path - if not a token path, return original + basePath := getTokenBasePath(path) + if basePath == "" { + return path + } + + // Get the final component (usually "token", "ca.crt", etc.) + finalComponent := filepath.Base(path) + + // Split the middle part (between base path and final component) + middle := strings.TrimPrefix(filepath.Dir(path), basePath) + if middle == "" { + return filepath.Join(basePath, finalComponent) + } + + // Process middle parts + var normalizedMiddle strings.Builder + parts := strings.Split(middle, "/") + for _, part := range parts { + if part == "" { + continue + } + // Skip timestamp directories (starting with ".." and containing "_") + if strings.HasPrefix(part, "..") && strings.Contains(part, "_") { + continue + } + normalizedMiddle.WriteString("/") + normalizedMiddle.WriteString(part) + } + + // If no middle parts remain, join base and final + if normalizedMiddle.Len() == 0 { + return filepath.Join(basePath, finalComponent) + } + + // Join all parts + return basePath + normalizedMiddle.String() + "/" + finalComponent +} + func CreateRuleR0006UnexpectedServiceAccountTokenAccess() *R0006UnexpectedServiceAccountTokenAccess { return &R0006UnexpectedServiceAccountTokenAccess{} } + func (rule *R0006UnexpectedServiceAccountTokenAccess) Name() string { return R0006Name } @@ -59,24 +111,36 @@ func (rule *R0006UnexpectedServiceAccountTokenAccess) ID() string { return R0006ID } -func (rule *R0006UnexpectedServiceAccountTokenAccess) DeleteRule() { -} +func (rule *R0006UnexpectedServiceAccountTokenAccess) DeleteRule() {} func (rule *R0006UnexpectedServiceAccountTokenAccess) generatePatchCommand(event *traceropentype.Event, ap *v1beta1.ApplicationProfile) string { - flagList := "[" - for _, arg := range event.Flags { - flagList += "\"" + arg + "\"," + if len(event.Flags) == 0 { + return fmt.Sprintf( + "kubectl patch applicationprofile %s --namespace %s --type merge -p '{\"spec\": {\"containers\": [{\"name\": \"%s\", \"opens\": [{\"path\": \"%s\"}]}]}}'", + ap.GetName(), + ap.GetNamespace(), + event.GetContainer(), + event.FullPath, + ) } - // remove the last comma - if len(flagList) > 1 { - flagList = flagList[:len(flagList)-1] + + flagList := make([]string, len(event.Flags)) + for i, flag := range event.Flags { + flagList[i] = fmt.Sprintf("%q", flag) } - baseTemplate := "kubectl patch applicationprofile %s --namespace %s --type merge -p '{\"spec\": {\"containers\": [{\"name\": \"%s\", \"opens\": [{\"path\": \"%s\", \"flags\": %s}]}]}}'" - return fmt.Sprintf(baseTemplate, ap.GetName(), ap.GetNamespace(), - event.GetContainer(), event.FullPath, flagList) + + return fmt.Sprintf( + "kubectl patch applicationprofile %s --namespace %s --type merge -p '{\"spec\": {\"containers\": [{\"name\": \"%s\", \"opens\": [{\"path\": \"%s\", \"flags\": [%s]}]}]}}'", + ap.GetName(), + ap.GetNamespace(), + event.GetContainer(), + event.FullPath, + strings.Join(flagList, ","), + ) } func (rule *R0006UnexpectedServiceAccountTokenAccess) ProcessEvent(eventType utils.EventType, event utils.K8sEvent, objCache objectcache.ObjectCache) ruleengine.RuleFailure { + // Quick type checks first if eventType != utils.OpenEventType { return nil } @@ -86,19 +150,12 @@ func (rule *R0006UnexpectedServiceAccountTokenAccess) ProcessEvent(eventType uti return nil } - shouldCheckEvent := false - - for _, prefix := range serviceAccountTokenPathsPrefix { - if strings.HasPrefix(openEvent.FullPath, prefix) { - shouldCheckEvent = true - break - } - } - - if !shouldCheckEvent { + // Check if this is a token path - using optimized check + if getTokenBasePath(openEvent.FullPath) == "" { return nil } + // Get the application profile ap := objCache.ApplicationProfileCache().GetApplicationProfile(openEvent.Runtime.ContainerID) if ap == nil { return nil @@ -109,24 +166,33 @@ func (rule *R0006UnexpectedServiceAccountTokenAccess) ProcessEvent(eventType uti return nil } + // Normalize the accessed path once + normalizedAccessedPath := normalizeTokenPath(openEvent.FullPath) + + // Check against whitelisted paths for _, open := range appProfileOpenList.Opens { - for _, prefix := range serviceAccountTokenPathsPrefix { - if strings.HasPrefix(open.Path, prefix) { - return nil - } + if normalizedAccessedPath == normalizeTokenPath(open.Path) { + return nil } } - ruleFailure := GenericRuleFailure{ + // If we get here, the access was not whitelisted - create an alert + return &GenericRuleFailure{ BaseRuntimeAlert: apitypes.BaseRuntimeAlert{ AlertName: rule.Name(), Arguments: map[string]interface{}{ "path": openEvent.FullPath, "flags": openEvent.Flags, }, - InfectedPID: openEvent.Pid, - FixSuggestions: fmt.Sprintf("If this is a valid behavior, please add the open call \"%s\" to the whitelist in the application profile for the Pod \"%s\". You can use the following command: %s", openEvent.FullPath, openEvent.GetPod(), rule.generatePatchCommand(openEvent, ap)), - Severity: R0006UnexpectedServiceAccountTokenAccessRuleDescriptor.Priority, + InfectedPID: openEvent.Pid, + FixSuggestions: fmt.Sprintf( + "If this is a valid behavior, please add the open call \"%s\" to the whitelist in the application profile for the Pod \"%s\". "+ + "You can use the following command:\n%s", + openEvent.FullPath, + openEvent.GetPod(), + rule.generatePatchCommand(openEvent, ap), + ), + Severity: R0006UnexpectedServiceAccountTokenAccessRuleDescriptor.Priority, }, RuntimeProcessDetails: apitypes.ProcessTree{ ProcessTree: apitypes.Process{ @@ -139,7 +205,12 @@ func (rule *R0006UnexpectedServiceAccountTokenAccess) ProcessEvent(eventType uti }, TriggerEvent: openEvent.Event, RuleAlert: apitypes.RuleAlert{ - RuleDescription: fmt.Sprintf("Unexpected access to service account token: %s with flags: %s in: %s", openEvent.FullPath, strings.Join(openEvent.Flags, ","), openEvent.GetContainer()), + RuleDescription: fmt.Sprintf( + "Unexpected access to service account token: %s with flags: %s in: %s", + openEvent.FullPath, + strings.Join(openEvent.Flags, ","), + openEvent.GetContainer(), + ), }, RuntimeAlertK8sDetails: apitypes.RuntimeAlertK8sDetails{ PodName: openEvent.GetPod(), @@ -147,8 +218,6 @@ func (rule *R0006UnexpectedServiceAccountTokenAccess) ProcessEvent(eventType uti }, RuleID: rule.ID(), } - - return &ruleFailure } func (rule *R0006UnexpectedServiceAccountTokenAccess) Requirements() ruleengine.RuleSpec { diff --git a/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access_test.go b/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access_test.go index 8f58a9f8..5feb69ba 100644 --- a/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access_test.go +++ b/pkg/ruleengine/v1/r0006_unexpected_service_account_token_access_test.go @@ -3,66 +3,173 @@ package ruleengine import ( "testing" - "github.com/kubescape/node-agent/pkg/utils" - traceropentype "github.com/inspektor-gadget/inspektor-gadget/pkg/gadgets/trace/open/types" - "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" - eventtypes "github.com/inspektor-gadget/inspektor-gadget/pkg/types" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" ) -func TestR0006UnexpectedServiceAccountTokenMount(t *testing.T) { - // Create a new rule - r := CreateRuleR0006UnexpectedServiceAccountTokenAccess() - // Assert r is not nil - if r == nil { - t.Errorf("Expected r to not be nil") - } - - // Create a file access event - e := &traceropentype.Event{ +func createTestEvent0006(containerName, path string, flags []string) *traceropentype.Event { + return &traceropentype.Event{ Event: eventtypes.Event{ CommonData: eventtypes.CommonData{ K8s: eventtypes.K8sMetadata{ BasicK8sMetadata: eventtypes.BasicK8sMetadata{ - ContainerName: "test", + ContainerName: containerName, }, }, }, }, - Path: "/test", - FullPath: "/test", - Flags: []string{"O_RDONLY"}, - } - - // Test with nil appProfileAccess - ruleResult := r.ProcessEvent(utils.OpenEventType, e, &RuleObjectCacheMock{}) - if ruleResult != nil { - t.Errorf("Expected ruleResult to not be nil since no appProfile") - return + Path: path, + FullPath: path, + Flags: flags, } +} - // Test with whitelisted file - e.FullPath = "/run/secrets/kubernetes.io/serviceaccount/asdasd" - objCache := RuleObjectCacheMock{} - profile := objCache.ApplicationProfileCache().GetApplicationProfile("test") - if profile == nil { - profile = &v1beta1.ApplicationProfile{} - profile.Spec.Containers = append(profile.Spec.Containers, v1beta1.ApplicationProfileContainer{ - Name: "test", - Opens: []v1beta1.OpenCalls{ +func createTestProfile0006(containerName string, openCalls []v1beta1.OpenCalls) *v1beta1.ApplicationProfile { + return &v1beta1.ApplicationProfile{ + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{ { - Path: "/run/secrets/kubernetes.io/serviceaccount", - Flags: []string{"O_RDONLY"}, + Name: containerName, + Opens: openCalls, }, }, - }) + }, + } +} + +func TestR0006UnexpectedServiceAccountTokenMount(t *testing.T) { + tests := []struct { + name string + event *traceropentype.Event + profile *v1beta1.ApplicationProfile + expectFailure bool + }{ + // Non-token path tests + { + name: "non-token path access", + event: createTestEvent0006("test", "/test", []string{"O_RDONLY"}), + expectFailure: false, + }, + { + name: "path with similar prefix but not token path", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/other", []string{"O_RDONLY"}), + expectFailure: false, + }, + + // Basic token access tests - Kubernetes paths + { + name: "basic whitelisted kubernetes token access", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/serviceaccount/token", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/kubernetes.io/serviceaccount/token", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, + { + name: "unauthorized kubernetes token access", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/serviceaccount/token", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/some/other/path", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: true, + }, + + // EKS token path tests with timestamps + { + name: "whitelisted eks token access - different timestamps", + event: createTestEvent0006("test", + "/run/secrets/eks.amazonaws.com/serviceaccount/..2024_11_1111_24_34_58.850095521/token", + []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/eks.amazonaws.com/serviceaccount/..2024_11_21_04_30_58.850095521/token", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, + { + name: "whitelisted eks token access - base path whitelist", + event: createTestEvent0006("test", + "/run/secrets/eks.amazonaws.com/serviceaccount/..2024_11_1111_24_34_58.850095521/token", + []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/eks.amazonaws.com/serviceaccount/token", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, + // Alternative token files tests + { + name: "whitelisted ca.crt access", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/serviceaccount/ca.crt", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/kubernetes.io/serviceaccount/ca.crt", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, + { + name: "whitelisted namespace access", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/serviceaccount/namespace", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/kubernetes.io/serviceaccount/namespace", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, - objCache.SetApplicationProfile(profile) + // Container name mismatch tests + { + name: "different container name", + event: createTestEvent0006("test2", "/run/secrets/kubernetes.io/serviceaccount/token", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/run/secrets/kubernetes.io/serviceaccount/token", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, // No profile for the container + }, + + // Alternative path formats + { + name: "var/run path variant", + event: createTestEvent0006("test", "/var/run/secrets/kubernetes.io/serviceaccount/token", []string{"O_RDONLY"}), + profile: createTestProfile0006("test", []v1beta1.OpenCalls{{ + Path: "/var/run/secrets/kubernetes.io/serviceaccount/token", + Flags: []string{"O_RDONLY"}, + }}), + expectFailure: false, + }, + + // Edge cases + { + name: "no application profile", + event: createTestEvent0006("test", "/run/secrets/kubernetes.io/serviceaccount/token", []string{"O_RDONLY"}), + profile: nil, + expectFailure: false, + }, } - ruleResult = r.ProcessEvent(utils.OpenEventType, e, &objCache) - if ruleResult != nil { - t.Errorf("Expected ruleResult to be nil since file is whitelisted") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := CreateRuleR0006UnexpectedServiceAccountTokenAccess() + mockCache := &RuleObjectCacheMock{} + + if tt.profile != nil { + mockCache.SetApplicationProfile(tt.profile) + } + + result := r.ProcessEvent(utils.OpenEventType, tt.event, mockCache) + + if tt.expectFailure && result == nil { + t.Error("Expected rule failure but got nil") + } + + if !tt.expectFailure && result != nil { + t.Errorf("Expected no failure but got: %v", result) + } + }) } }