Skip to content

Commit

Permalink
fix: default ctx.allowPrivilegeEscalation to false if undefined (
Browse files Browse the repository at this point in the history
…#698)

## Description
The default behavior when admitting pod containers that do not have
`securityContext.allowPrivilegeEscalation` explicitly defined is to
admit the request. As noted in #527, if not included,
`allowPrivilegeEscalation` defaults to `true`.

This PR updates the `DisallowPrivileged` policy to match any containers
that do not have a `securityContext` and/or `allowPrivilegeEscalation`
defined and mutates to explicitly set to `false`.

Configuring the policy to deny resources that do not have
`allowPrivilegeEscalation` explicitly defined could break existing
deployments. Adding a mutation is a safe bet, assuming existing
workloads are not already taking advantage of privilege escalation.

Read more about the default behavior:

https://medium.com/pareture/how-allowprivilegeescalation-works-in-kubernetes-ce696494f87b

## Related Issue
Fixes #527 

## 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

- [ ] Test, docs, adr added or updated as needed
- [ ] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

---------

Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
Co-authored-by: Palassis <40472433+MxNxPx@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 3, 2024
1 parent d71cb44 commit 7ecd130
Showing 1 changed file with 28 additions and 2 deletions.
30 changes: 28 additions & 2 deletions src/pepr/policies/security.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,40 @@ import { exemptionAnnotationPrefix, isExempt, markExemption } from "./exemptions
*/
When(a.Pod)
.IsCreatedOrUpdated()
.Mutate(markExemption(Policy.DisallowPrivileged))
.Mutate(request => {
markExemption(Policy.DisallowPrivileged)(request);
if (request.HasAnnotation(`${exemptionAnnotationPrefix}.${Policy.DisallowPrivileged}`)) {
return;
}
let wasMutated = false;

// Check if any containers defined in the pod do not have the `allowPrivilegeEscalation` field present. If not, include it and set to false.
for (const container of containers(request)) {
container.securityContext = container.securityContext || {};
const mutateCriteria = [
container.securityContext.allowPrivilegeEscalation === undefined,
!container.securityContext.privileged,
!container.securityContext.capabilities?.add?.includes("CAP_SYS_ADMIN"),
];
// We are only mutating if the conditions above are all satisfied
if (mutateCriteria.every(priv => priv === true)) {
container.securityContext.allowPrivilegeEscalation = false;
wasMutated = true;
}
}
if (wasMutated) {
annotateMutation(request, Policy.DisallowPrivileged);
}
})
.Validate(request => {
if (isExempt(request, Policy.DisallowPrivileged)) {
return request.Approve();
}

const violations = securityContextContainers(request).filter(
c => c.ctx.allowPrivilegeEscalation || c.ctx.privileged,
// Checking if allowPrivilegeEscalation is undefined. If yes, fallback to true as the default behavior in k8s is to allow if undefined.
// Checks the three different ways a container could escalate to admin privs
c => (c.ctx.allowPrivilegeEscalation ?? true) || c.ctx.privileged,
);

if (violations.length) {
Expand Down

0 comments on commit 7ecd130

Please sign in to comment.