diff --git a/go.mod b/go.mod index 014958883..b721ec513 100644 --- a/go.mod +++ b/go.mod @@ -511,3 +511,7 @@ replace github.com/cilium/ebpf => github.com/matthyx/ebpf v0.0.0-20260421101317- replace github.com/anchore/syft => github.com/kubescape/syft v1.32.0-ks.2 replace github.com/kubescape/storage => github.com/k8sstormcenter/storage v0.0.240-0.20260513133617-b23d85f00f6a + +replace github.com/anchore/stereoscope => github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385 + +replace github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.2.1 diff --git a/go.sum b/go.sum index b5cafe303..bb4a12f3f 100644 --- a/go.sum +++ b/go.sum @@ -198,8 +198,8 @@ github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4 h1:rmZG77uXgE github.com/anchore/go-version v1.2.2-0.20210903204242-51efa5b487c4/go.mod h1:Bkc+JYWjMCF8OyZ340IMSIi2Ebf3uwByOk6ho4wne1E= github.com/anchore/packageurl-go v0.1.1-0.20250220190351-d62adb6e1115 h1:ZyRCmiEjnoGJZ1+Ah0ZZ/mKKqNhGcUZBl0s7PTTDzvY= github.com/anchore/packageurl-go v0.1.1-0.20250220190351-d62adb6e1115/go.mod h1:KoYIv7tdP5+CC9VGkeZV4/vGCKsY55VvoG+5dadg4YI= -github.com/anchore/stereoscope v0.1.9 h1:Nhvk8g6PRx9ubaJU4asAhD3fGcY5HKXZCDGkxI2e0sI= -github.com/anchore/stereoscope v0.1.9/go.mod h1:YkrCtDgz7A+w6Ggd0yxU9q58CerqQFwYARS+F2RvLQQ= +github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385 h1:icCqbvAKGZXf29lEi8JmwvHVCBCYkiyZMuSnk+5ajYo= +github.com/anchore/stereoscope v0.1.9-0.20250826202322-ef061ea78385/go.mod h1:0UCjLz5MdPNiH9F0h2tSNf3yGF6/MnK8ZCPo0YfDQVc= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= diff --git a/pkg/objectcache/containerprofilecache/containerprofilecache.go b/pkg/objectcache/containerprofilecache/containerprofilecache.go index c539fad0e..eeb401586 100644 --- a/pkg/objectcache/containerprofilecache/containerprofilecache.go +++ b/pkg/objectcache/containerprofilecache/containerprofilecache.go @@ -409,8 +409,17 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry( // 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. + // CodeRabbit upstream PR #808 / containerprofilecache.go:414 (Major): + // when EnableSignatureVerification=true and the overlay fails + // verification, verifyUserApplicationProfile returns false. Drop the + // failed overlay before merging so a tampered profile does not + // silently project into the cache. In permissive mode the verifier + // always returns true and the overlay still merges (alert-only + // behaviour preserved). if userAP != nil { - c.verifyUserApplicationProfile(userAP, sharedData.Wlid) + if !c.verifyUserApplicationProfile(userAP, sharedData.Wlid) { + userAP = nil + } } var userNNErr error _ = c.refreshRPC(ctx, func(rctx context.Context) error { @@ -426,7 +435,9 @@ func (c *ContainerProfileCacheImpl) tryPopulateEntry( userNN = nil } if userNN != nil { - c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid) + if !c.verifyUserNetworkNeighborhood(userNN, sharedData.Wlid) { + userNN = nil + } } } diff --git a/pkg/objectcache/containerprofilecache/tamper_alert_test.go b/pkg/objectcache/containerprofilecache/tamper_alert_test.go index 03fa7b0a8..c8af3ce9b 100644 --- a/pkg/objectcache/containerprofilecache/tamper_alert_test.go +++ b/pkg/objectcache/containerprofilecache/tamper_alert_test.go @@ -17,6 +17,7 @@ import ( "sync" "testing" + "github.com/kubescape/node-agent/pkg/config" "github.com/kubescape/node-agent/pkg/hostfimsensor" "github.com/kubescape/node-agent/pkg/malwaremanager" rmtypes "github.com/kubescape/node-agent/pkg/rulemanager/types" @@ -279,3 +280,72 @@ func TestVerifyAP_OperationalError_DoesNotEmit(t *testing.T) { t.Errorf("unsigned AP produced %d R1016 alerts; want 0", got) } } + +// TestVerifyAP_StrictMode_ReturnsFalseOnTamper pins CodeRabbit upstream +// PR #808 / containerprofilecache.go:414 (Major). The fix wires the +// verifyUserApplicationProfile boolean result into the caller so that +// in EnableSignatureVerification=true (strict) mode a tampered overlay +// is NOT merged into the projected profile. This unit-level test pins +// the verifier's strict-mode return contract; the call-site honors the +// return value (drop tampered overlay → userAP = nil). +func TestVerifyAP_StrictMode_ReturnsFalseOnTamper(t *testing.T) { + profile := &v1beta1.ApplicationProfile{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tampered-strict", + Namespace: "test-ns", + ResourceVersion: "1", + UID: "ap-uid-strict", + }, + Spec: v1beta1.ApplicationProfileSpec{ + Containers: []v1beta1.ApplicationProfileContainer{{Name: "test"}}, + }, + } + + adapter := profiles.NewApplicationProfileAdapter(profile) + if err := signature.SignObjectDisableKeyless(adapter); err != nil { + t.Fatalf("sign profile: %v", err) + } + profile.Spec.Containers[0].Name = "TAMPERED" + + // Strict mode: EnableSignatureVerification = true + c := &ContainerProfileCacheImpl{ + cfg: config.Config{EnableSignatureVerification: true}, + } + ok := c.verifyUserApplicationProfile(profile, "wlid://test/cluster/ns/Pod/p") + if ok { + t.Errorf("verify returned true on tampered profile in strict mode; expected false (caller drops overlay)") + } +} + +// TestVerifyNN_StrictMode_ReturnsFalseOnTamper — symmetric pin for the +// NetworkNeighborhood overlay verification path. +func TestVerifyNN_StrictMode_ReturnsFalseOnTamper(t *testing.T) { + nn := &v1beta1.NetworkNeighborhood{ + ObjectMeta: metav1.ObjectMeta{ + Name: "tampered-strict-nn", + Namespace: "test-ns", + ResourceVersion: "1", + UID: "nn-uid-strict", + }, + Spec: v1beta1.NetworkNeighborhoodSpec{ + Containers: []v1beta1.NetworkNeighborhoodContainer{{Name: "test"}}, + }, + } + + adapter := profiles.NewNetworkNeighborhoodAdapter(nn) + if err := signature.SignObjectDisableKeyless(adapter); err != nil { + t.Fatalf("sign nn: %v", err) + } + nn.Spec.Containers[0].Name = "TAMPERED" + + c := &ContainerProfileCacheImpl{ + cfg: config.Config{EnableSignatureVerification: true}, + } + ok := c.verifyUserNetworkNeighborhood(nn, "wlid://test/cluster/ns/Pod/p") + if ok { + t.Errorf("verify returned true on tampered nn in strict mode; expected false (caller drops overlay)") + } +} + +// cfgRef is a minimal config shim for the strict-mode tests. Mirrors the +// concrete config.Config struct shape only in the field the verifier reads. diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go index ce86d7ab8..99ab11831 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/ap.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/ap.go @@ -130,6 +130,28 @@ func (l *apLibrary) Declarations() map[string][]cel.FunctionOpt { }), ), }, + // ap.was_path_opened_with_suffix and ap.was_path_opened_with_prefix + // — rule-author contract (CodeRabbit upstream PR #807 finding #7): + // + // These helpers answer "did any RECORDED concrete path open match + // this suffix/prefix?". When the profile-projection cache is in + // pass-through mode (no rule declared an Opens-projection slice, + // so cp.Opens.All == true), wildcard patterns in cp.Opens.Patterns + // are NOT scanned via string-level HasSuffix/HasPrefix because the + // pattern text contains '*' / '⋯' tokens whose string shape doesn't + // safely answer suffix/prefix questions (see open.go comment). + // Concrete-only Values are scanned. + // + // False-negative gap: if a profile entry is `/var/log/pods/*/foo.log`, + // the runtime path `/var/log/pods/web-7d6f/volumes/foo.log` actually + // matches this pattern, but `was_path_opened_with_suffix("/foo.log")` + // returns FALSE because the pattern text doesn't end in `/foo.log` + // literally. Rule authors who need wildcard-aware coverage should + // either: (a) declare an Opens projection slice in the rule's + // ProfileDataRequired (then SuffixHits/PrefixHits become authoritative + // and the projector pre-computes the hit map for wildcard entries), + // or (b) use ap.was_path_opened(path) which DOES run dynamic-segment + // matching over Patterns via CompareDynamic. "ap.was_path_opened_with_suffix": { cel.Overload( "ap_was_path_opened_with_suffix", []*cel.Type{cel.StringType, cel.StringType}, cel.BoolType, diff --git a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go index 5f5736922..10e8d4c97 100644 --- a/pkg/rulemanager/cel/libraries/applicationprofile/exec.go +++ b/pkg/rulemanager/cel/libraries/applicationprofile/exec.go @@ -92,14 +92,40 @@ func (l *apLibrary) wasExecutedWithArgs(containerID, path, args ref.Val) ref.Val // Exact path match: walk the profile's Args for that path via // CompareExecArgs (handles ⋯ single-arg and * zero-or-more tokens). + // + // ExecsByPath absent-vs-empty asymmetry — CodeRabbit upstream PR + // #807 finding #8. Three states to distinguish: + // + // 1. Path absent from cp.Execs.Values: + // Profile doesn't allow this exec at all → fall through to + // the pattern-match loop, then to false. + // + // 2. Path in Values, ABSENT from ExecsByPath (map lookup ok=false): + // Legacy / pre-args-projection profiles. Treated as + // "no argv constraint" — back-compat MATCH any args. + // This is the intentional fallback for profiles compiled + // against older storage versions that didn't populate the + // composite ExecsByPath surface. + // + // 3. Path in Values, PRESENT in ExecsByPath with an EMPTY arg + // list ([]): + // Profile explicitly captured "this path ran with no args". + // CompareExecArgs matches only when runtimeArgs is also + // empty. NOT a back-compat fallback — a deliberately tight + // constraint authored by the profile producer. + // + // The distinction matters for rule-author intuition: producing a + // signed profile that lists `{Path: /usr/bin/foo, Args: []}` is a + // CONSTRAINT, not a wildcard. Authors who want "any args" must + // omit the ExecsByPath entry (rare) or use an explicit `*` + // wildcard token in Args. if _, ok := cp.Execs.Values[pathStr]; ok { if profileArgs, ok := cp.ExecsByPath[pathStr]; ok { if dynamicpathdetector.CompareExecArgs(profileArgs, runtimeArgs) { return types.Bool(true) } } else { - // No ExecsByPath entry for this path — back-compat: treat as - // "no argv constraint", match. + // State 2: ExecsByPath absent → back-compat "no argv constraint". return types.Bool(true) } }