search

kubeflow / manifests

A repository for Kustomize manifests
Apache License 2.0
815 stars 875 forks source link

Find an automated way to set JWKS keys in RequestAuthentication for oauth2-proxy M2M (EKS, AKS, GKE, Rancher, Openshift) #2850

Closed thesuperzapper closed 1 month ago

thesuperzapper commented 2 months ago

Related Issues

What's the problem?

We have a RequestAuthentication/m2m-token-issuer in the istio-system namespace which is there to allow Istio to validate JWTs which are actually Kubernetes ServiceAccount tokens.

This is only relevant to requests to http://ml-pipeline-ui.kubeflow.svc.cluster.local, as this is the only place we enforce a source.requestPrincipals (see: AuthorizationPolicy/ml-pipeline from the kubeflow namespace).

Retrieving the JWKS keys

Currently, we are using a CronJob called kubeflow-m2m-oidc-configurator to populate the jwks information in the RequestAuthentication/m2m-token-issuer:

This is a bad idea for many reasons as it can result in failures if the JWKS tokens are rotated, or if the strange script fails for any reason (which seems like what is happening on all EKS clusters).

Setting the correct Issuer

Some clusters don't use https://kubernetes.default.svc.cluster.local as their cluster's JWT issuer (notably EKS and GKE), so any RquestAuthentication which assumes this will fail to validate JWTs issued by these clusters.

The only way to know what a cluster's issuer should be is to retrieve it from the /.well-known/openid-configuration cluster endpoint.

What's the solution?

We can instead directly set the spec.jwtRules[0].jwksUri of the RequestAuthentication to https://KUBERNETES_API/openid/v1/jwks as this always serves the cluster's JWSK.

However, the /openid/v1/jwks endpoint is not available without authentication (read: accessed by a Pod in the cluster, presenting its ServiceAccount token as authorization).

So as a workaround we can create a kubectl proxy ... pod which only exposes this endpoint of the API (note, this is not a security risk, as these are just public signing keys).

ALSO, in terms of restricting proxy service to specific Pods, I don't think we can easily do that, because each Istio sidecar will need to access it to retrieve the JWSK for its requests.

0 - Create the required RBAC

First, create this ServiceAccount, Role, and RoleBinding:

apiVersion: v1
kind: ServiceAccount
metadata:
  name: cluster-jwks-proxy
  namespace: istio-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
  name: cluster-jwks-proxy
  namespace: istio-system
rules:
  - apiGroups:
      - security.istio.io
    resources:
      - requestauthentications
    verbs:
      - get
      - patch
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: cluster-jwks-proxy
  namespace: istio-system
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: Role
  name: cluster-jwks-proxy
subjects:
  - kind: ServiceAccount
    name: cluster-jwks-proxy
    namespace: istio-system

1 - Create the Proxy Service

First, create this Deployment:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: cluster-jwks-proxy
  namespace: istio-system
spec:
  replicas: 1
  selector:
    matchLabels:
      app: cluster-jwks-proxy
  template:
    metadata:
      labels:
        app: cluster-jwks-proxy
    spec:
      serviceAccountName: cluster-jwks-proxy
      initContainers:
        ## NOTE: clusters don't all use the same JWT 'issuer', so we need to retrieve it from the cluster
        - name: patch-requestauthentication
          image: bitnami/kubectl:1.30.4
          command:
            - /bin/bash
            - -c
          args:
            - |
              # get the issuer from '/.well-known/openid-configuration'
              echo "INFO: Getting cluster issuer from '/.well-known/openid-configuration'"
              ISSUER=$(kubectl get --raw /.well-known/openid-configuration | jq -r '.issuer')

              # patch the RequestAuthentication to use the issuer
              RA_NAME="m2m-token-issuer"
              RA_NAMESPACE="istio-system"
              JSON_PATCH=$(cat <<EOF
              [
                {
                  "op": "replace",
                  "path": "/spec/jwtRules/0/issuer",
                  "value": "$ISSUER"
                }
              ]
              EOF
              )
              echo "INFO: Patching RequestAuthentication '$RA_NAME' in '$RA_NAMESPACE' to use issuer '$ISSUER'"
              kubectl patch requestauthentication "$RA_NAME" -n "$RA_NAMESPACE" --type json --patch "$JSON_PATCH"
      containers:
        ## NOTE: some clusters dont make their JWKS endpoint public, so we need to proxy it
        - name: kubectl-proxy
          image: bitnami/kubectl:1.30.4
          ports:
            - name: http
              containerPort: 8080
          startupProbe:
            initialDelaySeconds: 5
            periodSeconds: 5
            timeoutSeconds: 2
            httpGet:
              path: /openid/v1/jwks
              port: http
          livenessProbe:
            initialDelaySeconds: 15
            periodSeconds: 15
            timeoutSeconds: 5
            httpGet:
              path: /openid/v1/jwks
              port: http
          readinessProbe:
            initialDelaySeconds: 15
            periodSeconds: 15
            timeoutSeconds: 5
            httpGet:
              path: /openid/v1/jwks
              port: http
          args:
            - proxy
            - --address=0.0.0.0
            - --port=8080
            ## accept all hosts (default is local only)
            - --accept-hosts=.*
            ## only accept requests to '/openid/v1/jwks' and '/.well-known/openid-configuration'
            - --accept-paths=^(?:/openid/v1/jwks)|(?:/.well-known/openid-configuration)$
            ## reject all methods except 'GET'
            - --reject-methods=^(POST|PUT|PATCH|DELETE|HEAD|OPTIONS|CONNECT|TRACE)$

Next, create this Service:

apiVersion: v1
kind: Service
metadata:
  name: cluster-jwks-proxy
  namespace: istio-system
spec:
  ports:
    - name: http
      port: 80
      targetPort: http
  selector:
    app: cluster-jwks-proxy

2 - Delete existing CronJob

Run this command to delete the CronJob:

kubectl delete cronjob "kubeflow-m2m-oidc-configurator" -n "istio-system"

3 - Update the RequestAuthentication

Patch the RequestAuthentication as follows:

apiVersion: security.istio.io/v1
kind: RequestAuthentication
metadata:
  name: m2m-token-issuer
  namespace: istio-system
spec:
  jwtRules:
    - forwardOriginalToken: true

      ## \/ this part will be patched by the deployment \/
      issuer: https://kubernetes.default.svc.cluster.local

      ## \/ this part is new, and must be patched manually \/
      jwksUri: http://cluster-jwks-proxy.istio-system.svc.cluster.local/openid/v1/jwks

      outputClaimToHeaders:
        - claim: sub
          header: x-auth-request-user
        - claim: sub
          header: kubeflow-userid
thesuperzapper commented 2 months ago

@juliusvonkohout @kimwnasptd we need to release a Kubeflow 1.9.1 patch with the CronJob removed as described above.

thesuperzapper commented 2 months ago

The only remaining question is if we should do the issuer patching in a loop, so that even if someone accidentally reverts it after syncing, it will go back to being correct after x seconds.

thesuperzapper commented 2 months ago

Ok, it seems like EKS does not actually serve all of its JWKS keys under /openid/v1/jwks, and some are only found under https://oidc.eks.AWS_REGION.amazonaws.com/id/CLUSTER_ID/keys, I am really at a loss... It seems like this is another case where EKS needs specific attention from every downstream project.

At least you can automatically get the issuer URL from the cluster's /.well-known/openid-configuration and construct the real discovery URL or JWKS endpoint (so you don't have to figure out the CLUSTER_ID).

But honestly, if EKS needs a hard coded workaround, I think many other distributions are going to need one also.

Perhaps we should just go back to having KFP do the JWT validation itself, and let all requests with a JWT and no userid header reach it again.

Or at very least disable the m2m feature by default, and have people configure it for their cluster when they enable it, because each cluster will be different.

Plus, this is an advanced feature that most users will never need.

jaffe-fly commented 2 months ago

i have follow step from 0 - Create the required RBAC to 3 - Update the RequestAuthentication but in my-profile ns of kubeflow 1.9 by my test in notebook kubeflownotebookswg/jupyter-pytorch-full:latest

from kfp import dsl
from kfp.client import Client
import kfp

client = Client()
print(client.get_kfp_healthz())
print(client.get_kfp_healthz())
print(client.list_experiments())

the client.list_experiments() will get error

ApiException: (401)
Reason: Unauthorized
HTTP response headers: HTTPHeaderDict({'content-type': 'application/json', 'date': 'Wed, 21 Aug 2024 09:47:11 GMT', 'content-length': '811', 'x-envoy-upstream-service-time': '4', 'server': 'envoy'})
HTTP response body: {"error":"List experiments failed: Failed to authorize with API: Failed to authorize with API: Unauthenticated: Failed to check authorization. User identity is empty in the request header: Unauthenticated: Request header error: there is no user identity header.: Request header error: there is no user identity header.","code":16,"message":"List experiments failed: Failed to authorize with API: Failed to authorize with API: Unauthenticated: Failed to check authorization. User identity is empty in the request header: Unauthenticated: Request header error: there is no user identity header.: Request header error: there is no user identity header.","details":[{"@type":"type.googleapis.com/google.rpc.Status","code":16,"message":"Failed to check authorization. User identity is empty in the request header"}]}
thesuperzapper commented 2 months ago

@jaffe-fly what distribution of Kubernetes are you using?

Because as I was saying in https://github.com/kubeflow/manifests/issues/2850#issuecomment-2300774003, this fix does not work yet on EKS.

juliusvonkohout commented 2 months ago

@thesuperzapper first of all thank you for the PR. I addresed most other issues and PRs in this repository, but my time is currently limited. Since none of this came up during the distribution testing I want to wait for input from @kromanow94 and the other lead @kimwnasptd.

I am fine with changing the technical implementation for 1.9.1, but we need to have it enabled by default and we rely on it in many tests already.

jaffe-fly commented 2 months ago

@jaffe-fly what distribution of Kubernetes are you using?

Because as I was saying in #2850 (comment), this fix does not work yet on EKS.

v1.29.5 Sorry I didn't see that , I will restore the previous settings

thesuperzapper commented 2 months ago

@juliusvonkohout you can just have a "kind overlay" which you additionally apply in your tests that enables M2M with the https://kubernetes.default.svc.cluster.local RequestAuthentication as it does now. We can even use the proxy approach I have demonstrated above (to avoid the need for a CronJob).

But as pretty much every K8s distribution will need a custom workaround (because each of them implements cluster OIDC in a different way), I think it's best to leave the specific implementation up to the vendors, and not apply the "kind overlay" in the default install (to avoid users installing it on a cluster like EKS).

The fact that the current M2M implementation will fail on most managed Kubernetes services is a significant unacceptable regression in the default manifests for 1.9.0, the "in-cluster KFP SDK access" no longer works in most cases.

Because the M2M feature literally only affects the KFP API (and the API already validates JWTs that are passed to it), we can just revert to an AuthorizationPolicy that allows any request to the KFP API from inside the cluster, as long as it does not set the user-id header.

We should probably also add a basic non-m2m test for the default, to ensure this does not regress in the future.

juliusvonkohout commented 2 months ago

First of all we should document it in the main readme and provide a workaround until we find a long-term solution.

tarekabouzeid commented 2 months ago

Hi @thesuperzapper,

Would it be possible to use Kubeflow Dex as JWT issuer ? or that is bad idea and we need to use k8s cluster native jwt issuer ?

http://dex.auth.svc.cluster.local:5556/dex/.well-known/openid-configuration to get the jwks_uri or directly set it to http://dex.auth.svc.cluster.local:5556/dex/keys

My setup is, using example manifest and using RKE + rancher for k8s cluster.

Thanks

juliusvonkohout commented 2 months ago

@kimwnasptd can you help with the documentation or implementation to enable M2M issuers in other clusters than kind (AKS, EKS Etc.)? For the time being we could also apply a workaround or just document better how to disable it. Some users get it configured on EKS etc. while some are overwhelmed. Nevertheless no one complained during the long distribution testing. @kromanow94 pledged to help as well in the last platform meeting. I might have more time in October after my two GSOC students have finished.

thesuperzapper commented 2 months ago

@juliusvonkohout @kimwnasptd as there is not going to be a general solution for all clusters (each k8s distro uses a different auth system), I strongly believe our next steps need to be:

  1. Separate the M2M code into a "kind M2M" overlay which indicates that it's for Kind clusters only:
  2. Update the "kind M2M" overlay to not need a CronJob:
    • See the description of this issue for an example which uses kubectl proxy as an alternative.
  3. Remove the "kind M2M" overlay from the default while ! kustomize build example ... install:
    • Instead, we should revert to the Kubeflow Pipelines API only beeing accessible with a JWT if you DONT pass a kubeflow-userid header (enforced with an AuthorizationPolicy)
    • Note, the KFP API already does the only generic JWT validation available in the form of a TokenReview on requests without a kubeflow-userid header.
  4. Add some docs to the manifests readme about how to use the "kind m2m" overlay:
    • They must warn users that non-Kind clusters will need some custom changes to work.
  5. Release version 1.9.1 of the kubeflow/manifests with the above changes

Also, I can't think of a situation where you would be running on the cluster and not have the cluster-issued JWT available to talk to the ml-pipeline service (which is not available outside the cluster anyway).

If we want to enable JWT access from outside the cluster (e.g. through the Istio gateway), then we should NOT be allowing cluster JWTs anyway (it's very insecure to exfiltrate cluster-issued JWTs), so we only need to trust our own Dex/other JWTs which is trivial.

juliusvonkohout commented 2 months ago

Given my limited availability due to GSOC my current knowledge is the following:

The serviceaccount tokens for machine to machine communication is core functionality since Kubeflow 1.7, even with the oidc-authservice it was available. There is also Dex for user sessions. We rely in many of our tests on the authentication via Kubernetes serviceaccount tokens. Not just for KFP, but the whole Kubeflow API via the istio ingressgateway. So "Because the M2M feature literally only affects the KFP API" is not true. Again its core functionality and not something we will change for 1.9.1. These workflows which tests KFP, Jupyterlabs etc. programmatically are used as reference. https://github.com/kubeflow/manifests/blob/master/proposals/20240606-jwt-handling.md contains architectural information from Kimonas.

"Update the "kind M2M" overlay to not need a CronJob" This is a realistic target for 1.9.1. /openid/v1/jwks proxy and "At least you can automatically get the issuer URL from the cluster's /.well-known/openid-configuration and construct the real discovery URL or JWKS endpoint (so you don't have to figure out the CLUSTER_ID)." on EKS

"Remove the "kind M2M" overlay from the default while ! kustomize build example ... install" is not realistic and would violate the premises above.

"Add some docs to the manifests readme about how to use the "kind m2m" overlay. Instead, we should revert to the Kubeflow Pipelines API only beeing accessible with a JWT if you DONT pass a kubeflow-userid header (enforced with an AuthorizationPolicy)". We can add documentation and warning but keep it the default and not add the overlay. If it is possible to support both ways in parallel this would also be interesting. https://github.com/kubeflow/manifests/blob/master/proposals/20240606-jwt-handling.md#requiring-a-jwt might help here.

Another technical solution to support Kubernetes serviceaccount tokens as bearer tokens for the whole API would also be interesting, but please keep https://github.com/kubeflow/manifests/blob/master/proposals/20240606-jwt-handling.md in mind.

For 1.10 we can do larger changes, but for the 1.9.1 patch release we need to be realistic.

So is someone willing to update the documentation with a warning for now and tackle /openid/v1/jwks proxy and "At least you can automatically get the issuer URL from the cluster's /.well-known/openid-configuration and construct the real discovery URL or JWKS endpoint (so you don't have to figure out the CLUSTER_ID)." on EKS?

We can also investigate supporting both authentication mechanisms at the same time with https://github.com/kubeflow/manifests/blob/master/proposals/20240606-jwt-handling.md#requiring-a-jwt

Afterwards we need to test it with EKS, AKS, GKE, Rancher.

@tarekabouzeid do you want to start with the documentation and point to https://github.com/kubeflow/manifests/blob/afc358d6d473a24029149f2a0ca21671af4aca6d/common/oauth2-proxy/overlays/m2m/component-overwrite-m2m-token-issuer/kustomization.yaml#L8 and https://github.com/kubeflow/manifests/blob/master/common/oauth2-proxy/overlays/m2m/README.md for EKS clusters?

thesuperzapper commented 2 months ago

@juliusvonkohout and others watching, I have raised a PR which will stop Istio from verifying Kubernetes JWTs in the first place, it was unnecessary to enable M2M authentication so we can actually remove all this complexity.

https://github.com/kubeflow/manifests/pull/2864

thesuperzapper commented 2 months ago

@juliusvonkohout Ok, I have made a significant update to the PR https://github.com/kubeflow/manifests/pull/2864, which reworks the auth flow, and provides 3 options for users to pick from, depending on their needs:

  1. Dex Only (no Kubernetes JWTs from outside the cluster)
  2. Dex + Kind (allows Kubernetes JWTs on kind-like clusters)
  3. Dex + EKS (shows a basic example allowing JWTs from outside the cluster on EKS)
    • Note, I still recommend people use a distribution of Kubeflow if they are not a very advanced user, because we won't be officially "supporting" that EKS example.
kromanow94 commented 1 month ago

Hey, thank you for moving forward with this issue. At the same time, I'm sorry I didn't have the time to chime in earlier.

I totally agree that the CronJob approach is not the best, but, please note that the CronJob should only be used for self-signer issuers on clusters deployed by kind, vcluster, minikube and so on. If running on EKS, GKE or whatever other cluster that's not using self-signed certificates for in-cluster OIDC Issuer, the CronJob is not only not needed but also recommended against. For people that are trying to run on EKS and having issues, the solution would be to use the ../common/oauth2-proxy/overlays/m2m instead of the ../common/oauth2-proxy/overlays/m2m-self-signed in the example/kustomization.yaml and configure it with the proper OIDC Issuer, which when served behind publicly trusted certificates, doesn't need the CronJob.

For EKS, GKE and other cluster with OIDC Issuer served behind publicly trusted certificates, it should be enough to set the OIDC Issuer URL here: https://github.com/kubeflow/manifests/blob/da0255f10d875040c2d845cd61b7938236c0dfaa/common/oauth2-proxy/overlays/m2m/component-overwrite-m2m-token-issuer/kustomization.yaml#L8 and here: https://github.com/kubeflow/manifests/blob/da0255f10d875040c2d845cd61b7938236c0dfaa/common/oauth2-proxy/overlays/m2m/kustomization.yaml#L18 and not use the m2m-self-signed overlay. This does not deploy the CronJob and makes the m2m setup functional.

@jaffe-fly, the above paragraph should make the EKS setup functional. Basically, don't use the m2m-self-signed overlay and provide proper the EKS/IAM OIDC Issuer URL in the m2m overlay.

To add a crucial detail to the CronJob, it's sole reason was to provide the RequestAuthentication/m2m-token-issuer with the JWKS so then Istio doesn't have to call the OIDC Issuer for JWKS. If Istio were to call the OIDC Issuer for JWKS served behind a self-signed certs, the request would fail in istiod with self-signed certificates error. With publicly trusted certificates, the whole mechanism that puts the JWKS in RequestAuthentication/m2m-token-issuer is simply not needed.

I hoped to explain that in https://github.com/kubeflow/manifests/blob/da0255f10d875040c2d845cd61b7938236c0dfaa/common/oauth2-proxy/components/configure-self-signed-kubernetes-oidc-issuer/README.md but, it seems I didn't do a good job in there.

The idea behind RequestAuthentication/m2m-token-issuer in the istio-system namespace was to be used for more than just ml-pipeline. It's taking the sub (or other) claim with user email and puts to the kubeflow-userid. With this, you can use m2m tokens to also connect to other APIs, like notebooks API or central dashboard API from outside cluster. Technically it's the same as RequestAuthentication/dex-jwt but for M2M. The reason why we need the source.requestPrincipals in AuthorizationPolicy/ml-pipeline is because it's the only AuthorizationPolicy that uses the when.notValues for the Authorization Header. Technically, the source.requestPrincipals is a workaround for the when.notValues (it's also good for seurity because it enforces that any request going to ml-pipeline must contain an OIDC Issuer trusted by Istio, but that's an accidental benefit in this case).

I like the idea behind cluster-jwks-proxy and the kubectl-proxy container but, AFAIU, it's also only needed for the scenario with self-signed certificates. But, if we hope to make the example kustomization integrated with every K8s Provider and serve m2m in all scenarios (including self-signed issuer), maybe at the moment it's the most flexible solution. But, knowing that the JWKS can be rotated, I would circle back to the idea than a CronJob might be a better fit for the Deployment/cluster-jwks-proxy (because it's currently run only once in the initContainer). Putting that code in a loop and moving out of the initContainers is also ok, just a different mean of achieving the same goal.

So, my opinion in a summary would circle around a few points:

  1. Knowing that we only need the CronJob (or any other JWKS retrieval/configurator automation code) for self-signed issuer (kind, vcluster, minukube...), we could as well just leave it as it is and make a better job at explaining how to use it and configure. I agree the CronJob is not perfect but there is only one scenario where we need it.

  2. We can disable m2m functionality by default in the example/kustomization.yaml and provide documentation (and maybe other example kustomizations) explaining how to configure m2m functionality. With this, we omit the complexity of different providers (and the special attention needed by EKS) because user need to provide only the OIDC Issuer and the aud claim (the aud claim is needed for oauth2-proxy to trust the extra jwt issuer; maybe there is a possibility to set a wildcard for the aud claim but that's to be verified).

  3. We can add documentation steps on how to add the in-cluster Self-Signed CA to the Itiod, then the whole JWKS Injection to the RequestAuthentication/m2m-token-issuer is not needed, because Istio will retrieve it automatically. With this, the whole idea of JWKS injection is simply not needed!

  4. If we consider making a generic solution that would automatically work for every cluster behind every provider (EKS, GKE, kind, vcluster...), maybe it would be a better option to create some kind of a K8s Operator that would be deployed with every Kubeflow Installation and verify what kind of cluster/provider are we running on. This would solve for the special attention needed by some of the providers, for the rotating JWKS, and for the scenario where the OIDC Issuer is deployed behind self-signed certs.

juliusvonkohout commented 1 month ago

@kromanow94 I think some of your remarks are already answered in the PR instead of the issue.

imranrazakhan commented 1 month ago

@thesuperzapper I followed step from 0 to 3 and it works on AKS. I have to reload ml-pipeline-persistenceagent too as my run were always on pending state, Although all pipeline pods were completed.

AKS: 1.29.7 Kubeflow 1.9.0

juliusvonkohout commented 1 month ago

We can of course refine it in other issues/PRs so feel free to create them as needed.