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

Feat extra security for hosts #5836

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Feat extra security for hosts #5836

wants to merge 13 commits into from

Conversation

tofarr
Copy link
Collaborator

@tofarr tofarr commented Dec 26, 2024

By default, disallowing access to server by any domain except localhost

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

I figure the LLM API key is the crown jewels of what an intruder would want to steal / exploit with openhands. (Assuming they can't figure a way to escape the docker sandbox).

We recently changed over from using local storage to save config including API keys to using a server side settings store. Given that the stock openhands version has no concept of users or authentication, we need to prevent access to it by default from any domain except localhost. Although the LLM keys cannot be retrieved, sessions could be created by any remote machine with network access - since docker is not firewalled by default on mac at least:
image

After this change:

  • We check that each request has an approved host
  • By default, approved hosts are 'localhost' and '127.0.0.1'
  • Approved hosts can be overridden in the configuration
  • A wildcard of '*' can be used to approve any host

To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:f3cc95f-nikolaik   --name openhands-app-f3cc95f   docker.all-hands.dev/all-hands-ai/openhands:f3cc95f

@tofarr tofarr marked this pull request as ready for review December 27, 2024 15:54
Comment on lines +47 to +49
approved_hostnames: list[str] = field(
default_factory=lambda: ['localhost', '127.0.0.1']
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not rely on the uvicorn --host flag?

Copy link
Collaborator Author

@tofarr tofarr Dec 30, 2024

Choose a reason for hiding this comment

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

The uvicorn host flag is currently set to 0.0.0.0 by the default docker command (so that the server is accessible from outside the docker container). This means that unless docker is firewalled, anything within is it accessible on any network on which the machine is running. So, for example, if a person was running on a wifi hotspot at an internet cafe, somebody at a nearby table could access their server and use their LLM credits assuming there were not other security precautions in place and that they could figure out the IP (Maybe just search the subnet for an open port 3000). Or if somebody did not secure their home WIFI correctly, then their neighbor may be able to use their LLM credits.

config.template.toml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants