From 670917899cc00a6ad9ca226ed04f9bb08330960d Mon Sep 17 00:00:00 2001 From: lstocchi Date: Wed, 11 Dec 2024 15:54:17 +0100 Subject: [PATCH] Execute add/remove registry entries in elevated mode with hyperv Adding/removing a key in the Windows registry requires elevated rights. For this reason, using podman with hyperv had the constraint to run it as admin otherwise it was not possible to init/rm a hyperv machine. This patch adds a couple of functions to add/remove registry entries in bulk in privileged mode. When creating/deleting a machine the user is asked only once to elevate the rights to perform the action. So now podman can be started without being admin. This will also be leveraged by podman desktop so users could switch from wsl to hyperv without restarting desktop in elevated mode. Signed-off-by: lstocchi --- build_windows.md | 4 +- pkg/machine/hyperv/stubber.go | 42 +++++---- pkg/machine/hyperv/volumes.go | 23 ++--- pkg/machine/hyperv/vsock/vsock.go | 102 +++++++++++++++++++- pkg/machine/provider/platform_windows.go | 3 - pkg/machine/windows/windows.go | 113 +++++++++++++++++++++++ pkg/machine/wsl/machine.go | 13 +-- pkg/machine/wsl/util_windows.go | 96 +------------------ 8 files changed, 250 insertions(+), 146 deletions(-) create mode 100644 pkg/machine/windows/windows.go diff --git a/build_windows.md b/build_windows.md index b7b180a9e4..312ba28795 100644 --- a/build_windows.md +++ b/build_windows.md @@ -295,8 +295,8 @@ When `machine init` completes, run `machine start`: .\bin\windows\podman.exe machine start ``` -:information_source: If the virtualization provider is Hyperv-V, execute the -above commands in an administrator terminal. +:information_source: If the virtualization provider is Hyperv-V, you will be asked +to elevate privileges to add/remove specific Windows Registry podman settings ### Run a container using podman diff --git a/pkg/machine/hyperv/stubber.go b/pkg/machine/hyperv/stubber.go index ed538d006b..e517f19294 100644 --- a/pkg/machine/hyperv/stubber.go +++ b/pkg/machine/hyperv/stubber.go @@ -55,26 +55,31 @@ func (h HyperVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineC Memory: uint64(mc.Resources.Memory), } - networkHVSock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Network) + networkHVSock, err := vsock.CreateHVSockRegistryEntry(mc.Name, vsock.Network) if err != nil { return err } mc.HyperVHypervisor.NetworkVSock = *networkHVSock - // Add vsock port numbers to mounts - err = createShares(mc) + // Create vsock port numbers to mounts + sharesVsock, err := createShares(mc) if err != nil { return err } - removeShareCallBack := func() error { - return removeShares(mc) + // Add all vsock + err = vsock.AddHVSockRegistryEntries(append([]vsock.HVSockRegistryEntry{ + mc.HyperVHypervisor.ReadyVsock, + mc.HyperVHypervisor.NetworkVSock, + }, sharesVsock...)) + if err != nil { + return err } - callbackFuncs.Add(removeShareCallBack) removeRegistrySockets := func() error { - removeNetworkAndReadySocketsFromRegistry(mc) + sockets := getVsockShares(mc) + removeSocketsFromRegistry(mc, sockets) return nil } callbackFuncs.Add(removeRegistrySockets) @@ -144,7 +149,7 @@ func (h HyperVStubber) Remove(mc *vmconfigs.MachineConfig) ([]string, func() err rmFunc := func() error { // Tear down vsocks - removeNetworkAndReadySocketsFromRegistry(mc) + removeSocketsFromRegistry(mc, []vsock.HVSockRegistryEntry{}) // Remove ignition registry entries - not a fatal error // for vm removal @@ -358,7 +363,7 @@ func (h HyperVStubber) PrepareIgnition(mc *vmconfigs.MachineConfig, ignBuilder * // simply be derived. So we create the HyperVConfig here. mc.HyperVHypervisor = new(vmconfigs.HyperVConfig) var ignOpts ignition.ReadyUnitOpts - readySock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Events) + readySock, err := vsock.CreateHVSockRegistryEntry(mc.Name, vsock.Events) if err != nil { return nil, err } @@ -461,17 +466,16 @@ func resizeDisk(newSize strongunits.GiB, imagePath *define.VMFile) error { return nil } -// removeNetworkAndReadySocketsFromRegistry removes the Network and Ready sockets +// removeSocketsFromRegistry removes the Network, Ready and others (passed by the caller) sockets // from the Windows Registry -func removeNetworkAndReadySocketsFromRegistry(mc *vmconfigs.MachineConfig) { - // Remove the HVSOCK for networking - if err := mc.HyperVHypervisor.NetworkVSock.Remove(); err != nil { - logrus.Errorf("unable to remove registry entry for %s: %q", mc.HyperVHypervisor.NetworkVSock.KeyName, err) - } - - // Remove the HVSOCK for events - if err := mc.HyperVHypervisor.ReadyVsock.Remove(); err != nil { - logrus.Errorf("unable to remove registry entry for %s: %q", mc.HyperVHypervisor.ReadyVsock.KeyName, err) +func removeSocketsFromRegistry(mc *vmconfigs.MachineConfig, others []vsock.HVSockRegistryEntry) { + // remove all sockets from registry + err := vsock.RemoveHVSockRegistryEntries(append([]vsock.HVSockRegistryEntry{ + mc.HyperVHypervisor.ReadyVsock, + mc.HyperVHypervisor.NetworkVSock, + }, others...)) + if err != nil { + logrus.Errorf("unable to remove registry entries: %q", err) } } diff --git a/pkg/machine/hyperv/volumes.go b/pkg/machine/hyperv/volumes.go index ea94147dbd..c0ba66666b 100644 --- a/pkg/machine/hyperv/volumes.go +++ b/pkg/machine/hyperv/volumes.go @@ -14,8 +14,8 @@ import ( "github.com/sirupsen/logrus" ) -func removeShares(mc *vmconfigs.MachineConfig) error { - var removalErr error +func getVsockShares(mc *vmconfigs.MachineConfig) []vsock.HVSockRegistryEntry { + entries := []vsock.HVSockRegistryEntry{} for _, mount := range mc.Mounts { if mount.VSockNumber == nil { @@ -29,15 +29,10 @@ func removeShares(mc *vmconfigs.MachineConfig) error { continue } - if err := vsockReg.Remove(); err != nil { - if removalErr != nil { - logrus.Errorf("Error removing vsock: %v", removalErr) - } - removalErr = fmt.Errorf("removing vsock %d for mountpoint %s: %w", *mount.VSockNumber, mount.Target, err) - } + entries = append(entries, *vsockReg) } - return removalErr + return entries } func startShares(mc *vmconfigs.MachineConfig) error { @@ -70,14 +65,16 @@ func startShares(mc *vmconfigs.MachineConfig) error { return nil } -func createShares(mc *vmconfigs.MachineConfig) (err error) { +func createShares(mc *vmconfigs.MachineConfig) ([]vsock.HVSockRegistryEntry, error) { + vsockEntries := []vsock.HVSockRegistryEntry{} for _, mount := range mc.Mounts { - testVsock, err := vsock.NewHVSockRegistryEntry(mc.Name, vsock.Fileserver) + testVsock, err := vsock.CreateHVSockRegistryEntry(mc.Name, vsock.Fileserver) if err != nil { - return err + return nil, err } + vsockEntries = append(vsockEntries, *testVsock) mount.VSockNumber = &testVsock.Port logrus.Debugf("Going to share directory %s via 9p on vsock %d", mount.Source, testVsock.Port) } - return nil + return vsockEntries, nil } diff --git a/pkg/machine/hyperv/vsock/vsock.go b/pkg/machine/hyperv/vsock/vsock.go index 202faafb38..d4a17207d8 100644 --- a/pkg/machine/hyperv/vsock/vsock.go +++ b/pkg/machine/hyperv/vsock/vsock.go @@ -7,10 +7,13 @@ import ( "fmt" "io" "net" + "os" + "os/exec" "strings" "github.com/Microsoft/go-winio" "github.com/containers/podman/v5/pkg/machine/sockets" + "github.com/containers/podman/v5/pkg/machine/windows" "github.com/containers/podman/v5/utils" "github.com/sirupsen/logrus" "golang.org/x/sys/windows/registry" @@ -83,7 +86,7 @@ type HVSockRegistryEntry struct { } // Add creates a new Windows registry entry with string values from the -// HVSockRegistryEntry. +// HVSockRegistryEntry. Must have elevated rights. func (hv *HVSockRegistryEntry) Add() error { if err := hv.validate(); err != nil { return err @@ -186,9 +189,9 @@ func findOpenHVSockPort() (uint64, error) { return 0, errors.New("unable to find a free port for hvsock use") } -// NewHVSockRegistryEntry is a constructor to make a new registry entry in Windows. After making the new -// object, you must call the add() method to *actually* add it to the Windows registry. -func NewHVSockRegistryEntry(machineName string, purpose HVSockPurpose) (*HVSockRegistryEntry, error) { +// CreateHVSockRegistryEntry is a constructor to make an instance of a registry entry in Windows. After making the new +// object, you must call the add() method or AddHVSockRegistryEntries(...) to *actually* add it to the Windows registry. +func CreateHVSockRegistryEntry(machineName string, purpose HVSockPurpose) (*HVSockRegistryEntry, error) { // a so-called wildcard entry ... everything from FACB -> 6D3 is MS special sauce // for a " linux vm". this first segment is hexi for the hvsock port number // 00000400-FACB-11E6-BD58-64006A7986D3 @@ -202,10 +205,99 @@ func NewHVSockRegistryEntry(machineName string, purpose HVSockPurpose) (*HVSockR Port: port, MachineName: machineName, } + + return &r, nil +} + +// NewHVSockRegistryEntry is a constructor to make a new registry entry in Windows. After making the new +// object, it calls the add() method to *actually* add it to the Windows registry. +func NewHVSockRegistryEntry(machineName string, purpose HVSockPurpose) (*HVSockRegistryEntry, error) { + r, err := CreateHVSockRegistryEntry(machineName, purpose) + if err != nil { + return nil, err + } + if err := r.Add(); err != nil { return nil, err } - return &r, nil + + return r, nil +} + +// AddHVSockRegistryEntries allows to *actually* add multiple registry entries to the Windows registry +// As adding an entry to the HKLM path in the Registry requires elevated privileges, this func can be used for bulk insertion so to +// ask the user for elevated rights only once +func AddHVSockRegistryEntries(entries []HVSockRegistryEntry) error { + // create a script which will be executed with elevated rights + script := "" + for _, entry := range entries { + if err := entry.validate(); err != nil { + return err + } + exists, err := entry.exists() + if err != nil { + return err + } + if exists { + return fmt.Errorf("%q: %s", ErrVSockRegistryEntryExists, entry.KeyName) + } + parentKey, err := registry.OpenKey(registry.LOCAL_MACHINE, VsockRegistryPath, registry.QUERY_VALUE) + defer func() { + if err := parentKey.Close(); err != nil { + logrus.Error(err) + } + }() + if err != nil { + return err + } + + // for each entry it adds a purpose and machineName property + registryPath := fmt.Sprintf("HKLM:\\%s", VsockRegistryPath) + keyPath := fmt.Sprintf("%s\\%s", registryPath, entry.KeyName) + + createRegistryKeyCmd := fmt.Sprintf("New-Item -Path '%s' -Name '%s'", registryPath, entry.KeyName) + addPurposePropertyCmd := fmt.Sprintf("New-ItemProperty -Path '%s' -Name '%s' -Value '%s' -PropertyType String", keyPath, HvsockPurpose, entry.Purpose.string()) + addMachinePropertyCmd := fmt.Sprintf("New-ItemProperty -Path '%s' -Name '%s' -Value '%s' -PropertyType String", keyPath, HvsockMachineName, entry.MachineName) + + script += fmt.Sprintf("%s; %s; %s;", createRegistryKeyCmd, addPurposePropertyCmd, addMachinePropertyCmd) + } + + // launch the script in elevated mode + return launchElevated(script) +} + +// RemoveHVSockRegistryEntries allows to *actually* remove multiple registry entries from the Windows registry +// As removing an entry from the HKLM path in the Registry requires elevated privileges, this func can be used for bulk deletion so to +// ask the user for elevated rights only once +func RemoveHVSockRegistryEntries(entries []HVSockRegistryEntry) error { + // create a script which will be executed with elevated rights + script := "" + for _, entry := range entries { + // for each entry it calculate the path and the script to remove it + registryPath := fmt.Sprintf("HKLM:\\%s", VsockRegistryPath) + keyPath := fmt.Sprintf("%s\\%s", registryPath, entry.KeyName) + + removeRegistryKeyCmd := fmt.Sprintf("Remove-Item -Path '%s' -Force -Recurse", keyPath) + + script += fmt.Sprintf("%s;", removeRegistryKeyCmd) + } + + // launch the script in elevated mode + return launchElevated(script) +} + +func launchElevated(args string) error { + psPath, err := exec.LookPath("powershell.exe") + if err != nil { + return err + } + + d, err := os.Getwd() + if err != nil { + return err + } + + return windows.LaunchElevatedWait(psPath, d, args) } func portToKeyName(port uint64) string { diff --git a/pkg/machine/provider/platform_windows.go b/pkg/machine/provider/platform_windows.go index eab7502af5..a14f717104 100644 --- a/pkg/machine/provider/platform_windows.go +++ b/pkg/machine/provider/platform_windows.go @@ -34,9 +34,6 @@ func Get() (vmconfigs.VMProvider, error) { case define.WSLVirt: return new(wsl.WSLStubber), nil case define.HyperVVirt: - if !wsl.HasAdminRights() { - return nil, fmt.Errorf("hyperv machines require admin authority") - } return new(hyperv.HyperVStubber), nil default: return nil, fmt.Errorf("unsupported virtualization provider: `%s`", resolvedVMType.String()) diff --git a/pkg/machine/windows/windows.go b/pkg/machine/windows/windows.go new file mode 100644 index 0000000000..ed7576a01f --- /dev/null +++ b/pkg/machine/windows/windows.go @@ -0,0 +1,113 @@ +//go:build windows + +package windows + +import ( + "errors" + "fmt" + "syscall" + "unsafe" +) + +type SHELLEXECUTEINFO struct { + cbSize uint32 + fMask uint32 + hwnd syscall.Handle + lpVerb uintptr + lpFile uintptr + lpParameters uintptr + lpDirectory uintptr + nShow int + hInstApp syscall.Handle + lpIDList uintptr + lpClass uintptr + hkeyClass syscall.Handle + dwHotKey uint32 + hIconOrMonitor syscall.Handle + hProcess syscall.Handle +} + +// Cleaner to refer to the official OS constant names, and consistent with syscall +// Ref: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-shellexecuteinfow#members +const ( + //nolint:stylecheck + SEE_MASK_NOCLOSEPROCESS = 0x40 + //nolint:stylecheck + SE_ERR_ACCESSDENIED = 0x05 +) + +type ExitCodeError struct { + Code uint +} + +func (e *ExitCodeError) Error() string { + return fmt.Sprintf("Process failed with exit code: %d", e.Code) +} + +func LaunchElevatedWait(exe string, cwd string, args string) error { + exePtr, _ := syscall.UTF16PtrFromString(exe) + cwdPtr, _ := syscall.UTF16PtrFromString(cwd) + arg, _ := syscall.UTF16PtrFromString(args) + verb, _ := syscall.UTF16PtrFromString("runas") + + shell32 := syscall.NewLazyDLL("shell32.dll") + + info := &SHELLEXECUTEINFO{ + fMask: SEE_MASK_NOCLOSEPROCESS, + hwnd: 0, + lpVerb: uintptr(unsafe.Pointer(verb)), + lpFile: uintptr(unsafe.Pointer(exePtr)), + lpParameters: uintptr(unsafe.Pointer(arg)), + lpDirectory: uintptr(unsafe.Pointer(cwdPtr)), + nShow: syscall.SW_SHOWNORMAL, + } + info.cbSize = uint32(unsafe.Sizeof(*info)) + procShellExecuteEx := shell32.NewProc("ShellExecuteExW") + if ret, _, _ := procShellExecuteEx.Call(uintptr(unsafe.Pointer(info))); ret == 0 { // 0 = False + err := syscall.GetLastError() + if info.hInstApp == SE_ERR_ACCESSDENIED { + return wrapMaybe(err, "request to elevate privileges was denied") + } + return wrapMaybef(err, "could not launch process, ShellEX Error = %d", info.hInstApp) + } + + handle := info.hProcess + defer func() { + _ = syscall.CloseHandle(handle) + }() + + w, err := syscall.WaitForSingleObject(handle, syscall.INFINITE) + switch w { + case syscall.WAIT_OBJECT_0: + break + case syscall.WAIT_FAILED: + return fmt.Errorf("could not wait for process, failed: %w", err) + default: + return fmt.Errorf("could not wait for process, unknown error. event: %X, err: %v", w, err) + } + var code uint32 + if err := syscall.GetExitCodeProcess(handle, &code); err != nil { + return err + } + if code != 0 { + return &ExitCodeError{uint(code)} + } + + return nil +} + +func wrapMaybe(err error, message string) error { + if err != nil { + return fmt.Errorf("%v: %w", message, err) + } + + return errors.New(message) +} + +func wrapMaybef(err error, format string, args ...interface{}) error { + if err != nil { + return fmt.Errorf(format+": %w", append(args, err)...) + } + + return fmt.Errorf(format, args...) +} diff --git a/pkg/machine/wsl/machine.go b/pkg/machine/wsl/machine.go index 38c1a91261..01b6ded7d7 100644 --- a/pkg/machine/wsl/machine.go +++ b/pkg/machine/wsl/machine.go @@ -22,6 +22,7 @@ import ( "github.com/containers/podman/v5/pkg/machine/env" "github.com/containers/podman/v5/pkg/machine/ignition" "github.com/containers/podman/v5/pkg/machine/vmconfigs" + "github.com/containers/podman/v5/pkg/machine/windows" "github.com/containers/podman/v5/pkg/machine/wsl/wutil" "github.com/containers/podman/v5/utils" "github.com/containers/storage/pkg/homedir" @@ -35,14 +36,6 @@ var ( vmtype = define.WSLVirt ) -type ExitCodeError struct { - code uint -} - -func (e *ExitCodeError) Error() string { - return fmt.Sprintf("Process failed with exit code: %d", e.code) -} - //nolint:unused func getConfigPath(name string) (string, error) { return getConfigPathExt(name, "json") @@ -374,8 +367,8 @@ func launchElevate(operation string) error { } err := relaunchElevatedWait() if err != nil { - if eerr, ok := err.(*ExitCodeError); ok { - if eerr.code == ErrorSuccessRebootRequired { + if eerr, ok := err.(*windows.ExitCodeError); ok { + if eerr.Code == ErrorSuccessRebootRequired { fmt.Println("Reboot is required to continue installation, please reboot at your convenience") return nil } diff --git a/pkg/machine/wsl/util_windows.go b/pkg/machine/wsl/util_windows.go index d50f73b915..e9337a6c0c 100644 --- a/pkg/machine/wsl/util_windows.go +++ b/pkg/machine/wsl/util_windows.go @@ -6,16 +6,15 @@ import ( "bytes" "encoding/base64" "encoding/binary" - "errors" "fmt" "os" "path/filepath" "strings" "syscall" "unicode/utf16" - "unsafe" "github.com/Microsoft/go-winio" + wutil "github.com/containers/podman/v5/pkg/machine/windows" "github.com/containers/storage/pkg/fileutils" "github.com/containers/storage/pkg/homedir" "github.com/sirupsen/logrus" @@ -23,24 +22,6 @@ import ( "golang.org/x/sys/windows/registry" ) -type SHELLEXECUTEINFO struct { - cbSize uint32 - fMask uint32 - hwnd syscall.Handle - lpVerb uintptr - lpFile uintptr - lpParameters uintptr - lpDirectory uintptr - nShow int - hInstApp syscall.Handle - lpIDList uintptr - lpClass uintptr - hkeyClass syscall.Handle - dwHotKey uint32 - hIconOrMonitor syscall.Handle - hProcess syscall.Handle -} - type Luid struct { lowPart uint32 highPart int32 @@ -56,15 +37,6 @@ type TokenPrivileges struct { privileges [1]LuidAndAttributes } -// Cleaner to refer to the official OS constant names, and consistent with syscall -// Ref: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-shellexecuteinfow#members -const ( - //nolint:stylecheck - SEE_MASK_NOCLOSEPROCESS = 0x40 - //nolint:stylecheck - SE_ERR_ACCESSDENIED = 0x05 -) - const ( // ref: https://learn.microsoft.com/en-us/windows/win32/secauthz/privilege-constants#constants rebootPrivilege = "SeShutdownPrivilege" @@ -132,71 +104,7 @@ func HasAdminRights() bool { func relaunchElevatedWait() error { e, _ := os.Executable() d, _ := os.Getwd() - exe, _ := syscall.UTF16PtrFromString(e) - cwd, _ := syscall.UTF16PtrFromString(d) - arg, _ := syscall.UTF16PtrFromString(buildCommandArgs(true)) - verb, _ := syscall.UTF16PtrFromString("runas") - - shell32 := syscall.NewLazyDLL("shell32.dll") - - info := &SHELLEXECUTEINFO{ - fMask: SEE_MASK_NOCLOSEPROCESS, - hwnd: 0, - lpVerb: uintptr(unsafe.Pointer(verb)), - lpFile: uintptr(unsafe.Pointer(exe)), - lpParameters: uintptr(unsafe.Pointer(arg)), - lpDirectory: uintptr(unsafe.Pointer(cwd)), - nShow: syscall.SW_SHOWNORMAL, - } - info.cbSize = uint32(unsafe.Sizeof(*info)) - procShellExecuteEx := shell32.NewProc("ShellExecuteExW") - if ret, _, _ := procShellExecuteEx.Call(uintptr(unsafe.Pointer(info))); ret == 0 { // 0 = False - err := syscall.GetLastError() - if info.hInstApp == SE_ERR_ACCESSDENIED { - return wrapMaybe(err, "request to elevate privileges was denied") - } - return wrapMaybef(err, "could not launch process, ShellEX Error = %d", info.hInstApp) - } - - handle := info.hProcess - defer func() { - _ = syscall.CloseHandle(handle) - }() - - w, err := syscall.WaitForSingleObject(handle, syscall.INFINITE) - switch w { - case syscall.WAIT_OBJECT_0: - break - case syscall.WAIT_FAILED: - return fmt.Errorf("could not wait for process, failed: %w", err) - default: - return fmt.Errorf("could not wait for process, unknown error. event: %X, err: %v", w, err) - } - var code uint32 - if err := syscall.GetExitCodeProcess(handle, &code); err != nil { - return err - } - if code != 0 { - return &ExitCodeError{uint(code)} - } - - return nil -} - -func wrapMaybe(err error, message string) error { - if err != nil { - return fmt.Errorf("%v: %w", message, err) - } - - return errors.New(message) -} - -func wrapMaybef(err error, format string, args ...interface{}) error { - if err != nil { - return fmt.Errorf(format+": %w", append(args, err)...) - } - - return fmt.Errorf(format, args...) + return wutil.LaunchElevatedWait(e, d, buildCommandArgs(true)) } func reboot() error {