fix(vm): scope an unjailed Firecracker by its --api-sock
A Firecracker with --api-sock was ours on its id alone. Now the socket must be an absolute path below <state>/vm/<id>/ (cleaned, compared lexically; the resolved state path is accepted too), else it is someone else's. Tests: our dir, socket elsewhere, another id, a .. escape, a relative path, a missing value, a prefix sibling.
- commit
- b72d29f9e6b01e7bbb2740da6d6f2c6ab6fc2de6
- author
- brice · · 2026-10-12 22:15 UTC
- parent
- fd4fcdb
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +46 -9
M internal/sandbox/vm/proc.go +36 -6
| @@ -37,6 +37,7 @@ type vmmArgs struct { | |||
| 37 | 37 | id string | |
| 38 | 38 | markers map[string]bool | |
| 39 | 39 | chrootBase string // the jailer's --chroot-base-dir | |
| 40 | + | apiSock string // Firecracker's --api-sock value | |
| 40 | 41 | } | |
| 41 | 42 | ||
| 42 | 43 | func parseVMMArgs(args []string) vmmArgs { | |
| @@ -53,6 +54,9 @@ func parseVMMArgs(args []string) vmmArgs { | |||
| 53 | 54 | } | |
| 54 | 55 | case slices.Contains(VMMMarkers, args[i]): | |
| 55 | 56 | a.markers[args[i]] = true | |
| 57 | + | if args[i] == MarkerAPISock && i+1 < len(args) { | |
| 58 | + | a.apiSock = args[i+1] | |
| 59 | + | } | |
| 56 | 60 | } | |
| 57 | 61 | } | |
| 58 | 62 | return a | |
| @@ -73,17 +77,40 @@ func IDFromCmdline(args []string) string { | |||
| 73 | 77 | // (or have been started for) its own jailer chroot base, <state dir>/jail. | |
| 74 | 78 | type scope struct { | |
| 75 | 79 | jailBases []string // the base as configured, and with symlinks resolved | |
| 80 | + | vmBases []string // <state dir>/vm (unjailed VMs), likewise | |
| 76 | 81 | } | |
| 77 | 82 | ||
| 78 | 83 | func newScope(stateDir string) scope { | |
| 79 | - | base := jailBase(stateDir) | |
| 80 | - | sc := scope{jailBases: []string{base}} | |
| 81 | - | if r, err := filepath.EvalSymlinks(base); err == nil && r != base { | |
| 82 | - | sc.jailBases = append(sc.jailBases, r) | |
| 84 | + | sc := scope{} | |
| 85 | + | for _, b := range []struct { | |
| 86 | + | dir string | |
| 87 | + | to *[]string | |
| 88 | + | }{{jailBase(stateDir), &sc.jailBases}, {filepath.Join(stateDir, "vm"), &sc.vmBases}} { | |
| 89 | + | *b.to = []string{b.dir} | |
| 90 | + | if r, err := filepath.EvalSymlinks(b.dir); err == nil && r != b.dir { | |
| 91 | + | *b.to = append(*b.to, r) | |
| 92 | + | } | |
| 83 | 93 | } | |
| 84 | 94 | return sc | |
| 85 | 95 | } | |
| 86 | 96 | ||
| 97 | + | // apiSockIsOurs: an unjailed Firecracker's --api-sock must be a path below | |
| 98 | + | // <state dir>/vm/<id>/ (compared lexically, after cleaning), the directory the | |
| 99 | + | // driver puts it in. Another Firecracker with an id of our shape and a socket | |
| 100 | + | // elsewhere is someone else's. | |
| 101 | + | func (sc scope) apiSockIsOurs(sock, id string) bool { | |
| 102 | + | if sock == "" || !filepath.IsAbs(sock) { | |
| 103 | + | return false | |
| 104 | + | } | |
| 105 | + | sock = filepath.Clean(sock) | |
| 106 | + | for _, b := range sc.vmBases { | |
| 107 | + | if strings.HasPrefix(sock, filepath.Join(b, id)+"/") { | |
| 108 | + | return true | |
| 109 | + | } | |
| 110 | + | } | |
| 111 | + | return false | |
| 112 | + | } | |
| 113 | + | ||
| 87 | 114 | func (sc scope) isBase(p string) bool { | |
| 88 | 115 | return slices.Contains(sc.jailBases, filepath.Clean(p)) | |
| 89 | 116 | } | |
| @@ -105,7 +132,8 @@ func (sc scope) under(p string) bool { | |||
| 105 | 132 | // below our jail base (rootIsOurs). Anything else with that flag and an id | |
| 106 | 133 | // of our shape belongs to someone else's jailer and is never touched. | |
| 107 | 134 | // | |
| 108 | - | // A Firecracker started with --api-sock (no jailer) is ours by its id alone. | |
| 135 | + | // A Firecracker started with --api-sock (no jailer) is ours when the socket is | |
| 136 | + | // in <state dir>/vm/<id>/ (apiSockIsOurs). | |
| 109 | 137 | func (sc scope) match(dir string, args []string) string { | |
| 110 | 138 | a := parseVMMArgs(args) | |
| 111 | 139 | if a.id == "" || len(a.markers) == 0 { | |
| @@ -113,7 +141,9 @@ func (sc scope) match(dir string, args []string) string { | |||
| 113 | 141 | } | |
| 114 | 142 | switch { | |
| 115 | 143 | case a.markers[MarkerAPISock]: | |
| 116 | - | return a.id | |
| 144 | + | if sc.apiSockIsOurs(a.apiSock, a.id) { | |
| 145 | + | return a.id | |
| 146 | + | } | |
| 117 | 147 | case a.markers[MarkerExecFile]: | |
| 118 | 148 | if sc.isBase(a.chrootBase) { | |
| 119 | 149 | return a.id | |
| @@ -37,6 +37,7 @@ type vmmArgs struct { | |||||
| 37 | id string | 37 | id string | ||
| 38 | markers map[string]bool | 38 | markers map[string]bool | ||
| 39 | chrootBase string // the jailer's --chroot-base-dir | 39 | chrootBase string // the jailer's --chroot-base-dir | ||
| 40 | + | apiSock string // Firecracker's --api-sock value | |||
| 40 | } | 41 | } | ||
| 41 | 42 | ||||
| 42 | func parseVMMArgs(args []string) vmmArgs { | 43 | func parseVMMArgs(args []string) vmmArgs { | ||
| @@ -53,6 +54,9 @@ func parseVMMArgs(args []string) vmmArgs { | |||||
| 53 | } | 54 | } | ||
| 54 | case slices.Contains(VMMMarkers, args[i]): | 55 | case slices.Contains(VMMMarkers, args[i]): | ||
| 55 | a.markers[args[i]] = true | 56 | a.markers[args[i]] = true | ||
| 57 | + | if args[i] == MarkerAPISock && i+1 < len(args) { | |||
| 58 | + | a.apiSock = args[i+1] | |||
| 59 | + | } | |||
| 56 | } | 60 | } | ||
| 57 | } | 61 | } | ||
| 58 | return a | 62 | return a | ||
| @@ -73,17 +77,40 @@ func IDFromCmdline(args []string) string { | |||||
| 73 | // (or have been started for) its own jailer chroot base, <state dir>/jail. | 77 | // (or have been started for) its own jailer chroot base, <state dir>/jail. | ||
| 74 | type scope struct { | 78 | type scope struct { | ||
| 75 | jailBases []string // the base as configured, and with symlinks resolved | 79 | jailBases []string // the base as configured, and with symlinks resolved | ||
| 80 | + | vmBases []string // <state dir>/vm (unjailed VMs), likewise | |||
| 76 | } | 81 | } | ||
| 77 | 82 | ||||
| 78 | func newScope(stateDir string) scope { | 83 | func newScope(stateDir string) scope { | ||
| 79 | - | base := jailBase(stateDir) | 84 | + | sc := scope{} |
| 80 | - | sc := scope{jailBases: []string{base}} | 85 | + | for _, b := range []struct { |
| 81 | - | if r, err := filepath.EvalSymlinks(base); err == nil && r != base { | 86 | + | dir string |
| 82 | - | sc.jailBases = append(sc.jailBases, r) | 87 | + | to *[]string |
| 88 | + | }{{jailBase(stateDir), &sc.jailBases}, {filepath.Join(stateDir, "vm"), &sc.vmBases}} { | |||
| 89 | + | *b.to = []string{b.dir} | |||
| 90 | + | if r, err := filepath.EvalSymlinks(b.dir); err == nil && r != b.dir { | |||
| 91 | + | *b.to = append(*b.to, r) | |||
| 92 | + | } | |||
| 83 | } | 93 | } | ||
| 84 | return sc | 94 | return sc | ||
| 85 | } | 95 | } | ||
| 86 | 96 | ||||
| 97 | + | // apiSockIsOurs: an unjailed Firecracker's --api-sock must be a path below | |||
| 98 | + | // <state dir>/vm/<id>/ (compared lexically, after cleaning), the directory the | |||
| 99 | + | // driver puts it in. Another Firecracker with an id of our shape and a socket | |||
| 100 | + | // elsewhere is someone else's. | |||
| 101 | + | func (sc scope) apiSockIsOurs(sock, id string) bool { | |||
| 102 | + | if sock == "" || !filepath.IsAbs(sock) { | |||
| 103 | + | return false | |||
| 104 | + | } | |||
| 105 | + | sock = filepath.Clean(sock) | |||
| 106 | + | for _, b := range sc.vmBases { | |||
| 107 | + | if strings.HasPrefix(sock, filepath.Join(b, id)+"/") { | |||
| 108 | + | return true | |||
| 109 | + | } | |||
| 110 | + | } | |||
| 111 | + | return false | |||
| 112 | + | } | |||
| 113 | + | ||||
| 87 | func (sc scope) isBase(p string) bool { | 114 | func (sc scope) isBase(p string) bool { | ||
| 88 | return slices.Contains(sc.jailBases, filepath.Clean(p)) | 115 | return slices.Contains(sc.jailBases, filepath.Clean(p)) | ||
| 89 | } | 116 | } | ||
| @@ -105,7 +132,8 @@ func (sc scope) under(p string) bool { | |||||
| 105 | // below our jail base (rootIsOurs). Anything else with that flag and an id | 132 | // below our jail base (rootIsOurs). Anything else with that flag and an id | ||
| 106 | // of our shape belongs to someone else's jailer and is never touched. | 133 | // of our shape belongs to someone else's jailer and is never touched. | ||
| 107 | // | 134 | // | ||
| 108 | - | // A Firecracker started with --api-sock (no jailer) is ours by its id alone. | 135 | + | // A Firecracker started with --api-sock (no jailer) is ours when the socket is |
| 136 | + | // in <state dir>/vm/<id>/ (apiSockIsOurs). | |||
| 109 | func (sc scope) match(dir string, args []string) string { | 137 | func (sc scope) match(dir string, args []string) string { | ||
| 110 | a := parseVMMArgs(args) | 138 | a := parseVMMArgs(args) | ||
| 111 | if a.id == "" || len(a.markers) == 0 { | 139 | if a.id == "" || len(a.markers) == 0 { | ||
| @@ -113,7 +141,9 @@ func (sc scope) match(dir string, args []string) string { | |||||
| 113 | } | 141 | } | ||
| 114 | switch { | 142 | switch { | ||
| 115 | case a.markers[MarkerAPISock]: | 143 | case a.markers[MarkerAPISock]: | ||
| 116 | - | return a.id | 144 | + | if sc.apiSockIsOurs(a.apiSock, a.id) { |
| 145 | + | return a.id | |||
| 146 | + | } | |||
| 117 | case a.markers[MarkerExecFile]: | 147 | case a.markers[MarkerExecFile]: | ||
| 118 | if sc.isBase(a.chrootBase) { | 148 | if sc.isBase(a.chrootBase) { | ||
| 119 | return a.id | 149 | return a.id | ||
M internal/sandbox/vm/vm_test.go +10 -3
| @@ -652,8 +652,8 @@ func TestFirecrackerProcsScan(t *testing.T) { | |||
| 652 | 652 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | |
| 653 | 653 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | |
| 654 | 654 | } | |
| 655 | - | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/x\x00") | |
| 656 | - | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/x\x00--id\x00er44556677\x00") | |
| 655 | + | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/var/lib/er/vm/er00112233/api.sock\x00") | |
| 656 | + | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/var/lib/er/vm/er44556677/api.sock\x00--id\x00er44556677\x00") | |
| 657 | 657 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | |
| 658 | 658 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | |
| 659 | 659 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | |
| @@ -1210,7 +1210,14 @@ func TestScanIsScopedToOurJailBase(t *testing.T) { | |||
| 1210 | 1210 | mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail")) | |
| 1211 | 1211 | mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc") | |
| 1212 | 1212 | mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape | |
| 1213 | - | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", "/x") // unjailed: by id | |
| 1213 | + | vmDir := filepath.Join(state, "vm", "er66666666") | |
| 1214 | + | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", filepath.Join(vmDir, "api.sock")) // unjailed, in our dir | |
| 1215 | + | mk("111", "/", "firecracker", "--id", "er99999999", "--api-sock", "/x") // socket elsewhere | |
| 1216 | + | mk("112", "/", "firecracker", "--id", "erabababab", "--api-sock", filepath.Join(vmDir, "api.sock")) // our dir, another id | |
| 1217 | + | mk("113", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"/../../x/api.sock") // escapes by .. | |
| 1218 | + | mk("114", "/", "firecracker", "--id", "er66666666", "--api-sock", "vm/er66666666/api.sock") // relative | |
| 1219 | + | mk("115", "/", "firecracker", "--id", "er66666666", "--api-sock") // no value | |
| 1220 | + | mk("116", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"x/api.sock") // prefix sibling | |
| 1214 | 1221 | got := vmmProcs(proc, sc) | |
| 1215 | 1222 | want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666", 109: "er00112233"} | |
| 1216 | 1223 | if len(got) != len(want) { | |
| @@ -652,8 +652,8 @@ func TestFirecrackerProcsScan(t *testing.T) { | |||||
| 652 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | 652 | _ = os.MkdirAll(filepath.Join(proc, pid), 0o755) | ||
| 653 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | 653 | _ = os.WriteFile(filepath.Join(proc, pid, "cmdline"), []byte(cmdline), 0o644) | ||
| 654 | } | 654 | } | ||
| 655 | - | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/x\x00") | 655 | + | mk("100", "firecracker\x00--id\x00er00112233\x00--api-sock\x00/var/lib/er/vm/er00112233/api.sock\x00") |
| 656 | - | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/x\x00--id\x00er44556677\x00") | 656 | + | mk("101", "/usr/bin/firecracker\x00--api-sock\x00/var/lib/er/vm/er44556677/api.sock\x00--id\x00er44556677\x00") |
| 657 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | 657 | mk("102", "firecracker\x00--id\x00other-tool-vm\x00") // someone else's VM | ||
| 658 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | 658 | mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker | ||
| 659 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | 659 | mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched | ||
| @@ -1210,7 +1210,14 @@ func TestScanIsScopedToOurJailBase(t *testing.T) { | |||||
| 1210 | mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail")) | 1210 | mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail")) | ||
| 1211 | mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc") | 1211 | mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc") | ||
| 1212 | mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape | 1212 | mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape | ||
| 1213 | - | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", "/x") // unjailed: by id | 1213 | + | vmDir := filepath.Join(state, "vm", "er66666666") |
| 1214 | + | mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", filepath.Join(vmDir, "api.sock")) // unjailed, in our dir | |||
| 1215 | + | mk("111", "/", "firecracker", "--id", "er99999999", "--api-sock", "/x") // socket elsewhere | |||
| 1216 | + | mk("112", "/", "firecracker", "--id", "erabababab", "--api-sock", filepath.Join(vmDir, "api.sock")) // our dir, another id | |||
| 1217 | + | mk("113", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"/../../x/api.sock") // escapes by .. | |||
| 1218 | + | mk("114", "/", "firecracker", "--id", "er66666666", "--api-sock", "vm/er66666666/api.sock") // relative | |||
| 1219 | + | mk("115", "/", "firecracker", "--id", "er66666666", "--api-sock") // no value | |||
| 1220 | + | mk("116", "/", "firecracker", "--id", "er66666666", "--api-sock", vmDir+"x/api.sock") // prefix sibling | |||
| 1214 | got := vmmProcs(proc, sc) | 1221 | got := vmmProcs(proc, sc) | ||
| 1215 | want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666", 109: "er00112233"} | 1222 | want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666", 109: "er00112233"} | ||
| 1216 | if len(got) != len(want) { | 1223 | if len(got) != len(want) { | ||