Fix/improve security in the inference server start command #940
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bigbitbus
requested review from
PawelPeczek-Roboflow,
grzegorz-roboflow,
yeldarby,
probicheaux and
hansent
as code owners
bigbitbus
force-pushed
the
fix/improve-docker-container-security
branch
from
08489a0 to
6363864
Compare
| is_gpu = "gpu" in image and "jetson" not in image | ||
| is_jetson = "jetson" in image | ||
|
|
||
| if is_gpu: | ||
| device_requests = [ | ||
| docker.types.DeviceRequest(device_ids=["all"], capabilities=[["gpu"]]) |
There was a problem hiding this comment.
User can try to run inference-gpu docker image on non-gpu host, this will result in
docker: Error response from daemon: could not select device driver "" with capabilities: [[gpu]].
Probably nothing to worry about - trying to run gpu image on non-gpu hardware feels like unintended situation.
There was a problem hiding this comment.
The get_image() function checks which image is the appropriate one already. (line 134)
| @@ -167,7 +170,25 @@ def start_inference_container( | |||
| labels=labels, | |||
| ports=ports, | |||
| device_requests=device_requests, | |||
| environment=environment, | |||
| environment=environment + [ | |||
| "MODEL_CACHE_DIR=/tmp/model-cache", | |||
There was a problem hiding this comment.
is MODEL_CACHE_DIR host path or docker path? By default we cache models under /tmp/cache
There was a problem hiding this comment.
Its the path within the docker container.
bigbitbus
force-pushed
the
fix/improve-docker-container-security
branch
from
6363864 to
39ea380
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Harden the
inference server startcommand in several ways for the cpu and gpu containers. Notably:Container Privilege Restrictions:
Added security_opt=["no-new-privileges"] to prevent the container from gaining new privileges
Added cap_drop=["ALL"] to drop all Linux capabilities by default
Only adds back minimal required capabilities with cap_add=["NET_BIND_SERVICE"] (and SYS_ADMIN for GPU containers)
These restrictions are only applied when not running on Jetson devices (if not is_jetson)
Read-only Filesystem:
Added read_only=not is_jetson to make the container filesystem read-only
Only /tmp directory is mounted as writable for necessary runtime files
Explicitly defines cache directories to use /tmp for various components:
"MODEL_CACHE_DIR=/tmp/model-cache",
"TRANSFORMERS_CACHE=/tmp/huggingface",
"YOLO_CONFIG_DIR=/tmp/yolo",
"MPLCONFIGDIR=/tmp/matplotlib",
"HOME=/tmp/home",
Network Isolation:
Added network_mode="bridge" to ensure container uses bridge networking
Added ipc_mode="private" to isolate the IPC namespace (except for Jetson devices)
Type of change
Security fixes
Tested on CPU (mac), GPU (T4 Nvidia GPU VM with Intel architecture) and on Jetson 5.X
Any specific deployment considerations
For example, documentation changes, usability, usage/costs, secrets, etc.
Docs