Don't match on full header values (#53)

stackitcloud · Jun 14, 2024 · 2533753 · 2533753
1 parent cb40ec0
commit 2533753
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 16 deletions.
diff --git a/docs/adr/06_seperate_vpn_filter.md b/docs/adr/06_seperate_vpn_filter.md
@@ -17,13 +17,13 @@ The resulting filter chain in envoy looks like this:
     "@type": "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC",
     "rules": {
     "policies": {
-      "shoot-1": {
+      "shoot--1": {
       "permissions": [
         {
         "header": {
           "name": "reversed-vpn",
           "string_match": {
-          "exact": "outbound|1194||vpn-seed-server.shoot--shoot-1.svc.cluster.local"
+          "contains": ".shoot--shoot--1."
           }
         }
         }
@@ -38,14 +38,14 @@ The resulting filter chain in envoy looks like this:
 ...
       ]
       },
-      "shoot-1-inverse": {
+      "shoot--1-inverse": {
       "permissions": [
         {
         "not_rule": {
           "header": {
           "name": "reversed-vpn",
           "string_match": {
-            "exact": "outbound|1194||vpn-seed-server.shoot--shoot-1.svc.cluster.local"
+            "contains": ".shoot--shoot--1."
           }
           }
         }
@@ -70,13 +70,13 @@ The resulting filter chain in envoy looks like this:
     "@type": "type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC",
     "rules": {
     "policies": {
-      "shoot-2": {
+      "shoot--2": {
       "permissions": [
         {
         "header": {
           "name": "reversed-vpn",
           "string_match": {
-          "exact": "outbound|1194||vpn-seed-server.shoot--shoot-2.svc.cluster.local"
+          "contains": ".shoot--shoot--2."
           }
         }
         }
@@ -91,14 +91,14 @@ The resulting filter chain in envoy looks like this:
 ...
       ]
       },
-      "shoot-2-inverse": {
+      "shoot--2-inverse": {
       "permissions": [
         {
         "not_rule": {
           "header": {
           "name": "reversed-vpn",
           "string_match": {
-            "exact": "outbound|1194||vpn-seed-server.shoot--shoot-2.svc.cluster.local"
+            "contains": ".shoot--shoot--2."
           }
           }
         }

diff --git a/pkg/envoyfilters/envoyfilters.go b/pkg/envoyfilters/envoyfilters.go
@@ -79,8 +79,7 @@ func BuildIngressEnvoyFilterSpecForHelmChart(
 func BuildVPNEnvoyFilterSpecForHelmChart(
 	cluster *controller.Cluster, rule *ACLRule, alwaysAllowedCIDRs []string, istioLabels map[string]string,
 ) (map[string]interface{}, error) {
-	shootID := helper.ComputeShortShootID(cluster.Shoot)
-	vpnConfigPatch, err := CreateVPNConfigPatchFromRule(rule, shootID, alwaysAllowedCIDRs)
+	vpnConfigPatch, err := CreateVPNConfigPatchFromRule(rule, helper.ComputeShortShootID(cluster.Shoot), cluster.Shoot.Status.TechnicalID, alwaysAllowedCIDRs)
 	if err != nil {
 		return nil, err
 	}
@@ -214,13 +213,20 @@ func CreateIngressConfigPatchFromRule(
 // CreateVPNConfigPatchFromRule creates an HTTP filter patch that can be applied to the
 // `GATEWAY` HTTP filter chain for the VPN.
 func CreateVPNConfigPatchFromRule(rule *ACLRule,
-	shootID string, alwaysAllowedCIDRs []string,
+	shortShootID, technicalShootID string, alwaysAllowedCIDRs []string,
 ) (map[string]interface{}, error) {
 	rbacName := "acl-vpn"
 	headerMatcher := map[string]interface{}{
 		"name": "reversed-vpn",
 		"string_match": map[string]interface{}{
-			"exact": "outbound|1194||vpn-seed-server.shoot--" + shootID + ".svc.cluster.local",
+			// The actual header value will look something like
+			// `outbound|1194||vpn-seed-server.<technical-ID>.svc.cluster.local`.
+			// Include dots in the contains matcher as anchors, to always match the entire technical shoot ID.
+			// Otherwise, if there was one cluster named `foo` and one named `foo-bar` (in the same project),
+			// `foo` would effectively inherit the ACL of `foo-bar`.
+			// We don't match with the full header value to allow service names and ports to change while still making sure
+			// we catch all traffic targeting this shoot.
+			"contains": "." + technicalShootID + ".",
 		},
 	}
 	return map[string]interface{}{
@@ -240,7 +246,7 @@ func CreateVPNConfigPatchFromRule(rule *ACLRule,
 					"rules": map[string]interface{}{
 						"action": "ALLOW",
 						"policies": map[string]interface{}{
-							shootID + "-inverse": map[string]interface{}{
+							shortShootID + "-inverse": map[string]interface{}{
 								"permissions": []map[string]interface{}{{
 									"not_rule": map[string]interface{}{
 										"header": headerMatcher,
@@ -253,7 +259,7 @@ func CreateVPNConfigPatchFromRule(rule *ACLRule,
 									},
 								}},
 							},
-							shootID: map[string]interface{}{
+							shortShootID: map[string]interface{}{
 								"permissions": []map[string]interface{}{{
 									"header": headerMatcher,
 								}},

diff --git a/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml b/pkg/envoyfilters/testdata/vpnEnvoyFilterSpecWithOneAllowRule.yaml
@@ -19,7 +19,7 @@ configPatches:
                     header:
                       name: reversed-vpn
                       string_match:
-                        exact: outbound|1194||vpn-seed-server.shoot--bar--foo.svc.cluster.local
+                        contains: .shoot--bar--foo.
                 principals:
                 - remote_ip:
                     address_prefix: 0.0.0.0
@@ -29,7 +29,7 @@ configPatches:
                 - header:
                     name: reversed-vpn
                     string_match:
-                      exact: outbound|1194||vpn-seed-server.shoot--bar--foo.svc.cluster.local
+                      contains: .shoot--bar--foo.
                 principals:
                 - remote_ip:
                     address_prefix: 10.180.0.0