fix(vm): check every socket path against sun_path, in both jailer modes
The check only covered the jailed API socket. Without the jailer the API socket (<state>/vm/<id>/api.sock) and in both modes the vsock listener (<uds>_1024) can be the longest path. Check all of them at driver start. Boundary tests: 107 bytes accepted, 108 rejected, jailed and not.
- commit
- bbd315bd8db3729f8d05107e29955902d1979762
- author
- brice · · 2026-10-12 22:02 UTC
- parent
- 4f13c7a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +71 -13
M internal/sandbox/vm/driver.go +28 -5
| @@ -120,17 +120,17 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||
| 120 | 120 | } | |
| 121 | 121 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | |
| 122 | 122 | ||
| 123 | + | // A path over sun_path fails later as a 10 s "API not ready" (or a vsock | |
| 124 | + | // that never connects): refuse at start, in both modes. | |
| 125 | + | if err := checkSocketPaths(cfg.StateDir, filepath.Base(d.fcPath), !cfg.JailerOff); err != nil { | |
| 126 | + | return fail(err) | |
| 127 | + | } | |
| 123 | 128 | if !cfg.JailerOff { | |
| 124 | 129 | // Jailer 1.17 canonicalizes --chroot-base-dir and fails when it does not | |
| 125 | 130 | // exist ("Failed to canonicalize path"); older releases created it. | |
| 126 | 131 | if err := os.MkdirAll(jailBase(cfg.StateDir), 0o700); err != nil { | |
| 127 | 132 | return fail(fmt.Errorf("jailer chroot base: %w", err)) | |
| 128 | 133 | } | |
| 129 | - | // The API socket lives deep in the jail; a path over sun_path fails | |
| 130 | - | // as a 10 s "API not ready" timeout otherwise. | |
| 131 | - | if sock := newLayout(cfg.StateDir, filepath.Base(d.fcPath), "er00000000", true, 0).host("run/firecracker.socket"); len(sock) > maxSockPath { | |
| 132 | - | return fail(fmt.Errorf("firecracker API socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", sock, len(sock), maxSockPath, sandbox.EnvStateDir)) | |
| 133 | - | } | |
| 134 | 134 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | |
| 135 | 135 | if err := checkWorldReadable(p); err != nil { | |
| 136 | 136 | return fail(err) | |
| @@ -406,6 +406,29 @@ const udsName = "v.sock" | |||
| 406 | 406 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | |
| 407 | 407 | const maxSockPath = 107 | |
| 408 | 408 | ||
| 409 | + | // socketPaths lists the host paths of the unix sockets of a VM: the API socket | |
| 410 | + | // and the vsock listener (the guest's UDS path plus "_<port>"). With the jailer | |
| 411 | + | // they are deep in the chroot; without it they sit in <state>/vm/<id>. | |
| 412 | + | func socketPaths(stateDir, execName string, jailed bool) []string { | |
| 413 | + | lay := newLayout(stateDir, execName, "er00000000", jailed, 0) | |
| 414 | + | api := lay.host("api.sock") | |
| 415 | + | if jailed { | |
| 416 | + | api = lay.host("run/firecracker.socket") | |
| 417 | + | } | |
| 418 | + | return []string{api, lay.host(udsName) + fmt.Sprintf("_%d", sandbox.VsockPort)} | |
| 419 | + | } | |
| 420 | + | ||
| 421 | + | // checkSocketPaths refuses a state directory that makes a VM socket path longer | |
| 422 | + | // than a unix socket address. | |
| 423 | + | func checkSocketPaths(stateDir, execName string, jailed bool) error { | |
| 424 | + | for _, p := range socketPaths(stateDir, execName, jailed) { | |
| 425 | + | if len(p) > maxSockPath { | |
| 426 | + | return fmt.Errorf("socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", p, len(p), maxSockPath, sandbox.EnvStateDir) | |
| 427 | + | } | |
| 428 | + | } | |
| 429 | + | return nil | |
| 430 | + | } | |
| 431 | + | ||
| 409 | 432 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | |
| 410 | 433 | cfg := s.d.cfg | |
| 411 | 434 | return vmConfig{ | |
| @@ -120,17 +120,17 @@ func New(ctx context.Context, cfg sandbox.Config, opt Options) (*Driver, error) | |||||
| 120 | } | 120 | } | ||
| 121 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | 121 | fail := func(err error) (*Driver, error) { _ = d.Close(); return nil, err } | ||
| 122 | 122 | ||||
| 123 | + | // A path over sun_path fails later as a 10 s "API not ready" (or a vsock | |||
| 124 | + | // that never connects): refuse at start, in both modes. | |||
| 125 | + | if err := checkSocketPaths(cfg.StateDir, filepath.Base(d.fcPath), !cfg.JailerOff); err != nil { | |||
| 126 | + | return fail(err) | |||
| 127 | + | } | |||
| 123 | if !cfg.JailerOff { | 128 | if !cfg.JailerOff { | ||
| 124 | // Jailer 1.17 canonicalizes --chroot-base-dir and fails when it does not | 129 | // Jailer 1.17 canonicalizes --chroot-base-dir and fails when it does not | ||
| 125 | // exist ("Failed to canonicalize path"); older releases created it. | 130 | // exist ("Failed to canonicalize path"); older releases created it. | ||
| 126 | if err := os.MkdirAll(jailBase(cfg.StateDir), 0o700); err != nil { | 131 | if err := os.MkdirAll(jailBase(cfg.StateDir), 0o700); err != nil { | ||
| 127 | return fail(fmt.Errorf("jailer chroot base: %w", err)) | 132 | return fail(fmt.Errorf("jailer chroot base: %w", err)) | ||
| 128 | } | 133 | } | ||
| 129 | - | // The API socket lives deep in the jail; a path over sun_path fails | |||
| 130 | - | // as a 10 s "API not ready" timeout otherwise. | |||
| 131 | - | if sock := newLayout(cfg.StateDir, filepath.Base(d.fcPath), "er00000000", true, 0).host("run/firecracker.socket"); len(sock) > maxSockPath { | |||
| 132 | - | return fail(fmt.Errorf("firecracker API socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", sock, len(sock), maxSockPath, sandbox.EnvStateDir)) | |||
| 133 | - | } | |||
| 134 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | 134 | for _, p := range []string{cfg.KernelPath(), cfg.BaseImagePath()} { | ||
| 135 | if err := checkWorldReadable(p); err != nil { | 135 | if err := checkWorldReadable(p); err != nil { | ||
| 136 | return fail(err) | 136 | return fail(err) | ||
| @@ -406,6 +406,29 @@ const udsName = "v.sock" | |||||
| 406 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | 406 | // maxSockPath is the longest Unix socket path Linux accepts (108 with the NUL). | ||
| 407 | const maxSockPath = 107 | 407 | const maxSockPath = 107 | ||
| 408 | 408 | ||||
| 409 | + | // socketPaths lists the host paths of the unix sockets of a VM: the API socket | |||
| 410 | + | // and the vsock listener (the guest's UDS path plus "_<port>"). With the jailer | |||
| 411 | + | // they are deep in the chroot; without it they sit in <state>/vm/<id>. | |||
| 412 | + | func socketPaths(stateDir, execName string, jailed bool) []string { | |||
| 413 | + | lay := newLayout(stateDir, execName, "er00000000", jailed, 0) | |||
| 414 | + | api := lay.host("api.sock") | |||
| 415 | + | if jailed { | |||
| 416 | + | api = lay.host("run/firecracker.socket") | |||
| 417 | + | } | |||
| 418 | + | return []string{api, lay.host(udsName) + fmt.Sprintf("_%d", sandbox.VsockPort)} | |||
| 419 | + | } | |||
| 420 | + | ||||
| 421 | + | // checkSocketPaths refuses a state directory that makes a VM socket path longer | |||
| 422 | + | // than a unix socket address. | |||
| 423 | + | func checkSocketPaths(stateDir, execName string, jailed bool) error { | |||
| 424 | + | for _, p := range socketPaths(stateDir, execName, jailed) { | |||
| 425 | + | if len(p) > maxSockPath { | |||
| 426 | + | return fmt.Errorf("socket path %q is %d bytes, over the %d-byte limit of a Unix socket: use a shorter %s", p, len(p), maxSockPath, sandbox.EnvStateDir) | |||
| 427 | + | } | |||
| 428 | + | } | |||
| 429 | + | return nil | |||
| 430 | + | } | |||
| 431 | + | ||||
| 409 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | 432 | func (s *vmSandbox) vmConfig(spec sandbox.JobSpec, netw netpolicy.Net, f vmFiles) vmConfig { | ||
| 410 | cfg := s.d.cfg | 433 | cfg := s.d.cfg | ||
| 411 | return vmConfig{ | 434 | return vmConfig{ | ||
M internal/sandbox/vm/vm_test.go +43 -8
| @@ -1050,14 +1050,49 @@ func TestNewCreatesTheJailerChrootBase(t *testing.T) { | |||
| 1050 | 1050 | } | |
| 1051 | 1051 | } | |
| 1052 | 1052 | ||
| 1053 | - | func TestNewRejectsAStateDirThatMakesTheAPISocketPathTooLong(t *testing.T) { | |
| 1054 | - | e := newEnv(t, "ok") | |
| 1055 | - | e.cfg.JailerOff = false | |
| 1056 | - | e.cfg.Jailer = e.cfg.Firecracker | |
| 1057 | - | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | |
| 1058 | - | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |
| 1059 | - | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |
| 1060 | - | t.Fatalf("want a clear socket path error, got %v", err) | |
| 1053 | + | // GO-004: the longest socket path of a VM, in both jailer modes, 107 bytes ok / | |
| 1054 | + | // 108 rejected. | |
| 1055 | + | func TestSocketPathBoundaryInBothModes(t *testing.T) { | |
| 1056 | + | for _, jailed := range []bool{true, false} { | |
| 1057 | + | name := map[bool]string{true: "jailed", false: "unjailed"}[jailed] | |
| 1058 | + | // Find the longest socket path with an empty-ish state dir, then pad the | |
| 1059 | + | // state dir so that it is exactly maxSockPath, then one more. | |
| 1060 | + | longest := func(state string) int { | |
| 1061 | + | n := 0 | |
| 1062 | + | for _, p := range socketPaths(state, "firecracker", jailed) { | |
| 1063 | + | n = max(n, len(p)) | |
| 1064 | + | } | |
| 1065 | + | return n | |
| 1066 | + | } | |
| 1067 | + | base := "/s" | |
| 1068 | + | pad := maxSockPath - longest(base) | |
| 1069 | + | if pad < 0 { | |
| 1070 | + | t.Fatalf("%s: base already too long", name) | |
| 1071 | + | } | |
| 1072 | + | ok := base + strings.Repeat("d", pad) | |
| 1073 | + | if longest(ok) != maxSockPath { | |
| 1074 | + | t.Fatalf("%s: longest=%d want %d", name, longest(ok), maxSockPath) | |
| 1075 | + | } | |
| 1076 | + | if err := checkSocketPaths(ok, "firecracker", jailed); err != nil { | |
| 1077 | + | t.Errorf("%s: %d bytes rejected: %v", name, maxSockPath, err) | |
| 1078 | + | } | |
| 1079 | + | err := checkSocketPaths(ok+"d", "firecracker", jailed) | |
| 1080 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |
| 1081 | + | t.Errorf("%s: %d bytes accepted: %v", name, maxSockPath+1, err) | |
| 1082 | + | } | |
| 1083 | + | } | |
| 1084 | + | } | |
| 1085 | + | ||
| 1086 | + | func TestNewRejectsAStateDirThatMakesASocketPathTooLong(t *testing.T) { | |
| 1087 | + | for _, jailerOff := range []bool{false, true} { | |
| 1088 | + | e := newEnv(t, "ok") | |
| 1089 | + | e.cfg.JailerOff = jailerOff | |
| 1090 | + | e.cfg.Jailer = e.cfg.Firecracker | |
| 1091 | + | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | |
| 1092 | + | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |
| 1093 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |
| 1094 | + | t.Fatalf("jailerOff=%v: want a clear socket path error, got %v", jailerOff, err) | |
| 1095 | + | } | |
| 1061 | 1096 | } | |
| 1062 | 1097 | } | |
| 1063 | 1098 | ||
| @@ -1050,14 +1050,49 @@ func TestNewCreatesTheJailerChrootBase(t *testing.T) { | |||||
| 1050 | } | 1050 | } | ||
| 1051 | } | 1051 | } | ||
| 1052 | 1052 | ||||
| 1053 | - | func TestNewRejectsAStateDirThatMakesTheAPISocketPathTooLong(t *testing.T) { | 1053 | + | // GO-004: the longest socket path of a VM, in both jailer modes, 107 bytes ok / |
| 1054 | - | e := newEnv(t, "ok") | 1054 | + | // 108 rejected. |
| 1055 | - | e.cfg.JailerOff = false | 1055 | + | func TestSocketPathBoundaryInBothModes(t *testing.T) { |
| 1056 | - | e.cfg.Jailer = e.cfg.Firecracker | 1056 | + | for _, jailed := range []bool{true, false} { |
| 1057 | - | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | 1057 | + | name := map[bool]string{true: "jailed", false: "unjailed"}[jailed] |
| 1058 | - | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | 1058 | + | // Find the longest socket path with an empty-ish state dir, then pad the |
| 1059 | - | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | 1059 | + | // state dir so that it is exactly maxSockPath, then one more. |
| 1060 | - | t.Fatalf("want a clear socket path error, got %v", err) | 1060 | + | longest := func(state string) int { |
| 1061 | + | n := 0 | |||
| 1062 | + | for _, p := range socketPaths(state, "firecracker", jailed) { | |||
| 1063 | + | n = max(n, len(p)) | |||
| 1064 | + | } | |||
| 1065 | + | return n | |||
| 1066 | + | } | |||
| 1067 | + | base := "/s" | |||
| 1068 | + | pad := maxSockPath - longest(base) | |||
| 1069 | + | if pad < 0 { | |||
| 1070 | + | t.Fatalf("%s: base already too long", name) | |||
| 1071 | + | } | |||
| 1072 | + | ok := base + strings.Repeat("d", pad) | |||
| 1073 | + | if longest(ok) != maxSockPath { | |||
| 1074 | + | t.Fatalf("%s: longest=%d want %d", name, longest(ok), maxSockPath) | |||
| 1075 | + | } | |||
| 1076 | + | if err := checkSocketPaths(ok, "firecracker", jailed); err != nil { | |||
| 1077 | + | t.Errorf("%s: %d bytes rejected: %v", name, maxSockPath, err) | |||
| 1078 | + | } | |||
| 1079 | + | err := checkSocketPaths(ok+"d", "firecracker", jailed) | |||
| 1080 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |||
| 1081 | + | t.Errorf("%s: %d bytes accepted: %v", name, maxSockPath+1, err) | |||
| 1082 | + | } | |||
| 1083 | + | } | |||
| 1084 | + | } | |||
| 1085 | + | ||||
| 1086 | + | func TestNewRejectsAStateDirThatMakesASocketPathTooLong(t *testing.T) { | |||
| 1087 | + | for _, jailerOff := range []bool{false, true} { | |||
| 1088 | + | e := newEnv(t, "ok") | |||
| 1089 | + | e.cfg.JailerOff = jailerOff | |||
| 1090 | + | e.cfg.Jailer = e.cfg.Firecracker | |||
| 1091 | + | e.cfg.StateDir = filepath.Join(e.cfg.StateDir, strings.Repeat("d", 60)) | |||
| 1092 | + | _, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second}) | |||
| 1093 | + | if err == nil || !strings.Contains(err.Error(), "limit of a Unix socket") { | |||
| 1094 | + | t.Fatalf("jailerOff=%v: want a clear socket path error, got %v", jailerOff, err) | |||
| 1095 | + | } | |||
| 1061 | } | 1096 | } | ||
| 1062 | } | 1097 | } | ||
| 1063 | 1098 | ||||