Skip to content

Commit a527e8c

Browse files
authored
fix: verify blob digests and sandbox Python backends (#848)
Addresses two security vulnerabilities reported against Model Runner: - Blob digest verification: after downloading each layer, hash the complete file and reject it if the SHA256 does not match the digest declared in the manifest. A malicious registry could previously serve arbitrary bytes under any digest name and they would be stored without error. The full file is hashed (not just the streamed bytes) so that resumed downloads are also verified correctly. - Python backend sandboxing: add ConfigurationPython to the sandbox package and apply it to all Python-based inference backends (vLLM, vllm-metal, SGLang, MLX, Diffusers). On Darwin this uses sandbox-exec with a deny-by-default seatbelt profile matching the existing llama.cpp policy; on Windows the same job object limits are applied. Previously these backends ran completely unsandboxed, meaning attacker-controlled model files could execute arbitrary code on the host.
1 parent 9a742ad commit a527e8c

File tree

10 files changed

+160
-8
lines changed

10 files changed

+160
-8
lines changed

pkg/distribution/internal/store/blobs.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package store
22

33
import (
44
"context"
5+
"crypto/sha256"
6+
"encoding/hex"
57
"errors"
68
"fmt"
79
"io"
@@ -268,6 +270,35 @@ func (s *LocalStore) WriteBlobWithResume(diffID oci.Hash, r io.Reader, digestStr
268270

269271
f.Close() // Rename will fail on Windows if the file is still open.
270272

273+
// Verify the digest of the completed file before making it visible. This
274+
// must be done after closing the file (to flush all writes) and before the
275+
// rename so that a mismatch never results in a corrupt blob being stored.
276+
// We hash the whole file rather than the streamed bytes so that resumed
277+
// downloads (which append to an existing partial file) are verified
278+
// correctly over their entire contents.
279+
if diffID.Algorithm == "sha256" {
280+
completedFile, openErr := os.Open(incompletePath)
281+
if openErr != nil {
282+
_ = os.Remove(incompletePath)
283+
return fmt.Errorf("open completed blob file for verification: %w", openErr)
284+
}
285+
286+
hasher := sha256.New()
287+
if _, copyErr := io.Copy(hasher, completedFile); copyErr != nil {
288+
completedFile.Close()
289+
_ = os.Remove(incompletePath)
290+
return fmt.Errorf("hash completed blob file: %w", copyErr)
291+
}
292+
completedFile.Close()
293+
294+
computed := hex.EncodeToString(hasher.Sum(nil))
295+
if computed != diffID.Hex {
296+
_ = os.Remove(incompletePath)
297+
return fmt.Errorf("blob digest mismatch for %q: expected sha256:%s, got sha256:%s",
298+
diffID.String(), diffID.Hex, computed)
299+
}
300+
}
301+
271302
if renameFinalErr := os.Rename(incompletePath, path); renameFinalErr != nil {
272303
return fmt.Errorf("rename blob file: %w", renameFinalErr)
273304
}

pkg/distribution/internal/store/blobs_test.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,40 @@ func TestBlobs(t *testing.T) {
123123
}
124124
})
125125

126-
t.Run("WriteBlob reuses existing blob", func(t *testing.T) {
127-
// simulate existing blob
126+
t.Run("WriteBlob rejects blob with wrong digest", func(t *testing.T) {
127+
// Use a well-known sha256 hash (of empty string) but supply different content.
128128
hash := oci.Hash{
129129
Algorithm: "sha256",
130130
Hex: "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855",
131131
}
132+
blobPath, err := store.blobPath(hash)
133+
if err != nil {
134+
t.Fatalf("error getting blob path: %v", err)
135+
}
136+
// Remove any pre-existing blob so we hit the download path.
137+
_ = os.Remove(blobPath)
138+
_ = os.Remove(incompletePath(blobPath))
139+
140+
if err := store.WriteBlob(hash, bytes.NewBufferString("wrong content")); err == nil {
141+
t.Fatal("expected digest mismatch error, got nil")
142+
}
143+
144+
// The incomplete file must be cleaned up after a digest mismatch.
145+
if _, err := os.Stat(incompletePath(blobPath)); !errors.Is(err, os.ErrNotExist) {
146+
t.Fatal("expected incomplete file to be removed after digest mismatch")
147+
}
148+
// The final blob must not exist.
149+
if _, err := os.Stat(blobPath); !errors.Is(err, os.ErrNotExist) {
150+
t.Fatal("expected final blob file not to exist after digest mismatch")
151+
}
152+
})
153+
154+
t.Run("WriteBlob reuses existing blob", func(t *testing.T) {
155+
// Use the correct hash for "some-data" so digest verification passes.
156+
hash, _, err := oci.SHA256(bytes.NewReader([]byte("some-data")))
157+
if err != nil {
158+
t.Fatalf("error computing hash: %v", err)
159+
}
132160

133161
if err := store.WriteBlob(hash, bytes.NewReader([]byte("some-data"))); err != nil {
134162
t.Fatalf("error writing blob: %v", err)

pkg/inference/backends/diffusers/diffusers.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/docker/model-runner/pkg/internal/dockerhub"
1919
"github.com/docker/model-runner/pkg/internal/utils"
2020
"github.com/docker/model-runner/pkg/logging"
21+
"github.com/docker/model-runner/pkg/sandbox"
2122
)
2223

2324
const (
@@ -255,7 +256,7 @@ func (d *diffusers) Run(ctx context.Context, socket, model string, modelRef stri
255256
Socket: socket,
256257
BinaryPath: d.pythonPath,
257258
SandboxPath: "",
258-
SandboxConfig: "",
259+
SandboxConfig: sandbox.ConfigurationPython,
259260
Args: args,
260261
Logger: d.log,
261262
ServerLogWriter: logging.NewWriter(d.serverLog),

pkg/inference/backends/mlx/mlx.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/docker/model-runner/pkg/inference/models"
1414
"github.com/docker/model-runner/pkg/inference/platform"
1515
"github.com/docker/model-runner/pkg/logging"
16+
"github.com/docker/model-runner/pkg/sandbox"
1617
)
1718

1819
const (
@@ -140,7 +141,7 @@ func (m *mlx) Run(ctx context.Context, socket, model string, modelRef string, mo
140141
Socket: socket,
141142
BinaryPath: m.pythonPath,
142143
SandboxPath: "",
143-
SandboxConfig: "",
144+
SandboxConfig: sandbox.ConfigurationPython,
144145
Args: args,
145146
Logger: m.log,
146147
ServerLogWriter: logging.NewWriter(m.serverLog),

pkg/inference/backends/sglang/sglang.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/docker/model-runner/pkg/inference/models"
1717
"github.com/docker/model-runner/pkg/inference/platform"
1818
"github.com/docker/model-runner/pkg/logging"
19+
"github.com/docker/model-runner/pkg/sandbox"
1920
)
2021

2122
const (
@@ -169,7 +170,7 @@ func (s *sglang) Run(ctx context.Context, socket, model string, modelRef string,
169170
Socket: socket,
170171
BinaryPath: s.pythonPath,
171172
SandboxPath: sandboxPath,
172-
SandboxConfig: "",
173+
SandboxConfig: sandbox.ConfigurationPython,
173174
Args: args,
174175
Logger: s.log,
175176
ServerLogWriter: logging.NewWriter(s.serverLog),

pkg/inference/backends/vllm/vllm.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/docker/model-runner/pkg/inference/models"
1919
"github.com/docker/model-runner/pkg/inference/platform"
2020
"github.com/docker/model-runner/pkg/logging"
21+
"github.com/docker/model-runner/pkg/sandbox"
2122
)
2223

2324
const (
@@ -180,7 +181,7 @@ func (v *vLLM) Run(ctx context.Context, socket, model string, modelRef string, m
180181
Socket: socket,
181182
BinaryPath: v.binaryPath(),
182183
SandboxPath: vllmDir,
183-
SandboxConfig: "",
184+
SandboxConfig: sandbox.ConfigurationPython,
184185
Args: args,
185186
Logger: v.log,
186187
ServerLogWriter: logging.NewWriter(v.serverLog),

pkg/inference/backends/vllm/vllm_metal.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/docker/model-runner/pkg/internal/dockerhub"
2121
"github.com/docker/model-runner/pkg/internal/utils"
2222
"github.com/docker/model-runner/pkg/logging"
23+
"github.com/docker/model-runner/pkg/sandbox"
2324
)
2425

2526
const (
@@ -216,8 +217,8 @@ func (v *vllmMetal) Run(ctx context.Context, socket, model string, modelRef stri
216217
BackendName: "vllm-metal",
217218
Socket: socket,
218219
BinaryPath: v.pythonPath,
219-
SandboxPath: "",
220-
SandboxConfig: "",
220+
SandboxPath: v.installDir,
221+
SandboxConfig: sandbox.ConfigurationPython,
221222
Args: args,
222223
Logger: v.log,
223224
ServerLogWriter: logging.NewWriter(v.serverLog),

pkg/sandbox/sandbox_darwin.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,76 @@ import (
1010
"strings"
1111
)
1212

13+
// ConfigurationPython is the sandbox configuration for Python-based inference
14+
// backends (vLLM, vllm-metal, SGLang, MLX). It mirrors the llama.cpp profile
15+
// but additionally allows TCP loopback binding (used by vllm-metal and SGLang)
16+
// and read access to the Python runtime install directory supplied via
17+
// [UPDATEDBINPATH].
18+
const ConfigurationPython = `(version 1)
19+
20+
;;; Keep a default allow policy (because encoding things like DYLD support and
21+
;;; device access is quite difficult), but deny critical exploitation targets.
22+
;;; Python backends run with the same constraint philosophy as llama.cpp.
23+
(allow default)
24+
25+
;;; Deny network access except for our IPC sockets.
26+
;;; Python backends use either a Unix socket or a TCP loopback port.
27+
;;; Allow Unix socket paths that match the inference socket naming convention
28+
;;; as well as TCP loopback binding/inbound for backends that use TCP.
29+
(deny network*)
30+
(allow network-bind network-inbound
31+
(regex #"inference.*-[0-9]+\.sock$")
32+
(local tcp "localhost:*"))
33+
34+
;;; Deny access to the camera and microphone.
35+
(deny device*)
36+
37+
;;; Deny access to NVRAM settings.
38+
(deny nvram*)
39+
40+
;;; Deny access to system-level privileges.
41+
(deny system*)
42+
43+
;;; Deny access to job creation.
44+
(deny job-creation)
45+
46+
;;; Deny access to launchservicesd to prevent sandbox escape via open(1).
47+
(deny mach-lookup
48+
(global-name "com.apple.launchservicesd")
49+
(global-name "com.apple.coreservices.launchservicesd"))
50+
51+
;;; Don't allow new executable code to be created in memory at runtime.
52+
(deny dynamic-code-generation)
53+
54+
;;; Disable access to user preferences.
55+
(deny user-preference*)
56+
57+
;;; Restrict file access.
58+
(deny file-map-executable)
59+
(deny file-write*)
60+
(deny file-read*
61+
(subpath "/Applications")
62+
(subpath "/private/etc")
63+
(subpath "/Library")
64+
(subpath "/Users")
65+
(subpath "/Volumes"))
66+
(allow file-read* file-map-executable
67+
(subpath "/usr")
68+
(subpath "/System")
69+
(regex #"Docker\.app/Contents/Resources/model-runner")
70+
(subpath "[UPDATEDBINPATH]")
71+
(subpath "[UPDATEDLIBPATH]"))
72+
(allow file-write*
73+
(literal "/dev/null")
74+
(subpath "/private/var")
75+
(subpath "[HOMEDIR]/Library/Containers/com.docker.docker/Data")
76+
(subpath "[WORKDIR]"))
77+
(allow file-read*
78+
(subpath "[HOMEDIR]/.docker/models")
79+
(subpath "[HOMEDIR]/Library/Containers/com.docker.docker/Data")
80+
(subpath "[WORKDIR]"))
81+
`
82+
1383
// ConfigurationLlamaCpp is the sandbox configuration for llama.cpp processes.
1484
const ConfigurationLlamaCpp = `(version 1)
1585

pkg/sandbox/sandbox_other.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import (
88
"os/exec"
99
)
1010

11+
// ConfigurationPython is the sandbox configuration for Python-based inference
12+
// backends. On non-Darwin platforms no sandboxing is applied.
13+
const ConfigurationPython = ``
14+
1115
// ConfigurationLlamaCpp is the sandbox configuration for llama.cpp processes.
1216
const ConfigurationLlamaCpp = ``
1317

pkg/sandbox/sandbox_windows.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,20 @@ var limitTokenToGenerator = map[string]func() winjob.Limit{
2828
"(WithWriteClipboardLimit)": winjob.WithWriteClipboardLimit,
2929
}
3030

31+
// ConfigurationPython is the sandbox configuration for Python-based inference
32+
// backends (vLLM, SGLang, MLX) on Windows.
33+
const ConfigurationPython = `(WithDesktopLimit)
34+
(WithDieOnUnhandledException)
35+
(WithDisplaySettingsLimit)
36+
(WithExitWindowsLimit)
37+
(WithGlobalAtomsLimit)
38+
(WithHandlesLimit)
39+
(WithDisableOutgoingNetworking)
40+
(WithReadClipboardLimit)
41+
(WithSystemParametersLimit)
42+
(WithWriteClipboardLimit)
43+
`
44+
3145
// ConfigurationLlamaCpp is the sandbox configuration for llama.cpp processes.
3246
const ConfigurationLlamaCpp = `(WithDesktopLimit)
3347
(WithDieOnUnhandledException)

0 commit comments

Comments
 (0)