nginz deactivate unused upstreams (SQPIT-1174) (#2849)

Some upstreams are configured by default in nginz that have no backing service. This leads to many unnecessary DNS lookups and opens unused routes into our K8s clusters (which might be a security issue).

Upstream configuration in nginz goes from two entries (ignored_upstreams, upstreams) to four (ignored_upstreams, upstreams, enabled_extra_upstreams, extra_upstreams):

- ignored_upstreams : default upstreams that should be ignored (for backwards compatibility)
- upstreams : default upstreams (kept for backwards compatibility)
- enabled_extra_upstreams : Extra upstreams that should be enabled (routed / rendered; this one is new)
-  extra_upstreams : Contains upstreams that are usually not deployed by default (this is new, too)

Co-authored-by: jschaul <[email protected]>

changelog.d/2-features/disable-extra-nginz-upstreams-by-default

@@ -0,0 +1,10 @@
+**Nginz helm chart**: The list of upstreams is split into `nginx_conf.upstreams` and
+`nginx_conf.extra_upstreams`. Extra upstreams are disabled by default. They can
+be enabled by adding their name (entry's key) to
+`nginx_conf.enabled_extra_upstreams`. `nginx_conf.ignored_upstreams` is only
+applied to upstreams from `nginx_conf.upstreams`. In the default configuration
+of `nginz` extra upstreams are `ibis`, `galeb`, `calling-test` and `proxy`. If one
+of those is deployed, its name has be be added to
+`nginx_conf.enabled_extra_upstreams` (otherwise, it won't be reachable). Unless
+`nginx_conf.upstreams` hasn't been changed manually (overriding its default),
+this should be the only needed migration step.

charts/nginz/templates/_helpers.tpl

@@ -14,3 +14,30 @@ We truncate at 63 chars because some Kubernetes name fields are limited to this
 {{- $name := default .Chart.Name .Values.nameOverride -}}
 {{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
 {{- end -}}
+
+{{/*
+Takes no parameters and returns a merged map of upstreams ('upstreams' and 'extra_upstreams')
+that should be configured.
+*/}}
+{{- define "valid_upstreams" -}}
+    {{- range $e := $.Values.nginx_conf.ignored_upstreams }}
+        {{- if not (hasKey $.Values.nginx_conf.upstreams $e) }}
+            {{- fail (print "Upstream '" $e "' does not exist in 'upstreams'!") }}
+        {{- end }}
+    {{- end }}
+    {{- range $e := $.Values.nginx_conf.enabled_extra_upstreams }}
+        {{- if not (hasKey $.Values.nginx_conf.extra_upstreams $e) }}
+            {{- fail (print "Upstream '" $e "' does not exist in 'extra_upstreams'!") }}
+        {{- end }}
+    {{- end }}
+
+    {{- $validUpstreams := (deepCopy $.Values.nginx_conf.upstreams) }}
+    {{- range $key := $.Values.nginx_conf.ignored_upstreams }}
+        {{- $validUpstreams = unset $validUpstreams $key}}
+    {{- end }}
+    {{- range $key := $.Values.nginx_conf.enabled_extra_upstreams }}
+        {{- $validUpstreams = set $validUpstreams $key (get $.Values.nginx_conf.extra_upstreams $key)}}
+    {{- end }}
+
+    {{- toJson $validUpstreams}}
+{{- end -}}

charts/nginz/templates/conf/_nginx.conf.tpl

@@ -217,8 +217,8 @@ http {
     # Service Routing
     #
 
-  {{ range $name, $locations := .Values.nginx_conf.upstreams -}}
-    {{- if not (has $name $.Values.nginx_conf.ignored_upstreams) -}}
+  {{- $validUpstreams := include "valid_upstreams" . | fromJson }}
+  {{ range $name, $locations := $validUpstreams -}}
     {{- range $location := $locations -}}
       {{- if hasKey $location "envs" -}}
         {{- range $env := $location.envs -}}
@@ -324,7 +324,6 @@ http {
 
       {{- end -}}
     {{- end -}}
-    {{- end -}}
   {{- end }}
 
     {{ if not (eq $.Values.nginx_conf.env "prod")  }}

charts/nginz/templates/conf/_upstreams.txt.tpl

@@ -1,3 +1,4 @@
-{{ define "nginz_upstreams.txt" }}
-{{ range $key, $value := .Values.nginx_conf.upstreams }}{{ if not (has $key $.Values.nginx_conf.ignored_upstreams) }} {{ $key }}{{ if hasKey $.Values.nginx_conf.upstream_namespace $key }}.{{ get $.Values.nginx_conf.upstream_namespace $key }}{{end}} {{ end }}{{ end -}}
-{{ end }}
+{{- define "nginz_upstreams.txt" }}
+{{- $validUpstreams := include "valid_upstreams" . | fromJson }}
+{{- range $key, $value := $validUpstreams }}{{ $key }}{{- if hasKey $.Values.nginx_conf.upstream_namespace $key }}.{{ get $.Values.nginx_conf.upstream_namespace $key }}{{end}} {{ end -}}
+{{- end }}

charts/nginz/templates/configmap.yaml

@@ -2,8 +2,7 @@ apiVersion: v1
 data:
   nginx.conf: |2
 {{- include "nginz_nginx.conf" . | indent 4 }}
-  upstreams.txt: |2
-{{- include "nginz_upstreams.txt" . | indent 4 }}
+  upstreams.txt: "{{- include "nginz_upstreams.txt" . | trim }}"
   deeplink.json: |2
 {{- include "nginz_deeplink.json" . | indent 4 }}
   deeplink.html: |2

charts/nginz/values.yaml

@@ -128,6 +128,12 @@ nginx_conf:
       allow_credentials: true
       max_body_size: "0"
       disable_request_buffering: true
+    cannon:
+    - path: /await
+      envs:
+      - all
+      use_websockets: true
+      doc: true
     brig:
     - path: /api-version
       envs:
@@ -545,17 +551,23 @@ nginx_conf:
     - path: /scim
       envs:
       - all
+
+  # Possible 'extra_extra_upstreams' are:
+  #   - 'ibis'
+  #   - 'galeb'
+  #   - 'calling-test'
+  #   - 'proxy'
+  # For security reasons, these should only be enabled if they are deployed.
+  # (Otherwise, there are open routes into the cluster.)
+  enabled_extra_upstreams: []
+
+  # Services that are optionally deployed.
+  extra_upstreams:
     proxy:
     - path: /proxy
       envs:
       - all
       doc: true
-    cannon:
-    - path: /await
-      envs:
-      - all
-      use_websockets: true
-      doc: true
     ibis:
     - path: /billing
       envs:
@@ -592,7 +604,6 @@ nginx_conf:
       basic_auth: true
       envs:
       - staging
-
     calling-test:
     - path: /calling-test
       envs:

docs/src/how-to/install/configuration-options.rst

@@ -176,6 +176,79 @@ There is no way to entirely disable this behaviour, two extreme examples below
        millisecondsBetweenBatches: 50
        minBatchSize: 20
 
+
+Control nginz upstreams (routes) into the Kubernetes cluster
+------------------------------------------------------------
+
+Open unterminated upstreams (routes) into the Kubernetes cluster are a potential
+security issue. To prevent this, there are fine-grained settings in the nginz
+configuration defining which upstreams should exist.
+
+Default upstreams
+^^^^^^^^^^^^^^^^^
+
+Upstreams for services that exist in (almost) every Wire installation are
+enabled by default. These are:
+
+- ``brig``
+- ``cannon``
+- ``cargohold``
+- ``galley``
+- ``gundeck``
+- ``spar``
+
+For special setups (as e.g. described in separate-websocket-traffic_) the
+upstreams of these services can be ignored (disabled) with the setting
+``nginz.nginx_conf.ignored_upstreams``.
+
+The most common example is to disable the upstream of ``cannon``:
+
+.. code:: yaml
+
+   nginz:
+     nginx_conf:
+       ignored_upstreams: ["cannon"]
+
+
+Optional upstreams
+^^^^^^^^^^^^^^^^^^
+
+There are some services that are usually not deployed on most Wire installations
+or are specific to the Wire cloud:
+
+- ``ibis``
+- ``galeb``
+- ``calling-test``
+- ``proxy``
+
+The upstreams for those are disabled by default and can be enabled by the
+setting ``nginz.nginx_conf.enabled_extra_upstreams``.
+
+The most common example is to enable the (extra) upstream of ``proxy``:
+
+.. code:: yaml
+
+   nginz:
+     nginx_conf:
+       enabled_extra_upstreams: ["proxy"]
+
+
+Combining default and extra upstream configurations
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Default and extra upstream configurations are independent of each other. I.e.
+``nginz.nginx_conf.ignored_upstreams`` and
+``nginz.nginx_conf.enabled_extra_upstreams`` can be combined in the same
+configuration:
+
+.. code:: yaml
+
+   nginz:
+     nginx_conf:
+       ignored_upstreams: ["cannon"]
+       enabled_extra_upstreams: ["proxy"]
+
+
 .. _separate-websocket-traffic:
 
 Separate incoming websocket network traffic from the rest of the https traffic