From 7ecd130a84a5197842cfe96d4eec9791f07aced5 Mon Sep 17 00:00:00 2001
From: Noah <40781376+noahpb@users.noreply.github.com>
Date: Tue, 3 Sep 2024 18:57:36 -0400
Subject: [PATCH] fix: default `ctx.allowPrivilegeEscalation` to `false` if
 `undefined` (#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>
---
 src/pepr/policies/security.ts | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/pepr/policies/security.ts b/src/pepr/policies/security.ts
index e4b782433..6d614b58c 100644
--- a/src/pepr/policies/security.ts
+++ b/src/pepr/policies/security.ts
@@ -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) {