Skip to content

[Security] Don't bind tcp zmq socket to all interfaces #17197

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 28, 2025
Merged

[Security] Don't bind tcp zmq socket to all interfaces #17197

merged 1 commit into from
Apr 28, 2025

Conversation

russellb
Copy link
Member

This change addresses GHSA-9f8f-2vmf-885j

Stop binding to all interfaces for the XPUB socket in shm_broadcast.py.

We already determine the address that other hosts will use to connect to
this socket. If VLLM_HOST_IP is set, that will be the value. By binding
to all sockets, we expose private data to more networks than intended.

Signed-off-by: Russell Bryant [email protected]

@russellb
Don't bind tcp zmq socket to all interfaces
1d47516 
This change addresses GHSA-9f8f-2vmf-885j

Stop binding to all interfaces for the XPUB socket in shm_broadcast.py.

We already determine the address that other hosts will use to connect to
this socket. If `VLLM_HOST_IP` is set, that will be the value. By binding
to all sockets, we expose private data to more networks than intended.

Signed-off-by: Russell Bryant <[email protected]>
@github-actions GitHub Actions
Copy link

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

🚀

@russellb russellb mentioned this pull request Apr 25, 2025
Closed
2 tasks
@mergify mergify bot added the documentation Improvements or additions to documentation label Apr 25, 2025
@russellb russellb requested a review from youkaichao April 25, 2025 16:07
@russellb russellb added the security Security related issues and PRs label Apr 25, 2025
@russellb russellb changed the title Don't bind tcp zmq socket to all interfaces [Security] Don't bind tcp zmq socket to all interfaces Apr 25, 2025
@russellb russellb added this to the v0.8.5 milestone Apr 25, 2025
njhill
njhill approved these changes Apr 25, 2025
View reviewed changes
Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@youkaichao may have thoughts on this

@russellb russellb added the ready ONLY add when PR is ready to merge/full CI is needed label Apr 26, 2025
@simon-mo simon-mo merged commit a0304dc into vllm-project:main Apr 28, 2025
67 checks passed
jikunshang pushed a commit to jikunshang/vllm that referenced this pull request Apr 29, 2025
@russellb @dbyoung18
[Security] Don't bind tcp zmq socket to all interfaces (vllm-project#…
6743660 
…17197)

Signed-off-by: Russell Bryant <[email protected]>
lk-chen pushed a commit to lk-chen/vllm that referenced this pull request Apr 29, 2025
@russellb @lk-chen
[Security] Don't bind tcp zmq socket to all interfaces (vllm-project#…
7f24f0d 
…17197)

Signed-off-by: Russell Bryant <[email protected]>
RichardoMrMu pushed a commit to RichardoMrMu/vllm that referenced this pull request May 12, 2025
@russellb
[Security] Don't bind tcp zmq socket to all interfaces (vllm-project#…
ddda5fb 
…17197)

Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: Mu Huai <[email protected]>
@ckhordiasma ckhordiasma mentioned this pull request May 14, 2025
Merged
zzzyq pushed a commit to zzzyq/vllm that referenced this pull request May 24, 2025
@russellb
[Security] Don't bind tcp zmq socket to all interfaces (vllm-project#…
350ae9b 
…17197)

Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: Yuqi Zhang <[email protected]>
minpeter pushed a commit to minpeter/vllm that referenced this pull request Jun 24, 2025
@russellb @minpeter
[Security] Don't bind tcp zmq socket to all interfaces (vllm-project#…
f60c474 
…17197)

Signed-off-by: Russell Bryant <[email protected]>
Signed-off-by: minpeter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tlrmchlsmth tlrmchlsmth tlrmchlsmth approved these changes

@njhill njhill njhill approved these changes

@youkaichao youkaichao Awaiting requested review from youkaichao

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation ready ONLY add when PR is ready to merge/full CI is needed security Security related issues and PRs
Projects
None yet
Milestone
v0.8.5
Development

Successfully merging this pull request may close these issues.
4 participants
@russellb @tlrmchlsmth @njhill @simon-mo