[client] Classify Windows local accounts by NetBIOS name (#7628)

client/ssh/server/executor_windows.go

@@ -6,7 +6,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os"
 	"os/exec"
 	"os/user"
 	"strings"
@@ -506,15 +505,37 @@ func userExists(fullUsername, username, domain string) error {
 	return nil
 }
 
-// isLocalUser determines if this is a local user vs domain user
+// isLocalUser reports whether domain refers to this machine rather than to a
+// Windows domain.
 func (pd *PrivilegeDropper) isLocalUser(domain string) bool {
-	hostname, err := os.Hostname()
-	if err != nil {
-		hostname = "localhost"
+	return isLocalDomain(domain, netbiosComputerName)
+}
+
+// isLocalDomain compares against the NetBIOS name because Windows qualifies local
+// accounts with it, and it is the DNS host name truncated to 15 characters.
+// An unknown name falls back to the domain path: treating it as local could
+// authenticate a same named local account instead.
+// https://learn.microsoft.com/en-us/windows/win32/sysinfo/computer-names
+func isLocalDomain(domain string, machineName func() (string, error)) bool {
+	if domain == "" || domain == "." {
+		return true
 	}
 
-	return domain == "" || domain == "." ||
-		strings.EqualFold(domain, hostname)
+	name, err := machineName()
+	if err != nil {
+		log.Debugf("read NetBIOS computer name: %v", err)
+		return false
+	}
+	return strings.EqualFold(domain, name)
+}
+
+func netbiosComputerName() (string, error) {
+	buf := make([]uint16, windows.MAX_COMPUTERNAME_LENGTH+1)
+	size := uint32(len(buf))
+	if err := windows.GetComputerNameEx(windows.ComputerNamePhysicalNetBIOS, &buf[0], &size); err != nil {
+		return "", fmt.Errorf("GetComputerNameEx: %w", err)
+	}
+	return windows.UTF16ToString(buf[:size]), nil
 }
 
 // authenticateLocalUser handles authentication for local users

client/ssh/server/executor_windows_test.go

@@ -0,0 +1,48 @@
+//go:build windows
+
+package server
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/windows"
+)
+
+// Past 15 characters the DNS host name and the NetBIOS name differ, and Windows
+// qualifies local accounts with the NetBIOS one.
+func TestIsLocalDomain(t *testing.T) {
+	const dnsHostname = "WINTESTMACHINE01XYZ" // 19 characters
+	netbios := dnsHostname[:windows.MAX_COMPUTERNAME_LENGTH]
+	require.NotEqual(t, strings.ToLower(dnsHostname), strings.ToLower(netbios),
+		"a 19 character name must not equal its 15 character truncation")
+
+	name := func() (string, error) { return netbios, nil }
+	unreadable := func() (string, error) { return "", errors.New("name unavailable") }
+
+	tests := []struct {
+		name        string
+		domain      string
+		machineName func() (string, error)
+		want        bool
+	}{
+		{"empty_domain", "", unreadable, true},
+		{"dot_domain", ".", unreadable, true},
+		{"truncated_netbios_name", netbios, name, true},
+		{"netbios_name_lowercase", strings.ToLower(netbios), name, true},
+		{"untruncated_dns_host_name", dnsHostname, name, false},
+		{"real_domain", "CORP", name, false},
+		// Must not resolve to local: that could authenticate the wrong account.
+		{"unreadable_machine_name", netbios, unreadable, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, isLocalDomain(tt.domain, tt.machineName),
+				"classification of domain %q", tt.domain)
+		})
+	}
+}