Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions pkg/containerprofilemanager/v1/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ func (cpm *ContainerProfileManager) ContainerCallback(notif containercollection.
switch notif.Type {
case containercollection.EventTypeAddContainer:
if utils.IsHostContainer(notif.Container) {
logger.L().Debug("adding host container to the container profile manager",
helpers.String("containerID", notif.Container.Runtime.ContainerID))
return
}
if cpm.cfg.IgnoreContainer(notif.Container.K8s.Namespace, notif.Container.K8s.PodName, notif.Container.K8s.PodLabels) {
return
Expand Down Expand Up @@ -93,14 +92,17 @@ func (cpm *ContainerProfileManager) addContainer(container *containercollection.
return fmt.Errorf("failed to get shared data for container %s: %w", containerID, err)
}

// Check if the container should use a user-defined profile
// Check if the container should use a user-defined profile.
// When both an ApplicationProfile and a NetworkNeighborhood are
// user-provided, skip ALL recording — there is nothing to learn.
if sharedData.UserDefinedProfile != "" {
logger.L().Debug("ignoring container with a user-defined profile",
helpers.String("containerID", containerID),
helpers.String("containerName", container.Runtime.ContainerName),
helpers.String("podName", container.K8s.PodName),
helpers.String("namespace", container.K8s.Namespace),
helpers.String("userDefinedProfile", sharedData.UserDefinedProfile))
helpers.String("userDefinedProfile", sharedData.UserDefinedProfile),
helpers.String("userDefinedNetwork", sharedData.UserDefinedNetwork))
// Close ready channel before removing entry
if entry, exists := cpm.getContainerEntry(containerID); exists {
entry.readyOnce.Do(func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ type ContainerInfo struct {
InstanceTemplateHash string
Namespace string
SeenContainerFromTheStart bool // True if container was seen from the start
UserDefinedNetwork string // Non-empty when pod has a user-defined NN label
}

// NetworkNeighborhoodCacheImpl implements the NetworkNeighborhoodCache interface
Expand Down Expand Up @@ -204,6 +205,13 @@ func (nnc *NetworkNeighborhoodCacheImpl) updateAllNetworkNeighborhoods(ctx conte
continue
}

// Never overwrite a user-defined network neighborhood with an
// auto-learned one. Check if any container for this workload
// has a user-defined-network label.
if nnc.workloadHasUserDefinedNetwork(workloadID) {
continue
}
Comment on lines +208 to +213
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't make user-defined NNs write-once cache entries.

addContainer is the only place in this file that does GetNetworkNeighborhood(..., sharedData.UserDefinedNetwork). After that, Lines 208-213 and 768-778 only let the periodic sync skip the workload, not refresh the named object. That means a transient failure on Lines 441-452 is never retried here, and later edits to the user-defined NN also stay stale until the container restarts. It also runs after Lines 176-182 have already copied the auto-learned object's state into workloadIDToProfileState, so the reported state can diverge from the cached user-defined NN.

Also applies to: 436-470, 766-778

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go` around
lines 208 - 213, The code currently skips refreshing a named user-defined
network by returning early in the periodic sync when
workloadHasUserDefinedNetwork(workloadID) is true, making user-defined NNs
effectively write-once; change the logic so that addContainer still prefers
sharedData.UserDefinedNetwork on initial creation but the periodic refresh/Sync
routine (the function that calls GetNetworkNeighborhood and updates
workloadIDToProfileState) must always attempt to refresh the named network
object and retry transient failures rather than skipping updates entirely when
workloadHasUserDefinedNetwork(workloadID) is true; locate calls to
GetNetworkNeighborhood, the addContainer path, and the periodic sync code that
uses workloadHasUserDefinedNetwork and modify the sync path to fetch/update the
cached named NN (and retry on transient errors) while preserving the
initial-preference behavior in addContainer.


// If we have a "new" container (seen from start) and the network neighborhood is partial,
// skip it - we don't want to use partial profiles for containers we're tracking from the start
if hasNewContainer && nn.Annotations[helpersv1.CompletionMetadataKey] == helpersv1.Partial {
Expand Down Expand Up @@ -419,11 +427,48 @@ func (nnc *NetworkNeighborhoodCacheImpl) addContainer(container *containercollec
InstanceTemplateHash: sharedData.InstanceID.GetTemplateHash(),
Namespace: container.K8s.Namespace,
SeenContainerFromTheStart: !sharedData.PreRunningContainer,
UserDefinedNetwork: sharedData.UserDefinedNetwork,
}

// Add to container info map
nnc.containerIDToInfo.Set(containerID, containerInfo)

// If the container has a user-defined network neighborhood, load it
// directly into the cache — skip learning entirely for this workload.
if sharedData.UserDefinedNetwork != "" {
fullNN, err := nnc.storageClient.GetNetworkNeighborhood(
container.K8s.Namespace, sharedData.UserDefinedNetwork)
if err != nil {
logger.L().Error("failed to get user-defined network neighborhood",
helpers.String("containerID", containerID),
helpers.String("workloadID", workloadID),
helpers.String("namespace", container.K8s.Namespace),
helpers.String("nnName", sharedData.UserDefinedNetwork),
helpers.Error(err))
profileState := &objectcache.ProfileState{
Error: err,
}
nnc.workloadIDToProfileState.Set(workloadID, profileState)
return nil
}

nnc.workloadIDToNetworkNeighborhood.Set(workloadID, fullNN)
profileState := &objectcache.ProfileState{
Completion: helpersv1.Full,
Status: helpersv1.Completed,
Name: fullNN.Name,
Error: nil,
}
nnc.workloadIDToProfileState.Set(workloadID, profileState)

logger.L().Debug("added user-defined network neighborhood to cache",
helpers.String("containerID", containerID),
helpers.String("workloadID", workloadID),
helpers.String("namespace", container.K8s.Namespace),
helpers.String("nnName", sharedData.UserDefinedNetwork))
return nil
}

// Create workload ID to state mapping
if _, exists := nnc.workloadIDToProfileState.Load(workloadID); !exists {
nnc.workloadIDToProfileState.Set(workloadID, nil)
Expand Down Expand Up @@ -718,6 +763,20 @@ func (nnc *NetworkNeighborhoodCacheImpl) mergeNetworkPorts(normalPorts, userPort
return normalPorts
}

// workloadHasUserDefinedNetwork returns true if any container tracked for
// the given workloadID has a user-defined-network label set.
func (nnc *NetworkNeighborhoodCacheImpl) workloadHasUserDefinedNetwork(workloadID string) bool {
found := false
nnc.containerIDToInfo.Range(func(_ string, info *ContainerInfo) bool {
if info.WorkloadID == workloadID && info.UserDefinedNetwork != "" {
found = true
return false // stop iteration
}
return true
})
return found
}

func isUserManagedNN(nn *v1beta1.NetworkNeighborhood) bool {
return nn.Annotations != nil &&
nn.Annotations[helpersv1.ManagedByMetadataKey] == helpersv1.ManagedByUserValue &&
Expand Down
16 changes: 16 additions & 0 deletions pkg/objectcache/shared_container_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ import (
"k8s.io/apimachinery/pkg/util/validation"
)

// UserDefinedNetworkMetadataKey is the pod label that references a
// user-provided NetworkNeighborhood resource by name (analogous to
// helpersv1.UserDefinedProfileMetadataKey for ApplicationProfiles).
const UserDefinedNetworkMetadataKey = "kubescape.io/user-defined-network"

type ContainerType int

const (
Expand Down Expand Up @@ -82,6 +87,7 @@ type WatchedContainerData struct {
PreviousReportTimestamp time.Time
CurrentReportTimestamp time.Time
UserDefinedProfile string
UserDefinedNetwork string
}

type ContainerInfo struct {
Expand Down Expand Up @@ -167,6 +173,16 @@ func (watchedContainer *WatchedContainerData) SetContainerInfo(wl workloadinterf
watchedContainer.UserDefinedProfile = userDefinedProfile
}
}
// check for user defined network neighborhood
if userDefinedNetwork, ok := labels[UserDefinedNetworkMetadataKey]; ok {
if userDefinedNetwork != "" {
logger.L().Info("container has a user defined network neighborhood",
helpers.String("network", userDefinedNetwork),
helpers.String("container", containerName),
helpers.String("workload", wl.GetName()))
watchedContainer.UserDefinedNetwork = userDefinedNetwork
}
}
podSpec, err := wl.GetPodSpec()
if err != nil {
return fmt.Errorf("failed to get pod spec: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions tests/chart/templates/node-agent/default-rules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ spec:
profileDependency: 0
severity: 1
supportPolicy: false
isTriggerAlert: false
isTriggerAlert: true
mitreTactic: "TA0011"
mitreTechnique: "T1071.004"
tags:
Expand Down Expand Up @@ -245,7 +245,7 @@ spec:
- "anomaly"
- "applicationprofile"
- name: "Unexpected Egress Network Traffic"
enabled: false
enabled: true
id: "R0011"
description: "Detecting unexpected egress network traffic that is not whitelisted by application profile."
expressions:
Expand All @@ -257,7 +257,7 @@ spec:
profileDependency: 0
severity: 5 # Medium
supportPolicy: false
isTriggerAlert: false
isTriggerAlert: true
mitreTactic: "TA0010"
mitreTechnique: "T1041"
tags:
Expand Down
Loading