search

kubeflow / pipelines

Machine Learning Pipelines for Kubeflow
https://www.kubeflow.org/docs/components/pipelines/
Apache License 2.0
3.51k stars 1.58k forks source link

[backend] Security exploit in mlpipeline-UI #9889

Open juliusvonkohout opened 11 months ago

juliusvonkohout commented 11 months ago

Environment

Steps to reproduce

This image here shows Bobs S3 artifacts produced by a pipeline run (e.g. tensorflow model) Sadly the KFP UI allows Alice to read Bobs artifacts content But how? If Alice spies the S3 artifact location from Bob, as seen at the bottom of the image, She can just can remove the yellow namespace parameter and the UI server will skip permission checks So you still need to know the S3/GCS path which you can for example get from an insecure (that is the default in KFP) not-disabled ML-metadata

grafik

Expected result

The namespace parameter of the readartifact api call should not be user-configurable. That should have been achieved for the APIserver in another PR from me and @difince https://github.com/difince/pipelines/blob/d0424bf86de5217d41dd03b50f91ac5ec3489df3/backend/src/apiserver/server/run_server.go#L313

But with mlpipeline-ui you can circumvent the permission check in the apiserver as seen on the screenshot. You can just remove the namespace parameter and the artifact-proxy mode is disabled. So mlpipeline-ui must set the namespace parameter based on the namespace dropdown selection for an authenticated user and not expose the namespace parameter to the user in the URL.

https://github.com/kubeflow/pipelines/issues/8074 is also important to secure the apiserver in the future and in the long term i would really like to get rid of the artifactproxy altogether as stated in https://github.com/kubeflow/pipelines/issues/8406#issuecomment-1643579605 and https://github.com/kubeflow/pipelines/issues/4790#issuecomment-1643574994 but that is for another day.

Materials and Reference

This has been discussed in the last KFP meeting (16th August 2023) and acknowledged by @zijianjoy on a technical level.

From the Kubecon presentation https://static.sched.com/hosted_files/kccnceu2023/f8/Hardening%20Kubeflow%20Security%20for%20Enterprise%20Environments%20-%20Julius%20von%20Kohout%2C%20DPDHL%20%26%20Diana%20Dimitrova%20Atanasova%2C%20VMware.pdf grafik grafik

Impacted by this bug? Give it a 👍.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

juliusvonkohout commented 7 months ago

Let's keep it open

github-actions[bot] commented 4 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

juliusvonkohout commented 4 months ago

/lifecycle-frozen

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

juliusvonkohout commented 2 months ago

not stale

github-actions[bot] commented 2 weeks ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

juliusvonkohout commented 2 weeks ago

/lifecycle frozen

juliusvonkohout commented 2 weeks ago

@HumairAK where exactly did you fix https://github.com/kubeflow/pipelines/issues/9889 in https://github.com/kubeflow/pipelines/pull/10787 ?

I see only "Because we don't do namespace verification server side, https://github.com/kubeflow/pipelines/issues/9889 where users can dupe the namespace client side and access object stores from another profile namespace. Because we are storing the provider's creds location in mlmd and querying it from client side, client can technically dupe this, and attempt to read object stores if their secrets are stored else where on the cluster. However, once we resolve https://github.com/kubeflow/pipelines/issues/9889 this should no longer be an issue, because any attempt to access resources outside the user's profile namespace should be rejected." which says the opposite.

juliusvonkohout commented 2 weeks ago

/reopen

until this is clarified

google-oss-prow[bot] commented 2 weeks ago

@juliusvonkohout: Reopened this issue.

In response to [this](https://github.com/kubeflow/pipelines/issues/9889#issuecomment-2188118580): >/reopen > >until this is clarified Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.