[bug] rework Windows user password generation to match security constraints (#7507) (#7621)

* fix: re-implement Windows user password generation to match security constraints

* doc: add changelog fragment

* feat: bump user generated password to a range of 64-127

* feat: add null bytes checks in the generated password

* feat: add password charsets unit-tests

* fix: log hasLower, hasUpper, hasDigit, hasSpecial at the correct check

(cherry picked from commit b364d3d)

Co-authored-by: Panos Koutsovasilis <[email protected]>

Author: mergify[bot]

changelog/fragments/1742562383-Rework-Windows-user-password.yaml

@@ -0,0 +1,5 @@
+kind: bug-fix
+summary: Rework Windows user password generation to meet security policy constraints
+component: elastic-agent
+pr: https://github.com/elastic/elastic-agent/pull/7507
+issue: https://github.com/elastic/elastic-agent/issues/7506

internal/pkg/agent/install/user_windows.go

@@ -11,7 +11,6 @@ import (
 	"errors"
 	"fmt"
 	"math/big"
-	"strings"
 	"syscall"
 	"unsafe"
 
@@ -20,7 +19,16 @@ import (
 )
 
 const (
-	passwordLength = 127 // maximum length allowed by Windows
+	// Reference: https://learn.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/password-must-meet-complexity-requirements
+	passwordMinLength = 64  // Minimum length for better security
+	passwordMaxLength = 127 // Upper limit to avoid policy issues
+	// Character pools - ensuring a mix of character categories
+	passwordCharsLower   = "abcdefghijklmnopqrstuvwxyz"
+	passwordCharsUpper   = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	passwordCharsDigits  = "1234567890"
+	passwordCharsSpecial = "'-!\"#$%&()*,./:;?@[]^_`{|}~+<=>" //nolint:gosec // G101: False positive on Potential hardcoded credentials
+	// Combined pool for general randomization
+	passwordAllChars = passwordCharsLower + passwordCharsUpper + passwordCharsDigits + passwordCharsSpecial
 )
 
 var (
@@ -238,19 +246,81 @@ func SetUserPassword(name string, password string) error {
 	return nil
 }
 
-// RandomPassword generates a random password.
+// RandomPassword generates a secure password that meets Windows policy requirements.
 func RandomPassword() (string, error) {
-	runes := []rune("abcdefghijklmnopqrstuvwxyz1234567890!@#$%^&*ABCDEFGHIJKLMNOPQRSTUVWXYZ")
-	maxN := big.NewInt(int64(len(runes)))
-	var sb strings.Builder
-	for i := 0; i < passwordLength; i++ {
-		n, err := rand.Int(rand.Reader, maxN)
+	// Generate a random length within the allowed range
+	length, err := rand.Int(rand.Reader, big.NewInt(passwordMaxLength-passwordMinLength+1))
+	if err != nil {
+		return "", fmt.Errorf("failed to generate random length: %w", err)
+	}
+	passwordLength := int(length.Int64()) + passwordMinLength
+	password := make([]rune, passwordLength)
+
+	// Ensure Windows password constraints are met by guaranteeing at least one character from each category
+	filledPositions := make(map[int]any)
+	for _, chars := range []string{passwordCharsUpper, passwordCharsLower, passwordCharsDigits, passwordCharsSpecial} {
+		// Generate a random position for the character
+		var position int
+		for {
+			posBigInt, err := rand.Int(rand.Reader, big.NewInt(int64(passwordLength)))
+			if err != nil {
+				return "", fmt.Errorf("failed to generate random position: %w", err)
+			}
+			position = int(posBigInt.Int64())
+			if _, filled := filledPositions[position]; filled {
+				// Position already has a missing character, generate a new one
+				continue
+			}
+			break
+		}
+		// Generate a random character from the given characters
+		char, err := randomChar(chars)
+		if err != nil {
+			// err information is added by randomChar
+			return "", err
+		}
+		// Insert the character into the password
+		password[position] = char
+		filledPositions[position] = nil
+	}
+
+	// Fill the remaining positions with random characters
+	for i := 0; i < passwordLength; {
+		// Check if the current position has already been filled
+		if _, filled := filledPositions[i]; filled {
+			// Position already has a character
+			i++
+			continue
+		}
+		// Generate a random character from the combined pool of characters
+		char, err := randomChar(passwordAllChars)
 		if err != nil {
-			return "", fmt.Errorf("failed to generate random integer: %w", err)
+			// err information is added by randomChar
+			return "", err
+		}
+		// Ensure no consecutive duplicate characters to the left
+		if i > 0 && password[i-1] == char {
+			continue
 		}
-		sb.WriteRune(runes[n.Int64()])
+		// Ensure no consecutive duplicate characters to the right
+		if i+1 < passwordLength && password[i+1] == char {
+			continue
+		}
+		// Insert the character into the password
+		password[i] = char
+		i++
+	}
+
+	return string(password), nil
+}
+
+// randomChar selects a random character from a given string.
+func randomChar(charset string) (rune, error) {
+	n, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset))))
+	if err != nil {
+		return 0, fmt.Errorf("failed to generate random character: %w", err)
 	}
-	return sb.String(), nil
+	return rune(charset[n.Int64()]), nil
 }
 
 // LOCALGROUP_INFO_0 structure contains a local group name.

internal/pkg/agent/install/user_windows_test.go

@@ -0,0 +1,129 @@
+// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+// or more contributor license agreements. Licensed under the Elastic License 2.0;
+// you may not use this file except in compliance with the Elastic License 2.0.
+
+//go:build windows
+
+package install
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+	"unicode"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPasswordCharSets(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		charSet string
+		valid   func(rune) error
+	}{
+		{
+			name:    "lowercase characters",
+			charSet: passwordCharsLower,
+			valid: func(r rune) error {
+				if unicode.IsLower(r) {
+					return nil
+				}
+				return fmt.Errorf("character %q is not lowercase", r)
+			},
+		},
+		{
+			name:    "uppercase characters",
+			charSet: passwordCharsUpper,
+			valid: func(r rune) error {
+				if unicode.IsUpper(r) {
+					return nil
+				}
+				return fmt.Errorf("character %q is not uppercase", r)
+			},
+		},
+		{
+			name:    "digit characters",
+			charSet: passwordCharsDigits,
+			valid: func(r rune) error {
+				if unicode.IsDigit(r) {
+					return nil
+				}
+				return fmt.Errorf("character %q is not a digit", r)
+			},
+		},
+		{
+			name:    "special characters",
+			charSet: passwordCharsSpecial,
+			valid: func(r rune) error {
+				if unicode.IsPunct(r) || unicode.IsSymbol(r) {
+					return nil
+				}
+				return fmt.Errorf("character %q is not a special character", r)
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, char := range tc.charSet {
+				assert.NoError(t, tc.valid(char))
+			}
+		})
+	}
+}
+
+// TestRandomPassword tries to ensure that generated passwords meet the windows security constraints.
+func TestRandomPassword(t *testing.T) {
+	const testTimes = 100000
+	passwords := make(map[string]struct{})
+	for i := 0; i < testTimes; i++ {
+		password, err := RandomPassword()
+		if err != nil {
+			t.Fatalf("RandomPassword() returned an error: %v", err)
+		}
+
+		length := len(password)
+		if length < passwordMinLength || length > passwordMaxLength {
+			t.Fatalf("password %q is not within the allowed length range", password)
+		}
+
+		hasLower := false
+		hasUpper := false
+		hasDigit := false
+		hasSpecial := false
+
+		if strings.ContainsRune(password, 0) {
+			t.Fatalf("password %q contains null character", password)
+		}
+
+		for _, char := range password {
+			switch {
+			case strings.ContainsRune(passwordCharsLower, char):
+				hasLower = true
+			case strings.ContainsRune(passwordCharsUpper, char):
+				hasUpper = true
+			case strings.ContainsRune(passwordCharsDigits, char):
+				hasDigit = true
+			case strings.ContainsRune(passwordCharsSpecial, char):
+				hasSpecial = true
+			default:
+				t.Fatalf("password %q contains an invalid character %q", password, string(char))
+			}
+		}
+
+		if !hasLower || !hasUpper || !hasDigit || !hasSpecial {
+			t.Fatalf("password %q does not contain all required character categories (hasLower=%v, hasUpper=%v, hasDigit=%v, hasSpecial=%v)",
+				password, hasLower, hasUpper, hasDigit, hasSpecial)
+		}
+
+		// Check for consecutive duplicate digits
+		for j := 1; j < length; j++ {
+			if password[j] == password[j-1] {
+				t.Fatalf("password %q contains consecutive duplicate digits %q at positions %d %d", password, password[j], j, j-1)
+			}
+		}
+
+		if _, exists := passwords[password]; exists {
+			t.Fatalf("password %q is not unique", password)
+		}
+		passwords[password] = struct{}{}
+	}
+}