manager: rewrite service arguments when migrating configs

If a service is installed already, it's a good idea to migrate its conf
argument when migrating the file name, so that these don't get out of
sync. We're already doing this when migrating from the legacy config
store, so this is also an opportunity to clean up that code a bit and
quit using regexes.

Reported-by: Станислав Мацак <smatsak@mail.ru>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

4 files changed, 62 insertions, 58 deletions

diff --git a/conf/migration_windows.go b/conf/migration_windows.go
index 091efa32..e81fb5e4 100644
--- a/conf/migration_windows.go
+++ b/conf/migration_windows.go
@@ -21,9 +21,14 @@ import (
 var migrating sync.Mutex
 var lastMigrationTimer *time.Timer
 
-func MigrateUnencryptedConfigs() { migrateUnencryptedConfigs(3) }
+type MigrationCallback func(name, oldPath, newPath string)
 
-func migrateUnencryptedConfigs(sharingBase int) {
+func MigrateUnencryptedConfigs(migrated MigrationCallback) { migrateUnencryptedConfigs(3, migrated) }
+
+func migrateUnencryptedConfigs(sharingBase int, migrated MigrationCallback) {
+	if migrated == nil {
+		migrated = func(_, _, _ string) {}
+	}
 	migrating.Lock()
 	defer migrating.Unlock()
 	configFileDir, err := tunnelConfigurationsDirectory()
@@ -54,6 +59,7 @@ func migrateUnencryptedConfigs(sharingBase int) {
 
 		var bytes []byte
 		var config *Config
+		var newPath string
 		// We don't use os.ReadFile, because we actually want RDWR, so that we can take advantage
 		// of Windows file locking for ensuring the file is finished being written.
 		f, err := os.OpenFile(path, os.O_RDWR, 0)
@@ -65,7 +71,7 @@ func migrateUnencryptedConfigs(sharingBase int) {
 					if lastMigrationTimer != nil {
 						lastMigrationTimer.Stop()
 					}
-					lastMigrationTimer = time.AfterFunc(time.Second/time.Duration(sharingBase*sharingBase), func() { migrateUnencryptedConfigs(sharingBase - 1) })
+					lastMigrationTimer = time.AfterFunc(time.Second/time.Duration(sharingBase*sharingBase), func() { migrateUnencryptedConfigs(sharingBase-1, migrated) })
 					ignoreSharingViolations = true
 					continue
 				}
@@ -87,8 +93,13 @@ func migrateUnencryptedConfigs(sharingBase int) {
 		}
 		err = os.Remove(path)
 		if err != nil {
-			log.Printf("Unable to remove old path %#q: %v", path, err)
+			goto error
+		}
+		newPath, err = config.Path()
+		if err != nil {
+			goto error
 		}
+		migrated(config.Name, path, newPath)
 		continue
 	error:
 		log.Printf("Unable to ingest and encrypt %#q: %v", path, err)
diff --git a/manager/install.go b/manager/install.go
index bd7d23c7..f74fecf0 100644
--- a/manager/install.go
+++ b/manager/install.go
@@ -7,7 +7,9 @@ package manager
 
 import (
 	"errors"
+	"log"
 	"os"
+	"strings"
 	"time"
 
 	"golang.org/x/sys/windows"
@@ -197,3 +199,45 @@ func UninstallTunnel(name string) error {
 	}
 	return err2
 }
+
+func changeTunnelServiceConfigFilePath(name, oldPath, newPath string) {
+	var err error
+	defer func() {
+		if err != nil {
+			log.Printf("Unable to change tunnel service command line argument from %#q to %#q: %v", oldPath, newPath, err)
+		}
+	}()
+	m, err := serviceManager()
+	if err != nil {
+		return
+	}
+	serviceName, err := services.ServiceNameOfTunnel(name)
+	if err != nil {
+		return
+	}
+	service, err := m.OpenService(serviceName)
+	if err == windows.ERROR_SERVICE_DOES_NOT_EXIST {
+		err = nil
+		return
+	} else if err != nil {
+		return
+	}
+	defer service.Close()
+	config, err := service.Config()
+	if err != nil {
+		return
+	}
+	exePath, err := os.Executable()
+	if err != nil {
+		return
+	}
+	args, err := windows.DecomposeCommandLine(config.BinaryPathName)
+	if err != nil || len(args) != 3 ||
+		!strings.EqualFold(args[0], exePath) || args[1] != "/tunnelservice" || !strings.EqualFold(args[2], oldPath) {
+		err = nil
+		return
+	}
+	args[2] = newPath
+	config.BinaryPathName = windows.ComposeCommandLine(args)
+	err = service.UpdateConfig(config)
+}
diff --git a/manager/legacystore.go b/manager/legacystore.go
index 4125cb86..0996d15f 100644
--- a/manager/legacystore.go
+++ b/manager/legacystore.go
@@ -6,17 +6,13 @@
 package manager
 
 import (
-	"fmt"
 	"log"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"golang.org/x/sys/windows"
 	"golang.org/x/sys/windows/registry"
-	"golang.org/x/sys/windows/svc/mgr"
-
 	"golang.zx2c4.com/wireguard/windows/conf"
 )
 
@@ -39,7 +35,6 @@ func moveConfigsFromLegacyStore() {
 		}
 		key.Close()
 	}
-	migratedConfigs := make(map[string]string)
 	for i := range files {
 		if files[i].IsDir() {
 			continue
@@ -65,8 +60,8 @@ func moveConfigsFromLegacyStore() {
 		if err == nil {
 			windows.MoveFileEx(oldPath16, nil, windows.MOVEFILE_DELAY_UNTIL_REBOOT)
 		}
-		migratedConfigs[strings.ToLower(oldPath)] = newPath
-		log.Printf("Migrated configuration from ‘%s’ to ‘%s’", oldPath, newPath)
+		log.Printf("Migrated configuration from %#q to %#q", oldPath, newPath)
+		changeTunnelServiceConfigFilePath(config.Name, oldPath, newPath)
 	}
 	oldC16, err := windows.UTF16PtrFromString(oldC)
 	if err == nil {
@@ -80,50 +75,4 @@ func moveConfigsFromLegacyStore() {
 	if err == nil {
 		windows.MoveFileEx(oldRoot16, nil, windows.MOVEFILE_DELAY_UNTIL_REBOOT)
 	}
-	if len(migratedConfigs) == 0 {
-		return
-	}
-	m, err := mgr.Connect()
-	if err != nil {
-		return
-	}
-	defer m.Disconnect()
-	services, err := m.ListServices()
-	if err != nil {
-		return
-	}
-	matcher, err := regexp.Compile(" /tunnelservice \"?([^\"]+)\"?$")
-	if err != nil {
-		return
-	}
-	for _, svcName := range services {
-		if !strings.HasPrefix(svcName, "WireGuardTunnel$") {
-			continue
-		}
-		svc, err := m.OpenService(svcName)
-		if err != nil {
-			continue
-		}
-		config, err := svc.Config()
-		if err != nil {
-			continue
-		}
-		matches := matcher.FindStringSubmatchIndex(config.BinaryPathName)
-		if len(matches) != 4 {
-			svc.Close()
-			continue
-		}
-		newName, found := migratedConfigs[strings.ToLower(config.BinaryPathName[matches[2]:])]
-		if !found {
-			svc.Close()
-			continue
-		}
-		config.BinaryPathName = config.BinaryPathName[:matches[0]] + fmt.Sprintf(" /tunnelservice \"%s\"", newName)
-		err = svc.UpdateConfig(config)
-		svc.Close()
-		if err != nil {
-			continue
-		}
-		log.Printf("Migrated service command line arguments for ‘%s’", svcName)
-	}
 }
diff --git a/manager/service.go b/manager/service.go
index 99187eab..ac428c86 100644
--- a/manager/service.go
+++ b/manager/service.go
@@ -71,7 +71,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 		return
 	}
 
-	conf.RegisterStoreChangeCallback(conf.MigrateUnencryptedConfigs)
+	conf.RegisterStoreChangeCallback(func() { conf.MigrateUnencryptedConfigs(changeTunnelServiceConfigFilePath) })
 	conf.RegisterStoreChangeCallback(IPCServerNotifyTunnelsChange)
 
 	procs := make(map[uint32]*os.Process)