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
17 changes: 17 additions & 0 deletions pkg/objectcache/containerprofilecache/containerprofilecache.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/kubescape/go-logger/helpers"
helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers"
"github.com/kubescape/node-agent/pkg/config"
"github.com/kubescape/node-agent/pkg/exporters"
"github.com/kubescape/node-agent/pkg/metricsmanager"
"github.com/kubescape/node-agent/pkg/objectcache"
"github.com/kubescape/node-agent/pkg/objectcache/callstackcache"
Expand Down Expand Up @@ -122,6 +123,12 @@ type ContainerProfileCacheImpl struct {
specGeneration atomic.Int64 // bumped on each distinct spec hash change
nudge chan struct{} // buffered cap 1; signals reconciler on spec change
refreshPending atomic.Bool // set when a nudge arrives while refresh is running

// Tamper detection state (fork-only). See tamper_alert.go for the full
// description; reintroduced here on top of upstream's reshape so the
// legacy R1016 "Signed profile tampered" wiring keeps working.
tamperAlertExporter exporters.Exporter
tamperEmitted sync.Map // tamperKey -> struct{}
}

// NewContainerProfileCache creates a new ContainerProfileCacheImpl.
Expand Down Expand Up @@ -398,6 +405,13 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry(
helpers.Error(userAPErr))
userAP = nil
}
// Tamper detection: re-verify the signature on every load. Emits R1016
// when a signed overlay's signature no longer matches (i.e. content
// has been mutated post-sign). No-op when the overlay is unsigned or
// the tamper-alert exporter has not been wired.
if userAP != nil {
c.verifyUserApplicationProfile(userAP, sharedData.Wlid)
}
Comment on lines +412 to +414
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Honor verification result before merging user overlays.

Line 413 and Line 429 call verification but ignore the boolean outcome. In strict mode, failed verification should prevent projecting that overlay; otherwise tampered/failed overlays are still merged.

Suggested fix
 		if userAP != nil {
-			c.verifyUserApplicationProfile(userAP, sharedData.Wlid)
+			if !c.verifyUserApplicationProfile(userAP, sharedData.Wlid) {
+				userAP = nil
+			}
 		}
@@
 		if userNN != nil {
-			c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid)
+			if !c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid) {
+				userNN = nil
+			}
 		}

Also applies to: 428-430

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/objectcache/containerprofilecache/containerprofilecache.go` around lines
412 - 414, The verification calls to c.verifyUserApplicationProfile(userAP,
sharedData.Wlid) currently ignore its boolean result; update both call sites
(the one using userAP at lines ~412 and the similar one around ~428-430) to
check the returned bool and, when it is false and the cache is running in strict
verification mode (e.g., the instance flag controlling strict verification such
as c.strictVerify or equivalent), avoid projecting/merging the overlay (skip the
merge/projectOverlay path), log the verification failure with context (Wlid and
which overlay), and return or surface an error instead of continuing; if not in
strict mode, continue but log a warning. Ensure you reference and use
c.verifyUserApplicationProfile(...) and the strict-mode flag when implementing
the conditional.

var userNNErr error
_ = c.refreshRPC(ctx, func(rctx context.Context) error {
userNN, userNNErr = c.storageClient.GetNetworkNeighborhood(rctx, ns, overlayName)
Expand All @@ -411,6 +425,9 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry(
helpers.Error(userNNErr))
userNN = nil
}
if userNN != nil {
c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid)
}
}

// Need SOMETHING to cache. If we have nothing, stay pending and retry.
Expand Down
69 changes: 69 additions & 0 deletions pkg/objectcache/containerprofilecache/projection.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package containerprofilecache

import (
"strings"

helpersv1 "github.com/kubescape/k8s-interface/instanceidhandler/v1/helpers"
"github.com/kubescape/node-agent/pkg/utils"
"github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -62,9 +65,75 @@ func projectUserProfiles(
}
}

// Fold the user-overlay identity into the merged profile's SyncChecksum
// annotation. Apply (projection_apply.go) reads this into
// ProjectedContainerProfile.SyncChecksum which the rulemanager's
// function_cache uses as part of its invalidation key (see
// pkg/rulemanager/cel/libraries/cache/function_cache.go:
// HashForContainerProfile).
//
// Without this, an empty-baseline + user-overlay container has a
// constant SyncChecksum="" across both "no overlay yet" and "overlay
// merged" states. Stale ap.was_executed=false results computed during
// the no-overlay window would then persist in the cache and the rule
// evaluator would never see the merged user-overlay paths — which is
// the root cause behind Test_32_UnexpectedProcessArguments's R0001
// precondition failure and the latent R0001-on-nslookup noise in
// Test_28_UserDefinedNetworkNeighborhood.
if userAP != nil || userNN != nil {
stampOverlayIdentity(projected, userAP, userNN)
}

return projected, warnings
}

// stampOverlayIdentity appends user-overlay identity (kind/ns/name@RV)
// to the projected ContainerProfile's SyncChecksumMetadataKey annotation.
// Modifies projected.Annotations in place.
//
// The original baseline checksum (if present) is preserved as the prefix
// so distinct baselines still produce distinct keys. Format:
//
// <baseline-checksum>|ap=<ns>/<name>@<rv>|nn=<ns>/<name>@<rv>
//
// Either ap= or nn= segments are omitted when the corresponding overlay
// is nil. RV is the only piece that needs to change for the cache to
// invalidate, but namespace+name are kept so cross-overlay collisions
// (e.g. two different overlays happening to share RV across namespaces)
// don't alias.
//
// IDEMPOTENT: calling stampOverlayIdentity twice with the same overlay
// produces the same final annotation. The annotation is split on `|`
// and only the FIRST segment is kept as "baseline" — any existing
// ap= / nn= suffixes from prior stamps are discarded before being
// re-appended. (CodeRabbit PR #43 critical on projection.go:115:
// projectUserProfiles is called twice in succession in both
// reconciler.go and containerprofilecache.go, feeding the output of
// the first projection back as input to the second. Without this
// strip step, overlay suffixes accumulate on every reconcile tick,
// churning the function_cache.)
func stampOverlayIdentity(projected *v1beta1.ContainerProfile, userAP *v1beta1.ApplicationProfile, userNN *v1beta1.NetworkNeighborhood) {
if projected.Annotations == nil {
projected.Annotations = map[string]string{}
}
// Strip any prior ap= / nn= suffixes by taking only the first
// `|`-segment as the canonical baseline checksum. This is what
// makes repeat-stamping idempotent.
existing := projected.Annotations[helpersv1.SyncChecksumMetadataKey]
baseline := existing
if idx := strings.IndexByte(existing, '|'); idx >= 0 {
baseline = existing[:idx]
}
parts := []string{baseline}
if userAP != nil {
parts = append(parts, "ap="+userAP.Namespace+"/"+userAP.Name+"@"+userAP.ResourceVersion)
}
if userNN != nil {
parts = append(parts, "nn="+userNN.Namespace+"/"+userNN.Name+"@"+userNN.ResourceVersion)
}
projected.Annotations[helpersv1.SyncChecksumMetadataKey] = strings.Join(parts, "|")
}

// mergeApplicationProfile finds the container entry in userAP matching
// containerName (across Spec.Containers / InitContainers / EphemeralContainers)
// and merges its fields into projected.Spec. Returns the list of pod-spec
Expand Down
99 changes: 86 additions & 13 deletions pkg/objectcache/containerprofilecache/projection_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,30 +44,37 @@ func Apply(spec *objectcache.RuleProjectionSpec, cp *v1beta1.ContainerProfile, c
}

// Project each data surface.
// The third arg classifies an entry as "dynamic" — routes it to Patterns
// rather than Values. Path surfaces use the ⋯ DynamicIdentifier marker;
// network surfaces accept CIDRs, '*' sentinels, and DNS wildcard tokens
// per the v0.0.2 spec (matched at runtime by storage's networkmatch).
opensPaths := extractOpensPaths(cp)
pcp.Opens = projectField(s.Opens, opensPaths, true)
pcp.Opens = projectField(s.Opens, opensPaths, containsDynamicSegment)

execsPaths := extractExecsPaths(cp)
pcp.Execs = projectField(s.Execs, execsPaths, true)
pcp.Execs = projectField(s.Execs, execsPaths, containsDynamicSegment)
pcp.ExecsByPath = extractExecsByPath(cp)

endpointPaths := extractEndpointPaths(cp)
pcp.Endpoints = projectField(s.Endpoints, endpointPaths, true)
pcp.Endpoints = projectField(s.Endpoints, endpointPaths, containsDynamicSegment)

pcp.Capabilities = projectField(s.Capabilities, cp.Spec.Capabilities, false)
pcp.Syscalls = projectField(s.Syscalls, cp.Spec.Syscalls, false)
pcp.Capabilities = projectField(s.Capabilities, cp.Spec.Capabilities, nil)
pcp.Syscalls = projectField(s.Syscalls, cp.Spec.Syscalls, nil)

pcp.EgressDomains = projectField(s.EgressDomains, extractEgressDomains(cp), false)
pcp.EgressAddresses = projectField(s.EgressAddresses, extractEgressAddresses(cp), false)
pcp.EgressDomains = projectField(s.EgressDomains, extractEgressDomains(cp), isNetworkDNSWildcard)
pcp.EgressAddresses = projectField(s.EgressAddresses, extractEgressAddresses(cp), isNetworkIPWildcard)

pcp.IngressDomains = projectField(s.IngressDomains, extractIngressDomains(cp), false)
pcp.IngressAddresses = projectField(s.IngressAddresses, extractIngressAddresses(cp), false)
pcp.IngressDomains = projectField(s.IngressDomains, extractIngressDomains(cp), isNetworkDNSWildcard)
pcp.IngressAddresses = projectField(s.IngressAddresses, extractIngressAddresses(cp), isNetworkIPWildcard)

return pcp
}

// projectField is the per-surface transform. rawEntries are strings from the
// raw profile. isPathSurface enables retention of dynamic-segment entries.
func projectField(spec objectcache.FieldSpec, rawEntries []string, isPathSurface bool) objectcache.ProjectedField {
// raw profile. isDynamic, if non-nil, is called per entry: returning true
// routes the entry to Patterns rather than Values (cache-miss path runs the
// matcher rather than a map lookup).
func projectField(spec objectcache.FieldSpec, rawEntries []string, isDynamic func(string) bool) objectcache.ProjectedField {
if !spec.InUse {
// No rule declared a requirement for this field — pass all raw entries
// through so existing rules that omit profileDataRequired keep working.
Expand All @@ -92,9 +99,9 @@ func projectField(spec objectcache.FieldSpec, rawEntries []string, isPathSurface
seen := make(map[string]bool) // for Patterns dedup

for _, e := range rawEntries {
isDynamic := isPathSurface && containsDynamicSegment(e)
dynamic := isDynamic != nil && isDynamic(e)

if isDynamic {
if dynamic {
// Dynamic entries always go to Patterns on path surfaces (both
// pass-through and explicit InUse modes).
if !seen[e] {
Expand Down Expand Up @@ -148,6 +155,42 @@ func containsDynamicSegment(e string) bool {
return strings.Contains(e, dynamicpathdetector.DynamicIdentifier)
}

// isNetworkIPWildcard reports whether an IP-surface entry is a v0.0.2
// pattern (CIDR membership, '*' any-IP sentinel, or DynamicIdentifier).
// Literal IPv4/IPv6 addresses are NOT patterns; they go to Values for
// the cheap map lookup path. Spec §5.7.
func isNetworkIPWildcard(e string) bool {
if e == "" {
return false
}
if e == "*" {
return true
}
if strings.Contains(e, "/") {
return true
}
if strings.Contains(e, dynamicpathdetector.DynamicIdentifier) {
return true
}
return false
}

// isNetworkDNSWildcard reports whether a DNS-surface entry uses any of
// the v0.0.2 wildcard tokens — leading '*' (RFC 4592), mid '⋯', trailing
// '*'. Literal FQDNs go to Values. Spec §5.8.
func isNetworkDNSWildcard(e string) bool {
if e == "" {
return false
}
if strings.Contains(e, "*") {
return true
}
if strings.Contains(e, dynamicpathdetector.DynamicIdentifier) {
return true
}
return false
}

// --- Field extractors ---

func extractOpensPaths(cp *v1beta1.ContainerProfile) []string {
Expand All @@ -166,6 +209,32 @@ func extractExecsPaths(cp *v1beta1.ContainerProfile) []string {
return paths
}

// extractExecsByPath builds the path → args map used by the exec-args
// wildcard matcher (CompareExecArgs). Multiple ExecCalls entries with the
// same Path collapse to the last seen; this matches the prior fork-only
// behavior. nil-Args entries are stored as empty slices, which
// CompareExecArgs treats as "no argv constraint".
//
// Args slices are CLONED rather than aliased — Apply is contract-bound to
// be a pure transform, and an alias would let consumers mutate the source
// profile by editing the projected map. (CR #43 finding on this file.)
func extractExecsByPath(cp *v1beta1.ContainerProfile) map[string][]string {
if len(cp.Spec.Execs) == 0 {
return nil
}
m := make(map[string][]string, len(cp.Spec.Execs))
for _, e := range cp.Spec.Execs {
if e.Args == nil {
m[e.Path] = []string{}
continue
}
cloned := make([]string, len(e.Args))
copy(cloned, e.Args)
m[e.Path] = cloned
}
return m
}

func extractEndpointPaths(cp *v1beta1.ContainerProfile) []string {
endpoints := make([]string, len(cp.Spec.Endpoints))
for i, e := range cp.Spec.Endpoints {
Expand All @@ -191,6 +260,9 @@ func extractEgressAddresses(cp *v1beta1.ContainerProfile) []string {
if n.IPAddress != "" {
addrs = append(addrs, n.IPAddress)
}
// v0.0.2 IPAddresses[] — list form supporting CIDRs and '*' sentinel.
// Same semantics as the deprecated singular IPAddress, just plural.
addrs = append(addrs, n.IPAddresses...)
}
return addrs
}
Expand All @@ -212,6 +284,7 @@ func extractIngressAddresses(cp *v1beta1.ContainerProfile) []string {
if n.IPAddress != "" {
addrs = append(addrs, n.IPAddress)
}
addrs = append(addrs, n.IPAddresses...)
}
return addrs
}
Expand Down
Loading