From b6ebc4945f6eef132b3ae33fec106b4cb275574a Mon Sep 17 00:00:00 2001
From: Chance <139784371+UnicornChance@users.noreply.github.com>
Date: Tue, 10 Sep 2024 11:55:49 -0600
Subject: [PATCH] feat: investigate and restrict network policies (#719)

## Description
Our package should operate under a "least privilege" type model for
network access, and specifically egress network access should be limited
to specific services/addresses rather than "anywhere".

Investigated current `anywhere` policies, updated restrictions where
necessary. Added a new package CR field `remoteCidr` for defining a
custom cidr to be used in place of the anywhere cidr. Add some
validations to verify the use the `remoteGenerated`, `remoteSelector`,
`remoteNamespace`, and `remoteCidr` don't overlap or break each other.
They should be used individually except `remoteSelector` and
`remoteNamespace` being used together.

Potentially follow on issues for _KubeAPI ingress relation network
policy management, as well as utilizing service entries for known things
like S3 buckets.

## Related Issue

Fixes #558

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] 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
- [x] [Contributor
Guide](https://github.com/defenseunicorns/uds-template-capability/blob/main/CONTRIBUTING.md)
followed

---------

Co-authored-by: Micah Nagel <micah.nagel@defenseunicorns.com>
---
 .../chart/templates/uds-package.yaml          | 17 +++++-
 src/authservice/chart/values.yaml             |  8 +++
 src/grafana/chart/templates/uds-package.yaml  | 28 +++++----
 src/keycloak/chart/templates/_helpers.tpl     |  2 +-
 src/keycloak/chart/templates/uds-package.yaml | 11 +++-
 src/keycloak/chart/values.yaml                |  6 ++
 src/loki/chart/templates/uds-package.yaml     | 17 +++---
 src/loki/chart/values.yaml                    |  6 ++
 .../chart/templates/uds-package.yaml          | 12 ++--
 src/pepr/operator/README.md                   |  7 ---
 .../controllers/network/generate.spec.ts      | 58 +++++++++++++++++++
 .../operator/controllers/network/generate.ts  |  3 +
 .../network/generators/remoteCidr.ts          | 12 ++++
 .../crd/generated/package-v1alpha1.ts         |  4 ++
 .../operator/crd/sources/package/v1alpha1.ts  |  4 ++
 .../crd/validators/package-validator.ts       | 31 +++++++++-
 .../chart/templates/uds-package.yaml          | 10 +---
 src/promtail/chart/templates/uds-package.yaml |  7 ---
 src/test/app-tenant.yaml                      |  6 ++
 src/velero/chart/templates/uds-package.yaml   | 10 +++-
 src/velero/chart/values.yaml                  |  6 ++
 21 files changed, 210 insertions(+), 55 deletions(-)
 create mode 100644 src/pepr/operator/controllers/network/generators/remoteCidr.ts

diff --git a/src/authservice/chart/templates/uds-package.yaml b/src/authservice/chart/templates/uds-package.yaml
index bf1041bd3..0e4e583de 100644
--- a/src/authservice/chart/templates/uds-package.yaml
+++ b/src/authservice/chart/templates/uds-package.yaml
@@ -15,8 +15,23 @@ spec:
 
       # Egress must be allowed to the external facing Keycloak endpoint
       - direction: Egress
+        remoteSelector:
+          app: tenant-ingressgateway
+        remoteNamespace: istio-tenant-gateway
+        description: "SSO Provider"
+
+      {{- if .Values.redis.uri }}
+      - direction: Egress
+        description: Redis Session Store
+        {{- if .Values.redis.internal.enabled }}
+        remoteSelector: {{ .Values.redis.internal.remoteSelector }}
+        remoteNamespace: {{ .Values.redis.internal.remoteNamespace }}
+        {{- else if .Values.redis.egressCidr }}
+        remoteCidr: {{ .Values.redis.egressCidr }}
+        {{- else }}
         remoteGenerated: Anywhere
-        description: "SSO Provider & Redis Session Store"
+        {{- end }}
+      {{- end }}
 
       - direction: Ingress
         selector:
diff --git a/src/authservice/chart/values.yaml b/src/authservice/chart/values.yaml
index b28496153..06a631c9c 100644
--- a/src/authservice/chart/values.yaml
+++ b/src/authservice/chart/values.yaml
@@ -9,6 +9,14 @@ image:
 
 nameOverride: "authservice"
 
+redis:
+  uri: "###ZARF_VAR_AUTHSERVICE_REDIS_URI###"
+  egressCidr: ""
+  internal:
+    enabled: false
+    remoteSelector: {}
+    remoteNamespace: ""
+
 podAnnotations: {}
 
 podSecurityContext: {}
diff --git a/src/grafana/chart/templates/uds-package.yaml b/src/grafana/chart/templates/uds-package.yaml
index 713a103a0..ed5a08457 100644
--- a/src/grafana/chart/templates/uds-package.yaml
+++ b/src/grafana/chart/templates/uds-package.yaml
@@ -28,23 +28,31 @@ spec:
         targetPort: 3000
 
     allow:
-      - direction: Ingress
+      # Egress allowed to Loki
+      - direction: Egress
         selector:
           app.kubernetes.io/name: grafana
-        remoteNamespace: tempo
+        remoteNamespace: loki
         remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9090
-        description: "Tempo Datasource"
+          app.kubernetes.io/name: loki
+        description: "Loki Datasource"
+        port: 8080
 
+      # Egress allowed to Prometheus
       - direction: Egress
         selector:
           app.kubernetes.io/name: grafana
-        remoteGenerated: Anywhere
+        remoteNamespace: monitoring
+        remoteSelector:
+          app.kubernetes.io/name: prometheus
+        description: "Prometheus Datasource"
+        port: 9090
 
+      # Egress allowed to Keyclaok
       - direction: Egress
-        remoteNamespace: tempo
+        selector:
+          app.kubernetes.io/name: grafana
+        remoteNamespace: keycloak
         remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
+          app.kubernetes.io/name: keycloak
+        description: "SSO Provider"
diff --git a/src/keycloak/chart/templates/_helpers.tpl b/src/keycloak/chart/templates/_helpers.tpl
index bb0825a07..a5ce50f28 100644
--- a/src/keycloak/chart/templates/_helpers.tpl
+++ b/src/keycloak/chart/templates/_helpers.tpl
@@ -89,7 +89,7 @@ Check external PostgreSQL connection information. Fails when required values are
 {{- else -}}{{fail "You must define \"username\", \"password\", \"database\", \"host\", and \"port\" for \"postgresql\"."}}
 {{- end -}}
 {{- default "true" "" }}
-{{- else if not (empty (compact (values (omit .Values.postgresql "port")))) -}}
+{{- else if not (empty (compact (values (omit .Values.postgresql "port" "internal")))) -}}
 {{ fail "Cannot use an external PostgreSQL Database when devMode is enabled." -}}
 {{- else -}}
 {{ default "false" "" }}
diff --git a/src/keycloak/chart/templates/uds-package.yaml b/src/keycloak/chart/templates/uds-package.yaml
index 6ea6a2756..2c1c52e47 100644
--- a/src/keycloak/chart/templates/uds-package.yaml
+++ b/src/keycloak/chart/templates/uds-package.yaml
@@ -26,7 +26,7 @@ spec:
         port: 8080
 
       # Temp workaround for any cluster pod
-      # @todo: remove this once cluster pods is a remote generated target
+      # todo: remove this once cluster pods is a remote generated target
       - description: "Keycloak backchannel access"
         direction: Ingress
         selector:
@@ -34,6 +34,7 @@ spec:
         remoteGenerated: Anywhere
         port: 8080
 
+      # Keycloak OCSP to check certs cannot guarantee a static IP
       - description: "OCSP Lookup"
         direction: Egress
         selector:
@@ -58,8 +59,16 @@ spec:
         selector:
           app.kubernetes.io/name: keycloak
         port: {{ .Values.postgresql.port }}
+        {{- if .Values.postgresql.internal.enabled }}
+        remoteSelector: {{ .Values.postgresql.internal.remoteSelector }}
+        remoteNamespace: {{ .Values.postgresql.internal.remoteNamespace }}
+        {{- else if .Values.postgresql.egressCidr }}
+        remoteCidr: {{ .Values.postgresql.egressCidr }}
+        {{- else }}
         remoteGenerated: Anywhere
+        {{- end }}
       {{- end }}
+
       {{- if .Values.autoscaling.enabled }}
       # HA for keycloak
       - direction: Ingress
diff --git a/src/keycloak/chart/values.yaml b/src/keycloak/chart/values.yaml
index e5ee480c8..07d04ebe6 100644
--- a/src/keycloak/chart/values.yaml
+++ b/src/keycloak/chart/values.yaml
@@ -174,6 +174,12 @@ postgresql:
   host: ""
   # Port the database is listening on
   port: 5432
+  egressCidr: ""
+  # Configure internal postgresql deployment, requires keycloak not be deployed in dev-mode
+  internal:
+    enabled: false
+    remoteSelector: {}
+    remoteNamespace: ""
 
 serviceMonitor:
   # If `true`, a ServiceMonitor resource for the prometheus-operator is created
diff --git a/src/loki/chart/templates/uds-package.yaml b/src/loki/chart/templates/uds-package.yaml
index 8f30a3d0c..010f02cf7 100644
--- a/src/loki/chart/templates/uds-package.yaml
+++ b/src/loki/chart/templates/uds-package.yaml
@@ -44,15 +44,16 @@ spec:
           - 8080
         description: "Promtail Log Storage"
 
-      # Todo: wide open for now for pushing to s3
+      # Egress for S3 connections
       - direction: Egress
         selector:
           app.kubernetes.io/name: loki
+        description: Storage
+        {{- if .Values.storage.internal.enabled }}
+        remoteSelector: {{ .Values.storage.internal.remoteSelector }}
+        remoteNamespace: {{ .Values.storage.internal.remoteNamespace }}
+        {{- else if .Values.storage.egressCidr }}
+        remoteCidr: {{ .Values.storage.egressCidr }}
+        {{- else }}
         remoteGenerated: Anywhere
-
-      - direction: Egress
-        remoteNamespace: tempo
-        remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
+        {{- end }}
diff --git a/src/loki/chart/values.yaml b/src/loki/chart/values.yaml
index e69de29bb..fbb557b5a 100644
--- a/src/loki/chart/values.yaml
+++ b/src/loki/chart/values.yaml
@@ -0,0 +1,6 @@
+storage:
+  internal:
+    enabled: false
+    remoteSelector: {}
+    remoteNamespace: ""
+  egressCidr: ""
diff --git a/src/neuvector/chart/templates/uds-package.yaml b/src/neuvector/chart/templates/uds-package.yaml
index f9c4bd08e..1cdee101d 100644
--- a/src/neuvector/chart/templates/uds-package.yaml
+++ b/src/neuvector/chart/templates/uds-package.yaml
@@ -58,9 +58,12 @@ spec:
 
       # Access to SSO for OIDC
       - direction: Egress
-        remoteGenerated: Anywhere
         selector:
           app: neuvector-controller-pod
+        remoteSelector:
+          app: tenant-ingressgateway
+        remoteNamespace: istio-tenant-gateway
+        description: "SSO Provider"
 
       - direction: Egress
         remoteGenerated: KubeAPI
@@ -79,10 +82,3 @@ spec:
           app: neuvector-controller-pod
         port: 30443
         description: "Webhook"
-
-      - direction: Egress
-        remoteNamespace: tempo
-        remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
diff --git a/src/pepr/operator/README.md b/src/pepr/operator/README.md
index b03a84936..5b95cea2d 100644
--- a/src/pepr/operator/README.md
+++ b/src/pepr/operator/README.md
@@ -42,13 +42,6 @@ spec:
           app.kubernetes.io/name: grafana
         remoteGenerated: Anywhere
 
-      - direction: Egress
-        remoteNamespace: tempo
-        remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
-
   # SSO allows for the creation of Keycloak clients and with automatic secret generation
   sso:
     - name: Grafana Dashboard
diff --git a/src/pepr/operator/controllers/network/generate.spec.ts b/src/pepr/operator/controllers/network/generate.spec.ts
index 559bcf18a..9abeb1647 100644
--- a/src/pepr/operator/controllers/network/generate.spec.ts
+++ b/src/pepr/operator/controllers/network/generate.spec.ts
@@ -111,3 +111,61 @@ describe("network policy generate", () => {
     policyTypes: ["Egress"],
   } as kind.NetworkPolicy["spec"]);
 });
+
+describe("network policy generate with remoteCidr", () => {
+  it("should generate correct network policy with remoteCidr for Egress", async () => {
+    const policy = generate("test", {
+      description: "test",
+      direction: Direction.Egress,
+      selector: { app: "test" },
+      remoteCidr: "192.168.0.0/16",
+    });
+
+    expect(policy.metadata?.name).toEqual("Egress-test");
+    expect(policy.spec).toEqual({
+      egress: [
+        {
+          to: [
+            {
+              ipBlock: {
+                cidr: "192.168.0.0/16",
+                except: ["169.254.169.254/32"], // Include the except field here
+              },
+            },
+          ],
+          ports: [],
+        },
+      ],
+      podSelector: { matchLabels: { app: "test" } },
+      policyTypes: ["Egress"],
+    } as kind.NetworkPolicy["spec"]);
+  });
+
+  it("should generate correct network policy with remoteCidr for Ingress", async () => {
+    const policy = generate("test", {
+      description: "test",
+      direction: Direction.Ingress,
+      selector: { app: "test" },
+      remoteCidr: "10.0.0.0/8",
+    });
+
+    expect(policy.metadata?.name).toEqual("Ingress-test");
+    expect(policy.spec).toEqual({
+      ingress: [
+        {
+          from: [
+            {
+              ipBlock: {
+                cidr: "10.0.0.0/8",
+                except: ["169.254.169.254/32"], // Include the except field here
+              },
+            },
+          ],
+          ports: [],
+        },
+      ],
+      podSelector: { matchLabels: { app: "test" } },
+      policyTypes: ["Ingress"],
+    } as kind.NetworkPolicy["spec"]);
+  });
+});
diff --git a/src/pepr/operator/controllers/network/generate.ts b/src/pepr/operator/controllers/network/generate.ts
index e64f19402..ecba6d1cb 100644
--- a/src/pepr/operator/controllers/network/generate.ts
+++ b/src/pepr/operator/controllers/network/generate.ts
@@ -6,6 +6,7 @@ import { anywhere } from "./generators/anywhere";
 import { cloudMetadata } from "./generators/cloudMetadata";
 import { intraNamespace } from "./generators/intraNamespace";
 import { kubeAPI } from "./generators/kubeAPI";
+import { remoteCidr } from "./generators/remoteCidr";
 
 function isWildcardNamespace(namespace: string) {
   return namespace === "" || namespace === "*";
@@ -52,6 +53,8 @@ function getPeers(policy: Allow): V1NetworkPolicyPeer[] {
     }
 
     peers.push(peer);
+  } else if (policy.remoteCidr !== undefined) {
+    peers = [remoteCidr(policy.remoteCidr)];
   }
 
   return peers;
diff --git a/src/pepr/operator/controllers/network/generators/remoteCidr.ts b/src/pepr/operator/controllers/network/generators/remoteCidr.ts
new file mode 100644
index 000000000..031e43f16
--- /dev/null
+++ b/src/pepr/operator/controllers/network/generators/remoteCidr.ts
@@ -0,0 +1,12 @@
+import { V1NetworkPolicyPeer } from "@kubernetes/client-node";
+import { META_IP } from "./cloudMetadata";
+
+/** Matches a specific custom cidr EXCEPT the Cloud Meta endpoint */
+export function remoteCidr(cidr: string): V1NetworkPolicyPeer {
+  return {
+    ipBlock: {
+      cidr,
+      except: [META_IP],
+    },
+  };
+}
diff --git a/src/pepr/operator/crd/generated/package-v1alpha1.ts b/src/pepr/operator/crd/generated/package-v1alpha1.ts
index 311ec223f..20d896d92 100644
--- a/src/pepr/operator/crd/generated/package-v1alpha1.ts
+++ b/src/pepr/operator/crd/generated/package-v1alpha1.ts
@@ -144,6 +144,10 @@ export interface Allow {
    * A list of ports to allow (protocol is always TCP)
    */
   ports?: number[];
+  /**
+   * Custom generated policy CIDR
+   */
+  remoteCidr?: string;
   /**
    * Custom generated remote selector for the policy
    */
diff --git a/src/pepr/operator/crd/sources/package/v1alpha1.ts b/src/pepr/operator/crd/sources/package/v1alpha1.ts
index 33d288bc8..8af0ed32a 100644
--- a/src/pepr/operator/crd/sources/package/v1alpha1.ts
+++ b/src/pepr/operator/crd/sources/package/v1alpha1.ts
@@ -84,6 +84,10 @@ const allow = {
         type: "string",
         enum: ["KubeAPI", "IntraNamespace", "CloudMetadata", "Anywhere"],
       },
+      remoteCidr: {
+        description: "Custom generated policy CIDR",
+        type: "string",
+      },
       port: {
         description: "The port to allow (protocol is always TCP)",
         minimum: 1,
diff --git a/src/pepr/operator/crd/validators/package-validator.ts b/src/pepr/operator/crd/validators/package-validator.ts
index 4336fecfa..4ad6266fc 100644
--- a/src/pepr/operator/crd/validators/package-validator.ts
+++ b/src/pepr/operator/crd/validators/package-validator.ts
@@ -58,9 +58,34 @@ export async function validator(req: PeprValidateRequest<UDSPackage>) {
   const networkPolicyNames = new Set<string>();
 
   for (const policy of networkPolicy) {
-    // remoteGenerated cannot be combined with remoteNamespace or remoteSelector
-    if (policy.remoteGenerated && (policy.remoteNamespace || policy.remoteSelector)) {
-      return req.Deny("remoteGenerated cannot be combined with remoteNamespace or remoteSelector");
+    // If 'remoteGenerated' is set, it cannot be combined with 'remoteNamespace', 'remoteSelector', or 'remoteCidr'.
+    if (
+      policy.remoteGenerated &&
+      (policy.remoteNamespace || policy.remoteSelector || policy.remoteCidr)
+    ) {
+      return req.Deny(
+        "remoteGenerated cannot be combined with remoteNamespace, remoteSelector, or remoteCidr",
+      );
+    }
+
+    // If either 'remoteNamespace' or 'remoteSelector' is set, they cannot be combined with 'remoteGenerated' or 'remoteCidr'.
+    if (
+      (policy.remoteNamespace || policy.remoteSelector) &&
+      (policy.remoteGenerated || policy.remoteCidr)
+    ) {
+      return req.Deny(
+        "remoteNamespace and remoteSelector cannot be combined with remoteGenerated or remoteCidr",
+      );
+    }
+
+    // If 'remoteCidr' is set, it cannot be combined with 'remoteGenerated', 'remoteNamespace', or 'remoteSelector'.
+    if (
+      policy.remoteCidr &&
+      (policy.remoteGenerated || policy.remoteNamespace || policy.remoteSelector)
+    ) {
+      return req.Deny(
+        "remoteCidr cannot be combined with remoteGenerated, remoteNamespace, or remoteSelector",
+      );
     }
 
     // Ensure the policy name is unique
diff --git a/src/prometheus-stack/chart/templates/uds-package.yaml b/src/prometheus-stack/chart/templates/uds-package.yaml
index 746a08692..2dfda03fb 100644
--- a/src/prometheus-stack/chart/templates/uds-package.yaml
+++ b/src/prometheus-stack/chart/templates/uds-package.yaml
@@ -46,9 +46,9 @@ spec:
         port: 10250
         description: "Webhook"
 
-      # todo: lockdown egress to scrape targets
+      # Prometheus scrape targets
       - direction: Egress
-        remoteNamespace: ""
+        remoteNamespace: "" # todo: restrict this overly permissive netpol
         selector:
           app.kubernetes.io/name: prometheus
         description: "Metrics Scraping"
@@ -62,9 +62,3 @@ spec:
         port: 9090
         description: "Grafana Metrics Queries"
 
-      - direction: Egress
-        remoteNamespace: tempo
-        remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
diff --git a/src/promtail/chart/templates/uds-package.yaml b/src/promtail/chart/templates/uds-package.yaml
index 1a66b8490..98a46eca7 100644
--- a/src/promtail/chart/templates/uds-package.yaml
+++ b/src/promtail/chart/templates/uds-package.yaml
@@ -27,13 +27,6 @@ spec:
           app.kubernetes.io/name: promtail
         remoteGenerated: KubeAPI
 
-      - direction: Egress
-        remoteNamespace: tempo
-        remoteSelector:
-          app.kubernetes.io/name: tempo
-        port: 9411
-        description: "Tempo"
-
       - direction: Egress
         selector:
           app.kubernetes.io/name: promtail
diff --git a/src/test/app-tenant.yaml b/src/test/app-tenant.yaml
index 3eb203b99..7d37ac99a 100644
--- a/src/test/app-tenant.yaml
+++ b/src/test/app-tenant.yaml
@@ -23,6 +23,12 @@ spec:
         gateway: tenant
         host: demo-8081
         port: 8081
+      - service: test-tenant-app-cidr
+        selector:
+          app: test-tenant-app
+        remoteCidr: "192.168.0.0/16"
+        host: demo-8080
+        port: 8080
 ---
 apiVersion: v1
 kind: Service
diff --git a/src/velero/chart/templates/uds-package.yaml b/src/velero/chart/templates/uds-package.yaml
index 616559ebc..0326a863e 100644
--- a/src/velero/chart/templates/uds-package.yaml
+++ b/src/velero/chart/templates/uds-package.yaml
@@ -6,11 +6,19 @@ metadata:
 spec:
   network:
     allow:
-      # Todo: wide open for now for pushing to s3
+      # Egress for S3 connections
       - direction: Egress
         selector:
           app.kubernetes.io/name: velero
+        description: Storage
+        {{- if .Values.storage.internal.enabled }}
+        remoteSelector: {{ .Values.storage.internal.remoteSelector }}
+        remoteNamespace: {{ .Values.storage.internal.remoteNamespace }}
+        {{- else if .Values.storage.egressCidr }}
+        remoteCidr: {{ .Values.storage.egressCidr }}
+        {{- else }}
         remoteGenerated: Anywhere
+        {{- end }}
 
       - direction: Egress
         selector:
diff --git a/src/velero/chart/values.yaml b/src/velero/chart/values.yaml
index e69de29bb..fbb557b5a 100644
--- a/src/velero/chart/values.yaml
+++ b/src/velero/chart/values.yaml
@@ -0,0 +1,6 @@
+storage:
+  internal:
+    enabled: false
+    remoteSelector: {}
+    remoteNamespace: ""
+  egressCidr: ""