forked from kubescape/node-agent
-
Notifications
You must be signed in to change notification settings - Fork 1
Merge/exec arg wildcards #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
23d3d2f
test(component): port Test_28 to upstream's unified user-overlay label
a83bb69
feat(cel): re-port CompareExecArgs hookup onto upstream's CP cache
eb0145d
feat(rules): R0040 'Unexpected process arguments' + Test_32 e2e
0816393
deps(storage): bump to rebased feat/exec-arg-wildcards tip (0de34ebc)
fbf98b0
ci(component-tests): add Test_32_UnexpectedProcessArguments to matrix
6f2a5b4
fix(containerprofilecache): re-wire R1016 tamper alert + expand Test_31
fe80d73
test(component): Test_32 profile uses full-path argv[0]
f828777
test: AP-fixture linter (R-AP-* rules) + canonical reference profile
7c10baa
fix(tamper_alert): accept self-signed profiles, only flag actual tamper
4d374d5
test(component): make Test_30 30b deterministic by re-execing inside …
a59e284
deps(storage): bump replace to f44fed80 (analyzer trailing-* fix)
bcf41ea
deps(storage): bump replace to 4ab95fb8 (PR #25 merged on fork main)
9385562
test(component): Test_33_AnalyzeOpensWildcardAnchoring
cb57674
test(component): rework Test_33 negative cases to probe under R0002's…
484d11c
test(component): fix Test_28 + Test_31 31b flakiness
4dd0b39
test(component): sign-after-roundtrip in Test_31 to defeat content-dr…
6dda020
test(component): bump Test_33 WaitForReady to 180s for cluster-pressu…
a5af261
deps(storage): bump replace to 43795bb4 (storage feat/exec-arg-wildca…
c4ac7b5
test(aplint): drop redundant p := p loop var (Go 1.22+, copyloopvar l…
e7dc486
fix(tamper_alert): R1016 dedup + use real WLID
98bdf97
deps(storage): bump replace to b0d68d3d (empty-Args wildcard match)
0cf4a50
fix: address CodeRabbit second-review batch on PR #38
8d89240
fix: address CodeRabbit third-review batch on PR #38 (0cf4a503)
26bc4bc
Merge remote-tracking branch 'origin/main' into merge/exec-arg-wildcards
045940c
fix(test): make dedup-clearing assertion non-trivial
eb40bbf
fix(ci): drop 'permissions: read-all' from reusable workflows
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| // Tamper detection for user-supplied profile overlays loaded into the | ||
| // ContainerProfileCache. | ||
| // | ||
| // When a user references a signed ApplicationProfile or NetworkNeighborhood | ||
| // via the `kubescape.io/user-defined-profile` pod label, this code path | ||
| // re-verifies the signature on every cache load and emits an R1016 | ||
| // "Signed profile tampered" alert via the rule-alert exporter when the | ||
| // signature is present but no longer valid. | ||
| // | ||
| // This is the new home of the legacy applicationprofilecache's tamper | ||
| // detection (originally introduced in fork commit c2d681e0 — "Feat/ | ||
| // tamperalert"). Upstream PR #788 deleted the legacy cache; this re-wires | ||
| // the same behavior onto containerprofilecache without changing the alert | ||
| // shape so existing component tests (Test_31_TamperDetectionAlert) keep | ||
| // working. | ||
| package containerprofilecache | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "strings" | ||
|
|
||
| "github.com/armosec/armoapi-go/armotypes" | ||
| "github.com/kubescape/go-logger" | ||
| "github.com/kubescape/go-logger/helpers" | ||
| "github.com/kubescape/node-agent/pkg/exporters" | ||
| "github.com/kubescape/node-agent/pkg/rulemanager/types" | ||
| "github.com/kubescape/node-agent/pkg/signature" | ||
| "github.com/kubescape/node-agent/pkg/signature/profiles" | ||
| "github.com/kubescape/storage/pkg/apis/softwarecomposition/v1beta1" | ||
| ) | ||
|
|
||
| // SetTamperAlertExporter wires the rule-alert exporter used to emit R1016. | ||
| // Optional — when nil, signature verification still runs (and is logged) | ||
| // but no alert is emitted. Production wiring lives in cmd/main.go after the | ||
| // alert exporter is constructed. | ||
| func (c *ContainerProfileCacheImpl) SetTamperAlertExporter(e exporters.Exporter) { | ||
| c.tamperAlertExporter = e | ||
| } | ||
|
|
||
| // verifyUserApplicationProfile re-verifies the signature of a user-supplied | ||
| // ApplicationProfile overlay and emits R1016 if the signature is present | ||
| // but no longer valid (i.e. the profile was tampered after signing). | ||
| // | ||
| // Returns true iff the profile is acceptable for further use: | ||
| // - profile is signed and verifies → true | ||
| // - profile is not signed → true (signing is opt-in; the empty-signature | ||
| // case is handled by the caller's normal not-signed flow) | ||
| // - profile is signed but verification fails → false (and R1016 emitted) | ||
| // | ||
| // The boolean lets the caller decide whether to project the overlay into | ||
| // the cache. Today we always proceed (the legacy semantics don't actually | ||
| // gate loading on verification unless EnableSignatureVerification is true), | ||
| // but having the return value keeps the door open for stricter modes. | ||
| func (c *ContainerProfileCacheImpl) verifyUserApplicationProfile(profile *v1beta1.ApplicationProfile, containerID string) bool { | ||
| if profile == nil { | ||
| return true | ||
| } | ||
| adapter := profiles.NewApplicationProfileAdapter(profile) | ||
| if !signature.IsSigned(adapter) { | ||
| return true | ||
| } | ||
| // AllowUntrusted: accept self-signed/local-CA signatures as long as the | ||
| // signature itself verifies against the cert in the annotations. We only | ||
| // want to flag actual tampering, not the absence of a Sigstore Fulcio | ||
| // trust chain. Matches `cmd/sign-object`'s default verifier. | ||
| if err := signature.VerifyObjectAllowUntrusted(adapter); err != nil { | ||
| logger.L().Warning("user-defined ApplicationProfile signature verification failed (tamper detected)", | ||
| helpers.String("profile", profile.Name), | ||
| helpers.String("namespace", profile.Namespace), | ||
| helpers.String("containerID", containerID), | ||
| helpers.Error(err)) | ||
| c.emitTamperAlert(profile.Name, profile.Namespace, containerID, "ApplicationProfile", err) | ||
| return !c.cfg.EnableSignatureVerification | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // verifyUserNetworkNeighborhood is the NN-side counterpart to | ||
| // verifyUserApplicationProfile. Same contract, different object kind in | ||
| // the alert description. | ||
| func (c *ContainerProfileCacheImpl) verifyUserNetworkNeighborhood(nn *v1beta1.NetworkNeighborhood, containerID string) bool { | ||
| if nn == nil { | ||
| return true | ||
| } | ||
| adapter := profiles.NewNetworkNeighborhoodAdapter(nn) | ||
| if !signature.IsSigned(adapter) { | ||
| return true | ||
| } | ||
| if err := signature.VerifyObjectAllowUntrusted(adapter); err != nil { | ||
| logger.L().Warning("user-defined NetworkNeighborhood signature verification failed (tamper detected)", | ||
| helpers.String("profile", nn.Name), | ||
| helpers.String("namespace", nn.Namespace), | ||
| helpers.String("containerID", containerID), | ||
| helpers.Error(err)) | ||
| c.emitTamperAlert(nn.Name, nn.Namespace, containerID, "NetworkNeighborhood", err) | ||
| return !c.cfg.EnableSignatureVerification | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| // emitTamperAlert sends a single R1016 "Signed profile tampered" alert | ||
| // through the rule-alert exporter. No-op when the exporter is unset. | ||
| // | ||
| // Alert shape mirrors the legacy applicationprofilecache.emitTamperAlert | ||
| // (fork commit c2d681e0) so dashboards and component tests keep matching. | ||
| func (c *ContainerProfileCacheImpl) emitTamperAlert(profileName, namespace, containerID, objectKind string, verifyErr error) { | ||
| if c.tamperAlertExporter == nil { | ||
| return | ||
| } | ||
|
|
||
| ruleFailure := &types.GenericRuleFailure{ | ||
| BaseRuntimeAlert: armotypes.BaseRuntimeAlert{ | ||
| AlertName: "Signed profile tampered", | ||
| InfectedPID: 1, | ||
| Severity: 10, | ||
| FixSuggestions: "Investigate who modified the " + objectKind + " '" + profileName + "' in namespace '" + namespace + "'. Re-sign the profile after verifying its contents.", | ||
| }, | ||
| AlertType: armotypes.AlertTypeRule, | ||
| RuntimeProcessDetails: armotypes.ProcessTree{ | ||
| ProcessTree: armotypes.Process{ | ||
| PID: 1, | ||
| Comm: "node-agent", | ||
| }, | ||
| }, | ||
| RuleAlert: armotypes.RuleAlert{ | ||
| RuleDescription: fmt.Sprintf("Signed %s '%s' in namespace '%s' has been tampered with: %v", | ||
| objectKind, profileName, namespace, verifyErr), | ||
| }, | ||
| RuntimeAlertK8sDetails: armotypes.RuntimeAlertK8sDetails{ | ||
| Namespace: namespace, | ||
| }, | ||
| RuleID: "R1016", | ||
| } | ||
|
|
||
| // Best-effort workload identifier. The legacy cache used a wlid string; | ||
| // this cache is keyed on containerID, so we just stash that as the | ||
| // workload reference. Downstream consumers (Alertmanager, exporter | ||
| // pipelines) don't structurally depend on the wlid prefix. | ||
| ruleFailure.SetWorkloadDetails(extractWlidFromContainerID(containerID)) | ||
|
|
||
| c.tamperAlertExporter.SendRuleAlert(ruleFailure) | ||
| } | ||
|
|
||
| // extractWlidFromContainerID is a placeholder that returns the containerID | ||
| // as-is. The legacy cache had a richer "wlid://<cluster>/<namespace>/<kind>/ | ||
| // <name>/<templateHash>" string available; the new cache is keyed on | ||
| // containerID so callers that consume wlid get an opaque identifier here. | ||
| // Retained as a separate function so the alert path can be upgraded to a | ||
| // proper wlid lookup later without touching emitTamperAlert. | ||
| func extractWlidFromContainerID(containerID string) string { | ||
| if idx := strings.LastIndex(containerID, "/"); idx > 0 { | ||
| return containerID[:idx] | ||
| } | ||
| return containerID | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.