security: Prevent session hijacking through a malicious notebook. #3550
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| # This EnvoyFilter removes the cookie oauth2_proxy_kubeflow | ||
| # before the request is forwarded to the backend systems. | ||
| # This prevents token stealing: | ||
| # Otherwise, the bearer token would be leaked to the notebook's containers | ||
| # which would allow a contributor to access another contributor's token. | ||
| apiVersion: networking.istio.io/v1alpha3 | ||
| kind: EnvoyFilter | ||
| metadata: | ||
| name: gateway-remove-kubeflow-cookie | ||
| namespace: istio-system # Deployed in the gateway's namespace | ||
| spec: | ||
| workloadSelector: | ||
| labels: | ||
| istio: ingressgateway | ||
| configPatches: | ||
| - applyTo: HTTP_FILTER | ||
| match: | ||
| context: GATEWAY | ||
| listener: | ||
| filterChain: | ||
| filter: | ||
| name: "envoy.filters.network.http_connection_manager" | ||
| subFilter: | ||
| name: "envoy.filters.http.router" | ||
| patch: | ||
| operation: INSERT_BEFORE | ||
| value: | ||
| name: envoy.filters.http.lua | ||
| typed_config: | ||
| "@type": "type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua" | ||
| inlineCode: | | ||
| function envoy_on_request(request_handle) | ||
| local cookie_header = request_handle:headers():get("cookie") | ||
|
|
||
| if cookie_header then | ||
|
sven-ernw marked this conversation as resolved.
|
||
| local target_cookie = "oauth2_proxy_kubeflow" | ||
|
|
||
| -- Prefix the header so every cookie has an explicit boundary. | ||
| local new_cookie = string.gsub(";" .. cookie_header, ";%s*" .. target_cookie .. "=[^;]*", "") | ||
| new_cookie = string.gsub(new_cookie, "^;", "") | ||
|
Comment on lines
+38
to
+40
|
||
|
|
||
| -- Clean up any dangling semicolons or whitespace | ||
| new_cookie = string.gsub(new_cookie, "^[;%s]+", "") | ||
| new_cookie = string.gsub(new_cookie, "[;%s]+$", "") | ||
|
|
||
| if new_cookie == "" then | ||
| request_handle:headers():remove("cookie") | ||
| else | ||
| request_handle:headers():replace("cookie", new_cookie) | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| # Secure Notebook Setup | ||
|
|
||
| Authors: Lorin Lehawany (@lorinl), Sven Nobis (@SvenTo) | ||
|
|
||
| ## Goal/Motivation | ||
|
|
||
| These features add support for running notebooks on their own subdomains. This prevents session hijacking through a malicious notebook. It is recommended for production setups. | ||
|
|
||
| ## Description | ||
|
|
||
| The multi-domain setup is recommended for a production Kubeflow deployment because the default setup allows an authenticated attacker to hijack sessions. However, because the setup has prerequisites and is not suitable for local development environments, it cannot be enabled by default. | ||
|
|
||
| This setup hosts the Kubeflow dashboard and APIs on a separate domain from the notebooks to prevent session hijacking from malicious notebooks through the user’s agent (browser). If an attacker hosts malicious notebooks and convinces a victim to visit the notebook’s URL, the attacker cannot steal session data or make API requests in the victim’s name. | ||
|
|
||
| ## Prerequisite | ||
|
|
||
| If you want to enable the multi-domain setup, you need to meet the following prerequisites: | ||
|
|
||
| - A wildcard domain for Kubeflow or some kind of automated external domain management for the notebook domains (e.g., [ExternalDNS](https://github.com/kubernetes-sigs/external-dns)). | ||
| - A TLS certificate that covers both the Kubeflow dashboard domain and the wildcard notebook subdomains (for example, `kubeflow.example.org` and `*.kubeflow.example.org`), or some kind of automated certificate management for the notebook domains (e.g., [cert manager](https://cert-manager.io/docs/)). | ||
|
|
||
| This includes that Kubeflow is exposed externally (which is most likely given in a production environment): | ||
|
|
||
| - Istio ingress is exposed externally (required for subdomain routing). | ||
| - OAuth2 Proxy and Dex are exposed externally. | ||
|
|
||
| For instance, a minimal configuration to expose Kubeflow on an external domain requires the following settings: | ||
|
|
||
| **OAuth2/Dex** | ||
|
|
||
| The internal cluster Dex URLs should be replaced with the external Kubeflow ingress URL for OIDC issuer, token redemption, and JWKS endpoints in [``common/oauth2-proxy/base/oauth2_proxy.cfg``](../common/oauth2-proxy/base/oauth2_proxy.cfg). This example uses ``kubeflow.example.org`` as the external domain: | ||
|
|
||
| ```sh | ||
| oidc_issuer_url = "https://kubeflow.example.org/dex" | ||
| redeem_url = "https://kubeflow.example.org/dex/token" | ||
| oidc_jwks_url = "https://kubeflow.example.org/dex/keys" | ||
| ``` | ||
|
|
||
| As well as the JWT issuer in the file [``common/oauth2-proxy/components/istio-external-auth/requestauthentication.dex-jwt.yaml``](../common/oauth2-proxy/components/istio-external-auth/requestauthentication.dex-jwt.yaml): | ||
|
|
||
| ```sh | ||
| jwtRules: | ||
| - issuer: https://kubeflow.example.org/dex | ||
| ``` | ||
|
|
||
| As well as the JWT issuer in the dex configuration file [``common/dex/overlays/oauth2-proxy/config-map.yaml``](../common/dex/overlays/oauth2-proxy/config-map.yaml): | ||
|
|
||
| ```sh | ||
| data: | ||
| config.yaml: | | ||
| issuer: https://kubeflow.example.org/dex | ||
| ``` | ||
|
|
||
| Secondly, the Kubeflow Ingress Gateway needs to be exposed via HTTPS: | ||
|
|
||
| **Istio Service** | ||
|
|
||
| The Istio service needs to be exposed to the outside. Change the service type to ``LoadBalancer`` in the file [`common/istio/istio-install/base/patches/service.yaml`](../common/istio/istio-install/base/patches/service.yaml): | ||
|
|
||
| ```yaml | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: istio-ingressgateway | ||
| namespace: istio-system | ||
| spec: | ||
| type: LoadBalancer | ||
| ``` | ||
|
|
||
| **Kubeflow Gateway** | ||
|
|
||
| The Kubeflow gateway should enable HTTPS, and a TLS certificate needs to be referenced in the file [`common/istio/kubeflow-istio-resources/base/kf-istio-resources.yaml`](../common/istio/kubeflow-istio-resources/base/kf-istio-resources.yaml): | ||
|
sven-ernw marked this conversation as resolved.
|
||
|
|
||
| ```yaml | ||
| apiVersion: networking.istio.io/v1alpha3 | ||
| kind: Gateway | ||
| metadata: | ||
| name: kubeflow-gateway | ||
| spec: | ||
| selector: | ||
| istio: ingressgateway | ||
| servers: | ||
| - port: | ||
| number: 80 | ||
| name: http | ||
| protocol: HTTP | ||
| hosts: | ||
| - "*" | ||
| - port: | ||
| number: 443 | ||
| name: https | ||
| protocol: HTTPS | ||
| hosts: | ||
| - "*" | ||
| tls: | ||
| mode: SIMPLE | ||
| credentialName: https-credential # set this to the secret with the wildcard TLS certificate and key | ||
| ``` | ||
|
|
||
| This will make the setup work. Please ensure that you follow the recommendations in the [_Security Considerations_ section](../README.md#security-considerations) for guidance on a secure setup. | ||
|
|
||
| ## Implementation details to enable multi-domain setup | ||
|
|
||
| The list below shows all steps required to enable the multi-domain setup and shows how to configure them. | ||
|
|
||
| The notebook subdomains must be part of the Kubeflow authentication authority. In a default setup, this means that the notebook domain is a subdomain of the Kubeflow dashboard. Otherwise, the setup will not work. For instance, if the dashboard is hosted on ``kubeflow.example.org``, every Notebook in the `example` profile uses the host `example-notebook.kubeflow.example.org`, when `ISTIO_HOST_NOTEBOOK` is set to `${NAMESPACE}-notebook.kubeflow.example.org`. | ||
|
|
||
| **Environment parameters** | ||
|
|
||
| The following parameters need to be defined in the configuration file [`applications/notebooks-v1/upstream/notebook-controller/manager/params.env`](../applications/notebooks-v1/upstream/notebook-controller/manager/params.env): | ||
|
|
||
| | Parameter | Description | | ||
| | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `ISTIO_USE_NOTEBOOK_SUBDOMAINS` | Set this value to ``true``. | | ||
| | `ISTIO_HOST_NOTEBOOK` | Domain template used by Istio to host notebooks (e.g., `${NAMESPACE}-notebook.kubeflow.example.org`). `${NAMESPACE}` will be replaced with the Notebook's namespace (Kubeflow profile). | | ||
| | `ISTIO_HOST_AUTH` | Host used by Istio for handling authentication callbacks or login flows (e.g., `kubeflow.example.org`). | | ||
| | `ISTIO_AUTH_PATH` | Optional, defaults to `/oauth2/`; Can be used to change the base URL path used by Istio for authentication callbacks or login flows (e.g. `/oauth2/`). | | ||
|
Comment on lines
+114
to
+117
|
||
| | | This path must match the routing configured in the authentication provider (e.g., OAuth2 Proxy) so that login and callback requests are correctly handled. | | ||
|
|
||
| **Cookie domains** | ||
|
|
||
| Enable this setting for multi-domain notebook support in the configuration file [`common/oauth2-proxy/base/oauth2_proxy.cfg`](../common/oauth2-proxy/base/oauth2_proxy.cfg): | ||
|
|
||
| ```sh | ||
| cookie_domains = [ "kubeflow.example.org" ] | ||
| ``` | ||
|
|
||
| ## Does this break any existing functionality? | ||
|
|
||
| The multi-domain setup is not enabled by default and thus this change will not change any default behavior. It should not break any existing functionality if enabled, too. | ||
|
|
||
| ## Does this solve any outstanding security issues? | ||
|
|
||
| Yes. This implementation addresses a security issue that could allow session hijacking via a malicious notebook; see [the GitHub security advisory](https://github.com/kubeflow/notebooks/security/advisories/GHSA-qjw6-hpc7-w36h) for details. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.