Fix services list on Windows

* Run machine agent under LocalSystem on Windows. There is no need to run it under a regular user. * Remove ListServices() code from juju and use the upstream implementation. It seems 6 years later, the ability to list services has been added. No need to keep using the one in juju. * Only attempt to reset password on services that belong to juju unit agents. This fixes https://bugs.launchpad.net/juju/+bug/1900002
sdg0919 · Oct 15, 2020 · 942456e · 942456e
1 parent 97b9a76
commit 942456e
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 40 deletions.
diff --git a/cloudconfig/userdatacfg_win.go b/cloudconfig/userdatacfg_win.go
@@ -81,6 +81,7 @@ func (w *windowsConfigure) ConfigureBasic() error {
 		// This is necessary for setACLs to work
 		`$adminsGroup = (New-Object System.Security.Principal.SecurityIdentifier("S-1-5-32-544")).Translate([System.Security.Principal.NTAccount])`,
 		fmt.Sprintf(`icacls "%s" /inheritance:r /grant "${adminsGroup}:(OI)(CI)(F)" /t`, renderer.FromSlash(baseDir)),
+		fmt.Sprintf(`icacls "%s" /inheritance:e /grant "SYSTEM:(OI)(CI)(F)" /t`, renderer.FromSlash(baseDir)),
 	)
 
 	// TODO(bogdanteleaga): This, together with the call above, should be using setACLs, once it starts working across all windows versions properly.

diff --git a/service/windows/password_windows.go b/service/windows/password_windows.go
@@ -7,6 +7,7 @@
 package windows
 
 import (
+	"strings"
 	"syscall"
 	"time"
 
@@ -120,6 +121,16 @@ func (c *PasswordChanger) ChangeJujudServicesPassword(newPassword string, mgr Se
 		return errors.Trace(err)
 	}
 	for _, svc := range svcs {
+		// We only care about juju units. The machine agent runs under LocalSystem,
+		// because it does not need to run any hooks. Only the unit agents need to run
+		// under normal Windows accounts (jujud user created via userdata), as some applications
+		// and installers require an actual account, with a valid profile, to run properly.
+		// It also allows charms to run hooks that impersonate other users (similar to how sudo
+		// works on Linux), which is impossible under system accounts.
+		if strings.HasPrefix(svc, "jujud-unit-") == false {
+			continue
+		}
+		logger.Warningf("resetting password on %s", svc)
 		modifiedService := false
 		var err error
 		for attempt := changeServicePasswordAttempts.Start(); attempt.Next(); {

diff --git a/service/windows/service.go b/service/windows/service.go
@@ -249,10 +249,6 @@ func (s *Service) StartCommands() ([]string, error) {
 }
 
 const serviceCreateCommandTemplate = `
-if ($jujuCreds) {
-  New-Service -Credential $jujuCreds -Name %s -DependsOn Winmgmt -DisplayName %s %s
-} else {
-  New-Service -Name %s -DependsOn Winmgmt -DisplayName %s %s
-}
+New-Service -Name %s -DependsOn Winmgmt -DisplayName %s %s
 sc.exe failure %s reset=5 actions=restart/1000
 sc.exe failureflag %s 1`
diff --git a/service/windows/service_windows.go b/service/windows/service_windows.go
@@ -175,44 +175,12 @@ var getPassword = func() (string, error) {
 // the current system. It is defined as a variable to allow us to mock it out
 // for testing
 var listServices = func() (services []string, err error) {
-	host := syscall.StringToUTF16Ptr(".")
-
-	sc, err := windows.OpenSCManager(host, nil, windows.SC_MANAGER_ALL_ACCESS)
-	defer func() {
-		// The close service handle error is less important than others
-		if err == nil {
-			err = windows.CloseServiceHandle(sc)
-		}
-	}()
+	m, err := mgr.Connect()
 	if err != nil {
 		return nil, err
 	}
-
-	var needed uint32
-	var returned uint32
-	var resume uint32 = 0
-	var enum []enumService
-
-	for {
-		var buf [512]enumService
-		err := enumServicesStatus(sc, SC_ENUM_PROCESS_INFO, windows.SERVICE_WIN32,
-			windows.SERVICE_STATE_ALL, uintptr(unsafe.Pointer(&buf[0])), uint32(unsafe.Sizeof(buf)), &needed, &returned, &resume, nil)
-		if err != nil {
-			if err == windows.ERROR_MORE_DATA {
-				enum = append(enum, buf[:returned]...)
-				continue
-			}
-			return nil, err
-		}
-		enum = append(enum, buf[:returned]...)
-		break
-	}
-
-	services = make([]string, len(enum))
-	for i, v := range enum {
-		services[i] = v.Name()
-	}
-	return services, nil
+	defer m.Disconnect()
+	return m.ListServices()
 }
 
 // SvcManager implements ServiceManager interface