From 558e9d335a287a0ef985544cd583c0722d943c35 Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Wed, 28 Aug 2019 08:22:03 -0600 Subject: manager: fix nits in adapter cleanup logic and also handle ‘%s’ uniformly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- conf/migration_windows.go | 10 +++---- manager/adaptercleanup.go | 63 +++++++++++++++++++++++++++++++++++++++++++ manager/adapters.go | 69 ----------------------------------------------- manager/ipc_server.go | 3 +++ manager/service.go | 4 +-- tunnel/addressconfig.go | 2 +- ui/filesave.go | 2 +- 7 files changed, 75 insertions(+), 78 deletions(-) create mode 100644 manager/adaptercleanup.go delete mode 100644 manager/adapters.go diff --git a/conf/migration_windows.go b/conf/migration_windows.go index 9c070a08..4b7ffe30 100644 --- a/conf/migration_windows.go +++ b/conf/migration_windows.go @@ -38,27 +38,27 @@ func maybeMigrate(c string) { return } if err != nil { - log.Printf("Not migrating configuration from '%s' due to GetFileSecurity error: %v", oldRoot, err) + log.Printf("Not migrating configuration from ‘%s’ due to GetFileSecurity error: %v", oldRoot, err) return } var defaulted bool var sid *windows.SID err = getSecurityDescriptorOwner(&sd[0], &sid, &defaulted) if err != nil { - log.Printf("Not migrating configuration from '%s' due to GetSecurityDescriptorOwner error: %v", oldRoot, err) + log.Printf("Not migrating configuration from ‘%s’ due to GetSecurityDescriptorOwner error: %v", oldRoot, err) return } if defaulted || !sid.IsWellKnown(windows.WinLocalSystemSid) { sidStr, _ := sid.String() - log.Printf("Not migrating configuration from '%s', as it is not explicitly owned by SYSTEM, but rather '%s'", oldRoot, sidStr) + log.Printf("Not migrating configuration from ‘%s’, as it is not explicitly owned by SYSTEM, but rather ‘%s’", oldRoot, sidStr) return } err = windows.MoveFileEx(windows.StringToUTF16Ptr(oldC), windows.StringToUTF16Ptr(c), windows.MOVEFILE_COPY_ALLOWED) if err != nil { if err != windows.ERROR_FILE_NOT_FOUND && err != windows.ERROR_ALREADY_EXISTS { - log.Printf("Not migrating configuration from '%s' due to error when moving files: %v", oldRoot, err) + log.Printf("Not migrating configuration from ‘%s’ due to error when moving files: %v", oldRoot, err) } return } - log.Printf("Migrated configuration from '%s'", oldRoot) + log.Printf("Migrated configuration from ‘%s’", oldRoot) } diff --git a/manager/adaptercleanup.go b/manager/adaptercleanup.go new file mode 100644 index 00000000..779e4b80 --- /dev/null +++ b/manager/adaptercleanup.go @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: MIT + * + * Copyright (C) 2019 WireGuard LLC. All Rights Reserved. + */ + +package manager + +import ( + "log" + "strings" + + "golang.org/x/sys/windows" + "golang.org/x/sys/windows/svc" + "golang.org/x/sys/windows/svc/mgr" + + "golang.zx2c4.com/wireguard/tun/wintun" + "golang.zx2c4.com/wireguard/windows/services" +) + +const unnamedWintunInterface = "Local Area Connection" + +func cleanupStaleAdapters() { + defer printPanic() + + m, err := mgr.Connect() + if err != nil { + return + } + defer m.Disconnect() + + wintun.DeleteMatchingInterfaces(func(wintun *wintun.Wintun) bool { + interfaceName, err := wintun.InterfaceName() + if err != nil { + log.Printf("Removing Wintun interface %s because determining interface name failed: %v", wintun.GUID().String(), err) + return true + } + if strings.HasPrefix(interfaceName, unnamedWintunInterface) { + return false + } + serviceName, err := services.ServiceNameOfTunnel(interfaceName) + if err != nil { + log.Printf("Removing Wintun interface ‘%s’ because determining tunnel service name failed: %v", interfaceName, err) + return true + } + service, err := m.OpenService(serviceName) + if err == windows.ERROR_SERVICE_DOES_NOT_EXIST { + log.Printf("Removing Wintun interface ‘%s’ because no service for it exists", interfaceName) + return true + } else if err != nil { + return false + } + defer service.Close() + status, err := service.Query() + if err != nil { + return false + } + if status.State == svc.Stopped { + log.Printf("Removing Wintun interface ‘%s’ because its service is stopped", interfaceName) + return true + } + return false + }) +} diff --git a/manager/adapters.go b/manager/adapters.go deleted file mode 100644 index 5ac16465..00000000 --- a/manager/adapters.go +++ /dev/null @@ -1,69 +0,0 @@ -/* SPDX-License-Identifier: MIT - * - * Copyright (C) 2019 WireGuard LLC. All Rights Reserved. - */ - -package manager - -import ( - "log" - - "golang.org/x/sys/windows" - "golang.org/x/sys/windows/svc" - "golang.org/x/sys/windows/svc/mgr" - - "golang.zx2c4.com/wireguard/tun/wintun" - "golang.zx2c4.com/wireguard/windows/services" -) - -func cleanStaleAdapters() { - defer printPanic() - - m, err := mgr.Connect() - if err != nil { - log.Printf("Error connecting to Service Control Manager: %v", err) - return - } - defer m.Disconnect() - - wintun.DeleteMatchingInterfaces(func(wintun *wintun.Wintun) bool { - interfaceName, err := wintun.InterfaceName() - if err != nil { - log.Printf("Removing Wintun interface %s because determining interface name failed: %v", wintun.GUID().String(), err) - return true - } - serviceName, err := services.ServiceNameOfTunnel(interfaceName) - if err != nil { - log.Printf("Removing Wintun interface %s because determining tunnel service name failed: %v", interfaceName, err) - return true - } - service, err := m.OpenService(serviceName) - if err == windows.ERROR_SERVICE_DOES_NOT_EXIST { - log.Printf("Removing orphaned Wintun interface %s", interfaceName) - return true - } - if err != nil { - log.Printf("Error opening service %s: %v", serviceName, err) - return false - } - defer service.Close() - config, err := service.Config() - if err != nil { - log.Printf("Error getting service %s configuration: %v", serviceName, err) - return false - } - if config.StartType == mgr.StartAutomatic { - return false - } - status, err := service.Query() - if err != nil { - log.Printf("Error getting service %s status: %v", serviceName, err) - return false - } - if status.State == svc.Stopped { - log.Printf("Removing unused Wintun interface %s", interfaceName) - return true - } - return false - }) -} diff --git a/manager/ipc_server.go b/manager/ipc_server.go index 7691adb8..ed60d6b6 100644 --- a/manager/ipc_server.go +++ b/manager/ipc_server.go @@ -107,6 +107,7 @@ func (s *ManagerService) Start(tunnelName string, unused *uintptr) error { } } }() + go cleanupStaleAdapters() // After that process is started -- it's somewhat asynchronous -- we install the new one. c, err := conf.LoadFromName(tunnelName) @@ -121,6 +122,8 @@ func (s *ManagerService) Start(tunnelName string, unused *uintptr) error { } func (s *ManagerService) Stop(tunnelName string, _ *uintptr) error { + go cleanupStaleAdapters() + err := UninstallTunnel(tunnelName) if err == windows.ERROR_SERVICE_DOES_NOT_EXIST { _, notExistsError := conf.LoadFromName(tunnelName) diff --git a/manager/service.go b/manager/service.go index 7ab3db16..8a525b12 100644 --- a/manager/service.go +++ b/manager/service.go @@ -182,7 +182,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest return } - log.Printf("Starting UI process for user '%s@%s' for session %d", username, domain, session) + log.Printf("Starting UI process for user ‘%s@%s’ for session %d", username, domain, session) attr := &os.ProcAttr{ Sys: &syscall.SysProcAttr{ Token: syscall.Token(elevatedToken), @@ -247,7 +247,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest }() } - go cleanStaleAdapters() + go cleanupStaleAdapters() go checkForUpdates() var sessionsPointer *windows.WTS_SESSION_INFO diff --git a/tunnel/addressconfig.go b/tunnel/addressconfig.go index a1e5dc59..643e178e 100644 --- a/tunnel/addressconfig.go +++ b/tunnel/addressconfig.go @@ -50,7 +50,7 @@ func cleanupAddressesOnDisconnectedInterfaces(family winipcfg.AddressFamily, add ip := address.Address.IP() ipnet := net.IPNet{IP: ip, Mask: net.CIDRMask(int(address.OnLinkPrefixLength), 8*len(ip))} if includedInAddresses(ipnet) { - log.Printf("Cleaning up stale address %s from interface '%s'", ipnet.String(), iface.FriendlyName()) + log.Printf("Cleaning up stale address %s from interface ‘%s’", ipnet.String(), iface.FriendlyName()) iface.LUID.DeleteIPAddress(ipnet) } } diff --git a/ui/filesave.go b/ui/filesave.go index 5f78cfee..4b5c2947 100644 --- a/ui/filesave.go +++ b/ui/filesave.go @@ -26,7 +26,7 @@ func writeFileWithOverwriteHandling(owner walk.Form, filePath string, write func file, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0600) if err != nil { if os.IsExist(err) { - if walk.DlgCmdNo == walk.MsgBox(owner, "Writing file failed", fmt.Sprintf(`File "%s" already exists. + if walk.DlgCmdNo == walk.MsgBox(owner, "Writing file failed", fmt.Sprintf(`File ‘%s’ already exists. Do you want to overwrite it?`, filePath), walk.MsgBoxYesNo|walk.MsgBoxDefButton2|walk.MsgBoxIconWarning) { return false -- cgit v1.2.3-59-g8ed1b