Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Add Internal TLS Support for Milvus Components under cluster mode #36864

Open
1 task done
nish112022 opened this issue Oct 14, 2024 · 13 comments
Open
1 task done
Assignees
Labels
kind/feature Issues related to feature request from users

Comments

@nish112022
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe.

Currently, internal communication between Milvus components (such as DataCoord, QueryCoord, etc.) is not secured. In environments where enhanced security is required, having communication encrypted with TLS is crucial to protect data and credentials in transit.

Describe the solution you'd like.

I would like to see internal TLS support added to Milvus, allowing all internal gRPC calls between Milvus components to be encrypted.This would involve:

  1. Adding configuration options for TLS certificates and keys like TLSMode
  2. Ensuring all components (Proxy, DataCoord, QueryCoord, etc.) are capable of verifying certificates and establishing secure connections.

Describe an alternate solution.

No response

Anything else? (Additional Context)

No response

@nish112022 nish112022 added the kind/feature Issues related to feature request from users label Oct 14, 2024
@nish112022
Copy link
Contributor Author

assign @nish112022

@nish112022
Copy link
Contributor Author

/assign @nish112022

sre-ci-robot pushed a commit that referenced this issue Nov 19, 2024
issue : #36864

I have a few questions regarding my approach.I will consolidate them
here for feedback and review.Thanks

---------

Signed-off-by: Nischay Yadav <[email protected]>
Signed-off-by: Nischay <[email protected]>
@haorenfsa
Copy link
Contributor

/assign

@haorenfsa
Copy link
Contributor

haorenfsa commented Nov 22, 2024

TODO:

@nish112022
Copy link
Contributor Author

@haorenfsa Can we have a meeting tomorrow?Let me know how and when.We can meet on Discord as well.

@haorenfsa
Copy link
Contributor

@nish112022 Yes. I'm available around 2-4 a.m. UTC. But I'm not sure what you want to discuss about in particular

@nish112022
Copy link
Contributor Author

Thank you for letting me know your availability @haorenfsa . Since it’s quite early for me, I’d prefer to discuss the approach asynchronously before making any changes. I’ll raise a PR or share a draft plan for your feedback afterward. Thanks!

@haorenfsa
Copy link
Contributor

I think we just need to add fields to enable user adding volumes & volumeMounts in Milvus components, so that one could mount custom certificates & keys for Milvus to use.

JsDove pushed a commit to JsDove/milvus that referenced this issue Nov 26, 2024
issue : milvus-io#36864

I have a few questions regarding my approach.I will consolidate them
here for feedback and review.Thanks

---------

Signed-off-by: Nischay Yadav <[email protected]>
Signed-off-by: Nischay <[email protected]>
@haorenfsa
Copy link
Contributor

One more thing, Seems only server-side TLS is supported for internal. Can we also support it mutual like we did for external TLS ? @nish112022

@nish112022
Copy link
Contributor Author

Yes, @haorenfsa, I can take this up when I have some bandwidth.

@nish112022
Copy link
Contributor Author

@haorenfsa, I have created the documentation for the official Milvus docs here: #2895.

I am planning to wait until your PR above gets merged so that I can update the link in the docs accordingly. Please let me know if any changes are required in my PR.

For the rest, I see that you have already made the changes—Thank you!

@haorenfsa
Copy link
Contributor

haorenfsa commented Dec 2, 2024

@nish112022 I checked your patch, it works only when all components run on the same host.

So I just realized one more things we've missed. Internally, milvus components uses IP to connect to each other, while TLS needs an server name indicator ( SNI ) for certificate verification. So we need another patch to make all internal gRPC clients to use a specific SNI when establishing tls connection.

@nish112022
Copy link
Contributor Author

@haorenfsa Yes, for standalone milvus it will work but for other deployments let's say Kubernetes, each pod has it's own hostname in some pattern(***.cluster.local.pod).So, I thought it will be done from the operator side to provide the hostname in the IP field under each component.
Regarding SNI, I missed.Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Issues related to feature request from users
Projects
None yet
Development

No branches or pull requests

3 participants