From 5fd68374fe055ede5ef1ecdf9f6104edc88a0830 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Thu, 16 Jul 2020 19:44:01 +0000 Subject: [PATCH 1/3] Optionally disable password authentication for Linux builds --- builder/azure/common/template/template_builder.go | 7 ++++++- ...plate_builder_test.TestBuildLinux00.approved.json | 2 +- ...plate_builder_test.TestBuildLinux02.approved.json | 2 +- ...late_builder_test.TestSetIdentity00.approved.json | 2 +- .../azure/common/template/template_builder_test.go | 12 ++++++------ 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/builder/azure/common/template/template_builder.go b/builder/azure/common/template/template_builder.go index 9c2e02d32..3974511a3 100644 --- a/builder/azure/common/template/template_builder.go +++ b/builder/azure/common/template/template_builder.go @@ -43,7 +43,7 @@ func NewTemplateBuilder(template string) (*TemplateBuilder, error) { }, nil } -func (s *TemplateBuilder) BuildLinux(sshAuthorizedKey string) error { +func (s *TemplateBuilder) BuildLinux(sshAuthorizedKey string, disablePasswordAuthentication bool) error { resource, err := s.getResourceByType(resourceVirtualMachine) if err != nil { return err @@ -61,6 +61,11 @@ func (s *TemplateBuilder) BuildLinux(sshAuthorizedKey string) error { }, } + if disablePasswordAuthentication { + profile.LinuxConfiguration.DisablePasswordAuthentication = to.BoolPtr(true) + profile.AdminPassword = nil + } + s.osType = compute.Linux return nil } diff --git a/builder/azure/common/template/template_builder_test.TestBuildLinux00.approved.json b/builder/azure/common/template/template_builder_test.TestBuildLinux00.approved.json index de63373bb..403de815c 100644 --- a/builder/azure/common/template/template_builder_test.TestBuildLinux00.approved.json +++ b/builder/azure/common/template/template_builder_test.TestBuildLinux00.approved.json @@ -126,10 +126,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/common/template/template_builder_test.TestBuildLinux02.approved.json b/builder/azure/common/template/template_builder_test.TestBuildLinux02.approved.json index f7f3bed74..fc1441816 100644 --- a/builder/azure/common/template/template_builder_test.TestBuildLinux02.approved.json +++ b/builder/azure/common/template/template_builder_test.TestBuildLinux02.approved.json @@ -87,10 +87,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/common/template/template_builder_test.TestSetIdentity00.approved.json b/builder/azure/common/template/template_builder_test.TestSetIdentity00.approved.json index a182d7e8f..606ebadba 100644 --- a/builder/azure/common/template/template_builder_test.TestSetIdentity00.approved.json +++ b/builder/azure/common/template/template_builder_test.TestSetIdentity00.approved.json @@ -132,10 +132,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/common/template/template_builder_test.go b/builder/azure/common/template/template_builder_test.go index 2532bcc06..386f13ef4 100644 --- a/builder/azure/common/template/template_builder_test.go +++ b/builder/azure/common/template/template_builder_test.go @@ -15,7 +15,7 @@ func TestBuildLinux00(t *testing.T) { t.Fatal(err) } - err = testSubject.BuildLinux("--test-ssh-authorized-key--") + err = testSubject.BuildLinux("--test-ssh-authorized-key--", true) if err != nil { t.Fatal(err) } @@ -43,7 +43,7 @@ func TestBuildLinux01(t *testing.T) { t.Fatal(err) } - err = testSubject.BuildLinux("--test-ssh-authorized-key--") + err = testSubject.BuildLinux("--test-ssh-authorized-key--", false) if err != nil { t.Fatal(err) } @@ -71,7 +71,7 @@ func TestBuildLinux02(t *testing.T) { t.Fatal(err) } - testSubject.BuildLinux("--test-ssh-authorized-key--") + testSubject.BuildLinux("--test-ssh-authorized-key--", true) testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux, compute.CachingTypesReadWrite) testSubject.SetOSDiskSizeGB(100) @@ -189,7 +189,7 @@ func TestSharedImageGallery00(t *testing.T) { t.Fatal(err) } - err = testSubject.BuildLinux("--test-ssh-authorized-key--") + err = testSubject.BuildLinux("--test-ssh-authorized-key--", false) if err != nil { t.Fatal(err) } @@ -218,7 +218,7 @@ func TestNetworkSecurityGroup00(t *testing.T) { t.Fatal(err) } - err = testSubject.BuildLinux("--test-ssh-authorized-key--") + err = testSubject.BuildLinux("--test-ssh-authorized-key--", false) if err != nil { t.Fatal(err) } @@ -251,7 +251,7 @@ func TestSetIdentity00(t *testing.T) { t.Fatal(err) } - if err = testSubject.BuildLinux("--test-ssh-authorized-key--"); err != nil { + if err = testSubject.BuildLinux("--test-ssh-authorized-key--", true); err != nil { t.Fatal(err) } From deca28c1583386c6d3fa6dc9a3dad3dbc519ff95 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Thu, 16 Jul 2020 22:26:06 +0000 Subject: [PATCH 2/3] Disable password auth on ssh key Linux builds --- builder/azure/arm/config.go | 6 +++--- builder/azure/arm/config_test.go | 12 ++++++++++-- builder/azure/arm/template_factory.go | 2 +- ...mplate_factory_test.TestPlanInfo01.approved.json | 2 +- ...mplate_factory_test.TestPlanInfo02.approved.json | 2 +- ...est.TestVirtualMachineDeployment04.approved.json | 2 +- ...est.TestVirtualMachineDeployment06.approved.json | 2 +- ...est.TestVirtualMachineDeployment08.approved.json | 2 +- ...est.TestVirtualMachineDeployment10.approved.json | 2 +- ...est.TestVirtualMachineDeployment12.approved.json | 2 +- ...est.TestVirtualMachineDeployment14.approved.json | 2 +- builder/azure/arm/template_factory_test.go | 13 ++++++++----- 12 files changed, 30 insertions(+), 19 deletions(-) diff --git a/builder/azure/arm/config.go b/builder/azure/arm/config.go index 5e6e8d1e7..4f31bdbeb 100644 --- a/builder/azure/arm/config.go +++ b/builder/azure/arm/config.go @@ -697,10 +697,10 @@ func setUserNamePassword(c *Config) error { } c.UserName = c.Comm.SSHUsername - if c.Comm.SSHPassword == "" { - c.Comm.SSHPassword = c.Password + // if user has an explicit wish to use an SSH password, we'll set it + if c.Comm.SSHPassword != "" { + c.Password = c.Comm.SSHPassword } - c.Password = c.Comm.SSHPassword if c.Comm.Type == "ssh" { return nil diff --git a/builder/azure/arm/config_test.go b/builder/azure/arm/config_test.go index fa03ecd97..3f6df81ff 100644 --- a/builder/azure/arm/config_test.go +++ b/builder/azure/arm/config_test.go @@ -71,8 +71,8 @@ func TestConfigUserNameOverride(t *testing.T) { if c.Password != c.tmpAdminPassword { t.Errorf("Expected 'Password' to be set to generated password, but found %q!", c.Password) } - if c.Comm.SSHPassword != c.tmpAdminPassword { - t.Errorf("Expected 'c.Comm.SSHPassword' to be set to generated password, but found %q!", c.Comm.SSHPassword) + if c.Comm.SSHPassword != "" { + t.Errorf("Expected 'c.Comm.SSHPassword' to be empty, 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) @@ -2093,6 +2093,14 @@ func getPackerCommunicatorConfiguration() map[string]string { return config } +func getPackerSSHPasswordCommunicatorConfiguration() map[string]string { + config := map[string]string{ + "ssh_password": "superS3cret", + } + + return config +} + func TestConfigShouldRejectMalformedUserAssignedManagedIdentities(t *testing.T) { config := map[string]interface{}{ "capture_name_prefix": "ignore", diff --git a/builder/azure/arm/template_factory.go b/builder/azure/arm/template_factory.go index dd2ae5637..3dbe8902e 100644 --- a/builder/azure/arm/template_factory.go +++ b/builder/azure/arm/template_factory.go @@ -55,7 +55,7 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error) switch config.OSType { case constants.Target_Linux: - builder.BuildLinux(config.sshAuthorizedKey) + builder.BuildLinux(config.sshAuthorizedKey, config.Comm.SSHPassword == "") // if ssh password is not explicitly specified, disable password auth case constants.Target_Windows: osType = compute.Windows builder.BuildWindows(config.tmpKeyVaultName, config.tmpWinRMCertificateUrl) diff --git a/builder/azure/arm/template_factory_test.TestPlanInfo01.approved.json b/builder/azure/arm/template_factory_test.TestPlanInfo01.approved.json index eb841ab4d..1b9d40d93 100644 --- a/builder/azure/arm/template_factory_test.TestPlanInfo01.approved.json +++ b/builder/azure/arm/template_factory_test.TestPlanInfo01.approved.json @@ -149,10 +149,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestPlanInfo02.approved.json b/builder/azure/arm/template_factory_test.TestPlanInfo02.approved.json index 9a9f6e523..20eda3180 100644 --- a/builder/azure/arm/template_factory_test.TestPlanInfo02.approved.json +++ b/builder/azure/arm/template_factory_test.TestPlanInfo02.approved.json @@ -153,10 +153,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment04.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment04.approved.json index 24d96726a..4c69b352a 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment04.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment04.approved.json @@ -126,10 +126,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment06.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment06.approved.json index 0f29794ce..74774714a 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment06.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment06.approved.json @@ -141,10 +141,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment08.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment08.approved.json index f411e5a6e..bd1445920 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment08.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment08.approved.json @@ -126,10 +126,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment10.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment10.approved.json index c330259ae..d87964955 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment10.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment10.approved.json @@ -104,10 +104,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment12.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment12.approved.json index 77c10f0af..c6addd27a 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment12.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment12.approved.json @@ -126,10 +126,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment14.approved.json b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment14.approved.json index 1b80997a0..36182b82d 100644 --- a/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment14.approved.json +++ b/builder/azure/arm/template_factory_test.TestVirtualMachineDeployment14.approved.json @@ -127,10 +127,10 @@ ] }, "osProfile": { - "adminPassword": "[parameters('adminPassword')]", "adminUsername": "[parameters('adminUsername')]", "computerName": "[parameters('vmName')]", "linuxConfiguration": { + "disablePasswordAuthentication": true, "ssh": { "publicKeys": [ { diff --git a/builder/azure/arm/template_factory_test.go b/builder/azure/arm/template_factory_test.go index 82b9b1cc9..f0e026a26 100644 --- a/builder/azure/arm/template_factory_test.go +++ b/builder/azure/arm/template_factory_test.go @@ -108,7 +108,10 @@ func TestVirtualMachineDeployment03(t *testing.T) { m["image_version"] = "ImageVersion" var c Config - c.Prepare(m, getPackerConfiguration()) + _, err := c.Prepare(m, getPackerConfiguration(), getPackerSSHPasswordCommunicatorConfiguration()) + if err != nil { + t.Fatal(err) + } deployment, err := GetVirtualMachineDeployment(&c) if err != nil { t.Fatal(err) @@ -168,7 +171,7 @@ func TestVirtualMachineDeployment05(t *testing.T) { } var c Config - _, err := c.Prepare(config, getPackerConfiguration()) + _, err := c.Prepare(config, getPackerConfiguration(), getPackerSSHPasswordCommunicatorConfiguration()) if err != nil { t.Fatal(err) } @@ -235,7 +238,7 @@ func TestVirtualMachineDeployment07(t *testing.T) { } var c Config - _, err := c.Prepare(config, getPackerConfiguration()) + _, err := c.Prepare(config, getPackerConfiguration(), getPackerSSHPasswordCommunicatorConfiguration()) if err != nil { t.Fatal(err) } @@ -312,7 +315,7 @@ func TestVirtualMachineDeployment09(t *testing.T) { } var c Config - _, err := c.Prepare(config, getPackerConfiguration()) + _, err := c.Prepare(config, getPackerConfiguration(), getPackerSSHPasswordCommunicatorConfiguration()) if err != nil { t.Fatal(err) } @@ -387,7 +390,7 @@ func TestVirtualMachineDeployment11(t *testing.T) { } var c Config - _, err := c.Prepare(config, getPackerConfiguration()) + _, err := c.Prepare(config, getPackerConfiguration(), getPackerSSHPasswordCommunicatorConfiguration()) if err != nil { t.Fatal(err) } From 708ca1c1266871eb577df6a59b9b6c8d650a2adb Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Thu, 16 Jul 2020 23:22:24 +0000 Subject: [PATCH 3/3] Add some error checking --- builder/azure/arm/template_factory.go | 66 +++++++++++++++---- .../common/template/template_builder_test.go | 15 ++++- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/builder/azure/arm/template_factory.go b/builder/azure/arm/template_factory.go index 3dbe8902e..e8ee00f0b 100644 --- a/builder/azure/arm/template_factory.go +++ b/builder/azure/arm/template_factory.go @@ -55,10 +55,16 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error) switch config.OSType { case constants.Target_Linux: - builder.BuildLinux(config.sshAuthorizedKey, config.Comm.SSHPassword == "") // if ssh password is not explicitly specified, disable password auth + err = builder.BuildLinux(config.sshAuthorizedKey, config.Comm.SSHPassword == "") // if ssh password is not explicitly specified, disable password auth + if err != nil { + return nil, err + } case constants.Target_Windows: osType = compute.Windows - builder.BuildWindows(config.tmpKeyVaultName, config.tmpWinRMCertificateUrl) + err = builder.BuildWindows(config.tmpKeyVaultName, config.tmpWinRMCertificateUrl) + if err != nil { + return nil, err + } } if len(config.UserAssignedManagedIdentities) != 0 { @@ -68,9 +74,15 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error) } if config.ImageUrl != "" { - builder.SetImageUrl(config.ImageUrl, osType, config.diskCachingType) + err = builder.SetImageUrl(config.ImageUrl, osType, config.diskCachingType) + if err != nil { + return nil, err + } } else if config.CustomManagedImageName != "" { - builder.SetManagedDiskUrl(config.customManagedImageID, config.managedImageStorageAccountType, config.diskCachingType) + err = builder.SetManagedDiskUrl(config.customManagedImageID, config.managedImageStorageAccountType, config.diskCachingType) + if err != nil { + return nil, err + } } else if config.ManagedImageName != "" && config.ImagePublisher != "" { imageID := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Compute/locations/%s/publishers/%s/ArtifactTypes/vmimage/offers/%s/skus/%s/versions/%s", config.ClientConfig.SubscriptionID, @@ -92,38 +104,62 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error) config.SharedGallery.ImageVersion) } - builder.SetSharedGalleryImage(config.Location, imageID, config.diskCachingType) + err = builder.SetSharedGalleryImage(config.Location, imageID, config.diskCachingType) + if err != nil { + return nil, err + } } else { - builder.SetMarketPlaceImage(config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion, config.diskCachingType) + err = builder.SetMarketPlaceImage(config.ImagePublisher, config.ImageOffer, config.ImageSku, config.ImageVersion, config.diskCachingType) + if err != nil { + return nil, err + } } if config.OSDiskSizeGB > 0 { - builder.SetOSDiskSizeGB(config.OSDiskSizeGB) + err = builder.SetOSDiskSizeGB(config.OSDiskSizeGB) + if err != nil { + return nil, err + } } if len(config.AdditionalDiskSize) > 0 { isManaged := config.CustomManagedImageName != "" || (config.ManagedImageName != "" && config.ImagePublisher != "") || config.SharedGallery.Subscription != "" - builder.SetAdditionalDisks(config.AdditionalDiskSize, config.tmpDataDiskName, isManaged, config.diskCachingType) + err = builder.SetAdditionalDisks(config.AdditionalDiskSize, config.tmpDataDiskName, isManaged, config.diskCachingType) + if err != nil { + return nil, err + } } if config.customData != "" { - builder.SetCustomData(config.customData) + err = builder.SetCustomData(config.customData) + if err != nil { + return nil, err + } } if config.PlanInfo.PlanName != "" { - builder.SetPlanInfo(config.PlanInfo.PlanName, config.PlanInfo.PlanProduct, config.PlanInfo.PlanPublisher, config.PlanInfo.PlanPromotionCode) + err = builder.SetPlanInfo(config.PlanInfo.PlanName, config.PlanInfo.PlanProduct, config.PlanInfo.PlanPublisher, config.PlanInfo.PlanPromotionCode) + if err != nil { + return nil, err + } } if config.VirtualNetworkName != "" && DefaultPrivateVirtualNetworkWithPublicIp != config.PrivateVirtualNetworkWithPublicIp { - builder.SetPrivateVirtualNetworkWithPublicIp( + err = builder.SetPrivateVirtualNetworkWithPublicIp( config.VirtualNetworkResourceGroupName, config.VirtualNetworkName, config.VirtualNetworkSubnetName) + if err != nil { + return nil, err + } } else if config.VirtualNetworkName != "" { - builder.SetVirtualNetwork( + err = builder.SetVirtualNetwork( config.VirtualNetworkResourceGroupName, config.VirtualNetworkName, config.VirtualNetworkSubnetName) + if err != nil { + return nil, err + } } if config.AllowedInboundIpAddresses != nil && len(config.AllowedInboundIpAddresses) >= 1 && config.Comm.Port() != 0 { @@ -140,7 +176,11 @@ func GetVirtualMachineDeployment(config *Config) (*resources.Deployment, error) } } - builder.SetTags(&config.AzureTags) + err = builder.SetTags(&config.AzureTags) + if err != nil { + return nil, err + } + doc, _ := builder.ToJSON() return createDeploymentParameters(*doc, params) } diff --git a/builder/azure/common/template/template_builder_test.go b/builder/azure/common/template/template_builder_test.go index 386f13ef4..94a4b3673 100644 --- a/builder/azure/common/template/template_builder_test.go +++ b/builder/azure/common/template/template_builder_test.go @@ -71,9 +71,18 @@ func TestBuildLinux02(t *testing.T) { t.Fatal(err) } - testSubject.BuildLinux("--test-ssh-authorized-key--", true) - testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux, compute.CachingTypesReadWrite) - testSubject.SetOSDiskSizeGB(100) + err = testSubject.BuildLinux("--test-ssh-authorized-key--", true) + if err != nil { + t.Fatal(err) + } + err = testSubject.SetImageUrl("http://azure/custom.vhd", compute.Linux, compute.CachingTypesReadWrite) + if err != nil { + t.Fatal(err) + } + err = testSubject.SetOSDiskSizeGB(100) + if err != nil { + t.Fatal(err) + } err = testSubject.SetVirtualNetwork("--virtual-network-resource-group--", "--virtual-network--", "--subnet-name--") if err != nil {