Fix azure winrm_password attribution and allow to set winrm_username (#8928)

builder/azure/arm/builder.go

@@ -254,7 +254,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack
 				WinRMConfig: func(multistep.StateBag) (*communicator.WinRMConfig, error) {
 					return &communicator.WinRMConfig{
 						Username: b.config.UserName,
-						Password: b.config.Comm.WinRMPassword,
+						Password: b.config.Password,
 					}, nil
 				},
 			},

builder/azure/arm/config.go

@@ -18,6 +18,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/hashicorp/packer/common/random"
+
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-04-01/compute"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/masterzen/winrm"
@@ -528,7 +530,10 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) {
 
 	provideDefaultValues(c)
 	setRuntimeValues(c)
-	setUserNamePassword(c)
+	err = setUserNamePassword(c)
+	if err != nil {
+		return nil, err
+	}
 
 	// copy singular blocks
 	for _, kv := range c.AzureTag {
@@ -651,23 +656,34 @@ func setRuntimeValues(c *Config) {
 	c.tmpKeyVaultName = tempName.KeyVaultName
 }
 
-func setUserNamePassword(c *Config) {
+func setUserNamePassword(c *Config) error {
+	// SSH comm
 	if c.Comm.SSHUsername == "" {
 		c.Comm.SSHUsername = DefaultUserName
 	}
-
 	c.UserName = c.Comm.SSHUsername
 
 	if c.Comm.SSHPassword != "" {
 		c.Password = c.Comm.SSHPassword
-		return
+		return nil
 	}
 
-	// Configure password settings using Azure generated credentials
+	// WinRM comm
-	c.Password = c.tmpAdminPassword
+	if c.Comm.WinRMUser == "" {
-	if c.Comm.WinRMPassword == "" {
+		c.Comm.WinRMUser = DefaultUserName
-		c.Comm.WinRMPassword = c.Password
 	}
+	c.UserName = c.Comm.WinRMUser
+
+	if c.Comm.WinRMPassword == "" {
+		// Configure password settings using Azure generated credentials
+		c.Comm.WinRMPassword = c.tmpAdminPassword
+	}
+	if !isValidPassword(c.Comm.WinRMPassword) {
+		return fmt.Errorf("The supplied \"winrm_password\" must be between 8-123 characters long and must satisfy at least 3 from the following: \n1) Contains an uppercase character \n2) Contains a lowercase character\n3) Contains a numeric digit\n4) Contains a special character\n5) Control characters are not allowed")
+	}
+	c.Password = c.Comm.WinRMPassword
+
+	return nil
 }
 
 func setCustomData(c *Config) error {
@@ -1037,6 +1053,34 @@ func isValidAzureName(re *regexp.Regexp, rgn string) bool {
 		!strings.HasSuffix(rgn, "-")
 }
 
+// The supplied password must be between 8-123 characters long and must satisfy at least 3 of password complexity requirements from the following:
+// 1) Contains an uppercase character
+// 2) Contains a lowercase character
+// 3) Contains a numeric digit
+// 4) Contains a special character
+// 5) Control characters are not allowed (a very specific case - not included in this validation)
+func isValidPassword(password string) bool {
+	if !(len(password) >= 8 && len(password) <= 123) {
+		return false
+	}
+
+	requirements := 0
+	if strings.ContainsAny(password, random.PossibleNumbers) {
+		requirements++
+	}
+	if strings.ContainsAny(password, random.PossibleLowerCase) {
+		requirements++
+	}
+	if strings.ContainsAny(password, random.PossibleUpperCase) {
+		requirements++
+	}
+	if strings.ContainsAny(password, random.PossibleSpecialCharacter) {
+		requirements++
+	}
+
+	return requirements >= 3
+}
+
 func (c *Config) validateLocationZoneResiliency(say func(s string)) {
 	// Docs on regions that support Availibility Zones:
 	//   https://docs.microsoft.com/en-us/azure/availability-zones/az-overview#regions-that-support-availability-zones

builder/azure/arm/config_test.go

@@ -71,22 +71,6 @@ func TestConfigShouldBeAbleToOverrideDefaultedValues(t *testing.T) {
 		t.Fatalf("newConfig failed: %s", err)
 	}
 
-	if c.Password != "override_password" {
-		t.Errorf("Expected 'Password' to be set to 'override_password', but found %q!", c.Password)
-	}
-
-	if c.Comm.SSHPassword != "override_password" {
-		t.Errorf("Expected 'c.Comm.SSHPassword' to be set to 'override_password', but found %q!", c.Comm.SSHPassword)
-	}
-
-	if c.UserName != "override_username" {
-		t.Errorf("Expected 'UserName' to be set to 'override_username', but found %q!", c.UserName)
-	}
-
-	if c.Comm.SSHUsername != "override_username" {
-		t.Errorf("Expected 'c.Comm.SSHUsername' to be set to 'override_username', but found %q!", c.Comm.SSHUsername)
-	}
-
 	if c.VMSize != "override_vm_size" {
 		t.Errorf("Expected 'vm_size' to be set to 'override_vm_size', but found %q!", c.VMSize)
 	}
@@ -98,6 +82,43 @@ func TestConfigShouldBeAbleToOverrideDefaultedValues(t *testing.T) {
 	if c.diskCachingType != compute.CachingTypesNone {
 		t.Errorf("Expected 'disk_caching_type' to be set to 'None', but found %q!", c.diskCachingType)
 	}
+
+	// SSH comm
+	if c.Password != "override_password" {
+		t.Errorf("Expected 'Password' to be set to 'override_password', but found %q!", c.Password)
+	}
+	if c.Comm.SSHPassword != "override_password" {
+		t.Errorf("Expected 'c.Comm.SSHPassword' to be set to 'override_password', but found %q!", c.Comm.SSHPassword)
+	}
+	if c.UserName != "override_username" {
+		t.Errorf("Expected 'UserName' to be set to 'override_username', but found %q!", c.UserName)
+	}
+	if c.Comm.SSHUsername != "override_username" {
+		t.Errorf("Expected 'c.Comm.SSHUsername' to be set to 'override_username', but found %q!", c.Comm.SSHUsername)
+	}
+
+	// Winrm comm
+	c = Config{}
+	builderValues = getArmBuilderConfiguration()
+	builderValues["communicator"] = "winrm"
+	builderValues["winrm_password"] = "Override_winrm_password1"
+	builderValues["winrm_username"] = "override_winrm_username"
+	_, err = c.Prepare(builderValues, getPackerConfiguration())
+	if err != nil {
+		t.Fatalf("newConfig failed: %s", err)
+	}
+	if c.Password != "Override_winrm_password1" {
+		t.Errorf("Expected 'Password' to be set to 'Override_winrm_password1', but found %q!", c.Password)
+	}
+	if c.Comm.WinRMPassword != "Override_winrm_password1" {
+		t.Errorf("Expected 'c.Comm.WinRMPassword' to be set to 'Override_winrm_password1', but found %q!", c.Comm.SSHPassword)
+	}
+	if c.UserName != "override_winrm_username" {
+		t.Errorf("Expected 'UserName' to be set to 'override_winrm_username', but found %q!", c.UserName)
+	}
+	if c.Comm.WinRMUser != "override_winrm_username" {
+		t.Errorf("Expected 'c.Comm.WinRMUser' to be set to 'override_winrm_username', but found %q!", c.Comm.SSHUsername)
+	}
 }
 
 func TestConfigShouldDefaultVMSizeToStandardA1(t *testing.T) {
@@ -574,7 +595,7 @@ func TestWinRMConfigShouldSetRoundTripDecorator(t *testing.T) {
 	config := getArmBuilderConfiguration()
 	config["communicator"] = "winrm"
 	config["winrm_username"] = "username"
-	config["winrm_password"] = "password"
+	config["winrm_password"] = "Password123"
 
 	var c Config
 	_, err := c.Prepare(config, getPackerConfiguration())
@@ -1920,6 +1941,60 @@ func Test_GivenZoneSupportingResiliency_ConfigValidate_ShouldNotWarn(t *testing.
 	}
 }
 
+func TestConfig_PrepareProvidedWinRMPassword(t *testing.T) {
+	config := getArmBuilderConfiguration()
+	config["communicator"] = "winrm"
+
+	var c Config
+	tc := []struct {
+		name       string
+		password   string
+		shouldFail bool
+	}{
+		{
+			name:       "password should be longer than 8 characters",
+			password:   "packer",
+			shouldFail: true,
+		},
+		{
+			name:       "password should be shorter than 123 characters",
+			password:   "1Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			shouldFail: true,
+		},
+		{
+			name:       "password should have valid size but only lower and upper case letters",
+			password:   "AAAbbbCCC",
+			shouldFail: true,
+		},
+		{
+			name:       "password should have valid size but only digits and upper case letters",
+			password:   "AAA12345",
+			shouldFail: true,
+		},
+		{
+			name:       "password should have valid size, digits, upper and lower case letters",
+			password:   "AAA12345bbb",
+			shouldFail: false,
+		},
+		{
+			name:       "password should have valid size, digits, special characters and lower case letters",
+			password:   "//12345bbb",
+			shouldFail: false,
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.name, func(t *testing.T) {
+			config["winrm_password"] = tt.password
+			_, err := c.Prepare(config)
+			fail := err != nil
+			if tt.shouldFail != fail {
+				t.Fatalf("bad: %s. Expected fail is: %t but it was %t", tt.name, tt.shouldFail, fail)
+			}
+		})
+	}
+}
+
 func getArmBuilderConfiguration() map[string]string {
 	m := make(map[string]string)
 	for _, v := range requiredConfigValues {

common/random/string.go

@@ -7,9 +7,10 @@ import (
 )
 
 var (
-	PossibleNumbers   = "0123456789"
+	PossibleNumbers          = "0123456789"
-	PossibleLowerCase = "abcdefghijklmnopqrstuvwxyz"
+	PossibleLowerCase        = "abcdefghijklmnopqrstuvwxyz"
-	PossibleUpperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	PossibleUpperCase        = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+	PossibleSpecialCharacter = " !\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"
 
 	PossibleAlphaNum      = PossibleNumbers + PossibleLowerCase + PossibleUpperCase
 	PossibleAlphaNumLower = PossibleNumbers + PossibleLowerCase