diff --git a/.github/workflows/component-tests.yaml b/.github/workflows/component-tests.yaml index f3325394eb..b60ca4c1a5 100644 --- a/.github/workflows/component-tests.yaml +++ b/.github/workflows/component-tests.yaml @@ -105,6 +105,7 @@ jobs: Test_34_NetworkNeighborsCIDRCollapse, Test_35_ExecTTYFieldTest, Test_36_MultiContainerPerContainerBinding, + Test_37_CelStateStoreCorrelation, Test_43_RelativeOpenPathResolution, Test_48_MultiSubtypeGroupedProfileDocument, Test_49_EphemeralContainerFullTreatment diff --git a/docs/features/cel-rule-state-store.md b/docs/features/cel-rule-state-store.md new file mode 100644 index 0000000000..2f5a6cdb84 --- /dev/null +++ b/docs/features/cel-rule-state-store.md @@ -0,0 +1,350 @@ +# CEL rule state store + +Gives a CEL rule memory across events, so a detection can span more than one +event: remember something on `exec`, alert on `network`. Without it, every rule +is a pure predicate over a single event and multi-step behaviour — a process +launched from a mount that later connects out, a webshell chain, create/exec/ +delete of a pod — cannot be expressed at all. + +Design: `shared-designs-and-docs/projects/2026-07-28-cel-rule-state-store/spec.md`. + +> **Status: proven end-to-end on a cluster.** Verified against real eBPF events +> on kind by `Test_36_CelStateStoreCorrelation`: a rule that writes on `exec` and +> alerts on `network` fires, carries the remembered `value:` through to its +> message, and a negative control confirms it is not firing spuriously. +> +> **Deployment prerequisite:** the `Rules` CRD must declare `stateWrites`. The +> canonical CRD ships from **`kubescape/helm-charts`**, and until the property is +> added there the API server silently strips the clause — rules load cleanly and +> never fire, with no error anywhere. The copy in `tests/chart/crds/` is fixed; +> helm-charts is a separate, required change. + +## Writing state + +A rule declares what it remembers in a `stateWrites:` block. Each entry is driven +by one event type, which **need not** be an event type the rule alerts on: + +```yaml +id: R1089 +stateWrites: + - eventType: exec # the stream that drives this write + when: "" # optional; absent means always write + scope: container # container | pod | node + name: mount_exec # a literal, never an expression + key: "string(event.pid)" # optional CEL string: who the fact is about + value: # optional extras, CEL expression strings + argv: "event.args" + ttl: 10m # clamped to the configured maxTtl +expressions: + ruleExpression: + - eventType: network # alerts on a DIFFERENT stream + expression: | + state.has("mount_exec", string(event.pid)) && !net.is_private_ip(event.dstIP) +``` + +`name` is a literal rather than an expression on purpose: it stays statically +analysable and is safe to use as a metric label. + +### Writes are declarative, not a CEL setter + +There is no `state.set(...)` function, deliberately. A setter inside a predicate +would be skipped by boolean short-circuiting, could be reordered by the static +optimiser, and could never express "remember this **without** alerting" — which +is exactly what the first leg of a cross-event rule needs. + +### Writes run after the predicate + +For a given event, the rule's predicate is evaluated first and the writes second. +So a predicate only ever sees state from **earlier** events. Otherwise a rule +that reads and writes the same name on the same event type would satisfy itself +from its own write. + +### What suppresses a write + +| Condition | Writes still run? | +|---|---| +| Rule disabled, or does not apply to this context | no | +| `profileDependency: Required` and no profile | no | +| Pre-filter excluded the event | no — counted as `state_write_rejected_total{reason="prefiltered"}` | +| Rule policy suppressed it | no — counted as `reason="policy"` | +| **Alert cooldown** | **yes** | +| **Predicate returned false** | **yes** | +| Store at capacity | no — write rejected, `state_write_rejected_total` | + +Cooldown suppresses the *alert*, never the write: writes are evidence gathering, +and dropping them would break the next leg of the chain. + +**The pre-filter case is the one to watch when writing a correlation rule.** A +rule's pre-filter is built from the rule as a whole — in practice mostly from its +alerting leg's parameters — but it is applied to every event type the rule +touches. So a rule whose `network` leg carries `ignorePrefixes` or +`excludeProcesses` has those same parameters applied to its `exec` leg, and the +write is dropped. The chain then never forms, and nothing about the resulting +silence points at the pre-filter. That is why the two suppression paths are +counted: a non-zero `reason="prefiltered"` for a rule that is not firing is the +answer. Where the parameters are genuinely meant for one leg only, the fix is to +express them in that leg's `expression` instead of as rule-level pre-filter +parameters. + +### Validation + +An unknown event type, `eventType: all` (a binding wildcard, not a stream), +`scope: identity` (operator-only), a bad or non-positive TTL, or a `_`-prefixed +name or value key is rejected. Every one of those mistakes would otherwise produce +a rule that loads cleanly and silently never fires. + +Validation runs on the rule's first matching event rather than at load, because a +rule reaches the loop by two paths and only one of them populates load-time +derived fields. The compiled result is then cached, keyed by a fingerprint of the +clause, so an edited rule recompiles on its own without an invalidation hook. + +The practical consequence is that a malformed clause is reported the first time +the rule sees a matching event, not at apply time — and reported **once per +version of the clause**, not once per event. A TTL above `maxTtl` is clamped and +logged on the same terms. + +A rule with a malformed clause is degraded to non-correlating and logged; it does +not stop the other rules in the CRD from evaluating. + +### Bounds + +Over-capacity writes are **rejected, never satisfied by evicting** another +entry — eviction would let one container disable detection for its neighbours. +The per-scope cap is exact; the node-wide ceiling is approximate under +concurrency. At the ceiling a write first tries to reclaim expired entries, but at +most one such sweep runs per `sweepInterval` across all writers — otherwise every +write at a full store would walk all 16 shards under their locks, stalling the +whole rule loop at the busiest possible moment. A write arriving between sweeps is +treated as though the sweep freed nothing, which is what it would have found. Host processes share one `c:__host__` bucket with its own larger cap, +since it holds the whole node's process space and gets no removal purge; node scope +takes that same larger cap for the same reason. + +## Reading state + +Four functions, on a `state` receiver: + +```cel +state.has(name) // bool — for a fact about the whole scope +state.has(name, key) // bool — for a fact about one subject +state.get(name) // map — empty map on a miss, never an error +state.get(name, key) // map +state.has_ancestor(name) // bool — any ancestor PID carries the marker +state.get_ancestor(name) // map — the NEAREST matching ancestor +``` + +`name` is what kind of fact ("mount_exec"); `key` is who it is about, usually +`string(event.pid)`. A read takes **no scope argument**: state is rule-private, +so the name already determines its scope from the rule's own `stateWrites`. + +`state.get` on a miss returns an **empty map**, so guard provenance access with +`state.has` — `state.get("x", k)._pid` on a miss is a "no such key" error, and +that error fails the whole predicate: + +```cel +state.has("mount_exec", string(event.ppid)) && + state.get("mount_exec", string(event.ppid))._ts < timestamp +``` + +### What `state.get` returns + +Engine-stamped provenance uses reserved `_`-prefixed keys; the rule's own +`value:` entries sit alongside them at the top level. Author keys may not begin +with `_`, so they can never shadow provenance. + +| Key | Type | Meaning | +|---|---|---| +| `_ts` | timestamp | When the remembered event happened. Compare against `timestamp`. | +| `_eventType` | string | The event stream that wrote the entry. | +| `_container` | string | The scope ID the entry lives under. | +| `_pid` / `_ppid` | uint | Process and parent PID. | +| `_comm` / `_pcomm` | string | Process and parent command name. | +| `_exe` | string | Executable path. | +| `_cwd` | string | Working directory. | + +`_pid` and `_ppid` are unsigned, while `event.pid` is not, so +`state.get(...)._pid == event.pid` mixes numeric types. It is safe — cel-go +compares numbers across types by value, and a test pins that in the production +evaluator — so the comparison neither errors nor silently returns false. The join +itself should still go through `key: string(event.pid)`: it is the cheaper lookup +and does not depend on that behaviour holding. + +### The ancestor functions assume a PID key + +`has_ancestor` / `get_ancestor` probe each ancestor PID in turn, so they only +find entries whose `key` was a PID. That is an authoring contract, not something +the engine can check — write `key: string(event.pid)` for any name you intend to +read this way. + +They work identically for host and containerised processes. + +### Why `state` is a variable, not a function namespace + +Unlike `process.*` or `net.*`, `state` is a **variable** with member functions. +cel-go hands a function binding only its arguments, never the surrounding +context, so a global `state.has` could not know which rule or which container it +was evaluating for. + +That is also the security property: the rule ID, the scope IDs and the ancestor +list live in the receiver, and no CEL syntax supplies or overrides them. Reading +another rule's state or a neighbouring container's state is not merely forbidden +— it is inexpressible. + +## `timestamp` — the resolved event time + +A top-level CEL variable (not an `event` field) holding the authoritative time +for the event being evaluated: + +```cel +timestamp // a CEL timestamp +timestamp - duration("5m") // arithmetic and comparison work +``` + +It is the event's **kernel** timestamp where the event carries one, falling back +to node-agent's enrichment time only when that is zero. + +Two reasons it is defined this way: + +**Kernel time, not observation time.** Events are processed by a concurrent +worker pool, so the order node-agent *sees* events is not the order they +*happened*. Any ordering comparison has to be against when things happened or it +is meaningless. + +**One source of truth.** The same function populates both this variable and the +timestamp stamped onto stored state entries. If the two could disagree, an +ordering guard would be comparing different clocks and would silently never +fire — the worst failure mode for a detection rule. + +It is a top-level variable rather than `event.timestamp` because the `event` +field getters receive a wrapper around the raw event and cannot see +node-agent's enrichment time; an event field would therefore have to be a +second, divergent source of truth. + +### Timezone + +`string(timestamp)` renders in the node's local zone, so the offset in the text +depends on where the agent runs. Comparisons are instant-based and unaffected. +Assert on instants, not on rendered text. + +## Correlation evidence on the alert + +When a rule fires, the state entries its predicate **actually read** are attached +to the alert as `correlations[]`, so the alert describes both ends of the chain. +Without it, an exec-then-egress alert would say only "a process made an outbound +connection" and drop the exec that makes it interesting. + +Each entry carries `name`, `eventType`, `timestamp`, `scope`, `key`, the +remembered `process`, and any author `values`. Only hits are recorded — a miss is +not evidence of anything — and the record is reset per rule, so one rule never +cites another's entries. + +**Correlation enriches an incident; it does not re-key it.** `InfectedPID` and +`RuntimeProcessDetails` continue to describe the *triggering* event, so backend +incident grouping is unchanged. An alert with no correlations serializes exactly +as before, with no `correlations` key. + +### Which exporters carry it + +Only the **HTTP exporter**. That is a property of the payload shapes, not an +oversight, and it is not expected to change: + +| Exporter | Carries `correlations[]`? | +|---|---| +| HTTP | yes — the only nested-JSON payload | +| Alertmanager | no — labels are a flat `map[string]string` | +| stdout | no — a fixed set of structured log fields | +| CSV | no — a fixed column set | +| syslog | no — a flat message | + +Every exporter other than HTTP emits a flat, fixed-shape record, so carrying a +nested array would mean changing that exporter's schema rather than adding a +field. A rule that correlates still alerts normally on all of them; only the +evidence of the far leg is absent. If you are asserting on correlation evidence — +in a test or a downstream consumer — the HTTP exporter is the one to read. + +`message` and `uniqueId` are evaluated against the predicate's own context, so +`state.get()` in a message resolves against the same entries the predicate +matched — and `uniqueId` can be derived from the join key, which is what lets +cooldown collapse both legs of a bidirectional rule into one alert. + +## Configuration + +```yaml +celStateStore: + enabled: true + maxSize: 100000 # node-wide ceiling (approximate under concurrency) + maxEntriesPerContainer: 256 # exact, per container + maxEntriesForHost: 4096 # the c:__host__ bucket + maxTtl: 30m # every rule's ttl is clamped to this + sweepInterval: 30s + ancestorMaxDepth: 8 # probes per has_ancestor call +``` + +Disabling it makes writes no-ops and every read a miss, so correlation rules stop +firing while ordinary rules are unaffected. + +A container's entries are purged as soon as the container is removed, rather than +waiting for TTL. A **pod** bucket is purged once the pod's *last* container is +removed — it cannot go earlier, because pod-scoped state exists precisely to +outlive any one container in the pod. That check runs on the same 10-minute timer +as the rest of the pod-level cleanup, so a pod bucket can outlive its pod by up to +ten minutes before being reclaimed. + +The **host and node** buckets get no purge at all — they are node-wide and belong +to no workload, so they rely on TTL, which is why their cap is larger. + +Pod scope shares the per-container cap, and that cap covers the whole pod rather +than each container in it. Pod scope is meant for facts about the pod as a unit; if +a real workload needs more room, `state_write_rejected_total{reason="scope_cap"}` +is what will say so. + +## Metrics + +| Metric | Meaning | +|---|---| +| `node_agent_state_writes_total{rule_id,result}` | Entries written | +| `node_agent_state_write_rejected_total{rule_id,reason}` | **Alert on this** — a rule is being starved of the state it needs. `scope_cap` / `global_cap` are the store's own limits; `prefiltered` and `policy` mean a suppression meant for the rule's *alerting* leg silenced its *write* leg instead (see below); `guard_error`, `key_error` and `scope_unresolved` are the clause failing to evaluate | +| `node_agent_state_expired_total` | Reclaimed by TTL | +| `node_agent_state_purged_total` | Dropped by scope purge | +| `node_agent_state_entries{scope}` | Current entry count, published by each sweep. `scope` is the scope *kind* — `container`, `host`, `pod` or `node` — never a scope ID, which would be unbounded. The host bucket is reported apart from real containers because it is one of the two that TTL alone reclaims. Every kind is republished on each sweep, so a kind that drains reads as zero rather than keeping its last value. | + +Counters are labelled by rule ID only, never by state key — a key is unbounded +cardinality. + +## When a correlation rule does not fire + +Every failure mode here is silent — the rule applies, loads, and simply never +matches. Work down this list in order; each step is cheap and rules out one cause. + +**1. Is the clause even reaching the agent?** The `Rules` CRD must declare +`stateWrites`, or the API server prunes it. This is the most likely cause and the +hardest to guess, because `kubectl apply` reports success: + +```bash +kubectl get rules -n kubescape -o jsonpath='{.spec.rules[0].stateWrites}' +``` + +Empty output after a successful apply means the schema is missing the property. +`--validate=false` does not help — it skips client-side validation only. + +**2. Did the clause fail validation?** node-agent logs +`RuleManager - invalid stateWrites clause` with the rule ID and the offending +write name. A rule that fails validation is degraded to non-correlating; the rest +of the CRD keeps evaluating. + +**3. Is the rule bound?** Rules are inert until a `RuntimeRuleAlertBinding` lists +them by `ruleName`. A new rule ID does nothing on its own. + +**4. Is the write leg reaching the rule loop?** Add a temporary control rule that +alerts on the *write* event type with the same predicate as your `when:` guard. If +the control is silent, the problem is upstream of the state store. + +**5. Do the two legs agree on the join key?** Have the control rules print +`string(event.pid)` in their `message`, and compare. `has_ancestor` is the right +answer when the second leg is a *child* rather than the same process. + +**6. Is the write being rejected?** `state_write_rejected_total` counts caps and +guard errors, labelled by rule ID. + +**7. Could the events be reordered?** node-agent evaluates on a concurrent worker +pool, so two events milliseconds apart can be processed out of order. This is real +but usually not the cause — rule out everything above first. diff --git a/pkg/config/config.go b/pkg/config/config.go index cec9f41ab6..dfcd6f39aa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -16,6 +16,7 @@ import ( processtreecreator "github.com/kubescape/node-agent/pkg/processtree/config" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/cache" "github.com/kubescape/node-agent/pkg/rulemanager/rulecooldown" + "github.com/kubescape/node-agent/pkg/rulestate" "github.com/spf13/viper" ) @@ -54,6 +55,7 @@ type AlertDeduplicationConfig struct { type Config struct { BlockEvents bool `mapstructure:"blockEvents"` CelConfigCache cache.FunctionCacheConfig `mapstructure:"celConfigCache"` + CelStateStore rulestate.Config `mapstructure:"celStateStore"` ContainerEolNotificationBuffer int `mapstructure:"containerEolNotificationBuffer"` DBpf bool `mapstructure:"dBpf"` DCapSys bool `mapstructure:"dCapSys"` @@ -209,6 +211,17 @@ func LoadConfigOptional(path string, errNotFound bool) (Config, error) { viper.SetDefault("blockEvents", false) viper.SetDefault("celConfigCache::maxSize", 100000) viper.SetDefault("celConfigCache::ttl", 1*time.Minute) + + // CEL rule state store. maxEntriesForHost is larger than the per-container cap + // because the host bucket holds the whole node's process space rather than one + // workload, and never receives a container-removal purge -- it relies on TTL. + viper.SetDefault("celStateStore::enabled", true) + viper.SetDefault("celStateStore::maxSize", 100000) + viper.SetDefault("celStateStore::maxEntriesPerContainer", 256) + viper.SetDefault("celStateStore::maxEntriesForHost", 4096) + viper.SetDefault("celStateStore::maxTtl", 30*time.Minute) + viper.SetDefault("celStateStore::sweepInterval", 30*time.Second) + viper.SetDefault("celStateStore::ancestorMaxDepth", 8) viper.SetDefault("ignoreRuleBindings", false) viper.SetDefault("eventDedup::enabled", true) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 754b342279..105e8159fd 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -10,6 +10,7 @@ import ( processtreecreator "github.com/kubescape/node-agent/pkg/processtree/config" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/cache" "github.com/kubescape/node-agent/pkg/rulemanager/rulecooldown" + "github.com/kubescape/node-agent/pkg/rulestate" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -101,6 +102,15 @@ func TestLoadConfig(t *testing.T) { MaxSize: 100000, TTL: 1 * time.Minute, }, + CelStateStore: rulestate.Config{ + Enabled: true, + MaxSize: 100000, + MaxEntriesPerContainer: 256, + MaxEntriesForHost: 4096, + MaxTTL: 30 * time.Minute, + SweepInterval: 30 * time.Second, + AncestorMaxDepth: 8, + }, DNSCacheSize: 50000, ContainerEolNotificationBuffer: 100, FIM: FIMConfig{ diff --git a/pkg/exporters/http_exporter.go b/pkg/exporters/http_exporter.go index 8ecc9c751f..3305d08000 100644 --- a/pkg/exporters/http_exporter.go +++ b/pkg/exporters/http_exporter.go @@ -346,6 +346,7 @@ func (e *HTTPExporter) createRuleAlert(failedRule types.RuleFailure) armotypes.R RuleID: failedRule.GetRuleId(), IsTriggerAlert: failedRule.GetIsTriggerAlert(), HttpRuleAlert: httpDetails, + CorrelationAlert: failedRule.GetCorrelationAlert(), } } diff --git a/pkg/metricsmanager/metrics_manager_interface.go b/pkg/metricsmanager/metrics_manager_interface.go index 170d8cf320..5bf0155aae 100644 --- a/pkg/metricsmanager/metrics_manager_interface.go +++ b/pkg/metricsmanager/metrics_manager_interface.go @@ -65,4 +65,15 @@ type MetricsManager interface { // Alert suppression funnel — counts how many alerts were dropped and why. ReportAlertSuppressed(ruleID, reason string) + + // CEL rule state store. Labelled by ruleID only — never by state key, which is + // unbounded cardinality. + // + // ReportStateWriteRejected is the alert-worthy one: it means a rule is being + // silently starved of the state it needs to correlate. + ReportStateWrite(ruleID, result string) + ReportStateWriteRejected(ruleID, reason string) + ReportStateExpired(n int) + ReportStatePurged(n int) + ReportStateEntries(scope string, n int) } diff --git a/pkg/metricsmanager/metrics_manager_mock.go b/pkg/metricsmanager/metrics_manager_mock.go index 3da0978115..9a565b8e25 100644 --- a/pkg/metricsmanager/metrics_manager_mock.go +++ b/pkg/metricsmanager/metrics_manager_mock.go @@ -99,3 +99,9 @@ func (m *MetricsMock) ObserveSBOMScanDuration(_ string, _ time.Duration) func (m *MetricsMock) ReportSBOMScannerRestart() {} func (m *MetricsMock) SetSBOMScannerReady(_ bool) {} func (m *MetricsMock) ReportAlertSuppressed(_, _ string) {} + +func (m *MetricsMock) ReportStateWrite(_, _ string) {} +func (m *MetricsMock) ReportStateWriteRejected(_, _ string) {} +func (m *MetricsMock) ReportStateExpired(_ int) {} +func (m *MetricsMock) ReportStatePurged(_ int) {} +func (m *MetricsMock) ReportStateEntries(_ string, _ int) {} diff --git a/pkg/metricsmanager/metrics_manager_noop.go b/pkg/metricsmanager/metrics_manager_noop.go index 09c8f89ecb..5541dc6dc0 100644 --- a/pkg/metricsmanager/metrics_manager_noop.go +++ b/pkg/metricsmanager/metrics_manager_noop.go @@ -55,3 +55,9 @@ func (m *MetricsNoop) ObserveSBOMScanDuration(_ string, _ time.Duration) func (m *MetricsNoop) ReportSBOMScannerRestart() {} func (m *MetricsNoop) SetSBOMScannerReady(_ bool) {} func (m *MetricsNoop) ReportAlertSuppressed(_, _ string) {} + +func (m *MetricsNoop) ReportStateWrite(_, _ string) {} +func (m *MetricsNoop) ReportStateWriteRejected(_, _ string) {} +func (m *MetricsNoop) ReportStateExpired(_ int) {} +func (m *MetricsNoop) ReportStatePurged(_ int) {} +func (m *MetricsNoop) ReportStateEntries(_ string, _ int) {} diff --git a/pkg/metricsmanager/otel/otel_metrics_manager.go b/pkg/metricsmanager/otel/otel_metrics_manager.go index b0e5cfccf4..9adddef552 100644 --- a/pkg/metricsmanager/otel/otel_metrics_manager.go +++ b/pkg/metricsmanager/otel/otel_metrics_manager.go @@ -77,6 +77,13 @@ type OTELMetricsManager struct { // Alert suppression funnel alertSuppressedTotal metric.Int64Counter + // CEL rule state store + stateWritesTotal metric.Int64Counter + stateWriteRejectedTotal metric.Int64Counter + stateExpiredTotal metric.Int64Counter + statePurgedTotal metric.Int64Counter + stateEntries metric.Float64Gauge + // Live container count — incremented on start, decremented on stop. // Exposed as node_agent.container.count observable gauge. containerCount atomic.Int64 @@ -247,6 +254,16 @@ func NewOTELMetricsManager(ownContainerID, ownPodUID string, hostCgroupMounted b m.alertSuppressedTotal = mustCounter("node_agent.alert.suppressed.total", "Total alerts suppressed before delivery, labeled by rule_id and reason") + m.stateWritesTotal = mustCounter("node_agent.state.writes.total", + "Total CEL rule state entries written, labeled by rule_id") + m.stateWriteRejectedTotal = mustCounter("node_agent.state.write.rejected.total", + "Total CEL rule state writes rejected; a rule is being starved of the state it needs to correlate") + m.stateExpiredTotal = mustCounter("node_agent.state.expired.total", + "Total CEL rule state entries reclaimed by TTL expiry") + m.statePurgedTotal = mustCounter("node_agent.state.purged.total", + "Total CEL rule state entries dropped by scope purge, e.g. container removal") + m.stateEntries = mustGauge("node_agent.state.entries", + "Current CEL rule state entries, labeled by scope") registerResourceMetrics(meter, &m.containerCount, ownContainerID, ownPodUID, hostCgroupMounted) @@ -557,3 +574,27 @@ func (m *OTELMetricsManager) suppressedOption(ruleID, reason string) metric.Meas func (m *OTELMetricsManager) ReportAlertSuppressed(ruleID, reason string) { m.alertSuppressedTotal.Add(context.Background(), 1, m.suppressedOption(ruleID, reason)) } + +// The state counters reuse suppressedOption: it caches a (ruleID, reason) +// attribute set, which is exactly the label pair these need. Labelling by ruleID +// only is deliberate -- a state key is unbounded cardinality. +func (m *OTELMetricsManager) ReportStateWrite(ruleID, result string) { + m.stateWritesTotal.Add(context.Background(), 1, m.suppressedOption(ruleID, result)) +} + +func (m *OTELMetricsManager) ReportStateWriteRejected(ruleID, reason string) { + m.stateWriteRejectedTotal.Add(context.Background(), 1, m.suppressedOption(ruleID, reason)) +} + +func (m *OTELMetricsManager) ReportStateExpired(n int) { + m.stateExpiredTotal.Add(context.Background(), int64(n)) +} + +func (m *OTELMetricsManager) ReportStatePurged(n int) { + m.statePurgedTotal.Add(context.Background(), int64(n)) +} + +func (m *OTELMetricsManager) ReportStateEntries(scope string, n int) { + m.stateEntries.Record(context.Background(), float64(n), + metric.WithAttributes(attribute.String("scope", scope))) +} diff --git a/pkg/metricsmanager/prometheus/prometheus.go b/pkg/metricsmanager/prometheus/prometheus.go index 74bd0fb757..603e00d00d 100644 --- a/pkg/metricsmanager/prometheus/prometheus.go +++ b/pkg/metricsmanager/prometheus/prometheus.go @@ -107,6 +107,13 @@ type PrometheusMetric struct { // Alert suppression funnel alertSuppressedCounter *prometheus.CounterVec + // CEL rule state store + stateWritesCounter *prometheus.CounterVec + stateWriteRejectedCounter *prometheus.CounterVec + stateExpiredCounter prometheus.Counter + statePurgedCounter prometheus.Counter + stateEntriesGauge *prometheus.GaugeVec + // Cache to avoid allocating Labels maps on every call ruleCounterCache map[string]prometheus.Counter rulePrefilteredCounterCache map[string]prometheus.Counter @@ -387,6 +394,26 @@ func NewPrometheusMetric() *PrometheusMetric { Name: "node_agent_alert_suppressed_total", Help: "Total alerts suppressed before delivery, labeled by rule_id and reason", }, []string{prometheusRuleIdLabel, "reason"}), + stateWritesCounter: promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "node_agent_state_writes_total", + Help: "Total CEL rule state entries written, labeled by rule_id", + }, []string{prometheusRuleIdLabel, "result"}), + stateWriteRejectedCounter: promauto.NewCounterVec(prometheus.CounterOpts{ + Name: "node_agent_state_write_rejected_total", + Help: "Total CEL rule state writes rejected; a rule is being starved of the state it needs to correlate", + }, []string{prometheusRuleIdLabel, "reason"}), + stateExpiredCounter: promauto.NewCounter(prometheus.CounterOpts{ + Name: "node_agent_state_expired_total", + Help: "Total CEL rule state entries reclaimed by TTL expiry", + }), + statePurgedCounter: promauto.NewCounter(prometheus.CounterOpts{ + Name: "node_agent_state_purged_total", + Help: "Total CEL rule state entries dropped by scope purge, e.g. container removal", + }), + stateEntriesGauge: promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "node_agent_state_entries", + Help: "Current CEL rule state entries, labeled by scope", + }, []string{"scope"}), sbomScanDuration: promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "sbom_scan_duration_seconds", Help: "SBOM scan duration in seconds", @@ -479,6 +506,11 @@ func (p *PrometheusMetric) Destroy() { prometheus.Unregister(p.programPerCpuUsageGauge) prometheus.Unregister(p.sbomScanCounter) prometheus.Unregister(p.alertSuppressedCounter) + prometheus.Unregister(p.stateWritesCounter) + prometheus.Unregister(p.stateWriteRejectedCounter) + prometheus.Unregister(p.stateExpiredCounter) + prometheus.Unregister(p.statePurgedCounter) + prometheus.Unregister(p.stateEntriesGauge) prometheus.Unregister(p.sbomScanDuration) prometheus.Unregister(p.sbomRestarts) prometheus.Unregister(p.sbomReady) @@ -772,3 +804,23 @@ func (p *PrometheusMetric) SetSBOMScannerReady(ready bool) { func (p *PrometheusMetric) ReportAlertSuppressed(ruleID, reason string) { p.alertSuppressedCounter.WithLabelValues(ruleID, reason).Inc() } + +func (p *PrometheusMetric) ReportStateWrite(ruleID, result string) { + p.stateWritesCounter.WithLabelValues(ruleID, result).Inc() +} + +func (p *PrometheusMetric) ReportStateWriteRejected(ruleID, reason string) { + p.stateWriteRejectedCounter.WithLabelValues(ruleID, reason).Inc() +} + +func (p *PrometheusMetric) ReportStateExpired(n int) { + p.stateExpiredCounter.Add(float64(n)) +} + +func (p *PrometheusMetric) ReportStatePurged(n int) { + p.statePurgedCounter.Add(float64(n)) +} + +func (p *PrometheusMetric) ReportStateEntries(scope string, n int) { + p.stateEntriesGauge.WithLabelValues(scope).Set(float64(n)) +} diff --git a/pkg/objectcache/containerprofilecache/projection_compile_test.go b/pkg/objectcache/containerprofilecache/projection_compile_test.go index fa73e4c0e8..6eae4d6e35 100644 --- a/pkg/objectcache/containerprofilecache/projection_compile_test.go +++ b/pkg/objectcache/containerprofilecache/projection_compile_test.go @@ -3,6 +3,8 @@ package containerprofilecache import ( "testing" + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/objectcache" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" "github.com/stretchr/testify/assert" @@ -12,7 +14,7 @@ import ( // makeRule is a helper that builds a Rule with a ProfileDataRequired. func makeRule(pdr *typesv1.ProfileDataRequired) typesv1.Rule { return typesv1.Rule{ - ID: "test-rule", + RuntimeRule: armotypes.RuntimeRule{ID: "test-rule"}, ProfileDataRequired: pdr, } } @@ -63,8 +65,8 @@ func TestCompileSpec_Empty(t *testing.T) { // ProfileDataRequired do not contribute to the spec. func TestCompileSpec_NilProfileDataRequiredSkipped(t *testing.T) { rules := []typesv1.Rule{ - {ID: "no-pdr", ProfileDataRequired: nil}, - {ID: "also-no-pdr", ProfileDataRequired: nil}, + {RuntimeRule: armotypes.RuntimeRule{ID: "no-pdr"}, ProfileDataRequired: nil}, + {RuntimeRule: armotypes.RuntimeRule{ID: "also-no-pdr"}, ProfileDataRequired: nil}, } spec := CompileSpec(rules) @@ -90,7 +92,7 @@ func TestCompileSpec_DeterministicHash(t *testing.T) { pdr2 := &typesv1.ProfileDataRequired{ Execs: fieldReqAll(), } - rule2 := typesv1.Rule{ID: "r2", ProfileDataRequired: pdr2} + rule2 := typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{ID: "r2"}, ProfileDataRequired: pdr2} specAB := CompileSpec([]typesv1.Rule{rule, rule2}) specBA := CompileSpec([]typesv1.Rule{rule2, rule}) diff --git a/pkg/objectcache/containerprofilecache/projection_golden_test.go b/pkg/objectcache/containerprofilecache/projection_golden_test.go index 750d120bcb..f4106de730 100644 --- a/pkg/objectcache/containerprofilecache/projection_golden_test.go +++ b/pkg/objectcache/containerprofilecache/projection_golden_test.go @@ -7,6 +7,8 @@ import ( "sort" "testing" + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/objectcache" "github.com/kubescape/node-agent/pkg/objectcache/callstackcache" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" @@ -257,8 +259,7 @@ func networkProfile() *v1beta1.ContainerProfile { func mixedFilterRules() []typesv1.Rule { return []typesv1.Rule{ { - ID: "RULE-A", - Name: "mixed-file-and-net", + RuntimeRule: armotypes.RuntimeRule{ID: "RULE-A", Name: "mixed-file-and-net"}, ProfileDataRequired: &typesv1.ProfileDataRequired{ Opens: declaredPatterns( typesv1.PatternObject{Exact: "/etc/passwd"}, @@ -279,8 +280,7 @@ func mixedFilterRules() []typesv1.Rule { { // A second rule narrows capabilities further and adds an opens // prefix, proving the union merge across rules. - ID: "RULE-B", - Name: "extra-caps", + RuntimeRule: armotypes.RuntimeRule{ID: "RULE-B", Name: "extra-caps"}, ProfileDataRequired: &typesv1.ProfileDataRequired{ Capabilities: declaredPatterns(typesv1.PatternObject{Exact: "SYS_PTRACE"}), Opens: declaredPatterns(typesv1.PatternObject{Prefix: "/data/"}), @@ -294,8 +294,7 @@ func mixedFilterRules() []typesv1.Rule { func netAllRules() []typesv1.Rule { return []typesv1.Rule{ { - ID: "RULE-NET", - Name: "net-all", + RuntimeRule: armotypes.RuntimeRule{ID: "RULE-NET", Name: "net-all"}, ProfileDataRequired: &typesv1.ProfileDataRequired{ EgressDomains: declaredAll(), EgressAddresses: declaredAll(), diff --git a/pkg/processtree/ancestors.go b/pkg/processtree/ancestors.go new file mode 100644 index 0000000000..c75f052d6d --- /dev/null +++ b/pkg/processtree/ancestors.go @@ -0,0 +1,49 @@ +package processtree + +import ( + "github.com/armosec/armoapi-go/armotypes" +) + +// GetAncestorPIDs returns pid's ancestors, nearest first, up to maxDepth entries. +// pid itself is excluded. +// +// This walks the creator's global process map rather than +// containerTree.GetPidBranch, because GetPidBranch resolves a container shim and +// errors out when there is none -- which is every host / cgroup-0 process. Walking +// the map works identically for containerised and host processes. +// +// maxDepth also bounds the walk defensively: a reparenting race could in +// principle produce a parent cycle, and the evaluator must not hang. +func (ptm *ProcessTreeManagerImpl) GetAncestorPIDs(pid uint32, maxDepth int) []uint32 { + if maxDepth <= 0 { + return nil + } + + var out []uint32 + seen := make(map[uint32]struct{}, maxDepth) + current := pid + + for len(out) < maxDepth { + var node *armotypes.Process + func() { + ptm.mutex.RLock() + defer ptm.mutex.RUnlock() + node, _ = ptm.creator.GetProcessNode(int(current)) + }() + // PPID 0 means "parent unknown", not "parent is pid 0" -- recording it + // would add a key no state entry can ever be stored under. + if node == nil || node.PPID == 0 { + break + } + if _, dup := seen[node.PPID]; dup { + break + } + seen[node.PPID] = struct{}{} + out = append(out, node.PPID) + if node.PPID == 1 { + break + } + current = node.PPID + } + return out +} diff --git a/pkg/processtree/ancestors_test.go b/pkg/processtree/ancestors_test.go new file mode 100644 index 0000000000..1454e1355e --- /dev/null +++ b/pkg/processtree/ancestors_test.go @@ -0,0 +1,115 @@ +package processtree + +import ( + "fmt" + "testing" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/goradd/maps" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/processtree/conversion" + "github.com/stretchr/testify/assert" +) + +// stubCreator is a ProcessTreeCreator backed by a plain map, so ancestor walking +// can be tested without feeding synthetic events through the real creator. +type stubCreator struct { + tree map[uint32]*armotypes.Process +} + +func (s *stubCreator) FeedEvent(conversion.ProcessEvent) {} +func (s *stubCreator) Start() {} +func (s *stubCreator) Stop() {} + +func (s *stubCreator) GetRootTree() ([]armotypes.Process, error) { return nil, nil } + +// Ancestor walking does not consult process start times, so the stub reports +// "unknown" for every pid rather than inventing values. +func (s *stubCreator) GetProcessBootTimeNs(_ uint32) uint64 { return 0 } + +func (s *stubCreator) GetProcessMap() *maps.SafeMap[uint32, *armotypes.Process] { + m := &maps.SafeMap[uint32, *armotypes.Process]{} + for pid, p := range s.tree { + m.Set(pid, p) + } + return m +} + +func (s *stubCreator) GetProcessNode(pid int) (*armotypes.Process, error) { + p, ok := s.tree[uint32(pid)] + if !ok { + return nil, fmt.Errorf("process %d not found", pid) + } + return p, nil +} + +func newTestManagerWithTree(t *testing.T, tree map[uint32]*armotypes.Process) *ProcessTreeManagerImpl { + t.Helper() + return &ProcessTreeManagerImpl{ + creator: &stubCreator{tree: tree}, + config: config.Config{}, + } +} + +func TestGetAncestorPIDs(t *testing.T) { + // 900 (bash) -> 4471 (sh) -> 4530 (curl) + tree := map[uint32]*armotypes.Process{ + 900: {PID: 900, PPID: 1}, + 4471: {PID: 4471, PPID: 900}, + 4530: {PID: 4530, PPID: 4471}, + } + + tests := []struct { + name string + pid uint32 + maxDepth int + want []uint32 + }{ + {"full chain", 4530, 8, []uint32{4471, 900, 1}}, + {"depth bound respected", 4530, 2, []uint32{4471, 900}}, + {"leaf with one ancestor", 900, 8, []uint32{1}}, + {"unknown pid yields nothing", 99999, 8, nil}, + {"zero depth yields nothing", 4530, 0, nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ptm := newTestManagerWithTree(t, tree) + assert.Equal(t, tt.want, ptm.GetAncestorPIDs(tt.pid, tt.maxDepth)) + }) + } +} + +// Host processes have no container shim, so GetPidBranch errors for them and +// enrichedEvent.ProcessTree is zero-valued. GetAncestorPIDs must still work -- +// this is the whole reason it exists. +func TestGetAncestorPIDs_WorksForHostProcessesWithNoShim(t *testing.T) { + tree := map[uint32]*armotypes.Process{ + 2200: {PID: 2200, PPID: 1}, + 2300: {PID: 2300, PPID: 2200}, + } + ptm := newTestManagerWithTree(t, tree) + assert.Equal(t, []uint32{2200, 1}, ptm.GetAncestorPIDs(2300, 8)) +} + +func TestGetAncestorPIDs_TerminatesOnCycle(t *testing.T) { + // Defensive: reparenting races could in principle produce a loop. The depth + // bound must contain it rather than hanging the evaluator. + tree := map[uint32]*armotypes.Process{ + 10: {PID: 10, PPID: 11}, + 11: {PID: 11, PPID: 10}, + } + ptm := newTestManagerWithTree(t, tree) + assert.LessOrEqual(t, len(ptm.GetAncestorPIDs(10, 8)), 8) +} + +// A PPID of 0 means "parent unknown", not "parent is pid 0". Recording it would +// put a meaningless 0 in the ancestor list and make state lookups probe a key +// that can never exist. +func TestGetAncestorPIDs_StopsAtUnknownParent(t *testing.T) { + tree := map[uint32]*armotypes.Process{ + 7000: {PID: 7000, PPID: 0}, + } + ptm := newTestManagerWithTree(t, tree) + assert.Empty(t, ptm.GetAncestorPIDs(7000, 8)) +} diff --git a/pkg/processtree/process_tree_manager_interface.go b/pkg/processtree/process_tree_manager_interface.go index aa11122288..901a54059d 100644 --- a/pkg/processtree/process_tree_manager_interface.go +++ b/pkg/processtree/process_tree_manager_interface.go @@ -22,4 +22,7 @@ type ProcessTreeManager interface { // inherits btime's whole-second skew, so it must never be compared for // identity. GetProcessBootTimeNs(pid uint32) uint64 + // GetAncestorPIDs returns pid's ancestors, nearest first, bounded by maxDepth. + // Works for host processes too, unlike GetContainerProcessTree. + GetAncestorPIDs(pid uint32, maxDepth int) []uint32 } diff --git a/pkg/processtree/process_tree_manager_mock.go b/pkg/processtree/process_tree_manager_mock.go index 8d04552684..b1df311f8e 100644 --- a/pkg/processtree/process_tree_manager_mock.go +++ b/pkg/processtree/process_tree_manager_mock.go @@ -9,6 +9,7 @@ import ( type ProcessTreeManagerMock struct { pidList []uint32 bootTimeNs map[uint32]uint64 + ancestors []uint32 } var _ ProcessTreeManager = (*ProcessTreeManagerMock)(nil) @@ -63,3 +64,21 @@ func (m *ProcessTreeManagerMock) SetProcessBootTimeNs(pid uint32, ns uint64) { } m.bootTimeNs[pid] = ns } + +// SetAncestors sets the ancestor chain the mock reports, nearest first. Rule +// tests that exercise ancestor matching need to stub a chain rather than build a +// real process tree. +func (m *ProcessTreeManagerMock) SetAncestors(pids []uint32) { + m.ancestors = pids +} + +// GetAncestorPIDs returns the configured ancestor chain, truncated to maxDepth. +func (m *ProcessTreeManagerMock) GetAncestorPIDs(_ uint32, maxDepth int) []uint32 { + if maxDepth <= 0 || len(m.ancestors) == 0 { + return nil + } + if len(m.ancestors) > maxDepth { + return m.ancestors[:maxDepth] + } + return m.ancestors +} diff --git a/pkg/rulemanager/cel/cel.go b/pkg/rulemanager/cel/cel.go index ed151d4e8b..ebb97c8637 100644 --- a/pkg/rulemanager/cel/cel.go +++ b/pkg/rulemanager/cel/cel.go @@ -20,6 +20,7 @@ import ( "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/net" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/parse" "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/process" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/state" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" "github.com/kubescape/node-agent/pkg/utils" "github.com/picatz/xcel" @@ -58,6 +59,11 @@ func NewCEL(objectCache objectcache.ObjectCache, cfg config.Config, mm ...metric cel.Variable("event", eventTyp), // All events accessible via "event" variable cel.Variable("http", eventTyp), // HTTP events also accessible via "http" variable cel.Variable("eventType", cel.StringType), + // The resolved event time, as a top-level variable rather than an event + // field: CelFields getters receive an xcel wrapper around the event and + // cannot reach EnrichedEvent.Timestamp, so a field would be a second, + // divergent source of truth. See ResolveEventTime. + cel.Variable("timestamp", cel.TimestampType), cel.CustomTypeAdapter(ta), cel.CustomTypeProvider(tp), ext.Strings(), @@ -74,6 +80,10 @@ func NewCEL(objectCache objectcache.ObjectCache, cfg config.Config, mm ...metric parse.Parse(cfg), net.Net(cfg), process.Process(cfg), + // Declares the "state" variable and its read functions. The store and the + // per-rule receiver are injected into the eval context, not here -- see + // state.Accessor. + state.State(cfg), } env, err := cel.NewEnv(envOptions...) @@ -176,6 +186,7 @@ func (c *CEL) CreateEvalContext(event *events.EnrichedEvent) map[string]any { evalContext := map[string]any{ "eventType": string(eventType), "event": obj, + "timestamp": ResolveEventTime(event), } // For HTTP events, also add "http" variable @@ -209,6 +220,45 @@ func (c *CEL) evaluateProgramWithContext(expression string, evalContext map[stri return out, nil } +// EvaluateBoolExpressionWithContext evaluates a boolean expression against an +// already-built context. State-write guards use it so the guard sees exactly the +// same event view -- and the same state -- as the predicate did. +func (c *CEL) EvaluateBoolExpressionWithContext(evalContext map[string]any, expression string) (bool, error) { + out, err := c.evaluateProgramWithContext(expression, evalContext) + if err != nil { + return false, err + } + // A nil program means compilation failed and was cached as such. + if out == nil { + return false, nil + } + boolVal, ok := out.Value().(bool) + if !ok { + return false, fmt.Errorf("expression returned %T, expected bool", out.Value()) + } + return boolVal, nil +} + +// EvaluateStringExpressionWithContext evaluates expr against an already-built +// context. Message and uniqueId expressions must reuse the predicate's context so +// state.get() resolves against the same entries -- and so uniqueId can be derived +// from the join key, which is what lets rulecooldown collapse the two legs of a +// bidirectional rule into one alert. +func (c *CEL) EvaluateStringExpressionWithContext(evalContext map[string]any, expression string) (string, error) { + out, err := c.evaluateProgramWithContext(expression, evalContext) + if err != nil { + return "", err + } + if out == nil { + return "", nil + } + strVal, ok := out.Value().(string) + if !ok { + return "", fmt.Errorf("expression returned %T, expected string", out.Value()) + } + return strVal, nil +} + func (c *CEL) EvaluateRule(event *events.EnrichedEvent, expressions []typesv1.RuleExpression) (bool, error) { eventType := event.Event.GetEventType() evalContext := c.CreateEvalContext(event) diff --git a/pkg/rulemanager/cel/cel_interface.go b/pkg/rulemanager/cel/cel_interface.go index 935c7b830f..8a41f0f3a6 100644 --- a/pkg/rulemanager/cel/cel_interface.go +++ b/pkg/rulemanager/cel/cel_interface.go @@ -11,6 +11,8 @@ type RuleEvaluator interface { EvaluateRule(event *events.EnrichedEvent, expressions []typesv1.RuleExpression) (bool, error) EvaluateRuleWithContext(evalContext map[string]any, eventType utils.EventType, expressions []typesv1.RuleExpression) (bool, error) EvaluateExpression(event *events.EnrichedEvent, expression string) (string, error) + EvaluateBoolExpressionWithContext(evalContext map[string]any, expression string) (bool, error) + EvaluateStringExpressionWithContext(evalContext map[string]any, expression string) (string, error) CreateEvalContext(event *events.EnrichedEvent) map[string]any RegisterHelper(function cel.EnvOption) error RegisterCustomType(eventType utils.EventType, obj interface{}) error diff --git a/pkg/rulemanager/cel/eventtime.go b/pkg/rulemanager/cel/eventtime.go new file mode 100644 index 0000000000..c662686b30 --- /dev/null +++ b/pkg/rulemanager/cel/eventtime.go @@ -0,0 +1,57 @@ +package cel + +import ( + "time" + + "github.com/kubescape/node-agent/pkg/ebpf/events" +) + +// ResolveEventTime returns the single authoritative timestamp for an event. +// +// It prefers the event's own kernel timestamp, because ordering guards must +// compare when things HAPPENED, not when node-agent got around to seeing them -- +// events are processed by a concurrent worker pool, so observation order is not +// causal order. Some events report a zero timestamp; those fall back to the +// enrichment time rather than the epoch. +// +// Both the CEL "timestamp" variable and rulestate.Entry.Timestamp are populated +// from this function. They must never diverge: a mismatch would make the _ts +// join compare different clocks and silently never fire. +// +// The event clock must be a WALL clock, because ExpiresAt is derived from it +// while expiry is checked against time.Now(). Every producer today satisfies +// that -- DatasourceEvent.GetTimestamp goes through gadgets.WallTimeFromBootTime, +// and procfs and syscall events use time.Now().UnixNano() directly -- but that is +// a property of the current producers, not something the type system enforces. A +// future source handing over a raw boot-time value would yield 1970-plus-uptime, +// every entry from that stream would be born expired, and the rule would silently +// never fire. So the invariant is checked rather than assumed: a timestamp +// implausibly far from now is discarded in favour of the enrichment time. +// +// The window is deliberately wide. It exists to catch a wrong EPOCH -- off by +// decades -- not clock skew, and it must never reject a legitimately late or +// slightly-ahead event. +const maxEventTimeSkew = 24 * time.Hour + +func ResolveEventTime(enrichedEvent *events.EnrichedEvent) time.Time { + if enrichedEvent == nil { + return time.Time{} + } + if enrichedEvent.Event != nil { + if ns := int64(enrichedEvent.Event.GetTimestamp()); ns > 0 { + if t := time.Unix(0, ns); plausibleEventTime(t, time.Now()) { + return t + } + } + } + return enrichedEvent.Timestamp +} + +// plausibleEventTime reports whether t could be a wall-clock reading of now. +func plausibleEventTime(t, now time.Time) bool { + d := now.Sub(t) + if d < 0 { + d = -d + } + return d <= maxEventTimeSkew +} diff --git a/pkg/rulemanager/cel/eventtime_test.go b/pkg/rulemanager/cel/eventtime_test.go new file mode 100644 index 0000000000..219320675d --- /dev/null +++ b/pkg/rulemanager/cel/eventtime_test.go @@ -0,0 +1,143 @@ +package cel + +import ( + "testing" + "time" + + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// execEventAt builds an exec event whose kernel timestamp is ts. +// +// A real utils.StructEvent is used rather than a hand-rolled fake: the eval +// context casts the event to utils.CelEvent and calls GetEventType() on it, so a +// fake embedding a nil utils.K8sEvent panics before reaching the assertion. +// recentTime is a wall-clock instant close to now, truncated so equality +// assertions stay exact. +// +// Fixed calendar dates were used here originally, and they are wrong for this +// function: ResolveEventTime now rejects a timestamp implausibly far from now, so +// a hard-coded date silently becomes an "implausible epoch" the moment it ages +// past the bound. Real events always carry a near-now wall clock; the fixtures +// must too. +func recentTime() time.Time { + return time.Now().Add(-time.Second).Truncate(time.Millisecond) +} + +func execEventAt(ts int64) *utils.StructEvent { + return &utils.StructEvent{ + EventType: utils.ExecveEventType, + Comm: "curl", + Timestamp: ts, + } +} + +func TestResolveEventTime_PrefersEventTimestamp(t *testing.T) { + kernelTime := recentTime() + observed := kernelTime.Add(5 * time.Millisecond) + + ee := &events.EnrichedEvent{ + Event: execEventAt(kernelTime.UnixNano()), + Timestamp: observed, + } + assert.Equal(t, kernelTime.UTC(), ResolveEventTime(ee).UTC(), + "kernel time must win over observation time") +} + +func TestResolveEventTime_FallsBackWhenEventTimestampIsZero(t *testing.T) { + observed := recentTime() + ee := &events.EnrichedEvent{ + Event: execEventAt(0), + Timestamp: observed, + } + assert.Equal(t, observed.UTC(), ResolveEventTime(ee).UTC(), + "a zero event timestamp must fall back, not yield the epoch") +} + +func TestResolveEventTime_NilSafe(t *testing.T) { + assert.True(t, ResolveEventTime(nil).IsZero()) + assert.True(t, ResolveEventTime(&events.EnrichedEvent{}).IsZero(), + "a nil inner event must not panic on the hot path") +} + +func TestEvalContext_TimestampIsUsableFromCEL(t *testing.T) { + c := newTestCEL(t) + + kernelTime := recentTime() + ee := &events.EnrichedEvent{ + Event: execEventAt(kernelTime.UnixNano()), + Timestamp: kernelTime, + } + + out, err := c.EvaluateExpression(ee, `string(timestamp)`) + require.NoError(t, err) + + // CEL renders a timestamp in the location Go gives it, and time.Unix builds + // a local-zone Time -- so the rendered offset depends on the node's TZ. + // Assert the instant, not the spelling, or this test fails everywhere except + // a UTC machine. + got, err := time.Parse(time.RFC3339Nano, out) + require.NoError(t, err, "timestamp must render as RFC3339: %q", out) + assert.True(t, kernelTime.Equal(got), "want %s, got %s", kernelTime, got) +} + +// The whole point of the variable: comparing a remembered time against the +// current event's time. If timestamp were not a CEL timestamp this would not +// compile, and every _ts ordering guard would silently be dead. +func TestEvalContext_TimestampSupportsOrderingComparisons(t *testing.T) { + c := newTestCEL(t) + + kernelTime := recentTime() + ee := &events.EnrichedEvent{ + Event: execEventAt(kernelTime.UnixNano()), + Timestamp: kernelTime, + } + + out, err := c.EvaluateExpression(ee, + `string(timestamp - duration("1m") < timestamp)`) + require.NoError(t, err) + assert.Equal(t, "true", out) +} + +// A producer that ever hands over a raw boot-time value instead of a wall clock +// would yield 1970-plus-uptime. ExpiresAt is derived from the event clock while +// expiry is checked against time.Now(), so every entry from that stream would be +// born expired and the rule would silently never fire -- the failure mode this +// feature works hardest elsewhere to design out. No producer does this today; the +// bound is here so that staying true is not a matter of luck. +func TestResolveEventTime_RejectsAnImplausibleEpoch(t *testing.T) { + enrichmentTime := time.Now().Add(-2 * time.Second) + + bootTimeNs := (37 * time.Hour).Nanoseconds() // ~1.5 days of uptime, in 1970 + ee := &events.EnrichedEvent{ + Event: &utils.StructEvent{EventType: utils.ExecveEventType, Timestamp: bootTimeNs}, + Timestamp: enrichmentTime, + } + assert.Equal(t, enrichmentTime, ResolveEventTime(ee), + "a boot-time value must fall back to the enrichment time, not resolve to 1970") + + // A far-future reading is equally implausible and equally damaging: the entry + // would never expire. + future := time.Now().Add(72 * time.Hour) + ee.Event = &utils.StructEvent{EventType: utils.ExecveEventType, Timestamp: future.UnixNano()} + assert.Equal(t, enrichmentTime, ResolveEventTime(ee)) +} + +// The bound must not reject real events. It is there to catch a wrong epoch, not +// clock skew, so anything within a day either way is kept. +func TestResolveEventTime_KeepsLateAndSlightlyAheadEvents(t *testing.T) { + for _, offset := range []time.Duration{ + -23 * time.Hour, -time.Minute, -time.Millisecond, 0, time.Second, 23 * time.Hour, + } { + want := time.Now().Add(offset) + ee := &events.EnrichedEvent{ + Event: &utils.StructEvent{EventType: utils.ExecveEventType, Timestamp: want.UnixNano()}, + Timestamp: time.Now(), + } + assert.WithinDuration(t, want, ResolveEventTime(ee), time.Microsecond, + "an event %s from now is legitimate and must keep its own timestamp", offset) + } +} diff --git a/pkg/rulemanager/cel/libraries/state/accessor.go b/pkg/rulemanager/cel/libraries/state/accessor.go new file mode 100644 index 0000000000..1979ebac5c --- /dev/null +++ b/pkg/rulemanager/cel/libraries/state/accessor.go @@ -0,0 +1,184 @@ +package state + +import ( + "fmt" + "reflect" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types" + "github.com/google/cel-go/common/types/ref" + "github.com/kubescape/node-agent/pkg/rulestate" +) + +// AccessorType is the CEL type of the "state" variable. +var AccessorType = cel.ObjectType("rulestate.Accessor") + +// Accessor is the receiver behind the CEL "state" variable: state.has(...) is a +// member call on it, not a global function. +// +// It exists because cel-go hands a function binding only its arguments -- never +// the activation -- so a global function named "state.has" could not discover +// which rule or which container it was evaluating for. Making "state" a variable +// puts that context in the receiver, where the binding can reach it. +// +// The consequence is the security property the design depends on: ruleID, the +// scope IDs and the ancestor list live in the receiver, and there is no CEL +// syntax for supplying or overriding them. A rule author therefore cannot name +// another rule's state or another container's scope -- those reads are not merely +// forbidden, they are inexpressible. +// +// One Accessor is valid for one (rule, event) pair. The rule loop rebuilds it per +// rule, because ruleID and the declared-name scopes change per rule. +type Accessor struct { + store *rulestate.Store + ruleID string + + // scopeOf maps a state name to the scope the rule declared it in. Reads take + // no scope argument: because state is rule-private, a name uniquely + // determines its scope from the rule's own stateWrites. + scopeOf map[string]armotypes.StateScope + + // scopeIDs holds this event's resolved ID for each scope. + scopeIDs map[armotypes.StateScope]string + + // ancestors is called at most once per evaluation, lazily: most rules never + // call has_ancestor, and walking the process tree is not free. + ancestors func() []uint32 + ancestorsMemo []uint32 + ancestorsDone bool + + tracker *ReadTracker + adapter types.Adapter +} + +// NewAccessor builds the receiver for one (rule, event) pair. ancestors is +// invoked lazily and at most once. +func NewAccessor( + store *rulestate.Store, + ruleID string, + scopeOf map[string]armotypes.StateScope, + scopeIDs map[armotypes.StateScope]string, + ancestors func() []uint32, + tracker *ReadTracker, + adapter types.Adapter, +) *Accessor { + if adapter == nil { + adapter = types.DefaultTypeAdapter + } + return &Accessor{ + store: store, + ruleID: ruleID, + scopeOf: scopeOf, + scopeIDs: scopeIDs, + ancestors: ancestors, + tracker: tracker, + adapter: adapter, + } +} + +func (a *Accessor) ConvertToNative(typeDesc reflect.Type) (any, error) { + if typeDesc == reflect.TypeOf(a) { + return a, nil + } + return nil, fmt.Errorf("state accessor cannot be converted to %v", typeDesc) +} + +func (a *Accessor) ConvertToType(t ref.Type) ref.Val { + if t == types.TypeType { + return AccessorType + } + return types.NewErr("state accessor cannot be converted to %v", t) +} + +func (a *Accessor) Equal(other ref.Val) ref.Val { + o, ok := other.(*Accessor) + return types.Bool(ok && o == a) +} + +func (a *Accessor) Type() ref.Type { return AccessorType } +func (a *Accessor) Value() any { return a } + +// lookup resolves one entry. A name the rule never declared is a miss rather +// than an error: load-time validation is what rejects it, and failing the whole +// predicate here would take out an otherwise working rule. +func (a *Accessor) lookup(name, key string) (*rulestate.Entry, bool) { + if a == nil || a.store == nil { + return nil, false + } + scope, ok := a.scopeOf[name] + if !ok { + return nil, false + } + scopeID, ok := a.scopeIDs[scope] + if !ok { + return nil, false + } + e, ok := a.store.Get(a.ruleID, scopeID, name, key) + if !ok { + return nil, false + } + if a.tracker != nil { + a.tracker.record(e) + } + return e, true +} + +// lookupAncestor probes each ancestor PID in order and returns the first hit, so +// get_ancestor yields the NEAREST matching ancestor. +func (a *Accessor) lookupAncestor(name string) (*rulestate.Entry, bool) { + if a == nil { + return nil, false + } + for _, pid := range a.ancestorPIDs() { + if e, ok := a.lookup(name, fmt.Sprint(pid)); ok { + return e, true + } + } + return nil, false +} + +func (a *Accessor) ancestorPIDs() []uint32 { + if a.ancestorsDone { + return a.ancestorsMemo + } + a.ancestorsDone = true + if a.ancestors != nil { + a.ancestorsMemo = a.ancestors() + } + return a.ancestorsMemo +} + +// entryToMap renders an entry for CEL. Engine-stamped provenance uses reserved +// "_" keys; author values from the rule's `value:` sit alongside at top level. +// Author keys beginning with "_" are rejected at rule load, so they cannot +// shadow provenance here. +func entryToMap(e *rulestate.Entry) map[string]any { + m := map[string]any{ + "_ts": e.Timestamp, + "_eventType": string(e.EventType), + "_container": e.ScopeID, + } + if e.Process != nil { + m["_pid"] = e.Process.PID + m["_ppid"] = e.Process.PPID + m["_comm"] = e.Process.Comm + m["_pcomm"] = e.Process.Pcomm + m["_exe"] = e.Process.Path + m["_cwd"] = e.Process.Cwd + } + for k, v := range e.Value { + m[k] = v + } + return m +} + +// emptyMap is what a miss yields. Never an error: a message expression must +// degrade rather than abort evaluation of the whole rule. +func (a *Accessor) emptyMap() ref.Val { + return types.NewStringInterfaceMap(a.adapter, map[string]any{}) +} + +func (a *Accessor) entryVal(e *rulestate.Entry) ref.Val { + return types.NewStringInterfaceMap(a.adapter, entryToMap(e)) +} diff --git a/pkg/rulemanager/cel/libraries/state/readtracker.go b/pkg/rulemanager/cel/libraries/state/readtracker.go new file mode 100644 index 0000000000..ba7e4e7aac --- /dev/null +++ b/pkg/rulemanager/cel/libraries/state/readtracker.go @@ -0,0 +1,50 @@ +package state + +import ( + "sync" + + "github.com/kubescape/node-agent/pkg/rulestate" +) + +// AccessorContextKey is the eval-context key under which the caller injects the +// per-(rule, event) Accessor. It is the CEL variable name authors write as +// "state", and it is the ONLY state-related entry in the eval context. +const AccessorContextKey = "state" + +// ReadTracker records which entries a predicate actually read, so the alert can +// carry them as correlation evidence. Only hits are recorded: a miss is not +// evidence of anything. +// +// MUST be reset between rules. node-agent reuses one eval context across all +// rules for an event, so without a reset rule N inherits rule N-1's hits and +// alerts cite entries they never read. +type ReadTracker struct { + mu sync.Mutex + hits []*rulestate.Entry +} + +func (t *ReadTracker) record(e *rulestate.Entry) { + if e == nil { + return + } + t.mu.Lock() + defer t.mu.Unlock() + for _, h := range t.hits { + if h == e { + return // same entry read twice in one predicate + } + } + t.hits = append(t.hits, e) +} + +func (t *ReadTracker) Hits() []*rulestate.Entry { + t.mu.Lock() + defer t.mu.Unlock() + return append([]*rulestate.Entry(nil), t.hits...) +} + +func (t *ReadTracker) Reset() { + t.mu.Lock() + t.hits = t.hits[:0] + t.mu.Unlock() +} diff --git a/pkg/rulemanager/cel/libraries/state/statelib.go b/pkg/rulemanager/cel/libraries/state/statelib.go new file mode 100644 index 0000000000..79e50f21b0 --- /dev/null +++ b/pkg/rulemanager/cel/libraries/state/statelib.go @@ -0,0 +1,217 @@ +package state + +import ( + "github.com/google/cel-go/cel" + "github.com/google/cel-go/checker" + "github.com/google/cel-go/common/types" + "github.com/google/cel-go/common/types/ref" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries" +) + +func New(cfg config.Config) libraries.Library { + return &stateLibrary{cfg: cfg} +} + +// State registers the "state" variable and its read functions. +// +// Note what is NOT a parameter here: the store, the rule ID and the process tree. +// Reads are served by the per-(rule, event) Accessor injected into the eval +// context, so the library itself is immutable and safe to share across the +// worker pool. A library holding mutable per-evaluation state would race, since +// node-agent processes events concurrently against one shared cel.Env. +func State(cfg config.Config) cel.EnvOption { + return cel.Lib(New(cfg)) +} + +type stateLibrary struct { + cfg config.Config +} + +func (l *stateLibrary) LibraryName() string { + return "state" +} + +func (l *stateLibrary) Types() []*cel.Type { + return []*cel.Type{AccessorType} +} + +// accessorOf recovers the receiver. A missing or wrong-typed receiver means the +// caller did not seed the eval context; report it as a miss-shaped value rather +// than an error so one misconfigured rule cannot abort evaluation. +func accessorOf(v ref.Val) (*Accessor, bool) { + a, ok := v.Value().(*Accessor) + return a, ok && a != nil +} + +func stringOf(v ref.Val) (string, bool) { + s, ok := v.Value().(string) + return s, ok +} + +func (l *stateLibrary) Declarations() map[string][]cel.FunctionOpt { + return map[string][]cel.FunctionOpt{ + "has": { + cel.MemberOverload("state_has_name", + []*cel.Type{AccessorType, cel.StringType}, cel.BoolType, + cel.BinaryBinding(func(target, name ref.Val) ref.Val { + return hasImpl(target, name, types.String("")) + }), + ), + cel.MemberOverload("state_has_name_key", + []*cel.Type{AccessorType, cel.StringType, cel.StringType}, cel.BoolType, + cel.FunctionBinding(func(values ...ref.Val) ref.Val { + if len(values) != 3 { + return types.Bool(false) + } + return hasImpl(values[0], values[1], values[2]) + }), + ), + }, + "get": { + cel.MemberOverload("state_get_name", + []*cel.Type{AccessorType, cel.StringType}, cel.MapType(cel.StringType, cel.DynType), + cel.BinaryBinding(func(target, name ref.Val) ref.Val { + return getImpl(target, name, types.String("")) + }), + ), + cel.MemberOverload("state_get_name_key", + []*cel.Type{AccessorType, cel.StringType, cel.StringType}, cel.MapType(cel.StringType, cel.DynType), + cel.FunctionBinding(func(values ...ref.Val) ref.Val { + if len(values) != 3 { + return types.NewStringInterfaceMap(types.DefaultTypeAdapter, map[string]any{}) + } + return getImpl(values[0], values[1], values[2]) + }), + ), + }, + "has_ancestor": { + cel.MemberOverload("state_has_ancestor", + []*cel.Type{AccessorType, cel.StringType}, cel.BoolType, + cel.BinaryBinding(func(target, name ref.Val) ref.Val { + a, ok := accessorOf(target) + if !ok { + return types.Bool(false) + } + n, ok := stringOf(name) + if !ok { + return types.Bool(false) + } + _, hit := a.lookupAncestor(n) + return types.Bool(hit) + }), + ), + }, + "get_ancestor": { + cel.MemberOverload("state_get_ancestor", + []*cel.Type{AccessorType, cel.StringType}, cel.MapType(cel.StringType, cel.DynType), + cel.BinaryBinding(func(target, name ref.Val) ref.Val { + a, ok := accessorOf(target) + if !ok { + return types.NewStringInterfaceMap(types.DefaultTypeAdapter, map[string]any{}) + } + n, ok := stringOf(name) + if !ok { + return a.emptyMap() + } + e, hit := a.lookupAncestor(n) + if !hit { + return a.emptyMap() + } + return a.entryVal(e) + }), + ), + }, + } +} + +func hasImpl(target, name, key ref.Val) ref.Val { + a, ok := accessorOf(target) + if !ok { + return types.Bool(false) + } + n, ok := stringOf(name) + if !ok { + return types.Bool(false) + } + k, ok := stringOf(key) + if !ok { + return types.Bool(false) + } + _, hit := a.lookup(n, k) + return types.Bool(hit) +} + +func getImpl(target, name, key ref.Val) ref.Val { + a, ok := accessorOf(target) + if !ok { + return types.NewStringInterfaceMap(types.DefaultTypeAdapter, map[string]any{}) + } + n, ok := stringOf(name) + if !ok { + return a.emptyMap() + } + k, ok := stringOf(key) + if !ok { + return a.emptyMap() + } + e, hit := a.lookup(n, k) + if !hit { + return a.emptyMap() + } + return a.entryVal(e) +} + +func (l *stateLibrary) CompileOptions() []cel.EnvOption { + options := []cel.EnvOption{ + cel.Variable(AccessorContextKey, AccessorType), + } + for name, overloads := range l.Declarations() { + options = append(options, cel.Function(name, overloads...)) + } + return options +} + +func (l *stateLibrary) ProgramOptions() []cel.ProgramOption { + return []cel.ProgramOption{} +} + +func (l *stateLibrary) CostEstimator() checker.CostEstimator { + return &stateCostEstimator{cfg: l.cfg} +} + +// stateCostEstimator implements checker.CostEstimator for the 'state' library. +type stateCostEstimator struct { + cfg config.Config +} + +// EstimateCallCost keys off overloadID, not the function name: this library's +// member functions are called "has" and "get", which are far too generic to +// match on -- another library declaring a "get" would silently receive these +// costs. Unknown overloads return nil so a composite estimator can fall through +// to the library that actually owns the function. +func (e *stateCostEstimator) EstimateCallCost(function, overloadID string, target *checker.AstNode, args []checker.AstNode) *checker.CallEstimate { + var cost int64 + switch overloadID { + case "state_has_name", "state_has_name_key", "state_get_name", "state_get_name_key": + // One hash lookup under an RLock. + cost = 10 + case "state_has_ancestor", "state_get_ancestor": + // One probe per ancestor, so the depth bound is the multiplier. + depth := e.cfg.CelStateStore.AncestorMaxDepth + if depth <= 0 { + depth = 8 + } + cost = int64(10 * depth) + default: + return nil + } + return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: uint64(cost), Max: uint64(cost)}} +} + +func (e *stateCostEstimator) EstimateSize(element checker.AstNode) *checker.SizeEstimate { + return nil // Not providing size estimates for now. +} + +var _ checker.CostEstimator = (*stateCostEstimator)(nil) +var _ libraries.Library = (*stateLibrary)(nil) diff --git a/pkg/rulemanager/cel/libraries/state/statelib_test.go b/pkg/rulemanager/cel/libraries/state/statelib_test.go new file mode 100644 index 0000000000..9969058b96 --- /dev/null +++ b/pkg/rulemanager/cel/libraries/state/statelib_test.go @@ -0,0 +1,317 @@ +package state + +import ( + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const testRuleID = "R1089" + +// harness evaluates CEL expressions against a real store and a real cel.Env, so +// the tests exercise the actual dispatch path rather than the impl functions. +type harness struct { + t *testing.T + env *cel.Env + store *rulestate.Store + tracker *ReadTracker + scopeID string + ancestors []uint32 + // scopeOf stands in for the rule's own stateWrites declarations, which is + // what tells a read which scope a name lives in. + scopeOf map[string]armotypes.StateScope + now time.Time +} + +func newHarness(t *testing.T) *harness { + t.Helper() + + cfg := config.Config{} + cfg.CelStateStore = rulestate.Config{ + Enabled: true, + MaxSize: 1000, + MaxEntriesPerContainer: 100, + MaxEntriesForHost: 100, + MaxTTL: 30 * time.Minute, + AncestorMaxDepth: 8, + } + + env, err := cel.NewEnv( + cel.Variable("timestamp", cel.TimestampType), + State(cfg), + ) + require.NoError(t, err) + + return &harness{ + t: t, + env: env, + store: rulestate.NewStore(cfg.CelStateStore, rulestate.NoopMetrics{}), + tracker: &ReadTracker{}, + scopeID: "c:abc", + scopeOf: map[string]armotypes.StateScope{}, + now: time.Now(), + } +} + +// write stores an entry and declares its name as container-scoped, mirroring what +// a rule's stateWrites clause would have done at load time. +// +// ts sets the entry's logical event time, which tests compare against. ExpiresAt +// is deliberately derived from wall-clock now instead: expiry is enforced against +// time.Now(), so deriving it from a ts in the past (any fixed date literal, since +// these tests use them) would store an already-expired entry and every read would +// miss for a reason that has nothing to do with what is being tested. +func (h *harness) write(ruleID, scopeID, name, key string, ts time.Time) *rulestate.Entry { + h.t.Helper() + e := &rulestate.Entry{ + RuleID: ruleID, Name: name, Key: key, + Scope: armotypes.StateScopeContainer, ScopeID: scopeID, + EventType: armotypes.EventTypeExec, + Timestamp: ts, ExpiresAt: time.Now().Add(10 * time.Minute), + Process: &armotypes.Process{ + PID: 4471, PPID: 900, Comm: "xmrig", Pcomm: "sh", + Path: "/mnt/data/xmrig", Cwd: "/mnt/data", + }, + } + require.NoError(h.t, h.store.Set(e)) + h.scopeOf[name] = armotypes.StateScopeContainer + return e +} + +func (h *harness) accessor() *Accessor { + return NewAccessor( + h.store, testRuleID, h.scopeOf, + map[armotypes.StateScope]string{armotypes.StateScopeContainer: h.scopeID}, + func() []uint32 { return h.ancestors }, + h.tracker, + types.DefaultTypeAdapter, + ) +} + +func (h *harness) eval(expr string) any { + h.t.Helper() + ast, iss := h.env.Compile(expr) + require.NoError(h.t, iss.Err(), "expression must compile: %s", expr) + + prg, err := h.env.Program(ast) + require.NoError(h.t, err) + + out, _, err := prg.Eval(map[string]any{ + AccessorContextKey: h.accessor(), + "timestamp": h.now.Add(time.Minute), + }) + require.NoError(h.t, err, "expression must not error: %s", expr) + return out.Value() +} + +func (h *harness) evalBool(expr string) bool { + h.t.Helper() + v, ok := h.eval(expr).(bool) + require.True(h.t, ok, "expression must yield a bool: %s", expr) + return v +} + +func (h *harness) evalString(expr string) string { + h.t.Helper() + v, ok := h.eval(expr).(string) + require.True(h.t, ok, "expression must yield a string: %s", expr) + return v +} + +func TestStateHas_HitAndMiss(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "mount_exec", "4471", time.Now()) + + assert.True(t, h.evalBool(`state.has("mount_exec", "4471")`)) + assert.False(t, h.evalBool(`state.has("mount_exec", "9999")`)) + assert.False(t, h.evalBool(`state.has("nope", "4471")`)) +} + +func TestStateHas_OneArgFormForScopeWideMarkers(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "pkg_mgr_ran", "", time.Now()) + assert.True(t, h.evalBool(`state.has("pkg_mgr_ran")`)) +} + +func TestStateGet_ExposesProvenanceAndAuthorValues(t *testing.T) { + h := newHarness(t) + ts := time.Date(2026, 7, 28, 12, 0, 3, 100000000, time.UTC) + e := h.write(testRuleID, "c:abc", "mount_exec", "4471", ts) + e.Value = map[string]any{"argv": "-o pool:4444"} + + assert.Equal(t, "xmrig", h.evalString(`state.get("mount_exec", "4471")._comm`)) + assert.Equal(t, "sh", h.evalString(`state.get("mount_exec", "4471")._pcomm`)) + assert.Equal(t, "/mnt/data/xmrig", h.evalString(`state.get("mount_exec", "4471")._exe`)) + assert.Equal(t, "/mnt/data", h.evalString(`state.get("mount_exec", "4471")._cwd`)) + assert.Equal(t, "exec", h.evalString(`state.get("mount_exec", "4471")._eventType`)) + assert.Equal(t, "-o pool:4444", h.evalString(`state.get("mount_exec", "4471").argv`)) +} + +// _ts must be a CEL timestamp, not a string: the whole ordering-guard idiom is a +// comparison against the current event's time. +func TestStateGet_TimestampIsComparable(t *testing.T) { + h := newHarness(t) + h.now = time.Date(2026, 7, 28, 12, 0, 3, 0, time.UTC) + h.write(testRuleID, "c:abc", "mount_exec", "4471", h.now) + + assert.True(t, h.evalBool(`state.get("mount_exec", "4471")._ts < timestamp`), + "the remembered event happened before the current one") +} + +func TestStateGet_MissReturnsEmptyMapNotError(t *testing.T) { + h := newHarness(t) + // A message expression must degrade, not fail evaluation. + assert.Equal(t, int64(0), h.eval(`size(state.get("absent", "1"))`)) +} + +// A miss must not make a provenance access blow up the whole predicate -- this is +// the difference between a rule that under-fires and a rule that errors out. +func TestStateGet_MissTolerated_WithHasGuard(t *testing.T) { + h := newHarness(t) + assert.False(t, h.evalBool( + `state.has("absent", "1") && state.get("absent", "1")._pid == 1u`)) +} + +func TestStateHasAncestor_MatchesAnAncestorPID(t *testing.T) { + h := newHarness(t) + // nginx 900 -> sh 4471 -> curl 4530; marker is on 4471. + h.ancestors = []uint32{4471, 900, 1} + h.write(testRuleID, "c:abc", "webshell_parent", "4471", time.Now()) + + assert.True(t, h.evalBool(`state.has_ancestor("webshell_parent")`)) + assert.Equal(t, "xmrig", h.evalString(`state.get_ancestor("webshell_parent")._comm`)) +} + +func TestStateHasAncestor_NoMatchWhenNoAncestorCarriesTheMarker(t *testing.T) { + h := newHarness(t) + h.ancestors = []uint32{5000, 5001} + h.write(testRuleID, "c:abc", "webshell_parent", "4471", time.Now()) + assert.False(t, h.evalBool(`state.has_ancestor("webshell_parent")`)) +} + +func TestStateHasAncestor_WorksWithHostScope(t *testing.T) { + h := newHarness(t) + h.scopeID = rulestate.HostScopeID() + h.ancestors = []uint32{2200, 1} + h.write(testRuleID, rulestate.HostScopeID(), "sudo_ran", "2200", time.Now()) + assert.True(t, h.evalBool(`state.has_ancestor("sudo_ran")`)) +} + +// get_ancestor must return the NEAREST match, since the ancestor list is ordered +// nearest-first and a chain can carry the marker at several depths. +func TestStateGetAncestor_ReturnsNearestMatch(t *testing.T) { + h := newHarness(t) + h.ancestors = []uint32{4471, 900} + + near := h.write(testRuleID, "c:abc", "marker", "4471", time.Now()) + near.Process = &armotypes.Process{PID: 4471, Comm: "near"} + far := h.write(testRuleID, "c:abc", "marker", "900", time.Now()) + far.Process = &armotypes.Process{PID: 900, Comm: "far"} + + assert.Equal(t, "near", h.evalString(`state.get_ancestor("marker")._comm`)) +} + +func TestStateHasAncestor_EmptyAncestorListIsAMiss(t *testing.T) { + h := newHarness(t) + h.ancestors = nil + h.write(testRuleID, "c:abc", "marker", "4471", time.Now()) + assert.False(t, h.evalBool(`state.has_ancestor("marker")`)) +} + +func TestReadTracker_RecordsOnlyEntriesActuallyRead(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "mount_exec", "4471", time.Now()) + h.write(testRuleID, "c:abc", "unrelated", "4471", time.Now()) + + require.True(t, h.evalBool(`state.has("mount_exec", "4471")`)) + hits := h.tracker.Hits() + require.Len(t, hits, 1, "only the entry the predicate touched is evidence") + assert.Equal(t, "mount_exec", hits[0].Name) +} + +func TestReadTracker_MissesAreNotRecorded(t *testing.T) { + h := newHarness(t) + require.False(t, h.evalBool(`state.has("absent", "1")`)) + assert.Empty(t, h.tracker.Hits()) +} + +func TestReadTracker_ResetClearsBetweenRules(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "mount_exec", "4471", time.Now()) + require.True(t, h.evalBool(`state.has("mount_exec", "4471")`)) + require.Len(t, h.tracker.Hits(), 1) + + h.tracker.Reset() + assert.Empty(t, h.tracker.Hits(), + "without a per-rule reset, rule N inherits rule N-1's evidence") +} + +// Reading the same entry twice in one predicate must cite it once. +func TestReadTracker_DeduplicatesRepeatedReads(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "mount_exec", "4471", time.Now()) + require.True(t, h.evalBool( + `state.has("mount_exec", "4471") && state.get("mount_exec", "4471")._pid == 4471u`)) + assert.Len(t, h.tracker.Hits(), 1) +} + +func TestState_RuleIDIsNotExpressible(t *testing.T) { + h := newHarness(t) + // Stored under a DIFFERENT rule; the harness evaluates as R1089. There is no + // CEL syntax for naming another rule's state, which is what makes state + // rule-private. + h.write("R9999", "c:abc", "mount_exec", "4471", time.Now()) + assert.False(t, h.evalBool(`state.has("mount_exec", "4471")`)) +} + +// The neighbouring-container case, same argument as above: the scope ID comes +// from the receiver, so no expression can reach another container's bucket. +func TestState_ScopeIDIsNotExpressible(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:other", "mount_exec", "4471", time.Now()) + assert.False(t, h.evalBool(`state.has("mount_exec", "4471")`)) +} + +// A name the rule never declared has no scope to resolve against, so it reads as +// a miss instead of erroring. +func TestState_UndeclaredNameIsAMiss(t *testing.T) { + h := newHarness(t) + h.write(testRuleID, "c:abc", "declared", "1", time.Now()) + delete(h.scopeOf, "declared") + assert.False(t, h.evalBool(`state.has("declared", "1")`)) +} + +func TestState_ExpiredEntryIsAMiss(t *testing.T) { + h := newHarness(t) + e := h.write(testRuleID, "c:abc", "mount_exec", "4471", time.Now()) + e.ExpiresAt = time.Now().Add(-time.Second) + assert.False(t, h.evalBool(`state.has("mount_exec", "4471")`)) +} + +// The library declares member functions called "has" -- the same identifier as +// CEL's built-in has() macro. If declaring it ever shadowed the macro, +// has(event.field) would break across every existing rule. +func TestState_DoesNotShadowTheHasMacro(t *testing.T) { + env, err := cel.NewEnv( + cel.Variable("m", cel.MapType(cel.StringType, cel.StringType)), + State(config.Config{}), + ) + require.NoError(t, err) + + ast, iss := env.Compile(`has(m.present)`) + require.NoError(t, iss.Err(), "the has() macro must still parse alongside state.has") + + prg, err := env.Program(ast) + require.NoError(t, err) + + out, _, err := prg.Eval(map[string]any{"m": map[string]string{"present": "x"}}) + require.NoError(t, err) + assert.Equal(t, true, out.Value()) +} diff --git a/pkg/rulemanager/cel/statewiring_test.go b/pkg/rulemanager/cel/statewiring_test.go new file mode 100644 index 0000000000..89ea4ee350 --- /dev/null +++ b/pkg/rulemanager/cel/statewiring_test.go @@ -0,0 +1,195 @@ +package cel + +import ( + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/objectcache" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/state" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The state library's own tests build a bare cel.NewEnv. Production does not: +// NewCEL installs an xcel TypeAdapter/TypeProvider, every other library, and a +// static optimizer. This test exercises the state functions through THAT env, so +// a wiring problem that only appears in the real evaluator is caught here rather +// than on a cluster. +func newStateWiringCEL(t *testing.T) (*CEL, *rulestate.Store, config.Config) { + t.Helper() + + cfg := config.Config{} + cfg.CelStateStore = rulestate.Config{ + Enabled: true, + MaxSize: 1000, + MaxEntriesPerContainer: 100, + MaxEntriesForHost: 100, + MaxTTL: 30 * time.Minute, + AncestorMaxDepth: 8, + } + + c, err := NewCEL(objectcache.NewObjectCacheMock(), cfg) + require.NoError(t, err) + + return c, rulestate.NewStore(cfg.CelStateStore, rulestate.NoopMetrics{}), cfg +} + +func execProbeEvent(pid uint32) *events.EnrichedEvent { + return &events.EnrichedEvent{ + Event: &utils.StructEvent{ + EventType: utils.ExecveEventType, + ContainerID: "abc", + Comm: "sh", + Args: []string{"-c", "# CELSTATE_MARKER\nsleep 8\nexec nc -w 3 h p"}, + Pid: pid, + }, + ContainerID: "abc", + PID: pid, + } +} + +func networkProbeEvent(pid uint32) *events.EnrichedEvent { + return &events.EnrichedEvent{ + Event: &utils.StructEvent{ + EventType: utils.NetworkEventType, + ContainerID: "abc", + Comm: "nc", + PktType: "OUTGOING", + Pid: pid, + }, + ContainerID: "abc", + PID: pid, + } +} + +func seedState(c *CEL, ctx map[string]any, store *rulestate.Store, ee *events.EnrichedEvent, tracker *state.ReadTracker) { + ctx[state.AccessorContextKey] = state.NewAccessor( + store, "R9911", + map[string]armotypes.StateScope{"probe_exec": armotypes.StateScopeContainer}, + map[armotypes.StateScope]string{ + armotypes.StateScopeContainer: rulestate.ContainerScopeID(ee.ContainerID), + }, + func() []uint32 { return nil }, + tracker, nil, + ) +} + +// The exact predicates the R9911 component-test rule uses. +const ( + probeGuard = `event.comm == 'sh' && event.args.exists(a, a.contains('CELSTATE_MARKER'))` + probeRead = `event.pktType == 'OUTGOING' && state.has('probe_exec', string(event.pid))` +) + +func TestStateWiring_GuardCompilesAndMatchesInTheRealEnv(t *testing.T) { + c, _, _ := newStateWiringCEL(t) + + ctx := c.CreateEvalContext(execProbeEvent(4471)) + ok, err := c.EvaluateBoolExpressionWithContext(ctx, probeGuard) + require.NoError(t, err) + assert.True(t, ok, "the stateWrites guard must match the marker exec") +} + +func TestStateWiring_KeyExpressionEvaluates(t *testing.T) { + c, _, _ := newStateWiringCEL(t) + + ctx := c.CreateEvalContext(execProbeEvent(4471)) + key, err := c.EvaluateStringExpressionWithContext(ctx, "string(event.pid)") + require.NoError(t, err) + assert.Equal(t, "4471", key, "the join key must render as the bare pid") +} + +// The end-to-end shape: write on exec, read on network, through the production +// evaluator. This is the unit-level equivalent of the component test. +func TestStateWiring_WriteOnExecThenReadOnNetwork(t *testing.T) { + c, store, _ := newStateWiringCEL(t) + tracker := &state.ReadTracker{} + + // --- exec leg: evaluate the guard, then store the entry the executor would. + execEvent := execProbeEvent(4471) + execCtx := c.CreateEvalContext(execEvent) + seedState(c, execCtx, store, execEvent, tracker) + + guardOK, err := c.EvaluateBoolExpressionWithContext(execCtx, probeGuard) + require.NoError(t, err) + require.True(t, guardOK, "guard must match, or the write never happens") + + key, err := c.EvaluateStringExpressionWithContext(execCtx, "string(event.pid)") + require.NoError(t, err) + + now := time.Now() + require.NoError(t, store.Set(&rulestate.Entry{ + RuleID: "R9911", Name: "probe_exec", Key: key, + Scope: armotypes.StateScopeContainer, + ScopeID: rulestate.ContainerScopeID(execEvent.ContainerID), + EventType: armotypes.EventTypeExec, + Timestamp: now, ExpiresAt: now.Add(5 * time.Minute), + Process: &armotypes.Process{PID: 4471, Comm: "sh"}, + Value: map[string]any{"probeComm": "sh"}, + })) + + // --- network leg: the predicate must now see it. + netEvent := networkProbeEvent(4471) + netCtx := c.CreateEvalContext(netEvent) + seedState(c, netCtx, store, netEvent, tracker) + + fired, err := c.EvaluateBoolExpressionWithContext(netCtx, probeRead) + require.NoError(t, err) + assert.True(t, fired, + "state written on the exec leg must be readable on the network leg with the same pid") +} + +// _pid lands in the returned map as an unsigned value, while event.pid is what +// the event exposes. A rule author's first instinct is to compare them directly, +// so what cel-go actually does with that comparison is worth measuring rather +// than assuming -- if it errors, the rule does not merely miss, it aborts. +func TestStateWiring_ComparingStoredPidAgainstEventPid(t *testing.T) { + c, store, _ := newStateWiringCEL(t) + + now := time.Now() + require.NoError(t, store.Set(&rulestate.Entry{ + RuleID: "R9911", Name: "probe_exec", Key: "4471", + Scope: armotypes.StateScopeContainer, + ScopeID: rulestate.ContainerScopeID("abc"), + EventType: armotypes.EventTypeExec, + Timestamp: now, ExpiresAt: now.Add(5 * time.Minute), + Process: &armotypes.Process{PID: 4471, Comm: "sh"}, + })) + + netEvent := networkProbeEvent(4471) + ctx := c.CreateEvalContext(netEvent) + seedState(c, ctx, store, netEvent, nil) + + t.Run("direct numeric comparison", func(t *testing.T) { + got, err := c.EvaluateBoolExpressionWithContext(ctx, + `state.get('probe_exec', string(event.pid))._pid == event.pid`) + if err != nil { + t.Fatalf("comparing _pid to event.pid ABORTS evaluation: %v\n"+ + "the docs must steer authors away from this form", err) + } + assert.True(t, got, + "the same pid stored and read back must compare equal") + }) + + // The form the docs already recommend for the join, which cannot depend on + // numeric-type agreement at all. + t.Run("string comparison", func(t *testing.T) { + got, err := c.EvaluateBoolExpressionWithContext(ctx, + `string(state.get('probe_exec', string(event.pid))._pid) == string(event.pid)`) + require.NoError(t, err) + assert.True(t, got) + }) + + // A miss returns an empty map, so a read of a key that is not there must + // degrade rather than abort. + t.Run("miss degrades", func(t *testing.T) { + got, err := c.EvaluateBoolExpressionWithContext(ctx, + `!has(state.get('probe_exec', 'nope')._pid)`) + require.NoError(t, err, "a miss must not abort evaluation") + assert.True(t, got) + }) +} diff --git a/pkg/rulemanager/containercallbacks.go b/pkg/rulemanager/containercallbacks.go index 7a6a105d78..13b491ae06 100644 --- a/pkg/rulemanager/containercallbacks.go +++ b/pkg/rulemanager/containercallbacks.go @@ -12,6 +12,7 @@ import ( "github.com/kubescape/go-logger/helpers" "github.com/kubescape/node-agent/pkg/contextdetection/detectors" "github.com/kubescape/node-agent/pkg/objectcache" + "github.com/kubescape/node-agent/pkg/rulestate" "github.com/kubescape/node-agent/pkg/utils" ) @@ -109,30 +110,35 @@ func (rm *RuleManager) ContainerCallback(notif containercollection.PubSubEvent) close(done) rm.trackedContainerDone.Delete(k8sContainerID) } + + // Reclaim immediately rather than waiting for TTL: a churning node would + // otherwise hold markers for containers that no longer exist. + // + // This uses Runtime.ContainerID verbatim because that is exactly what the + // write path stored under -- EnrichedEvent.ContainerID is assigned from + // container.Runtime.ContainerID (containercallback.go), untrimmed. Do NOT + // pass it through utils.TrimRuntimePrefix: that helper returns "" for an ID + // with no "//" separator, and ContainerScopeID("") is the HOST bucket, so + // trimming here would purge every host process marker on each container + // exit. + if rm.stateStore != nil { + rm.stateStore.PurgeScope(rulestate.ContainerScopeID(notif.Container.Runtime.ContainerID)) + } + namespace := notif.Container.K8s.Namespace podName := notif.Container.K8s.PodName podID := utils.CreateK8sPodID(namespace, podName) time.AfterFunc(10*time.Minute, func() { - stillTracked := false - rm.trackedContainers.Each(func(id string) bool { - // Parse the container ID to reliably extract the pod info - parts := strings.Split(id, "/") - if len(parts) == 3 && parts[0] == namespace && parts[1] == podName { - stillTracked = true - return true // We found a match, can stop iteration - } - return false // No match yet, continue looking - }) - - if !stillTracked { - logger.L().Debug("RuleManager - removing pod from podToWlid map", - helpers.String("podID", podID)) - rm.podToWlid.Delete(podID) - } else { + if rm.podStillTracked(namespace, podName) { logger.L().Debug("RuleManager - keeping pod in podToWlid map due to active containers", helpers.String("podID", podID)) + return } + logger.L().Debug("RuleManager - removing pod from podToWlid map", + helpers.String("podID", podID)) + rm.podToWlid.Delete(podID) + rm.purgePodScopeIfPodGone(namespace, podName) }) rm.containerIdToShimPid.Delete(notif.Container.Runtime.ContainerID) @@ -148,3 +154,35 @@ func (rm *RuleManager) waitForSharedContainerData(containerID string) (*objectca return nil, fmt.Errorf("container %s not found in shared data", containerID) }, backoffv5.WithBackOff(backoffv5.NewExponentialBackOff())) } + +// podStillTracked reports whether any container of this pod is still tracked. +// A pod's identity spans its containers, so pod-level cleanup can only run once +// the last of them is gone. +func (rm *RuleManager) podStillTracked(namespace, podName string) bool { + stillTracked := false + rm.trackedContainers.Each(func(id string) bool { + // Parse the container ID to reliably extract the pod info + parts := strings.Split(id, "/") + if len(parts) == 3 && parts[0] == namespace && parts[1] == podName { + stillTracked = true + return true // We found a match, can stop iteration + } + return false // No match yet, continue looking + }) + return stillTracked +} + +// purgePodScopeIfPodGone reclaims a dead pod's state. +// +// Container scope is purged the moment a container goes, but pod scope outlives +// any single container by design, so it can only be reclaimed once the pod's LAST +// container has gone -- purging earlier would cut a correlation chain that is +// still legitimately in progress across a surviving sibling. Without this the +// bucket survived until TTL, and on a churning node many dead pods' worth of +// entries counted against the global ceiling at once. +func (rm *RuleManager) purgePodScopeIfPodGone(namespace, podName string) { + if rm.stateStore == nil || rm.podStillTracked(namespace, podName) { + return + } + rm.stateStore.PurgeScope(rulestate.PodScopeID(namespace, podName)) +} diff --git a/pkg/rulemanager/rule_manager.go b/pkg/rulemanager/rule_manager.go index 8db6069d00..8103d0ec6e 100644 --- a/pkg/rulemanager/rule_manager.go +++ b/pkg/rulemanager/rule_manager.go @@ -31,12 +31,15 @@ import ( "github.com/kubescape/node-agent/pkg/processtree" bindingcache "github.com/kubescape/node-agent/pkg/rulebindingmanager" "github.com/kubescape/node-agent/pkg/rulemanager/cel" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/state" "github.com/kubescape/node-agent/pkg/rulemanager/prefilter" "github.com/kubescape/node-agent/pkg/rulemanager/profilehelper" "github.com/kubescape/node-agent/pkg/rulemanager/ruleadapters" "github.com/kubescape/node-agent/pkg/rulemanager/rulecooldown" + "github.com/kubescape/node-agent/pkg/rulemanager/statewrites" "github.com/kubescape/node-agent/pkg/rulemanager/types" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" + "github.com/kubescape/node-agent/pkg/rulestate" "github.com/kubescape/node-agent/pkg/utils" "go.opentelemetry.io/otel/attribute" @@ -69,6 +72,12 @@ type RuleManager struct { detectorManager *detectors.DetectorManager alertLogDedup *expirable.LRU[string, struct{}] alertLogDedupMu sync.Mutex + // stateWritesCache holds compiled write clauses, keyed by rule ID plus a + // fingerprint of the clause, so an edited rule recompiles without an + // invalidation hook. expirable.LRU is already concurrency-safe. + stateWritesCache *expirable.LRU[string, *compiledWrites] + stateStore *rulestate.Store + stateWrites *statewrites.Executor } var _ RuleManagerClient = (*RuleManager)(nil) @@ -111,10 +120,19 @@ func CreateRuleManager( mntnsRegistry: mntnsRegistry, detectorManager: detectorManager, alertLogDedup: expirable.NewLRU[string, struct{}](1000, nil, 60*time.Second), + // No TTL: a compiled clause stays valid until the clause itself changes, + // which changes the key. The bound is what evicts stale versions. + stateWritesCache: expirable.NewLRU[string, *compiledWrites](512, nil, 0), } r.ruleFailureCreator = ruleadapters.NewRuleFailureCreator(enricher, dnsManager, adapterFactory, armotypes.AlertSourcePlatformK8sAgent, agentVersion, &r.containerIdToPid) + // The state store lives here rather than in main.go because the rule loop is + // its only writer and reader. Sweeping runs for the manager's lifetime. + r.stateStore = rulestate.NewStore(cfg.CelStateStore, newStateMetrics(metrics)) + r.stateWrites = statewrites.NewExecutor(r.stateStore, celEvaluator, newStateMetrics(metrics)) + go r.stateStore.Run(ctx) + // Compile the initial projection spec and start a goroutine that // recompiles whenever rule bindings change. r.recompileProjectionSpec() @@ -318,6 +336,9 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent) var eventFields prefilter.EventFields var evalContext map[string]any + // One tracker per event, reset per rule. Allocating per event rather than per + // rule keeps the common no-state path to a single allocation. + stateTracker := &state.ReadTracker{} for _, rule := range rules { if !rule.Enabled { @@ -335,7 +356,15 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent) } ruleExpressions := rm.getRuleExpressions(rule, eventType) - if len(ruleExpressions) == 0 { + + // Compile the write clause before the no-expressions bail-out below: a rule + // may legitimately have NO ruleExpression for this event type and still need + // to remember something. That is what makes write-without-alerting -- the + // first leg of every cross-event rule -- possible. + stateWrites, stateScopes := rm.compileStateWrites(&rule) + writesThisEvent := hasWriteFor(stateWrites, eventType) + + if len(ruleExpressions) == 0 && !writesThisEvent { continue } @@ -346,12 +375,21 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent) } if rule.Prefilter.ShouldSkip(&eventFields) { rm.metrics.ReportRulePrefiltered(rule.ID) + // A prefilter is built from the rule as a whole, in practice + // mostly from its alerting leg's params, but it is applied to + // every event type the rule touches. So a correlation rule whose + // network leg carries ignorePrefixes or excludeProcesses has those + // same params applied to its exec leg, and the write is dropped -- + // the chain then never forms, with nothing anywhere to say why. + // Counting it is what turns that into a visible failure. + rm.reportSuppressedWrite(&rule, stateWrites, eventType, "prefiltered") continue } } if rule.SupportPolicy && rm.validateRulePolicy(rule, enrichedEvent.Event, enrichedEvent.ContainerID) { rm.metrics.ReportAlertSuppressed(rule.ID, "policy") + rm.reportSuppressedWrite(&rule, stateWrites, eventType, "policy") continue } @@ -359,112 +397,219 @@ func (rm *RuleManager) ReportEnrichedEvent(enrichedEvent *events.EnrichedEvent) evalContext = rm.celEvaluator.CreateEvalContext(enrichedEvent) } - startTime := time.Now() - var shouldAlert bool - var err error - pprof.Do(context.Background(), pprof.Labels("rule", rule.ID), func(_ context.Context) { - shouldAlert, err = rm.celEvaluator.EvaluateRuleWithContext(evalContext, eventType, ruleExpressions) - }) - evaluationTime := time.Since(startTime) - // Slow-path tracing: only emit a span when evaluation exceeded the threshold. - // This protects the hot path from unconditional tracing overhead on millions of events/sec. - // errCtx tracks the spanned context (when a rule.evaluate span fires) so the - // failure log below inherits its trace_id/span_id — otherwise falls back to rm.ctx. - errCtx := rm.ctx - if evaluationTime >= otelsetup.SlowEvalThreshold() { - evalCtx, span := otelsetup.Tracer().Start(rm.ctx, "rule.evaluate", - trace.WithAttributes( - attribute.String("rule.id", rule.ID), - attribute.String("event.type", string(eventType)), - attribute.String("container.id", enrichedEvent.ContainerID), - attribute.Float64("eval.duration_ms", float64(evaluationTime.Milliseconds())), - attribute.Bool("alert_fired", shouldAlert), - )) - if err != nil { - span.SetStatus(codes.Error, err.Error()) - } - rm.metrics.ReportRuleEvaluationTime(evalCtx, rule.ID, eventType, evaluationTime) - span.End() - errCtx = evalCtx + // Rebuild the state receiver per rule: it carries the rule ID and the + // rule's own declared-name scopes, so it cannot be shared between rules. + // Resetting the tracker here is what stops rule N citing rule N-1's + // entries as its own evidence. + // + // Only rules that actually declare state pay for this. seedStateContext + // allocates a ScopeIDs map and an Accessor, and it ran for every rule on + // every event -- on a node running forty stateless rules that was eighty + // allocations per event for a feature none of them use. + // + // The delete is not optional. evalContext is built once per event and + // reused across rules, so skipping the seed without clearing the key would + // leave the PREVIOUS rule's accessor in place, and a stateless rule would + // read another rule's state through it -- the one thing the receiver design + // exists to make inexpressible. + if len(stateScopes) > 0 { + stateTracker.Reset() + rm.seedStateContext(evalContext, &rule, enrichedEvent, stateScopes, stateTracker) } else { - rm.metrics.ReportRuleEvaluationTime(rm.ctx, rule.ID, eventType, evaluationTime) + delete(evalContext, state.AccessorContextKey) + } + + // From here on, alerting must not skip the write clause: writes are + // evidence gathering, and dropping them because an alert was suppressed + // would break the NEXT leg of the chain. The predicate and alert path is + // therefore its own function -- its early exits return, and the writes + // below still run. + // processed mirrors the pre-refactor control flow exactly: the old loop + // reached ReportRuleProcessed only by falling off the end, so an + // eval error or a cooldown-suppressed alert did NOT count as processed. + // Those were continues; they are returns now, so the metric has to be + // gated or its meaning would silently change for every existing rule. + // + // It starts false, not true, for the same reason: the old loop also + // continued at len(ruleExpressions) == 0 and never counted that rule. A + // write-only leg -- a rule with a stateWrites clause but no + // ruleExpression for THIS event type -- reaches here now where it + // previously could not, so defaulting to true would inflate the counter + // for exactly the case this feature introduces. + processed := false + if len(ruleExpressions) > 0 { + processed = rm.evaluateRuleAndAlert(evaluateArgs{ + rule: rule, + ruleExpressions: ruleExpressions, + enrichedEvent: enrichedEvent, + evalContext: evalContext, + eventType: eventType, + namespace: namespace, + pod: pod, + details: details, + apChecksum: apChecksum, + tracker: stateTracker, + }) + } + + if writesThisEvent { + // Writes run AFTER the predicate, so a predicate only ever sees state + // from EARLIER events. Otherwise a rule that reads and writes the same + // name on the same event type would trivially satisfy itself. + rm.stateWrites.Apply(stateWrites, rule.ID, enrichedEvent, evalContext, + cel.ResolveEventTime(enrichedEvent)) + } + + if processed { + rm.metrics.ReportRuleProcessed(rule.ID) } + } +} + +type evaluateArgs struct { + rule typesv1.Rule + ruleExpressions []typesv1.RuleExpression + enrichedEvent *events.EnrichedEvent + evalContext map[string]any + eventType utils.EventType + namespace string + pod string + details string + apChecksum string + tracker *state.ReadTracker +} +// evaluateRuleAndAlert runs one rule's predicate and emits an alert if it fires. +// +// Split out of the rule loop so that every early exit in here is a return rather +// than a continue, which leaves the caller free to run the rule's state writes +// afterwards regardless of whether an alert was emitted or suppressed. +// The bool reports whether the path ran to completion. The caller uses it to +// decide whether to count the rule as processed, preserving the metric's +// pre-refactor meaning. +func (rm *RuleManager) evaluateRuleAndAlert(a evaluateArgs) bool { + rule := a.rule + enrichedEvent := a.enrichedEvent + evalContext := a.evalContext + eventType := a.eventType + namespace := a.namespace + pod := a.pod + apChecksum := a.apChecksum + + startTime := time.Now() + var shouldAlert bool + var err error + pprof.Do(context.Background(), pprof.Labels("rule", rule.ID), func(_ context.Context) { + shouldAlert, err = rm.celEvaluator.EvaluateRuleWithContext(evalContext, eventType, a.ruleExpressions) + }) + evaluationTime := time.Since(startTime) + // Slow-path tracing: only emit a span when evaluation exceeded the threshold. + // This protects the hot path from unconditional tracing overhead on millions of events/sec. + // errCtx tracks the spanned context (when a rule.evaluate span fires) so the + // failure log below inherits its trace_id/span_id — otherwise falls back to rm.ctx. + errCtx := rm.ctx + if evaluationTime >= otelsetup.SlowEvalThreshold() { + evalCtx, span := otelsetup.Tracer().Start(rm.ctx, "rule.evaluate", + trace.WithAttributes( + attribute.String("rule.id", rule.ID), + attribute.String("event.type", string(eventType)), + attribute.String("container.id", enrichedEvent.ContainerID), + attribute.Float64("eval.duration_ms", float64(evaluationTime.Milliseconds())), + attribute.Bool("alert_fired", shouldAlert), + )) if err != nil { - logger.L().Ctx(errCtx).Error("RuleManager.ReportEnrichedEvent - failed to evaluate rule", helpers.Error(err), helpers.String("rule", rule.ID), helpers.String("eventType", string(eventType))) - rm.metrics.ReportAlertSuppressed(rule.ID, "eval_error") - continue + span.SetStatus(codes.Error, err.Error()) } + rm.metrics.ReportRuleEvaluationTime(evalCtx, rule.ID, eventType, evaluationTime) + span.End() + errCtx = evalCtx + } else { + rm.metrics.ReportRuleEvaluationTime(rm.ctx, rule.ID, eventType, evaluationTime) + } - if shouldAlert { - state := rule.State - if eventType == utils.HTTPEventType { // TODO: Manage state evaluation in a better way (this is abuse of the state map, we need a better way to pass payloads from rules.) - state = rm.evaluateHTTPPayloadState(rule.State, enrichedEvent) - } - rm.metrics.ReportRuleAlert(rule.ID) - message, uniqueID, err := rm.getUniqueIdAndMessage(enrichedEvent, rule) - if err != nil { - logger.L().Error("RuleManager - failed to get unique ID and message", helpers.Error(err)) - continue - } + if err != nil { + logger.L().Ctx(errCtx).Error("RuleManager.ReportEnrichedEvent - failed to evaluate rule", helpers.Error(err), helpers.String("rule", rule.ID), helpers.String("eventType", string(eventType))) + rm.metrics.ReportAlertSuppressed(rule.ID, "eval_error") + return false + } - if shouldCooldown, _ := rm.ruleCooldown.ShouldCooldown(uniqueID, enrichedEvent.ContainerID, rule.ID); shouldCooldown { - rm.metrics.ReportAlertSuppressed(rule.ID, "cooldown") - continue - } + // A predicate that simply did not match still counts as processed, exactly as + // it did when this was a fall-through rather than a return. + if !shouldAlert { + return true + } - // Emit OTEL log after cooldown so suppressed alerts are not recorded. - // Dedup key includes eventType to avoid collapsing distinct alert types. - dedupKey := rule.ID + "|" + enrichedEvent.ContainerID + "|" + string(eventType) - rm.alertLogDedupMu.Lock() - alreadySeen := rm.alertLogDedup.Contains(dedupKey) - if !alreadySeen { - rm.alertLogDedup.Add(dedupKey, struct{}{}) - } - rm.alertLogDedupMu.Unlock() - if !alreadySeen { - var image, containerName string - if enrichable, ok := enrichedEvent.Event.(utils.EnrichEvent); ok { - image = enrichable.GetContainerImage() - containerName = enrichable.GetContainer() - } - alertCtx, alertSpan := otelsetup.Tracer().Start(rm.ctx, "rule.alert", - trace.WithAttributes( - attribute.String("rule.id", rule.ID), - attribute.String("rule.name", rule.Name), - attribute.String("k8s.namespace.name", namespace), - attribute.String("k8s.pod.name", pod), - attribute.String("container.id", enrichedEvent.ContainerID), - attribute.String("event.type", string(eventType)), - )) - otelsetup.EmitAlertLogRecord(alertCtx, otelsetup.AlertLogAttrs{ - RuleID: rule.ID, - AlertType: rule.Name, - ContainerID: enrichedEvent.ContainerID, - ContainerName: containerName, - Namespace: namespace, - PodName: pod, - Image: image, - EventType: string(eventType), - }) - alertSpan.End() - } + // ruleState, not "state": the local would otherwise shadow the state + // library package imported for the read tracker. + ruleState := rule.State + if eventType == utils.HTTPEventType { // TODO: Manage state evaluation in a better way (this is abuse of the state map, we need a better way to pass payloads from rules.) + ruleState = rm.evaluateHTTPPayloadState(rule.State, enrichedEvent) + } + rm.metrics.ReportRuleAlert(rule.ID) + message, uniqueID, err := rm.getUniqueIdAndMessage(evalContext, rule) + if err != nil { + logger.L().Error("RuleManager - failed to get unique ID and message", helpers.Error(err)) + return false + } - ruleFailure := rm.ruleFailureCreator.CreateRuleFailure(rule, enrichedEvent, rm.objectCache, message, uniqueID, apChecksum, state) - if ruleFailure == nil { - logger.L().Error("RuleManager - failed to create rule failure", helpers.String("rule", rule.Name), - helpers.String("message", message), - helpers.String("uniqueID", uniqueID), - helpers.String("enrichedEvent.EventType", string(eventType)), - ) - continue - } + if shouldCooldown, _ := rm.ruleCooldown.ShouldCooldown(uniqueID, enrichedEvent.ContainerID, rule.ID); shouldCooldown { + rm.metrics.ReportAlertSuppressed(rule.ID, "cooldown") + return false + } - ruleFailure.SetWorkloadDetails(details) - rm.exporter.SendRuleAlert(ruleFailure) - } - rm.metrics.ReportRuleProcessed(rule.ID) + // Emit OTEL log after cooldown so suppressed alerts are not recorded. + // Dedup key includes eventType to avoid collapsing distinct alert types. + dedupKey := rule.ID + "|" + enrichedEvent.ContainerID + "|" + string(eventType) + rm.alertLogDedupMu.Lock() + alreadySeen := rm.alertLogDedup.Contains(dedupKey) + if !alreadySeen { + rm.alertLogDedup.Add(dedupKey, struct{}{}) + } + rm.alertLogDedupMu.Unlock() + if !alreadySeen { + var image, containerName string + if enrichable, ok := enrichedEvent.Event.(utils.EnrichEvent); ok { + image = enrichable.GetContainerImage() + containerName = enrichable.GetContainer() + } + alertCtx, alertSpan := otelsetup.Tracer().Start(rm.ctx, "rule.alert", + trace.WithAttributes( + attribute.String("rule.id", rule.ID), + attribute.String("rule.name", rule.Name), + attribute.String("k8s.namespace.name", namespace), + attribute.String("k8s.pod.name", pod), + attribute.String("container.id", enrichedEvent.ContainerID), + attribute.String("event.type", string(eventType)), + )) + otelsetup.EmitAlertLogRecord(alertCtx, otelsetup.AlertLogAttrs{ + RuleID: rule.ID, + AlertType: rule.Name, + ContainerID: enrichedEvent.ContainerID, + ContainerName: containerName, + Namespace: namespace, + PodName: pod, + Image: image, + EventType: string(eventType), + }) + alertSpan.End() + } + + // The entries this rule's predicate actually read become the alert's + // correlation evidence. Harvested here, after the predicate ran and after + // cooldown, so a suppressed alert costs nothing. + ruleFailure := rm.ruleFailureCreator.CreateRuleFailure(rule, enrichedEvent, rm.objectCache, message, uniqueID, apChecksum, ruleState, a.tracker.Hits()) + if ruleFailure == nil { + logger.L().Error("RuleManager - failed to create rule failure", helpers.String("rule", rule.Name), + helpers.String("message", message), + helpers.String("uniqueID", uniqueID), + helpers.String("enrichedEvent.EventType", string(eventType)), + ) + return false } + + ruleFailure.SetWorkloadDetails(a.details) + rm.exporter.SendRuleAlert(ruleFailure) + return true } func (rm *RuleManager) enrichEventWithContext(enrichedEvent *events.EnrichedEvent) { @@ -600,19 +745,31 @@ func (rm *RuleManager) getRuleExpressions(rule typesv1.Rule, eventType utils.Eve return ruleExpressions } -func (rm *RuleManager) getUniqueIdAndMessage(enrichedEvent *events.EnrichedEvent, rule typesv1.Rule) (string, string, error) { - message, err := rm.celEvaluator.EvaluateExpression(enrichedEvent, rule.Expressions.Message) - if err != nil { - logger.L().Ctx(rm.ctx).Error("RuleManager - failed to evaluate message", helpers.Error(err)) +// getUniqueIdAndMessage renders the alert's message and uniqueId. +// +// It takes the predicate's evalContext rather than rebuilding one. That matters +// for two reasons: state.get() in a message must resolve against the SAME entries +// the predicate matched, and uniqueId can then be derived from the join key -- +// which is what lets rulecooldown collapse both legs of a bidirectional rule into +// a single alert instead of emitting one per leg. +func (rm *RuleManager) getUniqueIdAndMessage(evalContext map[string]any, rule typesv1.Rule) (string, string, error) { + message, msgErr := rm.celEvaluator.EvaluateStringExpressionWithContext(evalContext, rule.Expressions.Message) + if msgErr != nil { + logger.L().Ctx(rm.ctx).Error("RuleManager - failed to evaluate message", helpers.Error(msgErr)) } - uniqueID, err := rm.celEvaluator.EvaluateExpression(enrichedEvent, rule.Expressions.UniqueID) - if err != nil { - logger.L().Ctx(rm.ctx).Error("RuleManager - failed to evaluate unique ID", helpers.Error(err)) + uniqueID, idErr := rm.celEvaluator.EvaluateStringExpressionWithContext(evalContext, rule.Expressions.UniqueID) + if idErr != nil { + logger.L().Ctx(rm.ctx).Error("RuleManager - failed to evaluate unique ID", helpers.Error(idErr)) } uniqueID = hashStringToMD5(uniqueID) - return message, uniqueID, err + // Only the uniqueId error is returned, and the caller drops the alert on it. + // That asymmetry is deliberate: uniqueId drives cooldown and backend dedup, so + // a wrong one corrupts grouping, whereas a failed message costs description + // only. Dropping a real detection because its text did not render would be the + // worse failure, so a message error is logged and the alert still ships. + return message, uniqueID, idErr } func isSupportedEventType(rules []typesv1.Rule, enrichedEvent *events.EnrichedEvent) bool { @@ -623,6 +780,18 @@ func isSupportedEventType(rules []typesv1.Rule, enrichedEvent *events.EnrichedEv return true } } + // A write leg needs no ruleExpression for its event type -- that is what + // makes write-without-alerting possible. Without this, write-only legs are + // dropped before reaching the loop and the chain never forms. + // + // The string() casts are load-bearing: StateWrites carries + // armotypes.EventType while eventType is utils.EventType. They are the same + // strings, but not the same Go type. + for _, w := range rule.StateWrites { + if string(w.EventType) == string(eventType) { + return true + } + } } return false } diff --git a/pkg/rulemanager/ruleadapters/correlation_test.go b/pkg/rulemanager/ruleadapters/correlation_test.go new file mode 100644 index 0000000000..89d793b18d --- /dev/null +++ b/pkg/rulemanager/ruleadapters/correlation_test.go @@ -0,0 +1,130 @@ +package ruleadapters + +import ( + "encoding/json" + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/rulemanager/types" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func execHit() *rulestate.Entry { + return &rulestate.Entry{ + RuleID: "R1089", + Name: "mount_exec", + Scope: armotypes.StateScopeContainer, + ScopeID: "c:abc", + Key: "4471", + EventType: armotypes.EventTypeExec, + Timestamp: time.Date(2026, 7, 28, 12, 0, 3, 0, time.UTC), + Process: &armotypes.Process{ + PID: 4471, Comm: "xmrig", Path: "/mnt/data/xmrig", + }, + Value: map[string]any{"argv": "-o pool:4444"}, + } +} + +func TestCorrelationEvidence_OneHitBecomesOneEvidenceEntry(t *testing.T) { + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, []*rulestate.Entry{execHit()}) + + got := failure.GetCorrelationAlert() + require.Len(t, got.Correlations, 1) + + c := got.Correlations[0] + assert.Equal(t, "mount_exec", c.Name) + assert.Equal(t, armotypes.EventTypeExec, c.EventType) + assert.Equal(t, time.Date(2026, 7, 28, 12, 0, 3, 0, time.UTC), c.Timestamp) + assert.Equal(t, armotypes.StateScopeContainer, c.Scope) + assert.Equal(t, "4471", c.Key) + require.NotNil(t, c.Process) + assert.Equal(t, uint32(4471), c.Process.PID) + assert.Equal(t, "/mnt/data/xmrig", c.Process.Path) + assert.Equal(t, map[string]any{"argv": "-o pool:4444"}, c.Values) + assert.Nil(t, c.Admission, "node-agent entries carry a Process, never an Admission") +} + +func TestCorrelationEvidence_MultipleHitsArePreservedInOrder(t *testing.T) { + second := execHit() + second.Name = "egress_seen" + + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, []*rulestate.Entry{execHit(), second}) + + got := failure.GetCorrelationAlert() + require.Len(t, got.Correlations, 2) + assert.Equal(t, "mount_exec", got.Correlations[0].Name) + assert.Equal(t, "egress_seen", got.Correlations[1].Name) +} + +func TestCorrelationEvidence_NoHitsLeavesTheAlertUntouched(t *testing.T) { + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, nil) + assert.Empty(t, failure.GetCorrelationAlert().Correlations) + + // omitempty means an uncorrelated alert must not gain a "correlations" key -- + // every existing alert on the wire has to stay byte-identical. + data, err := json.Marshal(failure.GetCorrelationAlert()) + require.NoError(t, err) + assert.NotContains(t, string(data), "correlations") +} + +func TestCorrelationEvidence_NilEntriesAreSkipped(t *testing.T) { + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, []*rulestate.Entry{nil, execHit(), nil}) + assert.Len(t, failure.GetCorrelationAlert().Correlations, 1) +} + +func TestCorrelationEvidence_AllNilEntriesAddsNothing(t *testing.T) { + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, []*rulestate.Entry{nil, nil}) + assert.Empty(t, failure.GetCorrelationAlert().Correlations) +} + +// Correlation must ENRICH an incident, never re-key it. If InfectedPID or +// RuntimeProcessDetails shifted to the remembered process, backend incident +// grouping would move the alert to a different incident. +func TestCorrelationEvidence_DoesNotRekeyTheAlert(t *testing.T) { + failure := &types.GenericRuleFailure{ + BaseRuntimeAlert: armotypes.BaseRuntimeAlert{ + InfectedPID: 9999, // the TRIGGERING process + }, + RuntimeProcessDetails: armotypes.ProcessTree{ + ContainerID: "triggering-container", + ProcessTree: armotypes.Process{PID: 9999}, + }, + } + + setCorrelationEvidence(failure, []*rulestate.Entry{execHit()}) // remembered PID 4471 + + assert.Equal(t, uint32(9999), failure.GetBaseRuntimeAlert().InfectedPID, + "InfectedPID must still describe the triggering event") + assert.Equal(t, uint32(9999), failure.GetRuntimeProcessDetails().ProcessTree.PID) + assert.Equal(t, "triggering-container", failure.GetRuntimeProcessDetails().ContainerID) +} + +// The evidence must survive onto the wire alert, not just the internal failure. +func TestCorrelationEvidence_SerializesUnderCorrelations(t *testing.T) { + failure := &types.GenericRuleFailure{} + setCorrelationEvidence(failure, []*rulestate.Entry{execHit()}) + + alert := armotypes.RuntimeAlert{ + CorrelationAlert: failure.GetCorrelationAlert(), + } + data, err := json.Marshal(alert) + require.NoError(t, err) + + var round map[string]any + require.NoError(t, json.Unmarshal(data, &round)) + + raw, ok := round["correlations"] + require.True(t, ok, "CorrelationAlert is inlined, so evidence appears at the alert top level") + entries, ok := raw.([]any) + require.True(t, ok) + require.Len(t, entries, 1) + assert.Equal(t, "mount_exec", entries[0].(map[string]any)["name"]) +} diff --git a/pkg/rulemanager/ruleadapters/creator.go b/pkg/rulemanager/ruleadapters/creator.go index 391973de2d..3c4a0f8965 100644 --- a/pkg/rulemanager/ruleadapters/creator.go +++ b/pkg/rulemanager/ruleadapters/creator.go @@ -21,6 +21,7 @@ import ( "github.com/kubescape/node-agent/pkg/objectcache" "github.com/kubescape/node-agent/pkg/rulemanager/types" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" + "github.com/kubescape/node-agent/pkg/rulestate" "github.com/kubescape/node-agent/pkg/utils" ) @@ -60,7 +61,7 @@ func NewRuleFailureCreator(enricher types.Enricher, dnsManager dnsmanager.DNSRes } } -func (r *RuleFailureCreator) CreateRuleFailure(rule typesv1.Rule, enrichedEvent *events.EnrichedEvent, objectCache objectcache.ObjectCache, message, uniqueID, apChecksum string, state map[string]any) types.RuleFailure { +func (r *RuleFailureCreator) CreateRuleFailure(rule typesv1.Rule, enrichedEvent *events.EnrichedEvent, objectCache objectcache.ObjectCache, message, uniqueID, apChecksum string, state map[string]any, hits []*rulestate.Entry) types.RuleFailure { eventAdapter, ok := r.adapterFactory.GetAdapter(enrichedEvent.Event.GetEventType()) if !ok { logger.L().Error("RuleFailureCreator - no adapter registered for event type", helpers.String("eventType", string(enrichedEvent.Event.GetEventType()))) @@ -104,9 +105,48 @@ func (r *RuleFailureCreator) CreateRuleFailure(rule typesv1.Rule, enrichedEvent }) } + // Set here rather than in an event adapter: SetFailureMetadata is + // per-event-type, whereas correlation is orthogonal to event type. + // + // Note what is deliberately NOT touched -- InfectedPID and + // RuntimeProcessDetails still describe the TRIGGERING event. Correlation + // enriches an incident, it does not re-key it, so backend incident grouping is + // unchanged by this. + setCorrelationEvidence(ruleFailure, hits) + return ruleFailure } +// setCorrelationEvidence copies the state entries the predicate actually read +// onto the alert, so a correlation alert describes BOTH ends of the chain. +// Without it the alert would say only "a process made an outbound connection" +// and drop the exec that makes it interesting. +func setCorrelationEvidence(ruleFailure *types.GenericRuleFailure, hits []*rulestate.Entry) { + if len(hits) == 0 { + return + } + ev := make([]armotypes.CorrelationEvidence, 0, len(hits)) + for _, h := range hits { + if h == nil { + continue + } + ev = append(ev, armotypes.CorrelationEvidence{ + Name: h.Name, + EventType: h.EventType, + Timestamp: h.Timestamp, + Scope: h.Scope, + Key: h.Key, + Process: h.Process, + Admission: h.Admission, + Values: h.Value, + }) + } + if len(ev) == 0 { + return + } + ruleFailure.SetCorrelationAlert(armotypes.CorrelationAlert{Correlations: ev}) +} + func (r *RuleFailureCreator) enrichRuleFailure(ruleFailure *types.GenericRuleFailure) { if r.enricher != nil && !reflect.ValueOf(r.enricher).IsNil() { if err := r.enricher.EnrichRuleFailure(ruleFailure); err != nil { diff --git a/pkg/rulemanager/ruleadapters/creator_interface.go b/pkg/rulemanager/ruleadapters/creator_interface.go index 3e0781a32e..04c7e613a6 100644 --- a/pkg/rulemanager/ruleadapters/creator_interface.go +++ b/pkg/rulemanager/ruleadapters/creator_interface.go @@ -5,10 +5,11 @@ import ( "github.com/kubescape/node-agent/pkg/objectcache" "github.com/kubescape/node-agent/pkg/rulemanager/types" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" + "github.com/kubescape/node-agent/pkg/rulestate" ) type RuleFailureCreatorInterface interface { - CreateRuleFailure(rule typesv1.Rule, enrichedEvent *events.EnrichedEvent, objectCache objectcache.ObjectCache, message, uniqueID, apChecksum string, state map[string]any) types.RuleFailure + CreateRuleFailure(rule typesv1.Rule, enrichedEvent *events.EnrichedEvent, objectCache objectcache.ObjectCache, message, uniqueID, apChecksum string, state map[string]any, hits []*rulestate.Entry) types.RuleFailure } type EventMetadataSetter interface { diff --git a/pkg/rulemanager/rulecreator/context_match_test.go b/pkg/rulemanager/rulecreator/context_match_test.go index 23b9bb00ef..fba905e7ec 100644 --- a/pkg/rulemanager/rulecreator/context_match_test.go +++ b/pkg/rulemanager/rulecreator/context_match_test.go @@ -3,6 +3,8 @@ package rulecreator import ( "testing" + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/contextdetection" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" ) @@ -16,86 +18,86 @@ func TestRuleMatchesContext(t *testing.T) { }{ { name: "host tag matches host context", - rule: typesv1.Rule{Tags: []string{"context:host"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host"}}}, context: contextdetection.Host, expected: true, }, { name: "host tag does not match kubernetes context", - rule: typesv1.Rule{Tags: []string{"context:host"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host"}}}, context: contextdetection.Kubernetes, expected: false, }, { name: "kubernetes tag matches kubernetes context", - rule: typesv1.Rule{Tags: []string{"context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:kubernetes"}}}, context: contextdetection.Kubernetes, expected: true, }, { name: "kubernetes tag does not match host context", - rule: typesv1.Rule{Tags: []string{"context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:kubernetes"}}}, context: contextdetection.Host, expected: false, }, { name: "standalone tag matches standalone context", - rule: typesv1.Rule{Tags: []string{"context:standalone"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:standalone"}}}, context: contextdetection.Standalone, expected: true, }, { name: "ecs tag matches ecs context", - rule: typesv1.Rule{Tags: []string{"context:ecs"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:ecs"}}}, context: contextdetection.ECS, expected: true, }, { name: "container tag matches kubernetes context", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, context: contextdetection.Kubernetes, expected: true, }, { name: "container tag matches standalone context", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, context: contextdetection.Standalone, expected: true, }, { name: "container tag matches ecs context", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, context: contextdetection.ECS, expected: true, }, { name: "container tag matches container context", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, context: contextdetection.Container, expected: true, }, { name: "container tag does not match host context", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, context: contextdetection.Host, expected: false, }, { name: "no context tags defaults to kubernetes", - rule: typesv1.Rule{Tags: []string{"some-other-tag"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"some-other-tag"}}}, context: contextdetection.Kubernetes, expected: true, }, { name: "no context tags rejects host", - rule: typesv1.Rule{Tags: []string{"some-other-tag"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"some-other-tag"}}}, context: contextdetection.Host, expected: false, }, { name: "no context tags rejects standalone", - rule: typesv1.Rule{Tags: []string{}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{}}}, context: contextdetection.Standalone, expected: false, }, @@ -114,32 +116,32 @@ func TestRuleMatchesContext(t *testing.T) { { name: "multiple context tags: host+kubernetes matches host", - rule: typesv1.Rule{Tags: []string{"context:host", "context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host", "context:kubernetes"}}}, context: contextdetection.Host, expected: true, }, { name: "multiple context tags: host+kubernetes matches kubernetes", - rule: typesv1.Rule{Tags: []string{"context:host", "context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host", "context:kubernetes"}}}, context: contextdetection.Kubernetes, expected: true, }, { name: "multiple context tags: host+kubernetes rejects standalone", - rule: typesv1.Rule{Tags: []string{"context:host", "context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host", "context:kubernetes"}}}, context: contextdetection.Standalone, expected: false, }, { name: "mixed tags with context:host matches host", - rule: typesv1.Rule{Tags: []string{"severity:high", "context:host", "category:network"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"severity:high", "context:host", "category:network"}}}, context: contextdetection.Host, expected: true, }, { name: "mixed tags with context:host rejects kubernetes", - rule: typesv1.Rule{Tags: []string{"severity:high", "context:host", "category:network"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"severity:high", "context:host", "category:network"}}}, context: contextdetection.Kubernetes, expected: false, }, diff --git a/pkg/rulemanager/rulecreator/factory_context_test.go b/pkg/rulemanager/rulecreator/factory_context_test.go index dd9f696c12..0f31684bc8 100644 --- a/pkg/rulemanager/rulecreator/factory_context_test.go +++ b/pkg/rulemanager/rulecreator/factory_context_test.go @@ -3,19 +3,21 @@ package rulecreator import ( "testing" + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/contextdetection" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" ) func testRules() []typesv1.Rule { return []typesv1.Rule{ - {ID: "host-only", Name: "Host Rule", Enabled: true, Tags: []string{"context:host"}}, - {ID: "k8s-only", Name: "K8s Rule", Enabled: true, Tags: []string{"context:kubernetes"}}, - {ID: "standalone-only", Name: "Standalone Rule", Enabled: true, Tags: []string{"context:standalone"}}, - {ID: "ecs-only", Name: "ECS Rule", Enabled: true, Tags: []string{"context:ecs"}}, - {ID: "container-meta", Name: "Container Rule", Enabled: true, Tags: []string{"context:container"}}, - {ID: "no-context", Name: "Legacy Rule", Enabled: true, Tags: []string{"severity:high"}}, - {ID: "multi-context", Name: "Multi Context", Enabled: true, Tags: []string{"context:host", "context:standalone"}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "host-only", Name: "Host Rule", Enabled: true, Tags: []string{"context:host"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "k8s-only", Name: "K8s Rule", Enabled: true, Tags: []string{"context:kubernetes"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "standalone-only", Name: "Standalone Rule", Enabled: true, Tags: []string{"context:standalone"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "ecs-only", Name: "ECS Rule", Enabled: true, Tags: []string{"context:ecs"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "container-meta", Name: "Container Rule", Enabled: true, Tags: []string{"context:container"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "no-context", Name: "Legacy Rule", Enabled: true, Tags: []string{"severity:high"}}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "multi-context", Name: "Multi Context", Enabled: true, Tags: []string{"context:host", "context:standalone"}}}, } } @@ -107,11 +109,13 @@ func TestCreateRulesForContext_PrefilterInitialized(t *testing.T) { creator := &RuleCreatorImpl{ Rules: []typesv1.Rule{ { - ID: "host-rule", - Enabled: true, - Tags: []string{"context:host"}, - State: map[string]any{ - "ports": []uint16{443}, + RuntimeRule: armotypes.RuntimeRule{ + ID: "host-rule", + Enabled: true, + Tags: []string{"context:host"}, + State: map[string]any{ + "ports": []uint16{443}, + }, }, }, }, @@ -138,7 +142,7 @@ func TestCreateRulesForContext_EmptyRules(t *testing.T) { func TestCreateRulesForContext_NoMatchingRules(t *testing.T) { creator := &RuleCreatorImpl{ Rules: []typesv1.Rule{ - {ID: "k8s-only", Tags: []string{"context:kubernetes"}}, + {RuntimeRule: armotypes.RuntimeRule{ID: "k8s-only", Tags: []string{"context:kubernetes"}}}, }, } rules := creator.CreateRulesForContext(contextdetection.Host) diff --git a/pkg/rulemanager/rulecreator/ruleengine_mock.go b/pkg/rulemanager/rulecreator/ruleengine_mock.go index a004e0fa2f..74d55f7318 100644 --- a/pkg/rulemanager/rulecreator/ruleengine_mock.go +++ b/pkg/rulemanager/rulecreator/ruleengine_mock.go @@ -1,6 +1,7 @@ package rulecreator import ( + "github.com/armosec/armoapi-go/armotypes" "github.com/kubescape/node-agent/pkg/contextdetection" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" "github.com/kubescape/node-agent/pkg/utils" @@ -16,8 +17,10 @@ func (r *RuleCreatorMock) CreateRulesByTags(tags []string) []typesv1.Rule { var rl []typesv1.Rule for _, t := range tags { rl = append(rl, typesv1.Rule{ - Name: t, - Tags: []string{t}, + RuntimeRule: armotypes.RuntimeRule{ + Name: t, + Tags: []string{t}, + }, }) } return rl @@ -25,13 +28,13 @@ func (r *RuleCreatorMock) CreateRulesByTags(tags []string) []typesv1.Rule { func (r *RuleCreatorMock) CreateRuleByID(id string) typesv1.Rule { return typesv1.Rule{ - ID: id, + RuntimeRule: armotypes.RuntimeRule{ID: id}, } } func (r *RuleCreatorMock) CreateRuleByName(name string) typesv1.Rule { return typesv1.Rule{ - Name: name, + RuntimeRule: armotypes.RuntimeRule{Name: name}, } } diff --git a/pkg/rulemanager/rulepolicy_test.go b/pkg/rulemanager/rulepolicy_test.go index 8f0319c27a..a263055960 100644 --- a/pkg/rulemanager/rulepolicy_test.go +++ b/pkg/rulemanager/rulepolicy_test.go @@ -3,6 +3,7 @@ package rulemanager import ( "testing" + "github.com/armosec/armoapi-go/armotypes" "github.com/kubescape/node-agent/pkg/contextdetection" typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" ) @@ -13,7 +14,7 @@ type mockContextInfo struct { } func (m *mockContextInfo) Context() contextdetection.EventSourceContext { return m.ctx } -func (m *mockContextInfo) WorkloadID() string { return m.workloadID } +func (m *mockContextInfo) WorkloadID() string { return m.workloadID } func TestRuleAppliesToContext(t *testing.T) { tests := []struct { @@ -24,68 +25,68 @@ func TestRuleAppliesToContext(t *testing.T) { }{ { name: "nil contextInfo: rule with no tags defaults to kubernetes", - rule: typesv1.Rule{Tags: []string{}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{}}}, contextInfo: nil, expected: true, }, { name: "nil contextInfo: rule with host tag rejected", - rule: typesv1.Rule{Tags: []string{"context:host"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host"}}}, contextInfo: nil, expected: false, }, { name: "nil contextInfo: rule with kubernetes tag accepted", - rule: typesv1.Rule{Tags: []string{"context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:kubernetes"}}}, contextInfo: nil, expected: true, }, { name: "nil contextInfo: rule with container tag accepted (kubernetes is container-type)", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, contextInfo: nil, expected: true, }, { name: "host context: host tag matches", - rule: typesv1.Rule{Tags: []string{"context:host"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.Host}, expected: true, }, { name: "host context: kubernetes tag rejected", - rule: typesv1.Rule{Tags: []string{"context:kubernetes"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:kubernetes"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.Host}, expected: false, }, { name: "host context: no tags rejected (backward compat = k8s only)", - rule: typesv1.Rule{Tags: []string{"other"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"other"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.Host}, expected: false, }, { name: "standalone context: standalone tag matches", - rule: typesv1.Rule{Tags: []string{"context:standalone"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:standalone"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.Standalone}, expected: true, }, { name: "standalone context: container meta-tag matches", - rule: typesv1.Rule{Tags: []string{"context:container"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:container"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.Standalone}, expected: true, }, { name: "ecs context: ecs tag matches", - rule: typesv1.Rule{Tags: []string{"context:ecs"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:ecs"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.ECS}, expected: true, }, { name: "ecs context: host tag rejected", - rule: typesv1.Rule{Tags: []string{"context:host"}}, + rule: typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{Tags: []string{"context:host"}}}, contextInfo: &mockContextInfo{ctx: contextdetection.ECS}, expected: false, }, diff --git a/pkg/rulemanager/statecontext.go b/pkg/rulemanager/statecontext.go new file mode 100644 index 0000000000..c40133068f --- /dev/null +++ b/pkg/rulemanager/statecontext.go @@ -0,0 +1,194 @@ +package rulemanager + +import ( + "encoding/binary" + "fmt" + "hash/fnv" + "strconv" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/go-logger" + "github.com/kubescape/go-logger/helpers" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/metricsmanager" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/state" + "github.com/kubescape/node-agent/pkg/rulemanager/statewrites" + typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" + "github.com/kubescape/node-agent/pkg/utils" +) + +// compiledWrites is one cached compilation of a rule's write clause. A failed +// compilation is cached too, as a nil compiled slice -- otherwise a malformed +// rule would re-validate and re-log on every matching event. +type compiledWrites struct { + compiled []statewrites.Compiled + scopeOf map[string]armotypes.StateScope +} + +// compileStateWrites validates a rule's write clause and returns the compiled +// writes plus the name -> scope map that its reads resolve against. +// +// The result is cached, keyed by rule ID AND a fingerprint of the clause itself. +// Compilation cannot live on the Rule as a load-time field: rules reach the loop +// from two different paths and only the Kubernetes binding path populates +// load-time derived fields, so such a field would be silently empty for host +// rules. Fingerprinting instead of invalidating sidesteps that entirely -- an +// edited clause simply hashes differently and recompiles, from either path, with +// no invalidation hook to forget to call. +// +// Caching is what makes the logging below safe. Validation errors and TTL +// clamping used to be logged on EVERY matching event, so a single misconfigured +// rule could emit thousands of identical lines per second on a busy node. Now +// each distinct version of a clause is compiled, and therefore logged, once. +// +// A rule whose clause fails validation is treated as having no writes. It is not +// fatal: one malformed rule must not stop the other forty from evaluating. +func (rm *RuleManager) compileStateWrites(rule *typesv1.Rule) ([]statewrites.Compiled, map[string]armotypes.StateScope) { + if len(rule.StateWrites) == 0 { + return nil, nil + } + + key := stateWritesCacheKey(rule) + if cached, ok := rm.stateWritesCache.Get(key); ok { + return cached.compiled, cached.scopeOf + } + + compiled, scopeOf, err := statewrites.ValidateAll(rule.StateWrites, rule.ID, rm.cfg.CelStateStore.MaxTTL) + if err != nil { + logger.L().Error("RuleManager - invalid stateWrites clause; the rule will not correlate", + helpers.Error(err), helpers.String("rule", rule.ID)) + compiled, scopeOf = nil, nil + } + rm.stateWritesCache.Add(key, &compiledWrites{compiled: compiled, scopeOf: scopeOf}) + return compiled, scopeOf +} + +// stateWritesCacheKey identifies one version of one rule's write clause. +// +// It hashes every field validation reads, so any edit that could change the +// compiled result also changes the key. Hashing is far cheaper than the parsing +// and map building it replaces, and only rules that declare writes reach it. +func stateWritesCacheKey(rule *typesv1.Rule) string { + h := fnv.New64a() + _, _ = h.Write([]byte(rule.ID)) + for i := range rule.StateWrites { + w := &rule.StateWrites[i] + for _, part := range []string{ + string(w.EventType), string(w.Scope), w.Name, w.Key, w.When, w.TTL, + } { + _, _ = h.Write([]byte(part)) + _, _ = h.Write([]byte{0}) + } + // Value is a map, so iteration order varies; hash it order-independently + // by XOR-ing each pair's own digest into a running total. + var values uint64 + for k, v := range w.Value { + p := fnv.New64a() + _, _ = p.Write([]byte(k)) + _, _ = p.Write([]byte{0}) + _, _ = p.Write([]byte(fmt.Sprint(v))) + values ^= p.Sum64() + } + var buf [8]byte + binary.LittleEndian.PutUint64(buf[:], values) + _, _ = h.Write(buf[:]) + } + return strconv.FormatUint(h.Sum64(), 16) +} + +func hasWriteFor(compiled []statewrites.Compiled, eventType utils.EventType) bool { + for _, w := range compiled { + if w.EventType == eventType { + return true + } + } + return false +} + +// seedStateContext installs the per-rule state receiver into the eval context. +// +// The ancestor walk is passed as a closure and resolved lazily inside the +// accessor: most rules never call has_ancestor, and walking the process tree per +// event per rule would be a real cost for a feature few rules use. +func (rm *RuleManager) seedStateContext( + evalContext map[string]any, + rule *typesv1.Rule, + enrichedEvent *events.EnrichedEvent, + scopeOf map[string]armotypes.StateScope, + tracker *state.ReadTracker, +) { + evalContext[state.AccessorContextKey] = state.NewAccessor( + rm.stateStore, + rule.ID, + scopeOf, + statewrites.ScopeIDs(enrichedEvent), + func() []uint32 { + return rm.processManager.GetAncestorPIDs( + enrichedEvent.PID, rm.cfg.CelStateStore.AncestorMaxDepth) + }, + tracker, + nil, + ) +} + +// stateMetrics adapts the node-agent metrics manager to rulestate.Metrics. +// +// It is a separate type so pkg/rulestate stays free of any metrics dependency and +// remains unit-testable on its own. +type stateMetrics struct { + mm metricsmanager.MetricsManager +} + +func newStateMetrics(mm metricsmanager.MetricsManager) *stateMetrics { + return &stateMetrics{mm: mm} +} + +func (s *stateMetrics) ReportStateWrite(ruleID, result string) { + if s.mm != nil { + s.mm.ReportStateWrite(ruleID, result) + } +} + +func (s *stateMetrics) ReportStateWriteRejected(ruleID, reason string) { + if s.mm != nil { + s.mm.ReportStateWriteRejected(ruleID, reason) + } +} + +func (s *stateMetrics) ReportStateExpired(n int) { + if s.mm != nil { + s.mm.ReportStateExpired(n) + } +} + +func (s *stateMetrics) ReportStatePurged(n int) { + if s.mm != nil { + s.mm.ReportStatePurged(n) + } +} + +func (s *stateMetrics) ReportStateEntries(scope string, n int) { + if s.mm != nil { + s.mm.ReportStateEntries(scope, n) + } +} + +// reportSuppressedWrite counts a write leg that a suppression path dropped. +// +// Suppression is applied to the rule as a whole, so a gate meant for the rule's +// alerting leg also silences its write leg on a different event stream. That is +// the documented behaviour, but it is invisible: the chain simply never forms, +// and no metric, log or alert says so. This makes it countable, under the same +// metric as the store's own rejections, so step 4 of "when a correlation rule +// does not fire" has something to look at. +func (rm *RuleManager) reportSuppressedWrite( + rule *typesv1.Rule, + compiled []statewrites.Compiled, + eventType utils.EventType, + reason string, +) { + if !hasWriteFor(compiled, eventType) { + return + } + rm.metrics.ReportStateWriteRejected(rule.ID, reason) +} diff --git a/pkg/rulemanager/statecontext_test.go b/pkg/rulemanager/statecontext_test.go new file mode 100644 index 0000000000..ba5fc82d9d --- /dev/null +++ b/pkg/rulemanager/statecontext_test.go @@ -0,0 +1,353 @@ +package rulemanager + +import ( + "testing" + "time" + + mapset "github.com/deckarep/golang-set/v2" + "github.com/hashicorp/golang-lru/v2/expirable" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/config" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/rulemanager/cel/libraries/state" + typesv1 "github.com/kubescape/node-agent/pkg/rulemanager/types/v1" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func execEnriched() *events.EnrichedEvent { + return &events.EnrichedEvent{ + Event: &utils.StructEvent{EventType: utils.ExecveEventType}, + } +} + +// The subtle failure this exists to prevent: a rule whose ONLY reference to exec +// is a stateWrites entry must still let exec events reach the rule loop. Without +// it the first leg of every cross-event rule is filtered out before evaluation +// and no chain ever forms. +func TestIsSupportedEventType_WriteOnlyLegIsSupported(t *testing.T) { + rule := typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + TTL: "10m", + }}, + }, + // Alerts on network only -- there is deliberately no exec expression. + Expressions: typesv1.RuleExpressions{ + RuleExpression: []typesv1.RuleExpression{ + {EventType: utils.NetworkEventType, Expression: "true"}, + }, + }, + } + + assert.True(t, isSupportedEventType([]typesv1.Rule{rule}, execEnriched()), + "an exec event must reach the loop for a rule that only WRITES on exec") +} + +func TestIsSupportedEventType_UnrelatedEventStillUnsupported(t *testing.T) { + rule := typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeDNS, + Scope: armotypes.StateScopeContainer, + Name: "n", + TTL: "10m", + }}, + }, + Expressions: typesv1.RuleExpressions{ + RuleExpression: []typesv1.RuleExpression{ + {EventType: utils.NetworkEventType, Expression: "true"}, + }, + }, + } + + assert.False(t, isSupportedEventType([]typesv1.Rule{rule}, execEnriched()), + "exec appears in neither the expressions nor the writes") +} + +func TestIsSupportedEventType_ExpressionStillWorksWithoutWrites(t *testing.T) { + rule := typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ID: "R1004"}, + Expressions: typesv1.RuleExpressions{ + RuleExpression: []typesv1.RuleExpression{ + {EventType: utils.ExecveEventType, Expression: "true"}, + }, + }, + } + assert.True(t, isSupportedEventType([]typesv1.Rule{rule}, execEnriched())) +} + +func testRuleManager(t *testing.T) *RuleManager { + t.Helper() + cfg := config.Config{} + cfg.CelStateStore = rulestate.Config{ + Enabled: true, + MaxSize: 1000, + MaxEntriesPerContainer: 100, + MaxEntriesForHost: 100, + MaxTTL: 30 * time.Minute, + AncestorMaxDepth: 8, + } + return &RuleManager{ + cfg: cfg, + stateWritesCache: expirable.NewLRU[string, *compiledWrites](512, nil, 0), + } +} + +func TestCompileStateWrites_NoWritesIsNil(t *testing.T) { + rm := testRuleManager(t) + compiled, scopeOf := rm.compileStateWrites(&typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ID: "R1004"}, + }) + assert.Nil(t, compiled) + assert.Nil(t, scopeOf) +} + +func TestCompileStateWrites_ValidClause(t *testing.T) { + rm := testRuleManager(t) + compiled, scopeOf := rm.compileStateWrites(&typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + Key: "string(event.pid)", + TTL: "10m", + }}, + }, + }) + require.Len(t, compiled, 1) + assert.Equal(t, utils.ExecveEventType, compiled[0].EventType) + assert.Equal(t, map[string]armotypes.StateScope{ + "mount_exec": armotypes.StateScopeContainer, + }, scopeOf) +} + +// A malformed clause must degrade that one rule to non-correlating, not take down +// evaluation for every other rule in the CRD. +func TestCompileStateWrites_InvalidClauseDegradesToNoWrites(t *testing.T) { + rm := testRuleManager(t) + compiled, scopeOf := rm.compileStateWrites(&typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeIdentity, // operator-only + Name: "mount_exec", + TTL: "10m", + }}, + }, + }) + assert.Nil(t, compiled) + assert.Nil(t, scopeOf) +} + +// TTL clamping has to use the configured max, not the write's own value. +func TestCompileStateWrites_ClampsToConfiguredMaxTTL(t *testing.T) { + rm := testRuleManager(t) + compiled, _ := rm.compileStateWrites(&typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + TTL: "99h", + }}, + }, + }) + require.Len(t, compiled, 1) + assert.Equal(t, 30*time.Minute, compiled[0].TTL) +} + +func TestHasWriteFor(t *testing.T) { + rm := testRuleManager(t) + compiled, _ := rm.compileStateWrites(&typesv1.Rule{ + RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + TTL: "10m", + }}, + }, + }) + assert.True(t, hasWriteFor(compiled, utils.ExecveEventType)) + assert.False(t, hasWriteFor(compiled, utils.NetworkEventType)) + assert.False(t, hasWriteFor(nil, utils.ExecveEventType)) +} + +// Container removal must reclaim that container's markers immediately -- a +// churning node would otherwise hold state for containers that no longer exist +// until TTL. +func TestPurgeScope_OnContainerRemovalDropsOnlyThatContainer(t *testing.T) { + rm := testRuleManager(t) + rm.stateStore = rulestate.NewStore(rm.cfg.CelStateStore, rulestate.NoopMetrics{}) + + set := func(scopeID string) { + require.NoError(t, rm.stateStore.Set(&rulestate.Entry{ + RuleID: "R1089", Name: "n", Key: "1", + Scope: armotypes.StateScopeContainer, ScopeID: scopeID, + Timestamp: time.Now(), ExpiresAt: time.Now().Add(time.Minute), + })) + } + set(rulestate.ContainerScopeID("abc")) + set(rulestate.ContainerScopeID("def")) + set(rulestate.HostScopeID()) + + rm.stateStore.PurgeScope(rulestate.ContainerScopeID("abc")) + + _, ok := rm.stateStore.Get("R1089", rulestate.ContainerScopeID("abc"), "n", "1") + assert.False(t, ok, "the removed container's markers must be gone") + + _, ok = rm.stateStore.Get("R1089", rulestate.ContainerScopeID("def"), "n", "1") + assert.True(t, ok, "a neighbouring container must be untouched") + + _, ok = rm.stateStore.Get("R1089", rulestate.HostScopeID(), "n", "1") + assert.True(t, ok, "host markers must survive a container removal") +} + +// The trap this guards: utils.TrimRuntimePrefix returns "" for an ID with no +// "//" separator, and ContainerScopeID("") is the HOST bucket. If the removal +// path ever trims the runtime container ID again, every container exit would wipe +// all host-process state instead of that container's. +func TestContainerScopeID_TrimmedRuntimeIDWouldHitTheHostBucket(t *testing.T) { + bare := "1a2b3c4d5e6f" + assert.Empty(t, utils.TrimRuntimePrefix(bare), + "TrimRuntimePrefix yields empty for a bare runtime ID") + assert.Equal(t, rulestate.HostScopeID(), rulestate.ContainerScopeID(utils.TrimRuntimePrefix(bare)), + "which would resolve to the host bucket -- purge must use the untrimmed ID") + assert.NotEqual(t, rulestate.HostScopeID(), rulestate.ContainerScopeID(bare)) +} + +// Pod scope was reclaimed by nothing. PurgeScope's only production call site +// passes a CONTAINER scope ID, so `p:/` entries outlived the pod they +// describe and sat in the store until TTL -- on a churning node, many dead pods' +// worth at once, all of it counting against the global ceiling. +// +// The pod is gone only when its LAST container goes, which is why this reuses the +// same "is any container of this pod still tracked" test the podToWlid cleanup +// already makes. +func TestPurgePodScope_ReclaimsWhenTheLastContainerGoes(t *testing.T) { + rm := testRuleManager(t) + rm.stateStore = rulestate.NewStore(rm.cfg.CelStateStore, rulestate.NoopMetrics{}) + rm.trackedContainers = mapset.NewSet[string]() + + podEntry := func() (*rulestate.Entry, bool) { + return rm.stateStore.Get("R1089", rulestate.PodScopeID("prod", "web-1"), "n", "1") + } + seed := func() { + require.NoError(t, rm.stateStore.Set(&rulestate.Entry{ + RuleID: "R1089", Name: "n", Key: "1", + Scope: armotypes.StateScopePod, ScopeID: rulestate.PodScopeID("prod", "web-1"), + Timestamp: time.Now(), ExpiresAt: time.Now().Add(time.Minute), + })) + } + + // A sibling container of the same pod is still running: the pod is alive, so + // its state must survive. Purging here would break a correlation chain that + // is still legitimately in progress. + seed() + rm.trackedContainers.Add(utils.CreateK8sContainerID("prod", "web-1", "sidecar")) + rm.purgePodScopeIfPodGone("prod", "web-1") + _, ok := podEntry() + assert.True(t, ok, "a pod with a container still tracked must keep its state") + + // A container of a DIFFERENT pod must not keep this pod alive. + rm.trackedContainers.Clear() + rm.trackedContainers.Add(utils.CreateK8sContainerID("prod", "web-2", "app")) + rm.purgePodScopeIfPodGone("prod", "web-1") + _, ok = podEntry() + assert.False(t, ok, "another pod's container must not keep this pod's state alive") + + // And the neighbouring pod is untouched. + require.NoError(t, rm.stateStore.Set(&rulestate.Entry{ + RuleID: "R1089", Name: "n", Key: "1", + Scope: armotypes.StateScopePod, ScopeID: rulestate.PodScopeID("prod", "web-2"), + Timestamp: time.Now(), ExpiresAt: time.Now().Add(time.Minute), + })) + rm.purgePodScopeIfPodGone("prod", "web-1") + _, ok = rm.stateStore.Get("R1089", rulestate.PodScopeID("prod", "web-2"), "n", "1") + assert.True(t, ok, "purging one pod must not touch its neighbour") +} + +// Compiling the write clause on every event meant a malformed rule logged an +// error on every matching event, forever -- thousands of identical lines per +// second from one bad rule on a busy node. The cache is what makes that one line +// per clause version, so it is worth pinning that the key tracks the clause. +func TestStateWritesCacheKey_TracksEveryFieldValidationReads(t *testing.T) { + base := func() *typesv1.Rule { + return &typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{ + ID: "R1089", + StateWrites: []armotypes.StateWrite{{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + Key: "string(event.pid)", + When: "is_mount", + TTL: "10m", + Value: map[string]any{"argv": "event.args", "comm": "event.comm"}, + }}, + }} + } + + require.Equal(t, stateWritesCacheKey(base()), stateWritesCacheKey(base()), + "an unchanged clause must hash the same, or the cache never hits") + + // Value is a map: iteration order varies between runs, so the digest must not. + reordered := base() + reordered.StateWrites[0].Value = map[string]any{"comm": "event.comm", "argv": "event.args"} + assert.Equal(t, stateWritesCacheKey(base()), stateWritesCacheKey(reordered), + "map iteration order must not change the key, or the cache thrashes") + + mutations := map[string]func(*typesv1.Rule){ + "rule ID": func(r *typesv1.Rule) { r.ID = "R1090" }, + "event type": func(r *typesv1.Rule) { r.StateWrites[0].EventType = armotypes.EventTypeNetwork }, + "scope": func(r *typesv1.Rule) { r.StateWrites[0].Scope = armotypes.StateScopePod }, + "name": func(r *typesv1.Rule) { r.StateWrites[0].Name = "other" }, + "key": func(r *typesv1.Rule) { r.StateWrites[0].Key = "string(event.ppid)" }, + "guard": func(r *typesv1.Rule) { r.StateWrites[0].When = "other" }, + "ttl": func(r *typesv1.Rule) { r.StateWrites[0].TTL = "20m" }, + "value expr": func(r *typesv1.Rule) { r.StateWrites[0].Value["argv"] = "event.cwd" }, + "value key": func(r *typesv1.Rule) { r.StateWrites[0].Value["extra"] = "event.cwd" }, + "extra write": func(r *typesv1.Rule) { + r.StateWrites = append(r.StateWrites, r.StateWrites[0]) + }, + } + for what, mutate := range mutations { + mutated := base() + mutate(mutated) + assert.NotEqual(t, stateWritesCacheKey(base()), stateWritesCacheKey(mutated), + "changing the %s must change the key, or an edited rule keeps its stale compilation", what) + } +} + +// A rule that declares no state must not be handed the previous rule's accessor. +// evalContext is built once per event and reused down the rule list, so skipping +// the seed without clearing the key would let a stateless rule read another +// rule's state -- the exact thing the receiver design makes inexpressible. +func TestSeedStateContext_KeyIsClearedForRulesWithoutState(t *testing.T) { + rm := testRuleManager(t) + rm.stateStore = rulestate.NewStore(rm.cfg.CelStateStore, rulestate.NoopMetrics{}) + + evalContext := map[string]any{} + withState := &typesv1.Rule{RuntimeRule: armotypes.RuntimeRule{ID: "R1089"}} + rm.seedStateContext(evalContext, withState, execEnriched(), + map[string]armotypes.StateScope{"mount_exec": armotypes.StateScopeContainer}, nil) + require.Contains(t, evalContext, state.AccessorContextKey) + + // The loop's else branch: a stateless rule follows. + delete(evalContext, state.AccessorContextKey) + assert.NotContains(t, evalContext, state.AccessorContextKey, + "a rule with no declared state must not inherit the previous rule's accessor") +} diff --git a/pkg/rulemanager/statewrites/executor.go b/pkg/rulemanager/statewrites/executor.go new file mode 100644 index 0000000000..923c553a02 --- /dev/null +++ b/pkg/rulemanager/statewrites/executor.go @@ -0,0 +1,205 @@ +package statewrites + +import ( + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/go-logger" + "github.com/kubescape/go-logger/helpers" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/kubescape/node-agent/pkg/utils" +) + +// evaluator is the slice of the CEL evaluator the executor needs. Declared here +// rather than imported so this package does not depend on the whole rule engine. +type evaluator interface { + EvaluateBoolExpressionWithContext(evalContext map[string]any, expression string) (bool, error) + EvaluateStringExpressionWithContext(evalContext map[string]any, expression string) (string, error) +} + +// Executor applies a rule's validated write clause for one event. +type Executor struct { + store *rulestate.Store + eval evaluator + metrics rulestate.Metrics +} + +func NewExecutor(store *rulestate.Store, eval evaluator, metrics rulestate.Metrics) *Executor { + if metrics == nil { + metrics = rulestate.NoopMetrics{} + } + return &Executor{store: store, eval: eval, metrics: metrics} +} + +// ScopeIDs resolves this event's ID for every scope node-agent supports. +// +// Scope IDs are always derived here from the event, never from rule input, which +// is what stops a rule reaching another container's bucket. A host / cgroup-0 +// process has no container ID and maps to the explicit host bucket rather than to +// the empty string, which would otherwise collide with node scope. +func ScopeIDs(enriched *events.EnrichedEvent) map[armotypes.StateScope]string { + ids := map[armotypes.StateScope]string{ + armotypes.StateScopeContainer: rulestate.ContainerScopeID(enriched.ContainerID), + armotypes.StateScopeNode: rulestate.NodeScopeID(), + } + if ns, pod := podIdentity(enriched); pod != "" { + ids[armotypes.StateScopePod] = rulestate.PodScopeID(ns, pod) + } + return ids +} + +func podIdentity(enriched *events.EnrichedEvent) (string, string) { + if enriched.Event == nil { + return "", "" + } + return enriched.Event.GetNamespace(), enriched.Event.GetPod() +} + +// exePathGetter and cwdGetter are satisfied by the process-bearing event types +// (exec, open, dns, ...) but not by all of them, so they are probed rather than +// required. An event without them simply stores no path. +type exePathGetter interface{ GetExePath() string } +type cwdGetter interface{ GetCwd() string } + +// Apply evaluates every write whose event type matches this event and stores the +// ones whose guard passes. +// +// It must be called AFTER the predicate has been evaluated. If a write landed +// first, a rule that reads and writes the same name on the same event type would +// satisfy itself from its own write on a single event. +// +// A store rejection is logged at debug and counted, never propagated: a full +// store must degrade correlation, not break alerting. +func (e *Executor) Apply( + compiled []Compiled, + ruleID string, + enriched *events.EnrichedEvent, + evalContext map[string]any, + eventTime time.Time, +) { + if e == nil || e.store == nil || len(compiled) == 0 || evalContext == nil { + return + } + // podIdentity and processOf below both tolerate a nil Event; without this the + // very next line would panic instead, so the package's nil handling would be + // inconsistent. Not reachable from the rule loop, which dereferences Event + // earlier, but the exported entry point should not depend on that. + if enriched == nil || enriched.Event == nil { + return + } + eventType := enriched.Event.GetEventType() + scopeIDs := ScopeIDs(enriched) + + for _, w := range compiled { + if w.EventType != eventType { + continue + } + + if w.When != "" { + ok, err := e.eval.EvaluateBoolExpressionWithContext(evalContext, w.When) + if err != nil { + logger.L().Debug("statewrites - guard evaluation failed", + helpers.Error(err), + helpers.String("rule", ruleID), + helpers.String("name", w.Name)) + e.metrics.ReportStateWriteRejected(ruleID, "guard_error") + continue + } + if !ok { + continue + } + } + + scopeID, ok := scopeIDs[w.Scope] + if !ok { + // pod scope on an event with no pod identity, e.g. a host process. + e.metrics.ReportStateWriteRejected(ruleID, "scope_unresolved") + continue + } + + key := "" + if w.Key != "" { + k, err := e.eval.EvaluateStringExpressionWithContext(evalContext, w.Key) + if err != nil { + logger.L().Debug("statewrites - key evaluation failed", + helpers.Error(err), + helpers.String("rule", ruleID), + helpers.String("name", w.Name)) + e.metrics.ReportStateWriteRejected(ruleID, "key_error") + continue + } + key = k + } + + entry := &rulestate.Entry{ + RuleID: ruleID, + Name: w.Name, + Scope: w.Scope, + ScopeID: scopeID, + Key: key, + EventType: armotypes.EventType(w.EventType), + Timestamp: eventTime, + ExpiresAt: eventTime.Add(w.TTL), + Process: processOf(enriched), + Value: e.evaluateValues(w, ruleID, evalContext), + } + + if err := e.store.Set(entry); err != nil { + logger.L().Debug("statewrites - store rejected the write", + helpers.Error(err), + helpers.String("rule", ruleID), + helpers.String("name", w.Name)) + } + } +} + +func (e *Executor) evaluateValues(w Compiled, ruleID string, evalContext map[string]any) map[string]any { + if len(w.Value) == 0 { + return nil + } + out := make(map[string]any, len(w.Value)) + for k, expr := range w.Value { + v, err := e.eval.EvaluateStringExpressionWithContext(evalContext, expr) + if err != nil { + logger.L().Debug("statewrites - value evaluation failed", + helpers.Error(err), + helpers.String("rule", ruleID), + helpers.String("name", w.Name), + helpers.String("valueKey", k)) + continue + } + out[k] = v + } + if len(out) == 0 { + return nil + } + return out +} + +// processOf snapshots the acting process onto the entry, so an alert on the far +// leg of a chain can describe the process at the near leg -- which by then may be +// long gone. +func processOf(enriched *events.EnrichedEvent) *armotypes.Process { + p := &armotypes.Process{ + PID: enriched.PID, + PPID: enriched.PPID, + } + if e, ok := enriched.Event.(utils.EnrichEvent); ok { + if p.PID == 0 { + p.PID = e.GetPID() + } + if p.PPID == 0 { + p.PPID = e.GetPpid() + } + p.Comm = e.GetComm() + p.Pcomm = e.GetPcomm() + } + if e, ok := enriched.Event.(exePathGetter); ok { + p.Path = e.GetExePath() + } + if e, ok := enriched.Event.(cwdGetter); ok { + p.Cwd = e.GetCwd() + } + return p +} diff --git a/pkg/rulemanager/statewrites/executor_test.go b/pkg/rulemanager/statewrites/executor_test.go new file mode 100644 index 0000000000..7284576c4d --- /dev/null +++ b/pkg/rulemanager/statewrites/executor_test.go @@ -0,0 +1,313 @@ +package statewrites + +import ( + "errors" + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/ebpf/events" + "github.com/kubescape/node-agent/pkg/rulestate" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeEvaluator returns canned results per expression, so the executor's own +// logic is under test rather than CEL's. +type fakeEvaluator struct { + bools map[string]bool + strings map[string]string + boolErr map[string]bool + strErr map[string]bool +} + +func newFakeEvaluator() *fakeEvaluator { + return &fakeEvaluator{ + bools: map[string]bool{}, + strings: map[string]string{}, + boolErr: map[string]bool{}, + strErr: map[string]bool{}, + } +} + +func (f *fakeEvaluator) EvaluateBoolExpressionWithContext(_ map[string]any, expr string) (bool, error) { + if f.boolErr[expr] { + return false, errors.New("boom") + } + return f.bools[expr], nil +} + +func (f *fakeEvaluator) EvaluateStringExpressionWithContext(_ map[string]any, expr string) (string, error) { + if f.strErr[expr] { + return "", errors.New("boom") + } + return f.strings[expr], nil +} + +func testStore(t *testing.T) *rulestate.Store { + t.Helper() + return rulestate.NewStore(rulestate.Config{ + Enabled: true, + MaxSize: 1000, + MaxEntriesPerContainer: 100, + MaxEntriesForHost: 100, + MaxTTL: 30 * time.Minute, + }, rulestate.NoopMetrics{}) +} + +func execEvent(containerID string) *events.EnrichedEvent { + return &events.EnrichedEvent{ + Event: &utils.StructEvent{ + EventType: utils.ExecveEventType, + ContainerID: containerID, + Comm: "xmrig", + Pcomm: "sh", + ExePath: "/mnt/data/xmrig", + Cwd: "/mnt/data", + Namespace: "prod", + Pod: "web-1", + Pid: 4471, + Ppid: 900, + }, + ContainerID: containerID, + PID: 4471, + PPID: 900, + } +} + +func mustCompile(t *testing.T, w armotypes.StateWrite) []Compiled { + t.Helper() + c, err := Validate(w, "R1089", 30*time.Minute) + require.NoError(t, err) + return []Compiled{c} +} + +func TestApply_GuardTrueStoresEntry(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.bools["is_mount"] = true + ev.strings["string(event.pid)"] = "4471" + + w := base() + w.When = "is_mount" + eventTime := time.Now() + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, eventTime) + + got, ok := store.Get("R1089", "c:abc", "mount_exec", "4471") + require.True(t, ok) + assert.Equal(t, eventTime, got.Timestamp, + "the entry must carry the event time, so _ts guards compare the same clock the predicate saw") + assert.Equal(t, eventTime.Add(10*time.Minute), got.ExpiresAt) + require.NotNil(t, got.Process) + assert.Equal(t, uint32(4471), got.Process.PID) + assert.Equal(t, "xmrig", got.Process.Comm) + assert.Equal(t, "/mnt/data/xmrig", got.Process.Path) + assert.Equal(t, armotypes.EventTypeExec, got.EventType) +} + +func TestApply_GuardFalseStoresNothing(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.bools["is_mount"] = false + + w := base() + w.When = "is_mount" + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + assert.Equal(t, 0, store.Len()) +} + +func TestApply_AbsentGuardAlwaysStores(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + + w := base() // When == "" + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + _, ok := store.Get("R1089", "c:abc", "mount_exec", "4471") + assert.True(t, ok) +} + +// Host processes carry no container ID and must land in the explicit host bucket, +// not under the empty string. +func TestApply_HostProcessUsesHostScopeID(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + + NewExecutor(store, ev, nil).Apply(mustCompile(t, base()), "R1089", execEvent(""), + map[string]any{}, time.Now()) + + _, ok := store.Get("R1089", rulestate.HostScopeID(), "mount_exec", "4471") + assert.True(t, ok, "a host process must be addressable under %q", rulestate.HostScopeID()) +} + +// Write-without-alerting: the write leg's event type need not appear in any +// ruleExpression. The executor sees only compiled writes, so this must hold. +func TestApply_StoresForEventTypeWithNoRuleExpression(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + + // No ruleExpression exists anywhere for this rule; only the write clause. + NewExecutor(store, ev, nil).Apply(mustCompile(t, base()), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + assert.Equal(t, 1, store.Len()) +} + +func TestApply_SkipsWritesForOtherEventTypes(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + + w := base() + w.EventType = armotypes.EventTypeNetwork // event is exec + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + assert.Equal(t, 0, store.Len()) +} + +func TestApply_ValueExpressionsAreStored(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + ev.strings["event.args"] = "-o pool:4444" + + w := base() + w.Value = map[string]any{"argv": "event.args"} + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + got, ok := store.Get("R1089", "c:abc", "mount_exec", "4471") + require.True(t, ok) + assert.Equal(t, map[string]any{"argv": "-o pool:4444"}, got.Value) +} + +func TestApply_EmptyKeyYieldsOneScopeWideEntry(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + + w := base() + w.Key = "" + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + _, ok := store.Get("R1089", "c:abc", "mount_exec", "") + assert.True(t, ok) + assert.Equal(t, 1, store.Len()) +} + +// A broken guard must not store; a broken key must not store under a wrong key. +func TestApply_ExpressionErrorsSkipTheWrite(t *testing.T) { + t.Run("guard error", func(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.boolErr["bad"] = true + w := base() + w.When = "bad" + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + assert.Equal(t, 0, store.Len()) + }) + + t.Run("key error", func(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strErr["string(event.pid)"] = true + NewExecutor(store, ev, nil).Apply(mustCompile(t, base()), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + assert.Equal(t, 0, store.Len()) + }) +} + +// Pod scope on an event with no pod identity has nothing to resolve against; the +// write is dropped rather than landing in a bogus bucket. +func TestApply_PodScopeWithoutPodIdentityIsDropped(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + + w := base() + w.Scope = armotypes.StateScopePod + + enriched := execEvent("abc") + enriched.Event.(*utils.StructEvent).Pod = "" + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", enriched, + map[string]any{}, time.Now()) + + assert.Equal(t, 0, store.Len()) +} + +func TestApply_PodScopeUsesNamespacedID(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.strings["string(event.pid)"] = "4471" + + w := base() + w.Scope = armotypes.StateScopePod + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + _, ok := store.Get("R1089", rulestate.PodScopeID("prod", "web-1"), + "mount_exec", "4471") + assert.True(t, ok) +} + +// A guard that itself reads state is how multi-step chains are built; the +// executor must not treat a state-reading guard specially. +func TestApply_GuardMayItselfDependOnState(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + ev.bools[`state.has("step1")`] = true + ev.strings["string(event.pid)"] = "4471" + + w := base() + w.Name = "step2" + w.When = `state.has("step1")` + + NewExecutor(store, ev, nil).Apply(mustCompile(t, w), "R1089", execEvent("abc"), + map[string]any{}, time.Now()) + + _, ok := store.Get("R1089", "c:abc", "step2", "4471") + assert.True(t, ok) +} + +func TestApply_NilSafeOnMissingPieces(t *testing.T) { + store := testStore(t) + ev := newFakeEvaluator() + e := NewExecutor(store, ev, nil) + + // No writes, and a nil eval context: both must be no-ops, not panics. + e.Apply(nil, "R1089", execEvent("abc"), map[string]any{}, time.Now()) + e.Apply(mustCompile(t, base()), "R1089", execEvent("abc"), nil, time.Now()) + assert.Equal(t, 0, store.Len()) +} + +func TestScopeIDs_ResolvesFromTheEventOnly(t *testing.T) { + ids := ScopeIDs(execEvent("abc")) + assert.Equal(t, "c:abc", ids[armotypes.StateScopeContainer]) + assert.Equal(t, rulestate.NodeScopeID(), ids[armotypes.StateScopeNode]) + assert.Equal(t, "p:prod/web-1", ids[armotypes.StateScopePod]) + + // Host: container scope resolves to the host bucket, pod scope is absent. + hostIDs := ScopeIDs(&events.EnrichedEvent{ + Event: &utils.StructEvent{EventType: utils.ExecveEventType}, + ContainerID: "", + }) + assert.Equal(t, rulestate.HostScopeID(), hostIDs[armotypes.StateScopeContainer]) + _, hasPod := hostIDs[armotypes.StateScopePod] + assert.False(t, hasPod) +} diff --git a/pkg/rulemanager/statewrites/validate.go b/pkg/rulemanager/statewrites/validate.go new file mode 100644 index 0000000000..42bcb11d45 --- /dev/null +++ b/pkg/rulemanager/statewrites/validate.go @@ -0,0 +1,152 @@ +// Package statewrites validates and executes a rule's declarative `stateWrites` +// clause: the only way a CEL rule writes to the state store. +// +// Writes are declarative rather than a CEL setter function on purpose. A setter +// could be skipped by boolean short-circuiting, reordered by the static +// optimiser, and could never express "remember this without alerting" -- which is +// exactly what the first leg of a cross-event rule needs. +package statewrites + +import ( + "fmt" + "strings" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/go-logger" + "github.com/kubescape/go-logger/helpers" + "github.com/kubescape/node-agent/pkg/utils" +) + +// Compiled is a validated StateWrite ready for the rule loop. +// +// EventType is utils.EventType, not armotypes.EventType: the rule loop compares +// against utils.EventType, and doing the conversion once here keeps the +// comparison typed rather than a string cast at every event. +type Compiled struct { + EventType utils.EventType + Scope armotypes.StateScope + Name string + Key string + When string + Value map[string]string + TTL time.Duration +} + +// nodeAgentScopes are the scopes node-agent can resolve. identity is +// operator-only: there is no caller identity on an eBPF event. +var nodeAgentScopes = map[armotypes.StateScope]struct{}{ + armotypes.StateScopeContainer: {}, + armotypes.StateScopePod: {}, + armotypes.StateScopeNode: {}, +} + +// Validate checks one write and converts it for the rule loop. +// +// Everything here fails at rule load rather than at runtime, because every one of +// these mistakes otherwise produces a rule that loads cleanly and silently never +// fires -- the worst possible failure for a detection. +func Validate(w armotypes.StateWrite, ruleID string, maxTTL time.Duration) (Compiled, error) { + fail := func(format string, args ...any) (Compiled, error) { + return Compiled{}, fmt.Errorf("rule %s: stateWrites[%q]: "+format, + append([]any{ruleID, w.Name}, args...)...) + } + + if w.Name == "" { + return fail("name must not be empty") + } + if strings.HasPrefix(w.Name, "_") { + return fail("name must not begin with %q, which is reserved for engine provenance", "_") + } + + // IsValidStateWriteEventType, not IsKnownEventType: the latter accepts + // EventTypeAll, which is a rule-binding wildcard rather than an event stream, + // so it would produce a write leg that never matches a concrete event. + if !armotypes.IsValidStateWriteEventType(w.EventType) { + return fail("eventType %q is not an event stream that can drive a write", w.EventType) + } + eventType := utils.EventType(w.EventType) + if !utils.IsValidEventType(eventType) { + return fail("eventType %q is not emitted by node-agent", w.EventType) + } + + if _, ok := nodeAgentScopes[w.Scope]; !ok { + return fail("scope %q is not resolvable by node-agent (want container, pod or node)", w.Scope) + } + + ttl, err := time.ParseDuration(w.TTL) + if err != nil { + return fail("ttl %q is not a duration: %w", w.TTL, err) + } + if ttl <= 0 { + return fail("ttl %q must be positive; an entry born expired is a silently dead rule", w.TTL) + } + if ttl > maxTTL { + logger.L().Warning("statewrites - clamping ttl to the configured maximum", + helpers.String("rule", ruleID), + helpers.String("name", w.Name), + helpers.String("requested", ttl.String()), + helpers.String("maxTtl", maxTTL.String())) + ttl = maxTTL + } + + var values map[string]string + if len(w.Value) > 0 { + values = make(map[string]string, len(w.Value)) + for k, v := range w.Value { + if k == "" { + return fail("value keys must not be empty") + } + if strings.HasPrefix(k, "_") { + return fail("value key %q must not begin with %q, which is reserved for engine provenance", k, "_") + } + s, ok := v.(string) + if !ok { + return fail("value %q must be a CEL expression string, got %T", k, v) + } + values[k] = s + } + } + + return Compiled{ + EventType: eventType, + Scope: w.Scope, + Name: w.Name, + Key: w.Key, + When: w.When, + Value: values, + TTL: ttl, + }, nil +} + +// ValidateAll validates a rule's whole clause and returns the name -> scope map +// that reads resolve against. +// +// A name must map to exactly one scope. Reads take no scope argument -- they infer +// it from the name -- so the same name in two scopes would make every read of it +// ambiguous. Declaring one name across several event types in the SAME scope is +// the normal bidirectional idiom and is allowed. +func ValidateAll(writes []armotypes.StateWrite, ruleID string, maxTTL time.Duration) ([]Compiled, map[string]armotypes.StateScope, error) { + if len(writes) == 0 { + return nil, nil, nil + } + + compiled := make([]Compiled, 0, len(writes)) + scopeOf := make(map[string]armotypes.StateScope, len(writes)) + + for _, w := range writes { + c, err := Validate(w, ruleID, maxTTL) + if err != nil { + return nil, nil, err + } + if existing, ok := scopeOf[c.Name]; ok && existing != c.Scope { + return nil, nil, fmt.Errorf( + "rule %s: stateWrites[%q]: declared in both scope %q and scope %q; a name must have exactly one scope because reads infer it from the name", + ruleID, c.Name, existing, c.Scope) + } + scopeOf[c.Name] = c.Scope + compiled = append(compiled, c) + } + + return compiled, scopeOf, nil +} diff --git a/pkg/rulemanager/statewrites/validate_test.go b/pkg/rulemanager/statewrites/validate_test.go new file mode 100644 index 0000000000..2d576e2f64 --- /dev/null +++ b/pkg/rulemanager/statewrites/validate_test.go @@ -0,0 +1,203 @@ +package statewrites + +import ( + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func base() armotypes.StateWrite { + return armotypes.StateWrite{ + EventType: armotypes.EventTypeExec, + Scope: armotypes.StateScopeContainer, + Name: "mount_exec", + Key: "string(event.pid)", + TTL: "10m", + } +} + +func TestValidate_Accepts(t *testing.T) { + c, err := Validate(base(), "R1089", 30*time.Minute) + require.NoError(t, err) + assert.Equal(t, 10*time.Minute, c.TTL) + assert.Equal(t, "mount_exec", c.Name) + assert.Equal(t, utils.ExecveEventType, c.EventType, + "the rule loop compares utils.EventType, so Validate must convert") +} + +func TestValidate_ClampsTTLToMax(t *testing.T) { + w := base() + w.TTL = "24h" + c, err := Validate(w, "R1089", 30*time.Minute) + require.NoError(t, err) + assert.Equal(t, 30*time.Minute, c.TTL, "no rule may pin memory indefinitely") +} + +func TestValidate_RejectsUnknownEventType(t *testing.T) { + w := base() + w.EventType = armotypes.EventType("nonsense") + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err, "a rule naming a nonexistent event stream must fail loudly at load, not never match at runtime") +} + +// EventTypeAll is a rule-binding wildcard, not an event stream. Accepting it +// would yield a rule that loads cleanly and then never matches a concrete event. +func TestValidate_RejectsAllWildcardAsDriver(t *testing.T) { + w := base() + w.EventType = armotypes.EventTypeAll + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err, "the binding wildcard cannot drive a state write") +} + +func TestValidate_RejectsIdentityScopeInNodeAgent(t *testing.T) { + w := base() + w.Scope = armotypes.StateScopeIdentity + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err, "identity scope belongs to the operator") +} + +func TestValidate_AcceptsContainerPodAndNodeScopes(t *testing.T) { + for _, scope := range []armotypes.StateScope{ + armotypes.StateScopeContainer, + armotypes.StateScopePod, + armotypes.StateScopeNode, + } { + w := base() + w.Scope = scope + c, err := Validate(w, "R1089", 30*time.Minute) + require.NoError(t, err, "scope %q must be accepted", scope) + assert.Equal(t, scope, c.Scope) + } +} + +func TestValidate_RejectsEmptyScope(t *testing.T) { + w := base() + w.Scope = "" + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) +} + +func TestValidate_RejectsReservedValueKeys(t *testing.T) { + w := base() + w.Value = map[string]any{"_pid": "event.pid"} + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err, "author values must not shadow engine provenance") +} + +func TestValidate_RejectsReservedNamePrefix(t *testing.T) { + w := base() + w.Name = "_internal" + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) +} + +func TestValidate_RejectsEmptyNameAndBadTTL(t *testing.T) { + w := base() + w.Name = "" + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) + + w = base() + w.TTL = "not-a-duration" + _, err = Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) +} + +func TestValidate_RejectsNonPositiveTTL(t *testing.T) { + for _, ttl := range []string{"", "0s", "-5m"} { + w := base() + w.TTL = ttl + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err, "TTL %q must be rejected: an entry that is born expired is a silently dead rule", ttl) + } +} + +func TestValidate_AllowsEmptyKeyForScopeWideMarker(t *testing.T) { + w := base() + w.Key = "" + c, err := Validate(w, "R1089", 30*time.Minute) + require.NoError(t, err) + assert.Empty(t, c.Key) +} + +func TestValidate_AcceptsStringValueExpressions(t *testing.T) { + w := base() + w.Value = map[string]any{"argv": "event.args"} + c, err := Validate(w, "R1089", 30*time.Minute) + require.NoError(t, err) + assert.Equal(t, map[string]string{"argv": "event.args"}, c.Value) +} + +// Values are CEL expression strings. A non-string would otherwise be stored as a +// literal, which silently is not what the author asked for. +func TestValidate_RejectsNonStringValueExpressions(t *testing.T) { + w := base() + w.Value = map[string]any{"threshold": 5} + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) +} + +func TestValidate_RejectsEmptyValueKey(t *testing.T) { + w := base() + w.Value = map[string]any{"": "event.args"} + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) +} + +// Errors must name the rule and the write, or a bad rule in a 50-rule CRD is not +// diagnosable from the log line. +func TestValidate_ErrorNamesRuleAndWrite(t *testing.T) { + w := base() + w.TTL = "nope" + _, err := Validate(w, "R1089", 30*time.Minute) + require.Error(t, err) + assert.Contains(t, err.Error(), "R1089") + assert.Contains(t, err.Error(), "mount_exec") +} + +func TestValidateAll_ReportsFirstFailureAndScopeMap(t *testing.T) { + good := base() + other := base() + other.Name = "egress_seen" + other.Scope = armotypes.StateScopeNode + + compiled, scopeOf, err := ValidateAll([]armotypes.StateWrite{good, other}, "R1089", 30*time.Minute) + require.NoError(t, err) + require.Len(t, compiled, 2) + assert.Equal(t, map[string]armotypes.StateScope{ + "mount_exec": armotypes.StateScopeContainer, + "egress_seen": armotypes.StateScopeNode, + }, scopeOf, "the scope map is what lets a read resolve its scope from the name alone") + + bad := base() + bad.Name = "" + _, _, err = ValidateAll([]armotypes.StateWrite{good, bad}, "R1089", 30*time.Minute) + require.Error(t, err) +} + +// The same name declared twice with different scopes makes a read ambiguous. +func TestValidateAll_RejectsSameNameInTwoScopes(t *testing.T) { + a := base() + b := base() + b.EventType = armotypes.EventTypeNetwork + b.Scope = armotypes.StateScopeNode + + _, _, err := ValidateAll([]armotypes.StateWrite{a, b}, "R1089", 30*time.Minute) + require.Error(t, err, "a name must map to exactly one scope, or reads cannot resolve it") +} + +// The bidirectional idiom: one name, two event types, same scope. Must be legal. +func TestValidateAll_AllowsSameNameSameScopeAcrossEventTypes(t *testing.T) { + a := base() + b := base() + b.EventType = armotypes.EventTypeNetwork + + compiled, scopeOf, err := ValidateAll([]armotypes.StateWrite{a, b}, "R1089", 30*time.Minute) + require.NoError(t, err) + assert.Len(t, compiled, 2) + assert.Len(t, scopeOf, 1) +} diff --git a/pkg/rulemanager/types/failure.go b/pkg/rulemanager/types/failure.go index 9c632ec19a..361fa46f73 100644 --- a/pkg/rulemanager/types/failure.go +++ b/pkg/rulemanager/types/failure.go @@ -27,6 +27,7 @@ type GenericRuleFailure struct { Extra interface{} IsTriggerAlert bool SourceContext contextdetection.EventSourceContext + CorrelationAlert armotypes.CorrelationAlert } type RuleFailure interface { @@ -40,6 +41,8 @@ type RuleFailure interface { GetTriggerEvent() utils.EnrichEvent // Get Rule Description GetRuleAlert() armotypes.RuleAlert + // Get Correlation Alert -- the state entries this rule read to fire + GetCorrelationAlert() armotypes.CorrelationAlert // Get K8s Runtime Details GetRuntimeAlertK8sDetails() armotypes.RuntimeAlertK8sDetails // Get ECS Runtime Details @@ -85,6 +88,8 @@ type RuleFailure interface { SetIsTriggerAlert(isTriggerAlert bool) // Set Source Context SetSourceContext(sourceContext contextdetection.EventSourceContext) + // Set Correlation Alert + SetCorrelationAlert(correlationAlert armotypes.CorrelationAlert) } func (rule *GenericRuleFailure) GetBaseRuntimeAlert() armotypes.BaseRuntimeAlert { @@ -103,6 +108,14 @@ func (rule *GenericRuleFailure) GetRuleAlert() armotypes.RuleAlert { return rule.RuleAlert } +func (rule *GenericRuleFailure) GetCorrelationAlert() armotypes.CorrelationAlert { + return rule.CorrelationAlert +} + +func (rule *GenericRuleFailure) SetCorrelationAlert(correlationAlert armotypes.CorrelationAlert) { + rule.CorrelationAlert = correlationAlert +} + func (rule *GenericRuleFailure) GetRuntimeAlertK8sDetails() armotypes.RuntimeAlertK8sDetails { return rule.RuntimeAlertK8sDetails } diff --git a/pkg/rulemanager/types/v1/rule_embedding_test.go b/pkg/rulemanager/types/v1/rule_embedding_test.go new file mode 100644 index 0000000000..5b9c0a3300 --- /dev/null +++ b/pkg/rulemanager/types/v1/rule_embedding_test.go @@ -0,0 +1,168 @@ +package types + +import ( + "encoding/json" + "testing" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/kubescape/node-agent/pkg/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + k8sruntime "k8s.io/apimachinery/pkg/runtime" +) + +// A representative existing rule (R1004 shape). This must survive embedding +// completely unchanged -- it is the regression gate. +const existingRuleJSON = `{ + "enabled": true, + "id": "R1004", + "name": "Process executed from mount", + "description": "Detecting exec calls from mounted paths.", + "expressions": { + "message": "'msg'", + "uniqueId": "event.comm", + "ruleExpression": [{"eventType": "exec", "expression": "true"}] + }, + "profileDependency": 1, + "profileDataRequired": {"execs": "all"}, + "severity": 5, + "supportPolicy": false, + "isTriggerAlert": true, + "mitreTactic": "TA0002", + "mitreTechnique": "T1059", + "tags": ["exec", "mount"] +}` + +func TestRule_ExistingFieldsUnchangedAfterEmbedding(t *testing.T) { + var r Rule + require.NoError(t, json.Unmarshal([]byte(existingRuleJSON), &r)) + + // Promoted from the embedded RuntimeRule. + assert.True(t, r.Enabled) + assert.Equal(t, "R1004", r.ID) + assert.Equal(t, "Process executed from mount", r.Name) + assert.Equal(t, 5, r.Severity) + assert.Equal(t, armotypes.ProfileDependency(1), r.ProfileDependency) + assert.Equal(t, []string{"exec", "mount"}, r.Tags) + assert.True(t, r.IsTriggerAlert) + assert.Equal(t, "TA0002", r.MitreTactic) + + // Shadowed: still node-agent's own type, still utils.EventType. + require.Len(t, r.Expressions.RuleExpression, 1) + assert.Equal(t, utils.ExecveEventType, r.Expressions.RuleExpression[0].EventType) + assert.Equal(t, "event.comm", r.Expressions.UniqueID) + + // Shadowed: node-agent's FieldRequirement semantics, including Declared. + require.NotNil(t, r.ProfileDataRequired) + assert.True(t, r.ProfileDataRequired.Execs.All) + assert.True(t, r.ProfileDataRequired.Execs.Declared) + assert.False(t, r.ProfileDataRequired.Opens.Declared, + "an absent surface must stay undeclared -- this is the semantics armotypes lacks") +} + +func TestRule_StateWritesAreReadableViaEmbedding(t *testing.T) { + const withState = `{ + "id": "R1089", + "stateWrites": [{ + "eventType": "exec", + "when": "true", + "scope": "container", + "name": "mount_exec", + "key": "string(event.pid)", + "ttl": "10m" + }], + "expressions": {"message": "'m'", "uniqueId": "'u'", "ruleExpression": []} + }` + + var r Rule + require.NoError(t, json.Unmarshal([]byte(withState), &r)) + + require.Len(t, r.StateWrites, 1) + w := r.StateWrites[0] + assert.Equal(t, armotypes.EventTypeExec, w.EventType) + assert.Equal(t, armotypes.StateScopeContainer, w.Scope) + assert.Equal(t, "mount_exec", w.Name) + assert.Equal(t, "10m", w.TTL) +} + +func TestRule_PrefilterStillExcludedFromSerialization(t *testing.T) { + data, err := json.Marshal(Rule{}) + require.NoError(t, err) + assert.NotContains(t, string(data), "Prefilter") + assert.NotContains(t, string(data), "prefilter") +} + +func TestRule_ShadowedFieldWinsOverEmbedded(t *testing.T) { + // Proves the depth rule for encoding/json: the outer Expressions is + // populated, and the embedded RuntimeRule.Expressions is left at its zero + // value. If this ever inverts, EventType silently becomes + // armotypes.EventType and the rule loop stops matching. + var r Rule + require.NoError(t, json.Unmarshal([]byte(existingRuleJSON), &r)) + assert.Len(t, r.Expressions.RuleExpression, 1) + assert.Empty(t, r.RuntimeRule.Expressions.RuleExpression, + "embedded Expressions must stay unused; the shadow is deliberate") +} + +// unstructuredRule is the production decoding path: rules arrive from the CRD as +// unstructured maps and are converted by apimachinery, NOT by encoding/json. +// Unlike encoding/json, apimachinery has no depth-based conflict resolution -- +// it visits every field of the outer struct independently -- so the two decoders +// genuinely disagree about the shadowed fields. What must hold on BOTH paths is +// that the depth-0 shadow (the one all node-agent code reads) is correct. +func unstructuredRule(t *testing.T, raw string) Rule { + t.Helper() + var m map[string]any + require.NoError(t, json.Unmarshal([]byte(raw), &m)) + + var r Rule + require.NoError(t, k8sruntime.DefaultUnstructuredConverter.FromUnstructured(m, &r)) + return r +} + +func TestRule_DecodesViaApimachineryLikeTheCRDPath(t *testing.T) { + r := unstructuredRule(t, existingRuleJSON) + + assert.True(t, r.Enabled) + assert.Equal(t, "R1004", r.ID) + assert.Equal(t, 5, r.Severity) + assert.Equal(t, armotypes.ProfileDependency(1), r.ProfileDependency) + assert.Equal(t, []string{"exec", "mount"}, r.Tags) + assert.Equal(t, "TA0002", r.MitreTactic) + + // The field the rule loop actually compares against. + require.Len(t, r.Expressions.RuleExpression, 1) + assert.Equal(t, utils.ExecveEventType, r.Expressions.RuleExpression[0].EventType) + assert.Equal(t, "event.comm", r.Expressions.UniqueID) + + require.NotNil(t, r.ProfileDataRequired) + assert.True(t, r.ProfileDataRequired.Execs.All) + assert.True(t, r.ProfileDataRequired.Execs.Declared) + assert.False(t, r.ProfileDataRequired.Opens.Declared) +} + +func TestRule_StateWritesDecodeViaApimachinery(t *testing.T) { + const withState = `{ + "id": "R1089", + "stateWrites": [{ + "eventType": "exec", + "scope": "container", + "name": "mount_exec", + "key": "string(event.pid)", + "value": {"argv": "event.args"}, + "ttl": "10m" + }], + "expressions": {"message": "'m'", "uniqueId": "'u'", "ruleExpression": []} + }` + + r := unstructuredRule(t, withState) + + require.Len(t, r.StateWrites, 1) + w := r.StateWrites[0] + assert.Equal(t, armotypes.EventTypeExec, w.EventType) + assert.Equal(t, armotypes.StateScopeContainer, w.Scope) + assert.Equal(t, "mount_exec", w.Name) + assert.Equal(t, "string(event.pid)", w.Key) + assert.Equal(t, "10m", w.TTL) + assert.Equal(t, map[string]any{"argv": "event.args"}, w.Value) +} diff --git a/pkg/rulemanager/types/v1/types.go b/pkg/rulemanager/types/v1/types.go index 20e387552c..90e31ea2d4 100644 --- a/pkg/rulemanager/types/v1/types.go +++ b/pkg/rulemanager/types/v1/types.go @@ -18,23 +18,33 @@ type RulesSpec struct { Rules []Rule `json:"rules" yaml:"rules"` } +// Rule is node-agent's view of a rule from the Rules CRD. +// +// It embeds armotypes.RuntimeRule so the CRD contract -- including StateWrites -- +// has exactly one definition, shared with the operator. Two fields are +// deliberately SHADOWED because their types differ from the shared root: +// +// - Expressions: node-agent's RuleExpression uses utils.EventType, which covers +// all node-agent event streams and is the type the rule loop compares +// against. The embedded RuntimeRule.Expressions is unused. +// - ProfileDataRequired: node-agent's FieldRequirement carries a Declared flag +// distinguishing "absent" from "present but empty", and rejects unknown keys +// at unmarshal. armotypes.ProfileDataField has neither. +// +// The two decoders that reach this struct treat the shadows differently. +// encoding/json resolves same-tag conflicts by depth, so only these depth-0 +// fields are populated. apimachinery's converter -- the production CRD path -- +// has no depth rule and visits every field independently, so it populates the +// embedded copies as well. Either way the depth-0 fields are what all +// node-agent code reads; never read the embedded copies. rule_embedding_test.go +// pins the behaviour of both decoders. type Rule struct { - Enabled bool `json:"enabled" yaml:"enabled"` - ID string `json:"id" yaml:"id"` - Name string `json:"name" yaml:"name"` - Description string `json:"description" yaml:"description"` - Expressions RuleExpressions `json:"expressions" yaml:"expressions"` - ProfileDependency armotypes.ProfileDependency `json:"profileDependency" yaml:"profileDependency"` - ProfileDataRequired *ProfileDataRequired `json:"profileDataRequired,omitempty" yaml:"profileDataRequired,omitempty"` - Severity int `json:"severity" yaml:"severity"` - SupportPolicy bool `json:"supportPolicy" yaml:"supportPolicy"` - Tags []string `json:"tags" yaml:"tags"` - State map[string]any `json:"state,omitempty" yaml:"state,omitempty"` - AgentVersionRequirement string `json:"agentVersionRequirement" yaml:"agentVersionRequirement"` - IsTriggerAlert bool `json:"isTriggerAlert" yaml:"isTriggerAlert"` - MitreTactic string `json:"mitreTactic" yaml:"mitreTactic"` - MitreTechnique string `json:"mitreTechnique" yaml:"mitreTechnique"` - Prefilter *prefilter.Params `json:"-" yaml:"-"` + armotypes.RuntimeRule `json:",inline" yaml:",inline"` + + Expressions RuleExpressions `json:"expressions" yaml:"expressions"` + ProfileDataRequired *ProfileDataRequired `json:"profileDataRequired,omitempty" yaml:"profileDataRequired,omitempty"` + + Prefilter *prefilter.Params `json:"-" yaml:"-"` } type RuleExpressions struct { diff --git a/pkg/rulestate/store.go b/pkg/rulestate/store.go new file mode 100644 index 0000000000..0acba0ea5d --- /dev/null +++ b/pkg/rulestate/store.go @@ -0,0 +1,299 @@ +package rulestate + +import ( + "context" + "hash/fnv" + "sync" + "sync/atomic" + "time" +) + +const shardCount = 16 + +// defaultWriteSweepInterval bounds write-path sweeping when no sweep interval is +// configured, so a missing setting cannot reinstate the per-write full walk. +const defaultWriteSweepInterval = time.Second + +type entryKey struct{ ruleID, name, key string } + +type bucket struct { + entries map[entryKey]*Entry +} + +type shard struct { + mu sync.RWMutex + scopes map[string]*bucket +} + +// Store is a bounded, TTL-expiring set of Entries sharded by scope ID. +// +// Sharding by scope ID (not by full key) is deliberate: it makes the per-scope +// cap a plain len(), makes container-removal purge a single map delete, and keeps +// one container's write churn off its neighbours' locks. +type Store struct { + cfg Config + metrics Metrics + shards [shardCount]*shard + + // size is atomic rather than mutex-guarded: it is read on every write, and a + // single global mutex there would serialise exactly what the sharding exists + // to keep independent. + size atomic.Int64 + + // lastSweepNs bounds how often the WRITE path may sweep. Without it, every + // write at the ceiling triggers a full all-shard walk -- see sweepIfDue. + lastSweepNs atomic.Int64 +} + +func NewStore(cfg Config, metrics Metrics) *Store { + s := &Store{cfg: cfg, metrics: metrics} + for i := range s.shards { + s.shards[i] = &shard{scopes: make(map[string]*bucket)} + } + return s +} + +func (s *Store) shardFor(scopeID string) *shard { + h := fnv.New32a() + _, _ = h.Write([]byte(scopeID)) + return s.shards[h.Sum32()%shardCount] +} + +// scopeCap picks the cap for a bucket. +// +// The larger cap applies to the two node-wide buckets -- the host +// pseudo-container and node scope itself. Neither holds one workload: they are +// shared by every rule and every process on the node, and neither is ever +// reclaimed by PurgeScope, so both rely on TTL and need the headroom. Bounding +// node scope by the per-container cap would starve it far sooner than intended on +// a busy node. +// +// Pod scope deliberately takes the per-container cap. A pod is a single workload, +// like a container, and it IS reclaimed -- RuleManager purges the bucket once the +// pod's last container is gone. Note the cap is shared by every container in the +// pod, so a pod-scoped rule in a many-container pod has less room per container +// than a container-scoped one; that is intended, because pod scope exists for +// facts about the pod as a unit rather than per-process markers. If a real +// workload ever needs more, state_write_rejected_total{reason="scope_cap"} is +// what says so. +func (s *Store) scopeCap(scopeID string) int { + if IsHostScopeID(scopeID) || scopeID == NodeScopeID() { + return s.cfg.MaxEntriesForHost + } + return s.cfg.MaxEntriesPerContainer +} + +// Set stores e, replacing any live entry with the same (ruleID, name, key) in the +// same scope -- last write wins, which also resets the TTL. +func (s *Store) Set(e *Entry) error { + if !s.cfg.Enabled { + return nil + } + + // The global check is deliberately not atomic with the insert below: it is a + // backstop, and serialising every write on one lock to make the ceiling exact + // would cost more than the few entries of overshoot it prevents. Concurrent + // writers can each pass this check before any of them increments, so the size + // can exceed MaxSize by up to the number of in-flight writers. The per-scope + // cap, which IS exact, is what bounds any single workload. + sh := s.shardFor(e.ScopeID) + k := entryKey{e.RuleID, e.Name, e.Key} + + if s.currentSize() >= s.cfg.MaxSize { + if s.sweepIfDue() == 0 { + // A replacement does not grow the store, so the ceiling must not block + // it -- the same reasoning the per-scope cap already applies below. + // Otherwise a rule loses the ability to refresh an established marker + // exactly when the store is under most pressure, which is when an + // incident is most likely to be in progress. + // + // The peek costs an extra RLock, but only on this already-degraded + // path: at the ceiling with nothing reclaimable. The hot path is + // unchanged. + if !s.holds(sh, e.ScopeID, k) { + s.metrics.ReportStateWriteRejected(e.RuleID, "global_cap") + return ErrGlobalCapReached + } + } + } + + sh.mu.Lock() + b, ok := sh.scopes[e.ScopeID] + if !ok { + b = &bucket{entries: make(map[entryKey]*Entry)} + sh.scopes[e.ScopeID] = b + } + // Replacing an existing key does not grow the scope, so the cap must not + // block it -- otherwise a full scope could never update its own markers. + _, replacing := b.entries[k] + if !replacing && len(b.entries) >= s.scopeCap(e.ScopeID) { + sh.mu.Unlock() + s.metrics.ReportStateWriteRejected(e.RuleID, "scope_cap") + return ErrScopeCapReached + } + b.entries[k] = e + sh.mu.Unlock() + + if !replacing { + s.addSize(1) + } + s.metrics.ReportStateWrite(e.RuleID, "ok") + return nil +} + +// holds reports whether a live-or-expired entry already exists under k. Used only +// by the global-cap path to tell a replacement from a genuine insert. +func (s *Store) holds(sh *shard, scopeID string, k entryKey) bool { + sh.mu.RLock() + defer sh.mu.RUnlock() + b, ok := sh.scopes[scopeID] + if !ok { + return false + } + _, exists := b.entries[k] + return exists +} + +// Get returns a live entry, or false if absent or expired. Expiry is enforced +// here as well as by the sweeper so a read never sees a stale marker. +// +// It takes no scope: the scope is already encoded in scopeID's type prefix, and a +// second, ignored scope argument on the read path of a scope-keyed store invites a +// caller to pass one that disagrees and assume it is checked. +func (s *Store) Get(ruleID, scopeID, name, key string) (*Entry, bool) { + if !s.cfg.Enabled { + return nil, false + } + sh := s.shardFor(scopeID) + + sh.mu.RLock() + b, ok := sh.scopes[scopeID] + if !ok { + sh.mu.RUnlock() + return nil, false + } + e, ok := b.entries[entryKey{ruleID, name, key}] + sh.mu.RUnlock() + + if !ok || e.expired(time.Now()) { + return nil, false + } + return e, true +} + +// PurgeScope drops every entry for a scope. Called on container removal. +func (s *Store) PurgeScope(scopeID string) { + sh := s.shardFor(scopeID) + sh.mu.Lock() + n := 0 + if b, ok := sh.scopes[scopeID]; ok { + n = len(b.entries) + delete(sh.scopes, scopeID) + } + sh.mu.Unlock() + + if n > 0 { + s.addSize(-n) + s.metrics.ReportStatePurged(n) + } +} + +// Sweep removes expired entries and returns how many it reclaimed. Lazy +// expiry on read hides stale entries; only Sweep frees the memory. +// +// It is also where the per-kind occupancy gauge is published: the walk already +// visits every surviving bucket under the lock, so the counts are free here and +// would need a second full traversal anywhere else. +func (s *Store) Sweep() int { + now := time.Now() + s.lastSweepNs.Store(now.UnixNano()) + total := 0 + live := make(map[string]int, len(ScopeKinds())) + for _, kind := range ScopeKinds() { + live[kind] = 0 + } + for _, sh := range s.shards { + sh.mu.Lock() + for scopeID, b := range sh.scopes { + for k, e := range b.entries { + if e.expired(now) { + delete(b.entries, k) + total++ + } + } + if len(b.entries) == 0 { + delete(sh.scopes, scopeID) + continue + } + live[ScopeKind(scopeID)] += len(b.entries) + } + sh.mu.Unlock() + } + // Every kind is reported, including the ones now at zero: a gauge that is + // only written when non-zero keeps its last value forever once a kind drains. + for kind, n := range live { + s.metrics.ReportStateEntries(kind, n) + } + if total > 0 { + s.addSize(-total) + s.metrics.ReportStateExpired(total) + } + return total +} + +// sweepIfDue is the write path's sweep, rate-limited to at most one per sweep +// interval across all writers. +// +// Without the limit, reaching the global ceiling turns every subsequent write +// into a full walk of all 16 shards, each under its write lock -- up to MaxSize +// (100k by default) entries examined per write, while the rule loop is +// concurrent, so every worker stalls behind it. That is a cliff exactly when the +// node is busiest, and it does not even help: if the previous sweep a moment ago +// reclaimed nothing, neither will this one. +// +// Returning 0 when a sweep is not due is the honest answer for the caller's +// purposes -- nothing was reclaimed -- so the write falls through to the +// replacement-only rule, which is the same decision it would make after a sweep +// that freed nothing. +func (s *Store) sweepIfDue() int { + interval := s.cfg.SweepInterval + if interval <= 0 { + // No background sweeper is running, so the write path is the only + // reclamation there is; keep it cheap but do not disable it. + interval = defaultWriteSweepInterval + } + last := s.lastSweepNs.Load() + now := time.Now().UnixNano() + if now-last < int64(interval) { + return 0 + } + // CAS so that concurrent writers at the ceiling produce one sweep, not one + // per writer. + if !s.lastSweepNs.CompareAndSwap(last, now) { + return 0 + } + return s.Sweep() +} + +// Run sweeps until ctx is cancelled. +func (s *Store) Run(ctx context.Context) { + if !s.cfg.Enabled || s.cfg.SweepInterval <= 0 { + return + } + t := time.NewTicker(s.cfg.SweepInterval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + s.Sweep() + } + } +} + +func (s *Store) Len() int { return s.currentSize() } + +func (s *Store) currentSize() int { return int(s.size.Load()) } + +func (s *Store) addSize(d int) { s.size.Add(int64(d)) } diff --git a/pkg/rulestate/store_test.go b/pkg/rulestate/store_test.go new file mode 100644 index 0000000000..6f8eaf9a7f --- /dev/null +++ b/pkg/rulestate/store_test.go @@ -0,0 +1,477 @@ +package rulestate + +import ( + "fmt" + "sync" + "testing" + "time" + + "github.com/armosec/armoapi-go/armotypes" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testConfig() Config { + return Config{ + Enabled: true, + MaxSize: 1000, + MaxEntriesPerContainer: 4, + MaxEntriesForHost: 8, + MaxTTL: 30 * time.Minute, + SweepInterval: time.Second, + AncestorMaxDepth: 8, + } +} + +func entry(ruleID, scopeID, name, key string, ts time.Time, ttl time.Duration) *Entry { + return &Entry{ + RuleID: ruleID, Name: name, Key: key, + Scope: armotypes.StateScopeContainer, ScopeID: scopeID, + EventType: "exec", + Timestamp: ts, ExpiresAt: ts.Add(ttl), + Process: &armotypes.Process{PID: 4471, Comm: "xmrig"}, + } +} + +func TestStore_SetThenGet(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + require.NoError(t, s.Set(entry("R1089", "c:abc", "mount_exec", "4471", now, time.Minute))) + + got, ok := s.Get("R1089", "c:abc", "mount_exec", "4471") + require.True(t, ok) + assert.Equal(t, uint32(4471), got.Process.PID) +} + +func TestStore_IsolationAcrossRulesScopesAndKeys(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + require.NoError(t, s.Set(entry("R1089", "c:abc", "mount_exec", "4471", now, time.Minute))) + + // Different rule: state is rule-private. + _, ok := s.Get("R1090", "c:abc", "mount_exec", "4471") + assert.False(t, ok, "another rule must not see this entry") + + // Different container: the security property. + _, ok = s.Get("R1089", "c:def", "mount_exec", "4471") + assert.False(t, ok, "a neighbouring container must not see this entry") + + // Different key and different name. + _, ok = s.Get("R1089", "c:abc", "mount_exec", "9999") + assert.False(t, ok) + _, ok = s.Get("R1089", "c:abc", "other", "4471") + assert.False(t, ok) +} + +func TestStore_ExpiredEntryIsAMissOnRead(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + past := time.Now().Add(-2 * time.Minute) + require.NoError(t, s.Set(entry("R1089", "c:abc", "mount_exec", "4471", past, time.Minute))) + + _, ok := s.Get("R1089", "c:abc", "mount_exec", "4471") + assert.False(t, ok, "TTL must be enforced lazily on read, not only by the sweeper") +} + +func TestStore_SweepReclaimsExpired(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + past := time.Now().Add(-2 * time.Minute) + now := time.Now() + require.NoError(t, s.Set(entry("R1089", "c:abc", "expired", "1", past, time.Minute))) + require.NoError(t, s.Set(entry("R1089", "c:abc", "live", "1", now, time.Minute))) + + assert.Equal(t, 1, s.Sweep()) + assert.Equal(t, 1, s.Len()) +} + +func TestStore_ScopeCapRejectsRatherThanEvicting(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) // MaxEntriesPerContainer = 4 + now := time.Now() + for i := 0; i < 4; i++ { + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, time.Minute))) + } + + err := s.Set(entry("R1089", "c:abc", "n", "overflow", now, time.Minute)) + assert.ErrorIs(t, err, ErrScopeCapReached) + + // The critical assertion: nothing already stored was evicted. Evicting would + // let a hostile container silently disable its own -- or a neighbour's -- rules. + for i := 0; i < 4; i++ { + _, ok := s.Get("R1089", "c:abc", "n", fmt.Sprint(i)) + assert.True(t, ok, "entry %d was evicted; writes must be rejected instead", i) + } +} + +func TestStore_ScopeCapIsPerScopeNotGlobal(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + for i := 0; i < 4; i++ { + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, time.Minute))) + } + // A different container is unaffected by its neighbour hitting the cap. + require.NoError(t, s.Set(entry("R1089", "c:def", "n", "0", now, time.Minute))) +} + +// An over-cap scope must still accept an overwrite of a key it already holds: +// refusing would freeze the scope's newest observation out and make a +// bidirectional rule stop updating its own marker. +func TestStore_OverwriteSucceedsEvenAtCap(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + for i := 0; i < 4; i++ { + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, time.Minute))) + } + later := now.Add(time.Second) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "0", later, time.Minute)), + "replacing an existing key does not grow the scope, so the cap must not block it") + + got, ok := s.Get("R1089", "c:abc", "n", "0") + require.True(t, ok) + assert.Equal(t, later, got.Timestamp) +} + +func TestStore_OverwriteIsLastWriteWins(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + t1 := time.Now() + t2 := t1.Add(time.Second) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "1", t1, time.Minute))) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "1", t2, time.Minute))) + + got, ok := s.Get("R1089", "c:abc", "n", "1") + require.True(t, ok) + assert.Equal(t, t2, got.Timestamp) + assert.Equal(t, 1, s.Len(), "overwrite must not grow the store") +} + +func TestStore_HostBucketHasItsOwnLargerCap(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) // host cap 8, container cap 4 + now := time.Now() + for i := 0; i < 8; i++ { + require.NoError(t, s.Set(entry("R1089", HostScopeID(), "n", fmt.Sprint(i), now, time.Minute)), + "host bucket holds the whole node's processes, so it needs a bigger cap than one container") + } + assert.ErrorIs(t, s.Set(entry("R1089", HostScopeID(), "n", "8", now, time.Minute)), ErrScopeCapReached) +} + +// Node scope is a single node-wide bucket shared by every rule and workload, and +// PurgeScope is only ever called with a container's scope ID, so it is never +// reclaimed on container removal. It therefore needs the same headroom as the +// host bucket -- the per-container cap would starve it on a busy node. +func TestStore_NodeBucketGetsTheLargerCap(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) // host/node cap 8, container cap 4 + now := time.Now() + for i := 0; i < 8; i++ { + e := entry("R1089", NodeScopeID(), "n", fmt.Sprint(i), now, time.Minute) + e.Scope = armotypes.StateScopeNode + require.NoError(t, s.Set(e), + "node scope must not be bounded by the per-container cap") + } + over := entry("R1089", NodeScopeID(), "n", "8", now, time.Minute) + over.Scope = armotypes.StateScopeNode + assert.ErrorIs(t, s.Set(over), ErrScopeCapReached) +} + +// At the global ceiling with nothing reclaimable, a write that only REPLACES an +// existing key does not grow the store, so it must still be admitted -- otherwise +// a rule loses the ability to refresh a marker exactly when the store is under +// most pressure. Mirrors TestStore_OverwriteSucceedsEvenAtCap for the global cap. +func TestStore_GlobalCapAdmitsAReplacement(t *testing.T) { + cfg := testConfig() + cfg.MaxSize = 3 + cfg.MaxEntriesPerContainer = 100 + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + for i := 0; i < 3; i++ { + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, time.Minute))) + } + + // A genuine insert is still rejected... + assert.ErrorIs(t, s.Set(entry("R1089", "c:abc", "n", "new", now, time.Minute)), ErrGlobalCapReached) + + // ...but refreshing an existing key is not. + later := now.Add(time.Second) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "0", later, time.Minute)), + "a replacement does not grow the store, so the ceiling must not block it") + + got, ok := s.Get("R1089", "c:abc", "n", "0") + require.True(t, ok) + assert.Equal(t, later, got.Timestamp) + assert.Equal(t, 3, s.Len()) +} + +func TestScopeIDs_HostAndNodeDoNotCollide(t *testing.T) { + // Host processes carry ContainerID == "", and node scope has no ID. Without + // type prefixes both would be "" and share a bucket. + assert.Equal(t, "c:__host__", ContainerScopeID("")) + assert.Equal(t, "c:abc", ContainerScopeID("abc")) + assert.Equal(t, "n:", NodeScopeID()) + assert.Equal(t, "p:prod/web-1", PodScopeID("prod", "web-1")) + assert.NotEqual(t, ContainerScopeID(""), NodeScopeID()) + assert.True(t, IsHostScopeID(ContainerScopeID(""))) + assert.False(t, IsHostScopeID(ContainerScopeID("abc"))) +} + +func TestStore_PurgeScopeDropsOnlyThatScope(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "1", now, time.Minute))) + require.NoError(t, s.Set(entry("R1089", "c:def", "n", "1", now, time.Minute))) + + s.PurgeScope("c:abc") + _, ok := s.Get("R1089", "c:abc", "n", "1") + assert.False(t, ok) + _, ok = s.Get("R1089", "c:def", "n", "1") + assert.True(t, ok) + assert.Equal(t, 1, s.Len(), "purge must decrement the global size, not just drop the bucket") +} + +func TestStore_DisabledIsANoop(t *testing.T) { + cfg := testConfig() + cfg.Enabled = false + s := NewStore(cfg, NoopMetrics{}) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "1", time.Now(), time.Minute))) + _, ok := s.Get("R1089", "c:abc", "n", "1") + assert.False(t, ok, "disabled: writes are no-ops and reads always miss") + assert.Equal(t, 0, s.Len()) +} + +// The global ceiling is a backstop. It must reject rather than evict, for the +// same reason the per-scope cap does. +func TestStore_GlobalCapRejectsWhenNothingCanBeReclaimed(t *testing.T) { + cfg := testConfig() + cfg.MaxSize = 3 + cfg.MaxEntriesPerContainer = 100 + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + for i := 0; i < 3; i++ { + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, time.Minute))) + } + + assert.ErrorIs(t, s.Set(entry("R1089", "c:abc", "n", "3", now, time.Minute)), ErrGlobalCapReached) + assert.Equal(t, 3, s.Len()) +} + +// At the ceiling, an expiring entry should make room -- otherwise a node that +// once filled the store would stop correlating forever. +func TestStore_GlobalCapSweepsBeforeRejecting(t *testing.T) { + cfg := testConfig() + cfg.MaxSize = 3 + cfg.MaxEntriesPerContainer = 100 + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + past := now.Add(-2 * time.Minute) + + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "0", past, time.Minute))) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "1", now, time.Minute))) + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "2", now, time.Minute))) + + require.NoError(t, s.Set(entry("R1089", "c:abc", "n", "3", now, time.Minute)), + "the expired entry must be reclaimed to admit this write") + assert.Equal(t, 3, s.Len()) +} + +func TestStore_ConcurrentSetGetIsRaceFree(t *testing.T) { + cfg := testConfig() + cfg.MaxEntriesPerContainer = 10000 + // Both caps have to clear 8*200, or the assertion below is really measuring + // the global ceiling rejecting writes rather than concurrent correctness. + cfg.MaxSize = 100000 + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + + var wg sync.WaitGroup + for c := 0; c < 8; c++ { + wg.Add(1) + go func(c int) { + defer wg.Done() + scopeID := fmt.Sprintf("c:%d", c) + for i := 0; i < 200; i++ { + _ = s.Set(entry("R1089", scopeID, "n", fmt.Sprint(i), now, time.Minute)) + s.Get("R1089", scopeID, "n", fmt.Sprint(i)) + } + }(c) + } + wg.Wait() + assert.Equal(t, 8*200, s.Len()) +} + +// Sweep and Set race on the size counter and on bucket maps; a concurrent sweeper +// is exactly what Run does in production. +func TestStore_ConcurrentSweepIsRaceFree(t *testing.T) { + cfg := testConfig() + cfg.MaxEntriesPerContainer = 10000 + s := NewStore(cfg, NoopMetrics{}) + + stop := make(chan struct{}) + + var sweeper sync.WaitGroup + sweeper.Add(1) + go func() { + defer sweeper.Done() + for { + select { + case <-stop: + return + default: + s.Sweep() + } + } + }() + + var writer sync.WaitGroup + writer.Add(1) + go func() { + defer writer.Done() + now := time.Now() + for i := 0; i < 500; i++ { + // Half of these are born expired, so the sweeper has real work. + ttl := time.Minute + if i%2 == 0 { + ttl = -time.Minute + } + _ = s.Set(entry("R1089", "c:abc", "n", fmt.Sprint(i), now, ttl)) + } + }() + + writer.Wait() + close(stop) + sweeper.Wait() + assert.GreaterOrEqual(t, s.Len(), 0, "size must never go negative") +} + +// Pod scope taking the per-container cap is a decision, not a fallthrough: a pod +// is one workload and its bucket is reclaimed when the pod's last container goes, +// so it does not need the node-wide headroom. Pinned so that a future change to +// scopeCap has to be deliberate. +func TestStore_PodBucketTakesTheContainerCap(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) // host/node cap 8, container cap 4 + now := time.Now() + podID := PodScopeID("prod", "web-1") + + for i := 0; i < 4; i++ { + e := entry("R1089", podID, "n", fmt.Sprint(i), now, time.Minute) + e.Scope = armotypes.StateScopePod + require.NoError(t, s.Set(e)) + } + over := entry("R1089", podID, "n", "4", now, time.Minute) + over.Scope = armotypes.StateScopePod + assert.ErrorIs(t, s.Set(over), ErrScopeCapReached, + "pod scope must be bounded by the per-container cap, not the node-wide one") +} + +// The pod bucket must be reclaimable in full, the same way a container's is -- +// this is the store-side half of the pod-removal purge. +func TestStore_PurgeScopeReclaimsAPodBucket(t *testing.T) { + s := NewStore(testConfig(), NoopMetrics{}) + now := time.Now() + podID := PodScopeID("prod", "web-1") + + e := entry("R1089", podID, "n", "1", now, time.Minute) + e.Scope = armotypes.StateScopePod + require.NoError(t, s.Set(e)) + require.Equal(t, 1, s.Len()) + + s.PurgeScope(podID) + + _, ok := s.Get("R1089", podID, "n", "1") + assert.False(t, ok) + assert.Equal(t, 0, s.Len(), "purging a pod bucket must decrement the global size") +} + +// The occupancy gauge was plumbed through every metrics manager, documented, and +// never called -- so node_agent_state_entries silently did not exist. Sweep now +// publishes it, including zeroes, so a kind that drains does not keep its last +// non-zero reading forever. +func TestStore_SweepPublishesOccupancyPerScopeKind(t *testing.T) { + m := &recordingMetrics{entries: map[string]int{}} + s := NewStore(testConfig(), m) + now := time.Now() + + live := func(scopeID string, key string) *Entry { + return entry("R1089", scopeID, "n", key, now, time.Minute) + } + require.NoError(t, s.Set(live(ContainerScopeID("abc"), "1"))) + require.NoError(t, s.Set(live(ContainerScopeID("abc"), "2"))) + require.NoError(t, s.Set(live(ContainerScopeID("def"), "1"))) + require.NoError(t, s.Set(live(HostScopeID(), "1"))) + require.NoError(t, s.Set(live(PodScopeID("prod", "web-1"), "1"))) + require.NoError(t, s.Set(live(NodeScopeID(), "1"))) + // An entry that is already expired must not be counted as live. + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "old", now.Add(-time.Hour), time.Minute))) + + s.Sweep() + + assert.Equal(t, 3, m.entries[ScopeKindContainer], "two containers, three live entries; the host bucket is counted separately") + assert.Equal(t, 1, m.entries[ScopeKindHost]) + assert.Equal(t, 1, m.entries[ScopeKindPod]) + assert.Equal(t, 1, m.entries[ScopeKindNode]) + + // Drain everything: each kind must be republished as zero. + s.PurgeScope(ContainerScopeID("abc")) + s.PurgeScope(ContainerScopeID("def")) + s.PurgeScope(HostScopeID()) + s.PurgeScope(PodScopeID("prod", "web-1")) + s.PurgeScope(NodeScopeID()) + s.Sweep() + + for _, kind := range ScopeKinds() { + assert.Equal(t, 0, m.entries[kind], "%s must be republished as zero once it drains", kind) + } +} + +// recordingMetrics keeps only what the occupancy assertions need. +type recordingMetrics struct { + NoopMetrics + entries map[string]int +} + +func (r *recordingMetrics) ReportStateEntries(scope string, n int) { r.entries[scope] = n } + +// At the global ceiling the write path used to sweep on EVERY write: a full walk +// of all 16 shards, each under its write lock, per write, while the rule loop is +// concurrent. This pins the rate limit -- one sweep per interval, no matter how +// many writes arrive at the ceiling. +func TestStore_WritePathSweepIsRateLimited(t *testing.T) { + cfg := testConfig() + cfg.MaxSize = 2 + cfg.SweepInterval = time.Hour // nothing may sweep twice inside this test + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + + // Fill to the ceiling with entries that are ALREADY expired, so the first + // sweep has something to reclaim and later ones would too. + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "1", now.Add(-time.Hour), time.Minute))) + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "2", now.Add(-time.Hour), time.Minute))) + require.Equal(t, 2, s.Len()) + + // This write is at the ceiling, so it sweeps: both expired entries go, and it + // is admitted. + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "3", now, time.Minute))) + assert.Equal(t, 1, s.Len(), "the write-path sweep must reclaim the expired entries") + + // Re-fill to the ceiling with expired entries. A second sweep WOULD reclaim + // them, but it is not due, so the ceiling holds and the insert is rejected. + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "4", now.Add(-time.Hour), time.Minute))) + assert.Equal(t, 2, s.Len()) + err := s.Set(entry("R1089", ContainerScopeID("abc"), "n", "5", now, time.Minute)) + assert.ErrorIs(t, err, ErrGlobalCapReached, + "a second sweep inside the interval must not run, so the ceiling still rejects") + + // The rate limit must not break the replacement exemption: refreshing a key + // that already exists still does not grow the store, so it is still admitted. + assert.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "3", now, time.Minute)), + "a replacement must stay admissible even when the sweep is rate-limited") +} + +// An explicit Sweep() must still run on demand -- only the write path is +// rate-limited. The background sweeper calls Sweep directly. +func TestStore_ExplicitSweepIsNotRateLimited(t *testing.T) { + cfg := testConfig() + cfg.SweepInterval = time.Hour + s := NewStore(cfg, NoopMetrics{}) + now := time.Now() + + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "1", now.Add(-time.Hour), time.Minute))) + assert.Equal(t, 1, s.Sweep(), "a direct Sweep must always run") + require.NoError(t, s.Set(entry("R1089", ContainerScopeID("abc"), "n", "2", now.Add(-time.Hour), time.Minute))) + assert.Equal(t, 1, s.Sweep(), "and again immediately after") +} diff --git a/pkg/rulestate/types.go b/pkg/rulestate/types.go new file mode 100644 index 0000000000..119c119941 --- /dev/null +++ b/pkg/rulestate/types.go @@ -0,0 +1,136 @@ +// Package rulestate holds short-lived, TTL-bounded markers that let a CEL rule +// remember a fact from one event and read it back when a later event arrives. +// +// The package deliberately knows nothing about CEL, rules or Kubernetes: it is a +// bounded map with expiry, so it stays unit-testable without an evaluator. The +// CEL bindings live in pkg/rulemanager/cel/libraries/state, and write-clause +// execution in pkg/rulemanager/statewrites. +package rulestate + +import ( + "errors" + "strings" + "time" + + "github.com/armosec/armoapi-go/armotypes" +) + +var ( + // ErrScopeCapReached means this scope is at its entry cap. The write is + // rejected -- never satisfied by evicting an existing entry, which would let + // one workload silently disable detection for itself or a neighbour. + ErrScopeCapReached = errors.New("rulestate: scope entry cap reached") + // ErrGlobalCapReached means the node-wide ceiling is reached even after a sweep. + ErrGlobalCapReached = errors.New("rulestate: global entry cap reached") +) + +// Entry is one remembered fact. +// +// Process and Admission are mutually exclusive: node-agent entries carry a +// Process, operator entries carry an Admission. Both map straight onto +// armotypes.CorrelationEvidence, so store -> alert is a copy, not a translation. +type Entry struct { + RuleID string + Name string + Scope armotypes.StateScope + ScopeID string + Key string + EventType armotypes.EventType + + // Timestamp is when the remembered event HAPPENED (see cel.ResolveEventTime), + // not when it was observed. Ordering guards compare against it. + Timestamp time.Time + ExpiresAt time.Time + + Process *armotypes.Process + Admission *armotypes.AdmissionEvidence + Value map[string]any +} + +func (e *Entry) expired(now time.Time) bool { return now.After(e.ExpiresAt) } + +// Config bounds the store. Caps are the anti-abuse mechanism. +type Config struct { + Enabled bool `mapstructure:"enabled"` + // MaxSize is the node-wide ceiling, a backstop above the per-scope caps. + MaxSize int `mapstructure:"maxSize"` + // MaxEntriesPerContainer bounds one container, and also one pod bucket -- + // see Store.scopeCap for why pod scope shares this cap rather than the + // larger one. + MaxEntriesPerContainer int `mapstructure:"maxEntriesPerContainer"` + // MaxEntriesForHost bounds the c:__host__ bucket, which holds the whole + // node's process space rather than one workload, and never receives a + // container-removal purge -- so it needs a larger cap and relies on TTL. + MaxEntriesForHost int `mapstructure:"maxEntriesForHost"` + MaxTTL time.Duration `mapstructure:"maxTtl"` + SweepInterval time.Duration `mapstructure:"sweepInterval"` + AncestorMaxDepth int `mapstructure:"ancestorMaxDepth"` +} + +// Metrics is the observability surface. There is deliberately no per-read +// counter: reads are on the hot path. +type Metrics interface { + ReportStateWrite(ruleID, result string) + ReportStateWriteRejected(ruleID, reason string) + ReportStateExpired(n int) + ReportStatePurged(n int) + ReportStateEntries(scope string, n int) +} + +type NoopMetrics struct{} + +func (NoopMetrics) ReportStateWrite(string, string) {} +func (NoopMetrics) ReportStateWriteRejected(string, string) {} +func (NoopMetrics) ReportStateExpired(int) {} +func (NoopMetrics) ReportStatePurged(int) {} +func (NoopMetrics) ReportStateEntries(string, int) {} + +const hostScopeSuffix = "__host__" + +// ContainerScopeID maps a container ID to its bucket. The empty container ID is +// a host / cgroup-0 process, which gets an explicit pseudo-container bucket -- +// without the type prefix it would collide with node scope, whose ID is also "". +func ContainerScopeID(containerID string) string { + if containerID == "" { + return "c:" + hostScopeSuffix + } + return "c:" + containerID +} + +// Scope kinds, as reported on the node_agent_state_entries gauge. Deliberately a +// small fixed set: the label must never carry a scope ID, which is a container or +// pod identity and so unbounded. +const ( + ScopeKindContainer = "container" + ScopeKindHost = "host" + ScopeKindPod = "pod" + ScopeKindNode = "node" +) + +// ScopeKinds lists every kind the gauge reports, so a kind that has just fallen +// to zero is published as zero rather than keeping its last non-zero value. +func ScopeKinds() []string { + return []string{ScopeKindContainer, ScopeKindHost, ScopeKindPod, ScopeKindNode} +} + +// ScopeKind classifies a bucket for metrics. The host pseudo-container is +// reported separately from real containers: it is the one bucket that is never +// purged, so watching it against the others is how its TTL-only reclamation is +// observed. +func ScopeKind(scopeID string) string { + switch { + case IsHostScopeID(scopeID): + return ScopeKindHost + case scopeID == NodeScopeID(): + return ScopeKindNode + case strings.HasPrefix(scopeID, "p:"): + return ScopeKindPod + default: + return ScopeKindContainer + } +} + +func HostScopeID() string { return "c:" + hostScopeSuffix } +func NodeScopeID() string { return "n:" } +func PodScopeID(ns, pod string) string { return "p:" + ns + "/" + pod } +func IsHostScopeID(scopeID string) bool { return scopeID == HostScopeID() } diff --git a/pkg/utils/events.go b/pkg/utils/events.go index bc0a4cdf58..71683a2444 100644 --- a/pkg/utils/events.go +++ b/pkg/utils/events.go @@ -233,6 +233,29 @@ const ( UnshareEventType EventType = "unshare" ) +// nodeAgentEventTypes is every event stream node-agent can actually deliver. +// AllEventType is deliberately absent: it is a rule-binding wildcard, not a +// stream, so anything that must name a concrete stream has to reject it. +var nodeAgentEventTypes = map[EventType]struct{}{ + BpfEventType: {}, CapabilitiesEventType: {}, DnsEventType: {}, + ExecveEventType: {}, ExitEventType: {}, ForkEventType: {}, + HTTPEventType: {}, HardlinkEventType: {}, IoUringEventType: {}, + KmodEventType: {}, NetworkEventType: {}, OpenEventType: {}, + ProcfsEventType: {}, PtraceEventType: {}, RandomXEventType: {}, + SSHEventType: {}, SymlinkEventType: {}, SyscallEventType: {}, + UnshareEventType: {}, +} + +// IsValidEventType reports whether e names a concrete node-agent event stream. +// +// This is narrower than armotypes.IsKnownEventType, which spans both engines -- +// k8s-admission is a real armotypes event type that node-agent never emits, so a +// node-agent rule naming it must be rejected at load rather than never matching. +func IsValidEventType(e EventType) bool { + _, ok := nodeAgentEventTypes[e] + return ok +} + // Get the path of the file on the node. func GetHostFilePathFromEvent(event EnrichEvent, containerPid uint32) (string, error) { switch event.GetEventType() { diff --git a/tests/chart/crds/rules.crd.yaml b/tests/chart/crds/rules.crd.yaml index 90d5d56712..94aefa6825 100644 --- a/tests/chart/crds/rules.crd.yaml +++ b/tests/chart/crds/rules.crd.yaml @@ -71,6 +71,45 @@ spec: - message - uniqueId - ruleExpression + stateWrites: + type: array + description: >- + Facts this rule remembers across events, for + cross-event correlation. Each entry is driven by one + event type, which need not be an event type the rule + alerts on -- that is what allows a rule to remember on + exec and alert on network. + items: + type: object + properties: + eventType: + type: string + description: "Event stream that drives this write" + when: + type: string + description: "CEL boolean guard; empty means always write" + scope: + type: string + enum: ["container", "pod", "node", "identity"] + description: "Bucket the entry belongs to. identity is operator-only." + name: + type: string + description: "What kind of fact this is. A literal, never an expression." + key: + type: string + description: "CEL string expression naming the subject. Omit for a scope-wide fact." + value: + type: object + x-kubernetes-preserve-unknown-fields: true + description: "Optional author extras as CEL expressions. Keys may not begin with an underscore." + ttl: + type: string + description: "Go duration string; clamped to the agent's configured maxTtl at load" + required: + - eventType + - scope + - name + - ttl profileDependency: type: integer enum: [0, 1, 2] diff --git a/tests/component_test.go b/tests/component_test.go index 154e441e8b..7b7b805b50 100644 --- a/tests/component_test.go +++ b/tests/component_test.go @@ -3906,3 +3906,101 @@ func Test_49_EphemeralContainerFullTreatment(t *testing.T) { return countRuleAlerts(t, ns.Name, "R0001", "ephcon", "id") > 0 }, 2*time.Minute, 10*time.Second, "id was not in the ephemeral container's learned profile — it must fire R0001 (detected + alerted like any other container)") } + +// Test_37_CelStateStoreCorrelation is the end-to-end proof of the CEL state +// store: a rule that remembers a fact on one event stream and reads it back on +// another, against real eBPF events. +// +// The proof does not need to inspect the alert payload. R9911's network-leg +// predicate is state.has(...), so if the store does not work the predicate is +// false and no alert is emitted at all. The alert's EXISTENCE is the proof. +// Asserting the correlations[] evidence payload needs a payload-level receiver +// and is a separate, optional tier. +// +// The trigger puts an 8-second gap between the write and the read: +// +// sh -c '# CELSTATE_MARKER; sleep 8; exec nc -w 3 $HOST $PORT' +// +// T=0 the shell execs with the marker in argv -> R9911 writes state under pid P. +// T=8 `exec nc` replaces the shell's image IN THE SAME PID (exec does not fork), +// and nc connects -> the network event carries pid P and the read hits. +// +// Without that gap the exec and the connect are milliseconds apart, and +// node-agent evaluates events on a concurrent worker pool -- so a failure could +// be reordering rather than a defect. With it, a failure is a real defect. +func Test_37_CelStateStoreCorrelation(t *testing.T) { + start := time.Now() + defer tearDownTest(t, start) + + rulesPath := path.Join(utils.CurrentDir(), "resources/cel-state-rules.yaml") + bindingPath := path.Join(utils.CurrentDir(), "resources/cel-state-rulebinding.yaml") + require.Equal(t, 0, testutils.RunCommand("kubectl", "apply", "--validate=false", "-f", rulesPath), "apply state test rules") + defer testutils.RunCommand("kubectl", "delete", "--ignore-not-found", "-f", rulesPath) + require.Equal(t, 0, testutils.RunCommand("kubectl", "apply", "--validate=false", "-f", bindingPath), "apply state test rule binding") + defer testutils.RunCommand("kubectl", "delete", "--ignore-not-found", "-f", bindingPath) + // let the rules watcher and rule-binding watcher pick the new rules up + time.Sleep(20 * time.Second) + + ns := testutils.NewRandomNamespace() + wl, err := testutils.NewTestWorkload(ns.Name, path.Join(utils.CurrentDir(), "resources/cel-state-deployment.yaml")) + require.NoError(t, err, "Error creating workload") + require.NoError(t, wl.WaitForReady(80)) + time.Sleep(15 * time.Second) + + // Confirm nc exists before relying on it; without this a missing applet looks + // exactly like the state store failing. + _, _, err = wl.ExecIntoPodNoTTY([]string{"sh", "-c", "command -v nc"}, "probe") + require.NoError(t, err, "busybox nc must be present in the probe container") + + // Three probes rather than one. Each is independent (its own pid, its own + // state key), so a single flake does not fail the run, and three silent + // probes is clearly systematic rather than a race. + const probes = 3 + trigger := `# CELSTATE_MARKER +sleep 8 +exec nc -w 3 "$KUBERNETES_SERVICE_HOST" "$KUBERNETES_SERVICE_PORT"` + + for i := 0; i < probes; i++ { + go func() { + // nc is expected to be closed by the peer or time out; the connection + // attempt is the signal, its outcome is irrelevant. + _, _, _ = wl.ExecIntoPodNoTTY([]string{"sh", "-c", trigger}, "probe") + }() + time.Sleep(1 * time.Second) + } + + // 8s sleep + connect + export + Alertmanager group interval. + t.Log("waiting for the probes to connect and their alerts to land") + time.Sleep(60 * time.Second) + + alerts, err := testutils.GetAlerts(wl.Namespace) + require.NoError(t, err, "Error getting alerts") + + count := func(ruleID string) int { + n := 0 + for _, a := range alerts { + if a.Labels["rule_id"] == ruleID { + n++ + } + } + return n + } + for _, a := range alerts { + t.Logf("alert rule_id=%s rule_name=%q", a.Labels["rule_id"], a.Labels["rule_name"]) + } + + // Controls FIRST. If either is silent, R9911's silence says nothing about + // the state store, and these messages are the only diagnostic available. + require.Greater(t, count("R9913"), 0, + "exec control did not fire: the marker exec never reached the rule loop, so this test cannot say anything about state") + require.Greater(t, count("R9912"), 0, + "network control did not fire: the outbound connection never reached the rule loop, so this test cannot say anything about state") + + // The actual proof. + assert.Greater(t, count("R9911"), 0, + "correlation rule never fired: state written on exec was not readable on the network event") + + // And the negative control, which makes the assertion above meaningful. + assert.Equal(t, 0, count("R9914"), + "negative control fired: state.has returned true for a name no rule writes, so R9911 proves nothing") +} diff --git a/tests/resources/cel-state-deployment.yaml b/tests/resources/cel-state-deployment.yaml new file mode 100644 index 0000000000..541fdf57a3 --- /dev/null +++ b/tests/resources/cel-state-deployment.yaml @@ -0,0 +1,22 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: cel-state-app + name: cel-state-deployment +spec: + selector: + matchLabels: + app: cel-state-app + replicas: 1 + template: + metadata: + labels: + app: cel-state-app + spec: + containers: + # alpine's busybox provides nc, so the trigger needs no package install -- + # the kind node may have no outbound internet. + - name: probe + image: alpine:3.20 + command: ["sleep", "infinity"] diff --git a/tests/resources/cel-state-rulebinding.yaml b/tests/resources/cel-state-rulebinding.yaml new file mode 100644 index 0000000000..bc3b7e88d7 --- /dev/null +++ b/tests/resources/cel-state-rulebinding.yaml @@ -0,0 +1,22 @@ +# Binds the test-only state-store rules. The shipped default binding +# (chart/templates/node-agent/default-rule-binding.yaml) lists rules by explicit +# ruleName, so newly added rule IDs are inert until something binds them. +apiVersion: kubescape.io/v1 +kind: RuntimeRuleAlertBinding +metadata: + name: cel-state-test-binding +spec: + namespaceSelector: + matchExpressions: + - key: "kubernetes.io/metadata.name" + operator: "NotIn" + values: + - "kube-system" + - "kube-public" + - "kube-node-lease" + podSelector: + rules: + - ruleName: "TEST state correlation exec then connect" + - ruleName: "TEST state network control" + - ruleName: "TEST state exec control" + - ruleName: "TEST state negative control" diff --git a/tests/resources/cel-state-rules.yaml b/tests/resources/cel-state-rules.yaml new file mode 100644 index 0000000000..c85fafa7b0 --- /dev/null +++ b/tests/resources/cel-state-rules.yaml @@ -0,0 +1,123 @@ +# Test-only rules validating the CEL state store against real eBPF events. +# Applied by Test_36_CelStateStoreCorrelation and deleted on cleanup. IDs are in +# a deliberately test-only 99xx range so they cannot collide with the shipped +# R1xxx/R2xxx ranges. +# +# Four rules, because "no alert" is ambiguous on its own: an unresolvable CEL +# field or function does not error, it fails to compile and silently disables the +# whole expression (pkg/rulemanager/cel returns (false, nil) on compile failure), +# which looks exactly like "the predicate was false". +# +# R9911 The correlation rule under test. Writes on exec, alerts on network. +# It has NO exec ruleExpression at all -- that is the +# write-without-alerting shape every cross-event rule depends on. +# R9912 Network control. Fires on the same connection with no state +# predicate. If R9912 is silent the network leg never reached the rule +# loop, so R9911's silence says nothing about the state store. +# R9913 Exec control. Fires on the marker exec. Its message carries the pid, +# which is how the test verifies the exec and network legs share a pid. +# R9914 Negative control. Reads a name that is never written. Must NOT fire. +# If it does, state.has is returning true spuriously and R9911 proves +# nothing. +# +# All rules use profileDependency 2 (NotRequired) so the test never waits for +# application-profile completion, and uniqueId includes the pid so per-rule +# cooldown cannot swallow a later probe. +apiVersion: kubescape.io/v1 +kind: Rules +metadata: + name: cel-state-test-rules + namespace: kubescape + labels: + app: kubescape +spec: + rules: + - name: "TEST state correlation exec then connect" + enabled: true + id: "R9911" + description: "Test rule: remembers a marker exec, then alerts on an outbound connection from the same pid." + stateWrites: + - eventType: "exec" + scope: "container" + name: "probe_exec" + key: "string(event.pid)" + value: + probeComm: "event.comm" + ttl: "5m" + when: "event.comm == 'sh' && event.args.exists(a, a.contains('CELSTATE_MARKER'))" + expressions: + message: "'state correlation: pid=' + string(event.pid) + ' comm=' + event.comm + ' remembered=' + state.get('probe_exec', string(event.pid)).probeComm" + uniqueId: "'R9911_' + string(event.pid)" + ruleExpression: + - eventType: "network" + expression: | + event.pktType == 'OUTGOING' && state.has('probe_exec', string(event.pid)) + profileDependency: 2 + severity: 1 + supportPolicy: false + isTriggerAlert: true + mitreTactic: "TA0011" + mitreTechnique: "T1071" + tags: + - "test" + - "state" + - name: "TEST state network control" + enabled: true + id: "R9912" + description: "Test control rule: same outbound connection, no state predicate. Must fire whenever the probe connects." + expressions: + message: "'network control: pid=' + string(event.pid) + ' comm=' + event.comm" + uniqueId: "'R9912_' + string(event.pid)" + ruleExpression: + - eventType: "network" + expression: | + event.pktType == 'OUTGOING' && event.comm == 'nc' + profileDependency: 2 + severity: 1 + supportPolicy: false + isTriggerAlert: true + mitreTactic: "TA0011" + mitreTechnique: "T1071" + tags: + - "test" + - "state" + - name: "TEST state exec control" + enabled: true + id: "R9913" + description: "Test control rule: fires on the marker exec. Its message carries the pid, so the test can confirm both legs share one." + expressions: + message: "'exec control: pid=' + string(event.pid) + ' comm=' + event.comm" + uniqueId: "'R9913_' + string(event.pid)" + ruleExpression: + - eventType: "exec" + expression: | + event.comm == 'sh' && event.args.exists(a, a.contains('CELSTATE_MARKER')) + profileDependency: 2 + severity: 1 + supportPolicy: false + isTriggerAlert: true + mitreTactic: "TA0002" + mitreTechnique: "T1059" + tags: + - "test" + - "state" + - name: "TEST state negative control" + enabled: true + id: "R9914" + description: "Test negative control: reads a state name no rule ever writes. Must never fire." + expressions: + message: "'NEGATIVE CONTROL FIRED: pid=' + string(event.pid)" + uniqueId: "'R9914_' + string(event.pid)" + ruleExpression: + - eventType: "network" + expression: | + event.pktType == 'OUTGOING' && state.has('never_written', string(event.pid)) + profileDependency: 2 + severity: 1 + supportPolicy: false + isTriggerAlert: true + mitreTactic: "TA0011" + mitreTechnique: "T1071" + tags: + - "test" + - "state"