Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/vm/clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func (h *Handler) Clone(cmd *cobra.Command, args []string) error {
netMode, _ := cmd.Flags().GetString("net")
vnc, _ := cmd.Flags().GetInt("vnc")
vncPass, _ := cmd.Flags().GetString("vnc-password")
if netMode == netCNI && vnc >= 0 && vncPass == "" { // same pre-scaffold check as create
return errCNIVNCPassRequired
if err = requireCNIVNCPassword(netMode == netCNI, vnc, vncPass); err != nil { // same pre-scaffold check as create
return err
}
// the clone inherits SRC's data disks by name; extra --data-disk specs must not collide with them
// and the combined count still honors the AHCI cap. Parse before scaffolding to fail fast.
Expand Down
2 changes: 1 addition & 1 deletion cmd/vm/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type record struct {
CPUs int `json:"cpus"`
Memory string `json:"memory"`
VNCDisp int `json:"vnc"`
VNCPass string `json:"vnc_password,omitempty"`
VNCPass string `json:"-"` // launch-scoped, set from the flag each start; never persisted (would leak at rest)
SSHPort int `json:"ssh_port"`
NetMode string `json:"net_mode,omitempty"`
Hugepages bool `json:"hugepages,omitempty"`
Expand Down
3 changes: 2 additions & 1 deletion cmd/vm/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,8 @@ func setVNCPassword(ctx context.Context, monSock, pw string) error {
_ = conn.SetReadDeadline(time.Now().Add(5 * time.Second))
out, _ := readUntil(conn, "(qemu)")
if strings.Contains(out, "Could not") {
return fmt.Errorf("qemu: %s", strings.TrimSpace(out))
// out echoes the typed "set_password vnc <pw>" line — never surface it.
return errors.New("qemu rejected set_password (vnc display not active?)")
}
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/vm/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ func (h *Handler) create(cmd *cobra.Command, image string) (*record, error) {
vnc, _ := cmd.Flags().GetInt("vnc")
vncPass, _ := cmd.Flags().GetString("vnc-password")
netMode, _ := cmd.Flags().GetString("net")
if netMode == netCNI && vnc >= 0 && vncPass == "" { // fail before scaffolding leaves a half-made VM
return nil, errCNIVNCPassRequired
if err = requireCNIVNCPassword(netMode == netCNI, vnc, vncPass); err != nil { // fail before scaffolding leaves a half-made VM
return nil, err
}
oc, code, varsTmpl, err := resolveFirmware(cmd)
if err != nil {
Expand Down Expand Up @@ -182,8 +182,8 @@ func (h *Handler) create(cmd *cobra.Command, image string) (*record, error) {
func (h *Handler) launch(cmd *cobra.Command, dir string, r *record) error {
ctx := cliutil.CommandContext(cmd)
logger := log.WithFunc("cmd.vm.launch")
if r.Netns != "" && r.VNCDisp >= 0 && r.VNCPass == "" {
return errCNIVNCPassRequired
if err := requireCNIVNCPassword(r.Netns != "", r.VNCDisp, r.VNCPass); err != nil {
return err
}
if hostIsAMD() {
// macOS reads MSRs an AMD host lacks; without kvm.ignore_msrs KVM injects #GP. Best-effort,
Expand Down
16 changes: 15 additions & 1 deletion cmd/vm/vnc.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ const (
// never expose that unauthenticated.
var errCNIVNCPassRequired = errors.New("--vnc with --net cni serves VNC on a host port reachable off-box; --vnc-password is required")

// requireCNIVNCPassword rejects an unauthenticated VNC display on a CNI VM whose
// host port is reachable off-box. isCNI is the flag intent at create/clone
// (pre-scaffold) or the resolved Netns at launch (post-scaffold).
func requireCNIVNCPassword(isCNI bool, vncDisp int, vncPass string) error {
if isCNI && vncDisp >= 0 && vncPass == "" {
return errCNIVNCPassRequired
}
return nil
}

// vncProxyCommand is the hidden re-exec target that runs the forwarder for its lifetime, accepting
// on the TCP listener inherited as fd 3 (bound by the parent so bind errors fail the launch).
func vncProxyCommand() *cobra.Command {
Expand Down Expand Up @@ -72,7 +82,11 @@ func startVNCProxy(ctx context.Context, dir string, disp int) error {
if err := c.Start(); err != nil {
return err
}
return utils.WritePIDFile(filepath.Join(dir, vncProxyPID), c.Process.Pid)
if err := utils.WritePIDFile(filepath.Join(dir, vncProxyPID), c.Process.Pid); err != nil {
_ = c.Process.Kill() // no pidfile -> stopVNCProxy can't reap it; don't orphan the proxy
return err
}
return nil
}

// stopVNCProxy kills a running proxy (best-effort) and removes its pidfile. Zero grace: the CLI's
Expand Down
32 changes: 32 additions & 0 deletions cmd/vm/vnc_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package vm

import (
"errors"
"testing"
)

func TestRequireCNIVNCPassword(t *testing.T) {
tests := []struct {
name string
isCNI bool
vncDisp int
vncPass string
wantErr bool
}{
{"cni vnc no password rejected", true, 0, "", true},
{"cni vnc with password ok", true, 0, "s3cret", false},
{"cni vnc disabled ok", true, -1, "", false},
{"non-cni vnc no password ok", false, 0, "", false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := requireCNIVNCPassword(tt.isCNI, tt.vncDisp, tt.vncPass)
if tt.wantErr && !errors.Is(err, errCNIVNCPassRequired) {
t.Errorf("got %v, want errCNIVNCPassRequired", err)
}
if !tt.wantErr && err != nil {
t.Errorf("got %v, want nil", err)
}
})
}
}
Loading