-
Notifications
You must be signed in to change notification settings - Fork 21
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: exemption race conditions (#407)
## Description Fixes race conditions with exemptions that result in overwrites of previous exemptions in the Pepr store or mutating then allowing pods that were meant to be exempted from mutation. ## Issue Fixes #409 Fixes #314 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [x] Test, docs, adr added or updated as needed - [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)(https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md#submitting-a-pull-request) followed --------- Co-authored-by: Case Wylie <[email protected]> Co-authored-by: Micah Nagel <[email protected]> Co-authored-by: Rob Ferguson <[email protected]>
- Loading branch information
1 parent
a62f9a0
commit d1b3b56
Showing
34 changed files
with
773 additions
and
606 deletions.
There are no files selected for viewing
This file contains 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 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 |
---|---|---|
@@ -1,22 +1,42 @@ | ||
import { PeprModule } from "pepr"; | ||
import { Log, PeprModule } from "pepr"; | ||
|
||
import cfg from "./package.json"; | ||
|
||
import { istio } from "./src/pepr/istio"; | ||
import { operator } from "./src/pepr/operator"; | ||
import { policies } from "./src/pepr/policies"; | ||
import { Policy } from "./src/pepr/operator/crd"; | ||
import { registerCRDs } from "./src/pepr/operator/crd/register"; | ||
import { policies, startExemptionWatch } from "./src/pepr/policies"; | ||
import { prometheus } from "./src/pepr/prometheus"; | ||
|
||
new PeprModule(cfg, [ | ||
// UDS Core Operator | ||
operator, | ||
(async () => { | ||
// Apply the CRDs to the cluster | ||
await registerCRDs(); | ||
// KFC watch for exemptions and update in-memory map | ||
await startExemptionWatch(); | ||
new PeprModule(cfg, [ | ||
// UDS Core Operator | ||
operator, | ||
|
||
// UDS Core Policies | ||
policies, | ||
// UDS Core Policies | ||
policies, | ||
|
||
// Istio service mesh | ||
istio, | ||
// Istio service mesh | ||
istio, | ||
|
||
// Prometheus monitoring stack | ||
prometheus, | ||
]); | ||
// Prometheus monitoring stack | ||
prometheus, | ||
]); | ||
// Remove legacy policy entries from the pepr store for the 0.5.0 upgrade | ||
if (process.env.PEPR_WATCH_MODE === "true" && cfg.version === "0.5.0") { | ||
Log.debug("Clearing legacy pepr store exemption entries..."); | ||
policies.Store.onReady(() => { | ||
for (const p of Object.values(Policy)) { | ||
policies.Store.removeItem(p); | ||
} | ||
}); | ||
} | ||
})().catch(err => { | ||
Log.error(err); | ||
process.exit(1); | ||
}); |
This file contains 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
99 changes: 99 additions & 0 deletions
99
src/pepr/operator/controllers/exemptions/exemption-store.spec.ts
This file contains 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,99 @@ | ||
import { beforeEach, describe, expect, it } from "@jest/globals"; | ||
import { Matcher, MatcherKind, Policy } from "../../crd"; | ||
import { ExemptionStore } from "./exemption-store"; | ||
|
||
const enforcerMatcher = { | ||
namespace: "neuvector", | ||
name: "^neuvector-enforcer-pod.*", | ||
kind: MatcherKind.Pod, | ||
}; | ||
|
||
const controllerMatcher = { | ||
namespace: "neuvector", | ||
name: "^neuvector-controller-pod.*", | ||
kind: MatcherKind.Pod, | ||
}; | ||
|
||
const getExemption = (uid: string, matcher: Matcher, policies: Policy[]) => { | ||
return { | ||
metadata: { | ||
uid, | ||
}, | ||
spec: { | ||
exemptions: [ | ||
{ | ||
matcher, | ||
policies, | ||
}, | ||
], | ||
}, | ||
}; | ||
}; | ||
|
||
describe("Exemption Store", () => { | ||
beforeEach(() => { | ||
ExemptionStore.init(); | ||
}); | ||
|
||
it("Add exemption", async () => { | ||
const e = getExemption("uid", enforcerMatcher, [Policy.DisallowPrivileged]); | ||
ExemptionStore.add(e); | ||
const matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
|
||
expect(matchers).toHaveLength(1); | ||
}); | ||
|
||
it("Delete exemption", async () => { | ||
const e = getExemption("uid", enforcerMatcher, [Policy.DisallowPrivileged]); | ||
ExemptionStore.add(e); | ||
let matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(1); | ||
ExemptionStore.remove(e); | ||
|
||
matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(0); | ||
}); | ||
|
||
it("Update exemption", async () => { | ||
const enforcerException = getExemption("uid", enforcerMatcher, [Policy.DisallowPrivileged]); | ||
ExemptionStore.add(enforcerException); | ||
|
||
let matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(1); | ||
|
||
const controllerExemption = getExemption("uid", controllerMatcher, [Policy.RequireNonRootUser]); | ||
ExemptionStore.add(controllerExemption); | ||
|
||
matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(0); | ||
}); | ||
|
||
it("Add multiple policies", async () => { | ||
const enforcerException = getExemption("foo", enforcerMatcher, [Policy.DisallowPrivileged]); | ||
ExemptionStore.add(enforcerException); | ||
|
||
let matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(1); | ||
|
||
const controllerExemption = getExemption("bar", controllerMatcher, [Policy.RequireNonRootUser]); | ||
ExemptionStore.add(controllerExemption); | ||
|
||
matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(1); | ||
|
||
matchers = ExemptionStore.getByPolicy(Policy.RequireNonRootUser); | ||
expect(matchers).toHaveLength(1); | ||
}); | ||
|
||
it("Add duplicate exemptions owned by different owners", async () => { | ||
const enforcerException = getExemption("foo", enforcerMatcher, [Policy.DisallowPrivileged]); | ||
const otherEnforcerException = getExemption("bar", enforcerMatcher, [ | ||
Policy.DisallowPrivileged, | ||
]); | ||
ExemptionStore.add(enforcerException); | ||
ExemptionStore.add(otherEnforcerException); | ||
|
||
const matchers = ExemptionStore.getByPolicy(Policy.DisallowPrivileged); | ||
expect(matchers).toHaveLength(2); | ||
}); | ||
}); |
83 changes: 83 additions & 0 deletions
83
src/pepr/operator/controllers/exemptions/exemption-store.ts
This file contains 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,83 @@ | ||
import { Log } from "pepr"; | ||
import { StoredMatcher } from "../../../policies"; | ||
import { Matcher, Policy, UDSExemption } from "../../crd"; | ||
|
||
export type PolicyOwnerMap = Map<string, UDSExemption>; | ||
export type PolicyMap = Map<Policy, StoredMatcher[]>; | ||
let policyExemptionMap: PolicyMap; | ||
let policyOwnerMap: PolicyOwnerMap; | ||
|
||
function init(): void { | ||
policyExemptionMap = new Map(); | ||
policyOwnerMap = new Map(); | ||
for (const p of Object.values(Policy)) { | ||
policyExemptionMap.set(p, []); | ||
} | ||
} | ||
|
||
function getByPolicy(policy: Policy): StoredMatcher[] { | ||
return policyExemptionMap.get(policy) || []; | ||
} | ||
|
||
function setByPolicy(policy: Policy, matchers: StoredMatcher[]): void { | ||
policyExemptionMap.set(policy, matchers); | ||
} | ||
|
||
function addMatcher(matcher: Matcher, p: Policy, owner: string = ""): void { | ||
const storedMatcher = { | ||
...matcher, | ||
owner, | ||
}; | ||
|
||
const storedMatchers = getByPolicy(p); | ||
storedMatchers.push(storedMatcher); | ||
} | ||
|
||
// Iterate through each exemption block of CR and add matchers to PolicyMap | ||
function add(exemption: UDSExemption, log: boolean = true) { | ||
// Remove any existing exemption for this owner, in case of WatchPhase.Modified | ||
remove(exemption); | ||
const owner = exemption.metadata?.uid || ""; | ||
policyOwnerMap.set(owner, exemption); | ||
|
||
for (const e of exemption.spec?.exemptions ?? []) { | ||
const policies = e.policies ?? []; | ||
for (const p of policies) { | ||
// Append the matcher to the list of stored matchers for this policy | ||
addMatcher(e.matcher, p, owner); | ||
if (log) { | ||
Log.debug(`Added exemption to ${p}: ${JSON.stringify(e.matcher)}`); | ||
} | ||
} | ||
} | ||
} | ||
|
||
function remove(exemption: UDSExemption) { | ||
const owner = exemption.metadata?.uid || ""; | ||
const prevExemption = policyOwnerMap.get(owner); | ||
|
||
if (prevExemption) { | ||
for (const e of prevExemption.spec?.exemptions ?? []) { | ||
const policies = e.policies ?? []; | ||
for (const p of policies) { | ||
const existingMatchers = getByPolicy(p); | ||
const filteredList = existingMatchers.filter(m => { | ||
return m.owner !== owner; | ||
}); | ||
setByPolicy(p, filteredList); | ||
} | ||
} | ||
policyOwnerMap.delete(owner); | ||
Log.debug(`Removed all policy exemptions for ${owner}`); | ||
} else { | ||
Log.debug(`No existing exemption for owner ${owner}`); | ||
} | ||
} | ||
|
||
// export object with all included export as properties | ||
export const ExemptionStore = { | ||
init, | ||
add, | ||
remove, | ||
getByPolicy, | ||
}; |
Oops, something went wrong.