diff --git a/cmd/tapRunner.go b/cmd/tapRunner.go index 960b916e7..b61d02ef3 100644 --- a/cmd/tapRunner.go +++ b/cmd/tapRunner.go @@ -471,17 +471,18 @@ func updateConfig(kubernetesProvider *kubernetes.Provider) { ingressEnabled = "true" } - authEnabled := "" - if config.Config.Tap.Auth.Enabled { - authEnabled = "true" - } - _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_INGRESS_ENABLED, ingressEnabled) _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_INGRESS_HOST, config.Config.Tap.Ingress.Host) _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_PROXY_FRONT_PORT, fmt.Sprint(config.Config.Tap.Proxy.Front.Port)) - _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_AUTH_ENABLED, authEnabled) - _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_AUTH_TYPE, config.Config.Tap.Auth.Type) - _, _ = kubernetes.SetConfig(kubernetesProvider, kubernetes.CONFIG_AUTH_SAML_IDP_METADATA_URL, config.Config.Tap.Auth.Saml.IdpMetadataUrl) + // AUTH_ENABLED / AUTH_TYPE / AUTH_SAML_IDP_METADATA_URL are deliberately + // not written here. They are rendered by the Helm chart from the values + // the release was installed with, and this function runs on the + // "existing installation" path, where the CLI's own config is a separate + // source that may never have carried auth settings at all. Writing them + // meant a plain `kubeshark tap` against a release installed with SSO + // silently reconfigured, or disabled, its authentication. + // + // Change auth with `helm upgrade`, or `kubeshark tap` a fresh release. } diff --git a/helm-chart/templates/06-front-deployment.yaml b/helm-chart/templates/06-front-deployment.yaml index 9d94ca7cf..a8d29a384 100644 --- a/helm-chart/templates/06-front-deployment.yaml +++ b/helm-chart/templates/06-front-deployment.yaml @@ -25,17 +25,9 @@ spec: containers: - env: - name: REACT_APP_AUTH_ENABLED - value: '{{- if or (and .Values.cloudLicenseEnabled (not (empty .Values.license))) (not .Values.internetConnectivity) -}} - {{ (default false .Values.demoModeEnabled) | ternary true ((and .Values.tap.auth.enabled (or (eq .Values.tap.auth.type "oidc") (eq .Values.tap.auth.type "dex"))) | ternary true false) }} - {{- else -}} - {{ .Values.cloudLicenseEnabled | ternary "true" ((default false .Values.demoModeEnabled) | ternary "true" .Values.tap.auth.enabled) }} - {{- end }}' + value: '{{ include "kubeshark.authEnabled" . }}' - name: REACT_APP_AUTH_TYPE - value: '{{- if and .Values.cloudLicenseEnabled (not (or (eq .Values.tap.auth.type "oidc") (eq .Values.tap.auth.type "dex"))) -}} - default - {{- else -}} - {{ (default false .Values.demoModeEnabled) | ternary "default" .Values.tap.auth.type }} - {{- end }}' + value: '{{ .Values.tap.auth.type }}' - name: REACT_APP_COMPLETE_STREAMING_ENABLED value: '{{- if and (hasKey .Values.tap "dashboard") (hasKey .Values.tap.dashboard "completeStreamingEnabled") -}} {{ eq .Values.tap.dashboard.completeStreamingEnabled true | ternary "true" "false" }} @@ -74,7 +66,7 @@ spec: {{- end -}}' - name: 'REACT_APP_CLOUD_LICENSE_ENABLED' value: '{{- if or (and .Values.cloudLicenseEnabled (not (empty .Values.license))) (not .Values.internetConnectivity) -}} - "false" + false {{- else -}} {{ .Values.cloudLicenseEnabled }} {{- end }}' diff --git a/helm-chart/templates/09-worker-daemon-set.yaml b/helm-chart/templates/09-worker-daemon-set.yaml index 61485b249..e32f91f65 100644 --- a/helm-chart/templates/09-worker-daemon-set.yaml +++ b/helm-chart/templates/09-worker-daemon-set.yaml @@ -146,10 +146,8 @@ spec: value: '{{ (include "sentry.enabled" .) }}' - name: SENTRY_ENVIRONMENT value: '{{ .Values.tap.sentry.environment }}' - {{- if eq (include "kubeshark.authEnabled" .) "true" }} - name: HUB_INTERNAL_TOKEN_PATH value: /var/run/secrets/kubeshark/hub-token/token - {{- end }} resources: limits: {{ if ne (toString .Values.tap.resources.sniffer.limits.cpu) "0" }} @@ -239,11 +237,9 @@ spec: {{- if .Values.tap.persistentStorage }} subPathExpr: $(NODE_NAME) {{- end }} - {{- if eq (include "kubeshark.authEnabled" .) "true" }} - mountPath: /var/run/secrets/kubeshark/hub-token name: hub-internal-token readOnly: true - {{- end }} {{- if .Values.tap.tls }} - command: - ./tracer @@ -284,10 +280,8 @@ spec: value: '{{ (include "sentry.enabled" .) }}' - name: SENTRY_ENVIRONMENT value: '{{ .Values.tap.sentry.environment }}' - {{- if eq (include "kubeshark.authEnabled" .) "true" }} - name: HUB_INTERNAL_TOKEN_PATH value: /var/run/secrets/kubeshark/hub-token/token - {{- end }} resources: limits: {{ if ne (toString .Values.tap.resources.tracer.limits.cpu) "0" }} @@ -363,11 +357,9 @@ spec: mountPropagation: HostToContainer name: root readOnly: true - {{- if eq (include "kubeshark.authEnabled" .) "true" }} - mountPath: /var/run/secrets/kubeshark/hub-token name: hub-internal-token readOnly: true - {{- end }} {{- end }} dnsPolicy: ClusterFirstWithHostNet hostNetwork: {{ .Values.tap.hostNetwork }} @@ -448,7 +440,6 @@ spec: emptyDir: sizeLimit: {{ .Values.tap.storageLimit }} {{- end }} - {{- if eq (include "kubeshark.authEnabled" .) "true" }} - name: hub-internal-token projected: sources: @@ -456,4 +447,3 @@ spec: path: token audience: kubeshark-hub expirationSeconds: 3600 - {{- end }} diff --git a/helm-chart/templates/12-config-map.yaml b/helm-chart/templates/12-config-map.yaml index ecc396f5c..21fe8039a 100644 --- a/helm-chart/templates/12-config-map.yaml +++ b/helm-chart/templates/12-config-map.yaml @@ -18,12 +18,9 @@ data: INGRESS_ENABLED: '{{ .Values.tap.ingress.enabled }}' INGRESS_HOST: '{{ .Values.tap.ingress.host }}' PROXY_FRONT_PORT: '{{ .Values.tap.proxy.front.port }}' + {{- include "kubeshark.validateAuth" . }} AUTH_ENABLED: '{{ include "kubeshark.authEnabled" . }}' - AUTH_TYPE: '{{- if and .Values.cloudLicenseEnabled (not (or (eq .Values.tap.auth.type "oidc") (eq .Values.tap.auth.type "dex"))) -}} - default - {{- else -}} - {{ (default false .Values.demoModeEnabled) | ternary "default" .Values.tap.auth.type }} - {{- end }}' + AUTH_TYPE: '{{ .Values.tap.auth.type }}' AUTH_SAML_IDP_METADATA_URL: '{{ .Values.tap.auth.saml.idpMetadataUrl }}' AUTH_CLI_SERVICE_ACCOUNTS: '{{ if (((.Values.tap).auth).cli).enabled }}{{ .Release.Namespace }}:kubeshark-cli{{ end }}' AUTH_ROLES_CLAIM: '{{ .Values.tap.auth.rolesClaim }}' diff --git a/helm-chart/templates/NOTES.txt b/helm-chart/templates/NOTES.txt index e6d4c9d22..dc124a820 100644 --- a/helm-chart/templates/NOTES.txt +++ b/helm-chart/templates/NOTES.txt @@ -24,6 +24,16 @@ Overridden front image: {{ .Values.tap.docker.overrideImage.front }} Your deployment has been successful. The release is named `{{ .Release.Name }}` and it has been deployed in the `{{ .Release.Namespace }}` namespace. Notices: +{{- if not .Values.tap.auth.enabled }} +- API AUTHENTICATION IS DISABLED (tap.auth.enabled=false, the default). +- Anyone able to reach the Hub can read captured traffic and change settings. +- Enable it with `--set tap.auth.enabled=true` plus a `tap.auth.type` of `saml` or `oidc`, + and assign roles via `tap.auth.defaultRole` / `tap.auth.groupMapping`. +- See https://docs.kubeshark.com/en/roles +{{- else }} +- API authentication is enabled (tap.auth.type={{ .Values.tap.auth.type }}). +- Callers with no recognized role resolve to `{{ default "no role, deny-all" .Values.tap.auth.defaultRole }}`. +{{- end }} {{- if .Values.supportChatEnabled}} - Support chat using Intercom is enabled. It can be disabled using `--set supportChatEnabled=false` {{- end }} diff --git a/helm-chart/templates/_helpers.tpl b/helm-chart/templates/_helpers.tpl index fde9514e2..0272fc7be 100644 --- a/helm-chart/templates/_helpers.tpl +++ b/helm-chart/templates/_helpers.tpl @@ -112,15 +112,25 @@ Dex IdP: retrieve a secret for static client with a specific ID {{- end }} {{/* -Single source of truth for whether the hub enforces authentication. -Consumed by the hub ConfigMap (AUTH_ENABLED) and by the worker DaemonSet, which -must mount an internal hub token whenever the hub requires one. Keeping the two -in sync prevents workers from being issued no token while the hub demands one. +Whether the Hub enforces authentication and authorization on its API. +This is `tap.auth.enabled` and nothing else: licensing, demo mode and the +choice of identity provider do not affect whether the API is gated. */}} {{- define "kubeshark.authEnabled" -}} -{{- if and .Values.cloudLicenseEnabled (not (empty .Values.license)) -}} -{{ (default false .Values.demoModeEnabled) | ternary true ((and .Values.tap.auth.enabled (or (eq .Values.tap.auth.type "oidc") (eq .Values.tap.auth.type "dex"))) | ternary true false) }} -{{- else -}} -{{ .Values.cloudLicenseEnabled | ternary "true" ((default false .Values.demoModeEnabled) | ternary "true" .Values.tap.auth.enabled) }} +{{ .Values.tap.auth.enabled }} +{{- end -}} + +{{/* +Reject auth settings that cannot work, instead of rendering a Hub that +authenticates nobody. +*/}} +{{- define "kubeshark.validateAuth" -}} +{{- if .Values.tap.auth.enabled -}} + {{- if and (eq .Values.tap.auth.type "saml") (empty .Values.tap.auth.saml.idpMetadataUrl) -}} + {{- fail "tap.auth.enabled is true with tap.auth.type=saml but tap.auth.saml.idpMetadataUrl is empty. Set the IdP metadata URL, or pick another tap.auth.type (oidc, dex, descope)." -}} + {{- end -}} + {{- if and (or (eq .Values.tap.auth.type "oidc") (eq .Values.tap.auth.type "dex")) (empty (((.Values.tap).auth).oidc).issuer) -}} + {{- fail "tap.auth.enabled is true with tap.auth.type=oidc but tap.auth.oidc.issuer is empty. Set the OIDC issuer, or pick another tap.auth.type." -}} + {{- end -}} {{- end -}} {{- end -}} diff --git a/helm-chart/tests/auth_front_test.yaml b/helm-chart/tests/auth_front_test.yaml new file mode 100644 index 000000000..1b119edae --- /dev/null +++ b/helm-chart/tests/auth_front_test.yaml @@ -0,0 +1,108 @@ +## The dashboard needs two independent answers, and they are not the same one. +## +## REACT_APP_AUTH_ENABLED -> is the Hub API gated? Must agree with the +## Hub's own AUTH_ENABLED, or the dashboard +## and the Hub disagree about whether a +## login is needed. +## REACT_APP_CLOUD_LICENSE_ENABLED -> must the dashboard log in to fetch a +## licence key? True only when cloud +## licensing is on, no key was supplied, +## and there is internet to reach the cloud. +suite: front auth env +templates: + - templates/06-front-deployment.yaml +tests: + - it: reports auth off by default + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_AUTH_ENABLED + value: "false" + + - it: reports auth on when the Hub gates the API + set: + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_AUTH_ENABLED + value: "true" + + # The dashboard's value used to be computed by a second, hand-maintained + # copy of the Hub's expression, with an extra internetConnectivity term. An + # air-gapped install therefore rendered a gated Hub against a dashboard that + # never mounted a login, locking everyone out. + - it: agrees with the Hub when there is no internet connectivity + set: + internetConnectivity: false + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_AUTH_ENABLED + value: "true" + + - it: passes the auth type through unchanged + set: + tap.auth.enabled: true + tap.auth.type: saml + tap.auth.saml.idpMetadataUrl: https://idp.example.com/metadata + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_AUTH_TYPE + value: saml + + ## Cloud licensing. This flag is what tells the dashboard to run a login for + ## licence acquisition, independently of whether the API is gated. + + - it: asks for a cloud licence on a default install + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_CLOUD_LICENSE_ENABLED + value: "true" + + - it: does not ask for a cloud licence when a key was supplied + set: + license: ABC + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_CLOUD_LICENSE_ENABLED + value: "false" + + - it: does not ask for a cloud licence without internet connectivity + set: + internetConnectivity: false + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_CLOUD_LICENSE_ENABLED + value: "false" + + # Fetching a licence from the cloud does not gate the API. These two are + # rendered from different inputs and are expected to disagree here. + - it: asks for a cloud licence while leaving the API ungated + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_CLOUD_LICENSE_ENABLED + value: "true" + - contains: + path: spec.template.spec.containers[0].env + content: + name: REACT_APP_AUTH_ENABLED + value: "false" diff --git a/helm-chart/tests/auth_gating_test.yaml b/helm-chart/tests/auth_gating_test.yaml new file mode 100644 index 000000000..9f5b00af3 --- /dev/null +++ b/helm-chart/tests/auth_gating_test.yaml @@ -0,0 +1,139 @@ +## Whether the Hub gates its API is decided by `tap.auth.enabled` alone. +## +## Three separate questions used to share this one switch, and the answers +## disagreed with what operators had asked for: +## +## * Is the API gated? -> tap.auth.enabled +## * Does the dashboard need a login -> cloudLicenseEnabled + license + +## to fetch a licence key? internetConnectivity (front only) +## * Which features exist at all? -> demoModeEnabled +## +## These tests pin each answer to its own input, so a future change that +## reconnects them fails here rather than in someone's cluster. +suite: auth gating +templates: + - templates/12-config-map.yaml +tests: + - it: is off by default, so a default install gates nothing + asserts: + - equal: + path: data.AUTH_ENABLED + value: "false" + + - it: follows tap.auth.enabled and nothing else + set: + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - equal: + path: data.AUTH_ENABLED + value: "true" + + # A licence key is proof of entitlement, not an authentication decision. + # Supplying one used to switch AUTH_ENABLED off. + - it: is not turned off by supplying a licence key + set: + license: ABC + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - equal: + path: data.AUTH_ENABLED + value: "true" + + # Where the licence comes from is unrelated to whether callers are + # authenticated. Cloud licensing used to force AUTH_ENABLED on. + - it: is not turned on by cloud licensing + set: + cloudLicenseEnabled: true + license: "" + asserts: + - equal: + path: data.AUTH_ENABLED + value: "false" + + # This combination is the one that mattered most: an operator asks for SAML + # and previously received an unauthenticated Hub, with no error and no + # warning, because the licence terms in the old expression won. + - it: stays on for SAML alongside cloud licensing + set: + license: ABC + tap.auth.enabled: true + tap.auth.type: saml + tap.auth.saml.idpMetadataUrl: https://idp.example.com/metadata + asserts: + - equal: + path: data.AUTH_ENABLED + value: "true" + - equal: + path: data.AUTH_TYPE + value: saml + + - it: is not turned on by demo mode + set: + demoModeEnabled: true + asserts: + - equal: + path: data.AUTH_ENABLED + value: "false" + + ## AUTH_TYPE is the operator's choice, verbatim. + ## + ## It used to be rewritten to "default" whenever cloud licensing was on and + ## the type was not oidc/dex, and unconditionally under demo mode. An + ## operator's `saml` therefore became something else on a live cluster. + + - it: passes the configured auth type through unchanged + set: + tap.auth.enabled: true + tap.auth.type: descope + asserts: + - equal: + path: data.AUTH_TYPE + value: descope + + - it: does not rewrite the auth type under cloud licensing + set: + cloudLicenseEnabled: true + license: "" + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - equal: + path: data.AUTH_TYPE + value: oidc + + - it: does not rewrite the auth type under demo mode + set: + demoModeEnabled: true + tap.auth.enabled: true + tap.auth.type: descope + asserts: + - equal: + path: data.AUTH_TYPE + value: descope + + ## Role resolution inputs reach the Hub untouched, so `defaultRole` means + ## what it says for every auth type. + + - it: passes role resolution settings through + set: + tap.auth.enabled: true + tap.auth.type: descope + tap.auth.defaultRole: kubeshark-viewer + tap.auth.rolesClaim: role + tap.auth.groupMapping: + demo-guest: kubeshark-viewer + asserts: + - equal: + path: data.AUTH_DEFAULT_ROLE + value: kubeshark-viewer + - equal: + path: data.AUTH_ROLES_CLAIM + value: role + - equal: + path: data.AUTH_GROUP_MAPPING + value: '{"demo-guest":"kubeshark-viewer"}' diff --git a/helm-chart/tests/auth_validation_test.yaml b/helm-chart/tests/auth_validation_test.yaml new file mode 100644 index 000000000..e3cf6d2aa --- /dev/null +++ b/helm-chart/tests/auth_validation_test.yaml @@ -0,0 +1,54 @@ +## An auth type that cannot work should stop the install, not render a Hub +## that authenticates nobody. The chart previously rewrote such settings into +## a different auth type instead, which is how an operator could ask for SAML +## and get something else without being told. +suite: auth validation +templates: + - templates/12-config-map.yaml +tests: + - it: rejects SAML without an IdP metadata URL + set: + tap.auth.enabled: true + tap.auth.type: saml + asserts: + - failedTemplate: + errorPattern: tap.auth.saml.idpMetadataUrl is empty + + - it: accepts SAML with an IdP metadata URL + set: + tap.auth.enabled: true + tap.auth.type: saml + tap.auth.saml.idpMetadataUrl: https://idp.example.com/metadata + asserts: + - equal: + path: data.AUTH_TYPE + value: saml + + - it: rejects OIDC without an issuer + set: + tap.auth.enabled: true + tap.auth.type: oidc + asserts: + - failedTemplate: + errorPattern: tap.auth.oidc.issuer is empty + + - it: accepts OIDC with an issuer + set: + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - equal: + path: data.AUTH_TYPE + value: oidc + + # Incomplete auth settings are only a problem when auth is on. The default + # install ships tap.auth.type=saml with no IdP and must still render. + - it: ignores incomplete auth settings while auth is off + asserts: + - equal: + path: data.AUTH_ENABLED + value: "false" + - equal: + path: data.AUTH_TYPE + value: saml diff --git a/helm-chart/tests/worker_hub_token_test.yaml b/helm-chart/tests/worker_hub_token_test.yaml new file mode 100644 index 000000000..fe44dc7d5 --- /dev/null +++ b/helm-chart/tests/worker_hub_token_test.yaml @@ -0,0 +1,72 @@ +## Workers authenticate to a gated Hub with a projected ServiceAccount token, +## audience-bound to kubeshark-hub. +## +## The projection used to be conditional on the same expression that decided +## AUTH_ENABLED, which coupled a DaemonSet rollout to the Hub's auth decision: +## anything that turned auth on without re-rendering the workers left them +## holding no token against a Hub that required one. It is now unconditional. +## The Hub ignores the token when auth is off, so the only cost is a mounted +## volume, and the two can no longer drift apart. +suite: worker hub token +templates: + - templates/09-worker-daemon-set.yaml +tests: + - it: projects the token when auth is off + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: hub-internal-token + projected: + sources: + - serviceAccountToken: + path: token + audience: kubeshark-hub + expirationSeconds: 3600 + + - it: projects the token when auth is on + set: + tap.auth.enabled: true + tap.auth.type: oidc + tap.auth.oidc.issuer: https://issuer.example.com + asserts: + - contains: + path: spec.template.spec.volumes + content: + name: hub-internal-token + projected: + sources: + - serviceAccountToken: + path: token + audience: kubeshark-hub + expirationSeconds: 3600 + + - it: points the sniffer at the token regardless of auth + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: HUB_INTERNAL_TOKEN_PATH + value: /var/run/secrets/kubeshark/hub-token/token + - contains: + path: spec.template.spec.containers[0].volumeMounts + content: + mountPath: /var/run/secrets/kubeshark/hub-token + name: hub-internal-token + readOnly: true + + - it: points the tracer at the token regardless of auth + set: + tap.tls: true + asserts: + - contains: + path: spec.template.spec.containers[1].env + content: + name: HUB_INTERNAL_TOKEN_PATH + value: /var/run/secrets/kubeshark/hub-token/token + - contains: + path: spec.template.spec.containers[1].volumeMounts + content: + mountPath: /var/run/secrets/kubeshark/hub-token + name: hub-internal-token + readOnly: true diff --git a/manifests/complete.yaml b/manifests/complete.yaml index 5f71e2b3c..2ccf2287c 100644 --- a/manifests/complete.yaml +++ b/manifests/complete.yaml @@ -269,8 +269,8 @@ data: INGRESS_ENABLED: 'false' INGRESS_HOST: 'ks.svc.cluster.local' PROXY_FRONT_PORT: '8899' - AUTH_ENABLED: 'true' - AUTH_TYPE: 'default' + AUTH_ENABLED: 'false' + AUTH_TYPE: 'saml' AUTH_SAML_IDP_METADATA_URL: '' AUTH_CLI_SERVICE_ACCOUNTS: '' AUTH_ROLES_CLAIM: 'groups' @@ -990,9 +990,9 @@ spec: containers: - env: - name: REACT_APP_AUTH_ENABLED - value: 'true' + value: 'false' - name: REACT_APP_AUTH_TYPE - value: 'default' + value: 'saml' - name: REACT_APP_COMPLETE_STREAMING_ENABLED value: 'true' - name: REACT_APP_STREAMING_TYPE