runtime/qemu: Rework QMP/HMP support

PR #6146 added the possibility to control QEMU with an extra HMP socket
as an aid for debugging. This is great for development or bug chasing
but this raises some concerns in production.

The HMP monitor allows to temper with the VM state in a variety of ways.
This could be intentionally or mistakenly used to inject subtle bugs in
the VM that would be extremely hard if not even impossible to debug. We
definitely don't want that to be enabled by default.

The feature is currently wired to the `enable_debug` setting in the
`[hypervisor.qemu]` section of the configuration file. This setting has
historically been used to control "debug output" and it is used as such
by some downstream users (e.g. Openshift). Forcing people to have the
extra HMP backdoor at the same time is abusive and dangerous.

A new `extra_monitor_socket` is added to `[hypervisor.qemu]` to give
fine control on whether the HMP socket is wanted or not. This setting
is still gated by `enable_debug = true` to make it clear it is for
debug only. The default is to not have the HMP socket though. This
isn't backward compatible with #6416 but it is for the sake of "better
safe than sorry".

An extra monitor socket makes the QEMU instance untrusted. A warning is
thus logged to the journal when one is requested.

While here, also allow the user to choose between HMP and QMP for the
extra monitor socket. Motivation is that QMP offers way more options to
control or introspect the VM than HMP does. Users can also ask for
pretty json formatting well suited for human reading. This will improve
the debugging experience.

This feature is only made visible in the base and GPU configurations
of QEMU for now.

Fixes #7952

Signed-off-by: Greg Kurz <groug@kaod.org>

src/runtime/config/configuration-qemu-nvidia-gpu.toml.in

@@ -323,11 +323,26 @@ valid_file_mem_backends = @DEFVALIDFILEMEMBACKENDS@
 pflashes = []
 
 # This option changes the default hypervisor and kernel parameters
-# to enable debug output where available. And Debug also enable the hmp socket.
+# to enable debug output where available.
 #
 # Default false
 #enable_debug = true
 
+# This option allows to add an extra HMP or QMP socket when `enable_debug = true`
+#
+# WARNING: Anyone with access to the extra socket can take full control of
+# Qemu. This is for debugging purpose only and must *NEVER* be used in
+# production.
+#
+# Valid values are :
+# - "hmp"
+# - "qmp"
+# - "qmp-pretty" (same as "qmp" with pretty json formatting)
+#
+# If set to the empty string "", no extra monitor socket is added. This is
+# the default.
+#extra_monitor_socket = hmp
+
 # Disable the customizations done in the runtime when it detects
 # that it is running on top a VMM. This will result in the runtime
 # behaving as it would when running on bare metal.

src/runtime/config/configuration-qemu.toml.in

@@ -323,11 +323,26 @@ valid_file_mem_backends = @DEFVALIDFILEMEMBACKENDS@
 pflashes = []
 
 # This option changes the default hypervisor and kernel parameters
-# to enable debug output where available. And Debug also enable the hmp socket.
+# to enable debug output where available.
 #
 # Default false
 #enable_debug = true
 
+# This option allows to add an extra HMP or QMP socket when `enable_debug = true`
+#
+# WARNING: Anyone with access to the extra socket can take full control of
+# Qemu. This is for debugging purpose only and must *NEVER* be used in
+# production.
+#
+# Valid values are :
+# - "hmp"
+# - "qmp"
+# - "qmp-pretty" (same as "qmp" with pretty json formatting)
+#
+# If set to the empty string "", no extra monitor socket is added. This is
+# the default.
+#extra_monitor_socket = hmp
+
 # Disable the customizations done in the runtime when it detects
 # that it is running on top a VMM. This will result in the runtime
 # behaving as it would when running on bare metal.

src/runtime/pkg/govmm/qemu/qemu.go

@@ -2471,14 +2471,28 @@ const (
 	Unix QMPSocketType = "unix"
 )
 
-// QMPSocket represents a qemu QMP socket configuration.
+// MonitorProtocol tells what protocol is used on a QMPSocket
+type MonitorProtocol string
+
+const (
+	// Socket using a human-friendly text-based protocol.
+	Hmp MonitorProtocol = "hmp"
+
+	// Socket using a richer json-based protocol.
+	Qmp MonitorProtocol = "qmp"
+
+	// Same as Qmp with pretty json formatting.
+	QmpPretty MonitorProtocol = "qmp-pretty"
+)
+
+// QMPSocket represents a qemu QMP or HMP socket configuration.
 // nolint: govet
 type QMPSocket struct {
 	// Type is the socket type (e.g. "unix").
 	Type QMPSocketType
 
-	// Human Monitor Interface (HMP) (true for HMP, false for QMP, default false)
+	// Protocol is the protocol to be used on the socket.
-	IsHmp bool
+	Protocol MonitorProtocol
 
 	// QMP listener file descriptor to be passed to qemu
 	FD *os.File
@@ -2504,6 +2518,10 @@ func (qmp QMPSocket) Valid() bool {
 		return false
 	}
 
+	if qmp.Protocol != Hmp && qmp.Protocol != Qmp && qmp.Protocol != QmpPretty {
+		return false
+	}
+
 	return true
 }
 
@@ -2855,10 +2873,11 @@ func (config *Config) appendQMPSockets() {
 			}
 		}
 
-		if q.IsHmp {
+		switch q.Protocol {
+		case Hmp:
 			config.qemuParams = append(config.qemuParams, "-monitor")
-		} else {
+		default:
-			config.qemuParams = append(config.qemuParams, "-qmp")
+			config.qemuParams = append(config.qemuParams, fmt.Sprintf("-%s", q.Protocol))
 		}
 
 		config.qemuParams = append(config.qemuParams, strings.Join(qmpParams, ","))

src/runtime/pkg/govmm/qemu/qemu_test.go

@@ -730,6 +730,7 @@ func TestAppendSingleQMPSocketServer(t *testing.T) {
 		Name:     "cc-qmp",
 		Server:   true,
 		NoWait:   true,
+		Protocol: Qmp,
 	}
 
 	testAppend(qmp, qmpSingleSocketServerString, t)
@@ -740,6 +741,7 @@ func TestAppendSingleQMPSocket(t *testing.T) {
 		Type:     Unix,
 		Name:     "cc-qmp",
 		Server:   false,
+		Protocol: Qmp,
 	}
 
 	testAppend(qmp, qmpSingleSocketString, t)
@@ -760,6 +762,7 @@ func TestAppendQMPSocketServerFd(t *testing.T) {
 		FD:       foo,
 		Server:   true,
 		NoWait:   true,
+		Protocol: Qmp,
 	}
 
 	testAppend(qmp, qmpSocketServerFdString, t)
@@ -774,12 +777,14 @@ func TestAppendQMPSocketServer(t *testing.T) {
 			Name:     "cc-qmp-1",
 			Server:   true,
 			NoWait:   true,
+			Protocol: Qmp,
 		},
 		{
 			Type:     "unix",
 			Name:     "cc-qmp-2",
 			Server:   true,
 			NoWait:   true,
+			Protocol: Qmp,
 		},
 	}
 

src/runtime/pkg/katautils/config-settings.go.in

@@ -11,6 +11,7 @@ package katautils
 
 import (
 	config "github.com/kata-containers/kata-containers/src/runtime/pkg/device/config"
+	govmmQemu "github.com/kata-containers/kata-containers/src/runtime/pkg/govmm/qemu"
 )
 
 // name is the name of the runtime
@@ -79,6 +80,7 @@ const defaultEnableIOMMU bool = false
 const defaultEnableIOMMUPlatform bool = false
 const defaultFileBackedMemRootDir string = ""
 const defaultEnableDebug bool = false
+const defaultExtraMonitorSocket govmmQemu.MonitorProtocol = ""
 const defaultDisableNestingChecks bool = false
 const defaultMsize9p uint32 = 8192
 const defaultEntropySource = "/dev/urandom"

src/runtime/pkg/katautils/config.go

@@ -159,6 +159,7 @@ type hypervisor struct {
 	DisableSeLinux                 bool                      `toml:"disable_selinux"`
 	DisableGuestSeLinux            bool                      `toml:"disable_guest_selinux"`
 	LegacySerial                   bool                      `toml:"use_legacy_serial"`
+	ExtraMonitorSocket             govmmQemu.MonitorProtocol `toml:"extra_monitor_socket"`
 }
 
 type runtime struct {
@@ -516,6 +517,22 @@ func (h hypervisor) blockDeviceAIO() (string, error) {
 	return "", fmt.Errorf("Invalid hypervisor block storage I/O mechanism  %v specified (supported AIO: %v)", h.BlockDeviceAIO, supportedBlockAIO)
 }
 
+func (h hypervisor) extraMonitorSocket() (govmmQemu.MonitorProtocol, error) {
+	supportedExtraMonitor := []govmmQemu.MonitorProtocol{govmmQemu.Hmp, govmmQemu.Qmp, govmmQemu.QmpPretty}
+
+	if h.ExtraMonitorSocket == "" {
+		return "", nil
+	}
+
+	for _, extra := range supportedExtraMonitor {
+		if extra == h.ExtraMonitorSocket {
+			return extra, nil
+		}
+	}
+
+	return "", fmt.Errorf("Invalid hypervisor extra monitor socket %v specified (supported values: %v)", h.ExtraMonitorSocket, supportedExtraMonitor)
+}
+
 func (h hypervisor) sharedFS() (string, error) {
 	supportedSharedFS := []string{config.Virtio9P, config.VirtioFS, config.VirtioFSNydus, config.NoSharedFS}
 
@@ -819,6 +836,11 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 	rxRateLimiterMaxRate := h.getRxRateLimiterCfg()
 	txRateLimiterMaxRate := h.getTxRateLimiterCfg()
 
+	extraMonitorSocket, err := h.extraMonitorSocket()
+	if err != nil {
+		return vc.HypervisorConfig{}, err
+	}
+
 	return vc.HypervisorConfig{
 		HypervisorPath:          hypervisor,
 		HypervisorPathList:      h.HypervisorPathList,
@@ -887,6 +909,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		LegacySerial:            h.LegacySerial,
 		DisableSeLinux:          h.DisableSeLinux,
 		DisableGuestSeLinux:     h.DisableGuestSeLinux,
+		ExtraMonitorSocket:      extraMonitorSocket,
 	}, nil
 }
 
@@ -1284,6 +1307,7 @@ func GetDefaultHypervisorConfig() vc.HypervisorConfig {
 		IOMMUPlatform:            defaultEnableIOMMUPlatform,
 		FileBackedMemRootDir:     defaultFileBackedMemRootDir,
 		Debug:                    defaultEnableDebug,
+		ExtraMonitorSocket:       defaultExtraMonitorSocket,
 		DisableNestingChecks:     defaultDisableNestingChecks,
 		BlockDeviceDriver:        defaultBlockDeviceDriver,
 		BlockDeviceAIO:           defaultBlockDeviceAIO,

src/runtime/virtcontainers/hypervisor.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/device/config"
 	"github.com/kata-containers/kata-containers/src/runtime/pkg/govmm"
+	govmmQemu "github.com/kata-containers/kata-containers/src/runtime/pkg/govmm/qemu"
 	hv "github.com/kata-containers/kata-containers/src/runtime/pkg/hypervisors"
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/types"
 
@@ -575,7 +576,7 @@ type HypervisorConfig struct {
 	EnableIOThreads bool
 
 	// Debug changes the default hypervisor and kernel parameters to
-	// enable debug output where available. And Debug also enable the hmp socket.
+	// enable debug output where available.
 	Debug bool
 
 	// MemPrealloc specifies if the memory should be pre-allocated
@@ -641,6 +642,9 @@ type HypervisorConfig struct {
 
 	// Use legacy serial for the guest console
 	LegacySerial bool
+
+	// ExtraMonitorSocket allows to add an extra HMP or QMP socket when the VMM is Qemu
+	ExtraMonitorSocket govmmQemu.MonitorProtocol
 }
 
 // vcpu mapping from vcpu number to thread number

src/runtime/virtcontainers/qemu.go

@@ -121,7 +121,7 @@ type qemu struct {
 const (
 	consoleSocket      = "console.sock"
 	qmpSocket          = "qmp.sock"
-	hmpSocket     = "hmp.sock"
+	extraMonitorSocket = "extra-monitor.sock"
 	vhostFSSocket      = "vhost-fs.sock"
 	nydusdAPISock      = "nydusd-api.sock"
 
@@ -329,8 +329,8 @@ func (q *qemu) qmpSocketPath(id string) (string, error) {
 	return utils.BuildSocketPath(q.config.VMStorePath, id, qmpSocket)
 }
 
-func (q *qemu) hmpSocketPath(id string) (string, error) {
+func (q *qemu) extraMonitorSocketPath(id string) (string, error) {
-	return utils.BuildSocketPath(q.config.VMStorePath, id, hmpSocket)
+	return utils.BuildSocketPath(q.config.VMStorePath, id, extraMonitorSocket)
 }
 
 func (q *qemu) getQemuMachine() (govmmQemu.Machine, error) {
@@ -362,23 +362,30 @@ func (q *qemu) createQmpSocket() ([]govmmQemu.QMPSocket, error) {
 
 	sockets = append(sockets, govmmQemu.QMPSocket{
 		Type:     "unix",
+		Protocol: govmmQemu.Qmp,
 		Server:   true,
 		NoWait:   true,
 	})
 
-	if q.HypervisorConfig().Debug {
+	// The extra monitor socket allows an external user to take full
-		humanMonitorSockPath, err := q.hmpSocketPath(q.id)
+	// control on Qemu and silently break the VM in all possible ways.
+	// It should only ever be used for debugging purposes, hence the
+	// check on Debug.
+	if q.HypervisorConfig().Debug && q.config.ExtraMonitorSocket != "" {
+		extraMonitorSockPath, err := q.extraMonitorSocketPath(q.id)
 		if err != nil {
 			return nil, err
 		}
 
 		sockets = append(sockets, govmmQemu.QMPSocket{
 			Type:     "unix",
-			IsHmp:  true,
+			Protocol: q.config.ExtraMonitorSocket,
-			Name:   humanMonitorSockPath,
+			Name:     extraMonitorSockPath,
 			Server:   true,
 			NoWait:   true,
 		})
+
+		q.Logger().Warn("QEMU configured to start with an untrusted monitor")
 	}
 
 	return sockets, nil