Skip to content

Commit cc1c1e9

Browse files
committed
fix: refuse a running-origin restore for every password-gated VNC display
The guard keyed on CNI, but a user, tap or bridge VM can be started with --vnc-password too, and the password is never persisted: its relaunch came back unauthenticated. The record now carries a non-secret vnc_password_set bit written at launch and cleared at stop, and restore refuses any running origin whose display is password-gated. Linux test drives the refusal against the fake qemu and checks it never terminates.
1 parent c6ea5c5 commit cc1c1e9

4 files changed

Lines changed: 29 additions & 3 deletions

File tree

cmd/vm/handler.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ type record struct {
3737
Storage int64 `json:"storage,omitempty"` // system-disk virtual size in bytes
3838
VNCDisp int `json:"vnc"`
3939
VNCPass string `json:"-"` // launch-scoped, set from the flag each start; never persisted (would leak at rest)
40+
VNCPassSet bool `json:"vnc_password_set,omitempty"`
4041
SSHPort int `json:"ssh_port"`
4142
NetMode string `json:"net_mode,omitempty"`
4243
Hugepages bool `json:"hugepages,omitempty"`

cmd/vm/lifecycle.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ func (h *Handler) Stop(cmd *cobra.Command, args []string) error {
118118
terminate(ctx, r, grace)
119119
quiesceNet(cmd, r)
120120
stopVNCProxy(ctx, dir)
121-
r.PID, r.VNCDisp, r.VNCPass = 0, -1, "" // VNC is launch-scoped: gone with the qemu it belonged to
121+
r.PID, r.VNCDisp, r.VNCPass, r.VNCPassSet = 0, -1, "", false // VNC is launch-scoped: gone with the qemu it belonged to
122122
return saveRec(dir, r)
123123
}); err != nil {
124124
return err
@@ -296,6 +296,7 @@ func (h *Handler) launch(cmd *cobra.Command, dir string, r *record) error {
296296
return fmt.Errorf("start vnc proxy: %w", err)
297297
}
298298
}
299+
r.VNCPassSet = r.VNCPass != ""
299300
return saveRec(dir, r)
300301
}
301302

cmd/vm/lifecycle_test.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,30 @@ func TestRMAdoptsQEMUWhenRecordPIDWasNotCommitted(t *testing.T) {
5151
}
5252
}
5353

54+
func TestRestoreRefusesRunningPasswordedVNC(t *testing.T) {
55+
stateDir := t.TempDir()
56+
vmDir, pid := startUnrecordedQEMU(t, stateDir, "macos-demo")
57+
r, err := loadRec(vmDir)
58+
if err != nil {
59+
t.Fatal(err)
60+
}
61+
r.VNCDisp, r.VNCPassSet = 7, true
62+
if err := saveRec(vmDir, r); err != nil {
63+
t.Fatal(err)
64+
}
65+
cmd := newLifecycleTestCommand(t, stateDir)
66+
cmd.Flags().String("tag", "", "")
67+
cmd.Flags().Bool("force", true, "")
68+
69+
err = NewHandler().Restore(cmd, []string{"macos-demo"})
70+
if err == nil || !strings.Contains(err.Error(), "password-gated VNC") {
71+
t.Fatalf("Restore error = %v, want the password-gated VNC refusal", err)
72+
}
73+
if !utils.VerifyProcessCmdline(pid, qemuBinary, filepath.Join(vmDir, "disk.qcow2")) {
74+
t.Error("qemu was terminated by a refused restore")
75+
}
76+
}
77+
5478
func TestCloneRejectsRunningSource(t *testing.T) {
5579
stateDir := t.TempDir()
5680
startUnrecordedQEMU(t, stateDir, "macos-src")

cmd/vm/snapshot.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ func (h *Handler) Restore(cmd *cobra.Command, args []string) error {
6767
if force, _ := cmd.Flags().GetBool("force"); !force {
6868
return fmt.Errorf("vm %q is running; stop it first or pass --force to stop+restore", r.Name)
6969
}
70-
if r.Netns != "" && r.VNCDisp >= 0 {
71-
return fmt.Errorf("vm %q serves VNC over CNI and a relaunch cannot carry its password; stop it, restore, then start --vnc %d --vnc-password", r.Name, r.VNCDisp)
70+
if r.VNCDisp >= 0 && (r.VNCPassSet || r.Netns != "") {
71+
return fmt.Errorf("vm %q serves a password-gated VNC display and a relaunch cannot carry the password; stop it, restore, then start --vnc %d --vnc-password", r.Name, r.VNCDisp)
7272
}
7373
terminate(ctx, r, stopGracePeriod)
7474
r.PID = 0

0 commit comments

Comments
 (0)