From 6383e6cbbf75afb67eac72b613f76092c385df14 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Jul 2020 15:09:49 -0700 Subject: [PATCH 1/5] fix vsphere postprocessor password log filtering, write tests --- post-processor/vsphere/post-processor.go | 8 +++--- post-processor/vsphere/post-processor_test.go | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index eede07c0d..22f1b5f10 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -193,11 +193,13 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact func filterLog(s string, u *url.URL) string { password, passwordSet := u.User.Password() - if passwordSet && password != "" { - return strings.Replace(s, password, "", -1) + if !passwordSet || password == "" { + return s } + encodedUserPassword := strings.Split(u.User.String(), ":") + encodedPassword := encodedUserPassword[len(encodedUserPassword)-1] - return s + return strings.Replace(s, encodedPassword, "", -1) } func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) { diff --git a/post-processor/vsphere/post-processor_test.go b/post-processor/vsphere/post-processor_test.go index 8a14357d3..5cb8f707d 100644 --- a/post-processor/vsphere/post-processor_test.go +++ b/post-processor/vsphere/post-processor_test.go @@ -101,3 +101,30 @@ func TestGenerateURI_PasswordEscapes(t *testing.T) { } } } + +func TestFilterLogs(t *testing.T) { + + // Password is encoded, and contains a colon + ovftool_uri := fmt.Sprintf("vi://hostname/Datacenter/host/cluster") + + u, _ := url.Parse(ovftool_uri) + u.User = url.UserPassword("us:ername", "P@ssW:rd") + + logstring := "vi://us%3Aername:P%40ssW%3Ard@hostname/Datacenter/host/cluster" + outstring := filterLog(logstring, u) + expected := "vi://us%3Aername:@hostname/Datacenter/host/cluster" + if outstring != expected { + t.Fatalf("Should have successfully filtered encoded password. Expected: %s; recieved: %s", expected, outstring) + } + + // There is no password + u.User = url.UserPassword("us:ername", "") + + logstring = "vi://us%3Aername:@hostname/Datacenter/host/cluster" + outstring = filterLog(logstring, u) + expected = "vi://us%3Aername:@hostname/Datacenter/host/cluster" + if outstring != expected { + t.Fatalf("Should have ignored password filtering since it was not set. Expected: %s; recieved: %s", expected, outstring) + } + +} From 395a0c472e95b8f2f6c48b9e2240fd4d560c8d1a Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Jul 2020 16:36:12 -0700 Subject: [PATCH 2/5] improve postprocessor stdout --- builder/vmware/common/step_export.go | 2 +- post-processor/vsphere/post-processor.go | 80 ++++++++++++++----- post-processor/vsphere/post-processor_test.go | 22 ++--- .../pages/docs/post-processors/vsphere.mdx | 4 +- 4 files changed, 73 insertions(+), 35 deletions(-) diff --git a/builder/vmware/common/step_export.go b/builder/vmware/common/step_export.go index e60e0ea84..91d3a9552 100644 --- a/builder/vmware/common/step_export.go +++ b/builder/vmware/common/step_export.go @@ -48,7 +48,7 @@ func (s *StepExport) generateArgs(c *DriverConfig, displayName string, hidePassw password := c.RemotePassword if hidePassword { - password = "" + password = "" } u.User = url.UserPassword(c.RemoteUser, password) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index 22f1b5f10..39cc4ee58 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -6,17 +6,17 @@ import ( "bytes" "context" "fmt" - "io" "log" "net/url" - "os" "os/exec" "regexp" "runtime" "strings" + "time" "github.com/hashicorp/hcl/v2/hcldec" "github.com/hashicorp/packer/common" + shelllocal "github.com/hashicorp/packer/common/shell-local" "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" @@ -144,6 +144,17 @@ func (p *PostProcessor) generateURI() (*url.URL, error) { return u, nil } +func getEncodedPassword(u *url.URL) (string, bool) { + // filter password from all logging + password, passwordSet := u.User.Password() + if passwordSet && password != "" { + encodedUserPassword := strings.Split(u.User.String(), ":") + encodedPassword := encodedUserPassword[len(encodedUserPassword)-1] + return encodedPassword, true + } + return password, false +} + func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, bool, error) { source := "" for _, path := range artifact.Files() { @@ -161,6 +172,10 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact if err != nil { return nil, false, false, err } + encodedPassword, isSet := getEncodedPassword(ovftool_uri) + if isSet { + packer.LogSecretFilter.Set(encodedPassword) + } args, err := p.BuildArgs(source, ovftool_uri.String()) if err != nil { @@ -169,37 +184,58 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact ui.Message(fmt.Sprintf("Uploading %s to vSphere", source)) - log.Printf("Starting ovftool with parameters: %s", - filterLog(strings.Join(args, " "), ovftool_uri)) + log.Printf("Starting ovftool with parameters: %s", strings.Join(args, " ")) - var errWriter io.Writer - var errOut bytes.Buffer - cmd := exec.Command(ovftool, args...) - errWriter = io.MultiWriter(os.Stderr, &errOut) - cmd.Stdout = os.Stdout - cmd.Stderr = errWriter - - if err := cmd.Run(); err != nil { - err := fmt.Errorf("Error uploading virtual machine: %s\n%s\n", err, filterLog(errOut.String(), ovftool_uri)) + ui.Message("Validating Username and Password with dry-run") + err = p.ValidateOvfTool(args, ovftool) + if err != nil { return nil, false, false, err } - ui.Message(filterLog(errOut.String(), ovftool_uri)) + // Validation has passed, so run for real. + ui.Message("Calling OVFtool to upload vm") + commandAndArgs := []string{ovftool} + commandAndArgs = append(commandAndArgs, args...) + comm := &shelllocal.Communicator{ + ExecuteCommand: commandAndArgs, + } + flattenedCmd := strings.Join(commandAndArgs, " ") + cmd := &packer.RemoteCmd{Command: flattenedCmd} + log.Printf("[INFO] (vsphere): starting ovftool command: %s", flattenedCmd) + if err := cmd.RunWithUi(ctx, comm, ui); err != nil { + return nil, false, false, fmt.Errorf( + "Error uploading virtual machine: Please see output above for more information.") + } artifact = NewArtifact(p.config.Datastore, p.config.VMFolder, p.config.VMName, artifact.Files()) return artifact, false, false, nil } -func filterLog(s string, u *url.URL) string { - password, passwordSet := u.User.Password() - if !passwordSet || password == "" { - return s - } - encodedUserPassword := strings.Split(u.User.String(), ":") - encodedPassword := encodedUserPassword[len(encodedUserPassword)-1] +func (p *PostProcessor) ValidateOvfTool(args []string, ofvtool string) error { + args = append([]string{"--verifyOnly"}, args...) + var out bytes.Buffer + cmdCtx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + cmd := exec.CommandContext(cmdCtx, ovftool, args...) + cmd.Stdout = &out - return strings.Replace(s, encodedPassword, "", -1) + // Need to manually close stdin or else the ofvtool call will hang + // forever in a situation where the user has provided an invalid + // password or username + stdin, _ := cmd.StdinPipe() + defer stdin.Close() + + if err := cmd.Run(); err != nil { + outString := out.String() + if strings.Contains(outString, "Enter login information for") { + err = fmt.Errorf("Error performing OVFtool dry run; the username " + + "or password you provided to ovftool is likely invalid.") + return err + } + return nil + } + return nil } func (p *PostProcessor) BuildArgs(source, ovftool_uri string) ([]string, error) { diff --git a/post-processor/vsphere/post-processor_test.go b/post-processor/vsphere/post-processor_test.go index 5cb8f707d..c666cb625 100644 --- a/post-processor/vsphere/post-processor_test.go +++ b/post-processor/vsphere/post-processor_test.go @@ -102,7 +102,7 @@ func TestGenerateURI_PasswordEscapes(t *testing.T) { } } -func TestFilterLogs(t *testing.T) { +func TestGetEncodedPassword(t *testing.T) { // Password is encoded, and contains a colon ovftool_uri := fmt.Sprintf("vi://hostname/Datacenter/host/cluster") @@ -110,21 +110,21 @@ func TestFilterLogs(t *testing.T) { u, _ := url.Parse(ovftool_uri) u.User = url.UserPassword("us:ername", "P@ssW:rd") - logstring := "vi://us%3Aername:P%40ssW%3Ard@hostname/Datacenter/host/cluster" - outstring := filterLog(logstring, u) - expected := "vi://us%3Aername:@hostname/Datacenter/host/cluster" - if outstring != expected { - t.Fatalf("Should have successfully filtered encoded password. Expected: %s; recieved: %s", expected, outstring) + encoded, isSet := getEncodedPassword(u) + expected := "P%40ssW%3Ard" + if !isSet { + t.Fatalf("Password is set but test said it is not") + } + if encoded != expected { + t.Fatalf("Should have successfully gotten encoded password. Expected: %s; recieved: %s", expected, encoded) } // There is no password u.User = url.UserPassword("us:ername", "") - logstring = "vi://us%3Aername:@hostname/Datacenter/host/cluster" - outstring = filterLog(logstring, u) - expected = "vi://us%3Aername:@hostname/Datacenter/host/cluster" - if outstring != expected { - t.Fatalf("Should have ignored password filtering since it was not set. Expected: %s; recieved: %s", expected, outstring) + _, isSet = getEncodedPassword(u) + if isSet { + t.Fatalf("Should have determined that password was not set") } } diff --git a/website/pages/docs/post-processors/vsphere.mdx b/website/pages/docs/post-processors/vsphere.mdx index 7609380fe..0da5093f1 100644 --- a/website/pages/docs/post-processors/vsphere.mdx +++ b/website/pages/docs/post-processors/vsphere.mdx @@ -22,7 +22,9 @@ each category, the available configuration keys are alphabetized. Required: -- `cluster` (string) - The cluster to upload the VM to. +- `cluster` (string) - The cluster to upload the VM to. If you do not have a + cluster defined, you can instead provide the IP address of the esx host + that you want to upload to. - `datacenter` (string) - The name of the datacenter within vSphere to add the VM to. From c34e89aec769b68922fec8a46c218d98fb33ce82 Mon Sep 17 00:00:00 2001 From: Adrien Delorme Date: Mon, 27 Jul 2020 10:33:39 +0200 Subject: [PATCH 3/5] getEncodedPassword: simplify encodedPassword return --- post-processor/vsphere/post-processor.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index 39cc4ee58..ff6615ebb 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -148,8 +148,7 @@ func getEncodedPassword(u *url.URL) (string, bool) { // filter password from all logging password, passwordSet := u.User.Password() if passwordSet && password != "" { - encodedUserPassword := strings.Split(u.User.String(), ":") - encodedPassword := encodedUserPassword[len(encodedUserPassword)-1] + encodedPassword := strings.Split(u.User.String(), ":")[1] return encodedPassword, true } return password, false From 69b0e66b5d59d78a087aacc9970559fbbba71a4f Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 27 Jul 2020 08:52:29 -0700 Subject: [PATCH 4/5] Update post-processor/vsphere/post-processor.go Co-authored-by: Adrien Delorme --- post-processor/vsphere/post-processor.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index ff6615ebb..1074097db 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -222,7 +222,10 @@ func (p *PostProcessor) ValidateOvfTool(args []string, ofvtool string) error { // Need to manually close stdin or else the ofvtool call will hang // forever in a situation where the user has provided an invalid // password or username - stdin, _ := cmd.StdinPipe() + stdin, err := cmd.StdinPipe() + if err != nil { + err + } defer stdin.Close() if err := cmd.Run(); err != nil { From 02d3fb37e372c271ee9290b194166777107bb04a Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 27 Jul 2020 08:53:33 -0700 Subject: [PATCH 5/5] fix return --- post-processor/vsphere/post-processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/post-processor/vsphere/post-processor.go b/post-processor/vsphere/post-processor.go index 1074097db..c57a9e704 100644 --- a/post-processor/vsphere/post-processor.go +++ b/post-processor/vsphere/post-processor.go @@ -224,7 +224,7 @@ func (p *PostProcessor) ValidateOvfTool(args []string, ofvtool string) error { // password or username stdin, err := cmd.StdinPipe() if err != nil { - err + return err } defer stdin.Close()