From da23d8b54395340eba4d8bd5daa0e2e952c63c7e Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Tue, 24 May 2022 21:20:48 -0400 Subject: [PATCH 1/5] Update go-getter client options to disable symlinks --- multistep/commonsteps/step_download.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/multistep/commonsteps/step_download.go b/multistep/commonsteps/step_download.go index ae802a9cd..ea296b495 100644 --- a/multistep/commonsteps/step_download.go +++ b/multistep/commonsteps/step_download.go @@ -56,7 +56,8 @@ type StepDownload struct { } var defaultGetterClient = getter.Client{ - Getters: getter.Getters, + Getters: getter.Getters, + DisableSymlinks: true, } func init() { From ee2fe87a4e822607c47d6a0777503dde09e9c891 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Wed, 1 Jun 2022 15:21:33 -0400 Subject: [PATCH 2/5] Set resonable timeouts for remote storage getters --- multistep/commonsteps/step_download.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/multistep/commonsteps/step_download.go b/multistep/commonsteps/step_download.go index ea296b495..6bfe13572 100644 --- a/multistep/commonsteps/step_download.go +++ b/multistep/commonsteps/step_download.go @@ -11,6 +11,7 @@ import ( "path/filepath" "runtime" "strings" + "time" gcs "github.com/hashicorp/go-getter/gcs/v2" s3 "github.com/hashicorp/go-getter/s3/v2" @@ -61,8 +62,14 @@ var defaultGetterClient = getter.Client{ } func init() { - defaultGetterClient.Getters = append(defaultGetterClient.Getters, new(gcs.Getter)) - defaultGetterClient.Getters = append(defaultGetterClient.Getters, new(s3.Getter)) + gcsGetter := gcs.Getter{ + Timeout: 5 * time.Minute, + } + defaultGetterClient.Getters = append(defaultGetterClient.Getters, &gcsGetter) + s3Getter := s3.Getter{ + Timeout: 5 * time.Minute, + } + defaultGetterClient.Getters = append(defaultGetterClient.Getters, &s3Getter) } func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { From ffa44ec532c9c55313ae967f7148d6c8b9becf86 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Thu, 2 Jun 2022 06:30:51 -0400 Subject: [PATCH 3/5] Replace default Getters with explicit list of Getters --- multistep/commonsteps/step_download.go | 44 +++++++++++++++++++------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/multistep/commonsteps/step_download.go b/multistep/commonsteps/step_download.go index 6bfe13572..736447f5a 100644 --- a/multistep/commonsteps/step_download.go +++ b/multistep/commonsteps/step_download.go @@ -57,19 +57,39 @@ type StepDownload struct { } var defaultGetterClient = getter.Client{ - Getters: getter.Getters, + // Disable writing and reading through symlinks DisableSymlinks: true, -} - -func init() { - gcsGetter := gcs.Getter{ - Timeout: 5 * time.Minute, - } - defaultGetterClient.Getters = append(defaultGetterClient.Getters, &gcsGetter) - s3Getter := s3.Getter{ - Timeout: 5 * time.Minute, - } - defaultGetterClient.Getters = append(defaultGetterClient.Getters, &s3Getter) + // The order of the Getters in the list may affect the result + // depending if the Request.Src is detected as valid by multiple getters + Getters: []getter.Getter{ + &getter.GitGetter{ + Timeout: 5 * time.Minute, + Detectors: []getter.Detector{ + new(getter.GitHubDetector), + new(getter.GitDetector), + new(getter.BitBucketDetector), + new(getter.GitLabDetector), + }, + }, + &getter.HgGetter{ + Timeout: 5 * time.Minute, + }, + new(getter.SmbClientGetter), + new(getter.SmbMountGetter), + &getter.HttpGetter{ + Netrc: true, + XTerraformGetDisabled: true, + HeadFirstTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + }, + new(getter.FileGetter), + &gcs.Getter{ + Timeout: 5 * time.Minute, + }, + &s3.Getter{ + Timeout: 5 * time.Minute, + }, + }, } func (s *StepDownload) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { From faee09db4198a07a2f5ef3a3d01c0d2a58024968 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Thu, 2 Jun 2022 16:07:02 -0400 Subject: [PATCH 4/5] Update CHANGELOG --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f176eb518..aa2a2e8c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.3.0 (Upcoming) + +* multistep/commonsteps: Update settings for the default go-getter client to prevent arbitrary host access via go-getter's path traversal, symlink processing, and command injection flaws. +* multistep/commonsteps: Disable support for the `X-Terraform-Get` header to mitigate against protocol switching, endless redirect, and configuration bypass abuse of custom HTTP response header processing. +* multistep/commonsteps: Add default timeouts to the GitGetter, HgGetter, S3Getter, and GcsGetter getters to mitigate against resource exhaustion when calling out to external command line applications. +* sdk: Bump github.com/hashicorp/go-getter/v2, github.com/hashicorp/go-getter/gcs/v2, github.com/hashicorp/go-getter/s3/v2 to address a number of security vulnerabilities as defined in [HCSEC-2022-13](https://discuss.hashicorp.com/t/hcsec-2022-13-multiple-vulnerabilities-in-go-getter-library/39930) + ## 0.2.13 (May 11, 2022) * cmd/packer-sdc: Update golang.org/x/tools to fix internal package errors when running code generation commands with Go 1.18 [GH-108](https://github.com/hashicorp/packer-plugin-sdk/pull/108) From 900cdcf9474e61358e1ee2e399d6f2ea6e134ca9 Mon Sep 17 00:00:00 2001 From: Wilken Rivera Date: Mon, 6 Jun 2022 11:26:21 -0400 Subject: [PATCH 5/5] Bump default to accommodate long downloads --- multistep/commonsteps/step_download.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/multistep/commonsteps/step_download.go b/multistep/commonsteps/step_download.go index 736447f5a..a9c7cd561 100644 --- a/multistep/commonsteps/step_download.go +++ b/multistep/commonsteps/step_download.go @@ -56,14 +56,18 @@ type StepDownload struct { Extension string } +// defaultGetterReadTimeout is the read timeout for downloading operations via go-getter. +// The timeout must be long enough to accommodate large/slow downloads. +const defaultGetterReadTimeout time.Duration = 30 * time.Minute + var defaultGetterClient = getter.Client{ - // Disable writing and reading through symlinks + // Disable writing and reading through symlinks. DisableSymlinks: true, // The order of the Getters in the list may affect the result // depending if the Request.Src is detected as valid by multiple getters Getters: []getter.Getter{ &getter.GitGetter{ - Timeout: 5 * time.Minute, + Timeout: defaultGetterReadTimeout, Detectors: []getter.Detector{ new(getter.GitHubDetector), new(getter.GitDetector), @@ -72,22 +76,22 @@ var defaultGetterClient = getter.Client{ }, }, &getter.HgGetter{ - Timeout: 5 * time.Minute, + Timeout: defaultGetterReadTimeout, }, new(getter.SmbClientGetter), new(getter.SmbMountGetter), &getter.HttpGetter{ Netrc: true, XTerraformGetDisabled: true, - HeadFirstTimeout: 10 * time.Second, - ReadTimeout: 30 * time.Second, + HeadFirstTimeout: defaultGetterReadTimeout, + ReadTimeout: defaultGetterReadTimeout, }, new(getter.FileGetter), &gcs.Getter{ - Timeout: 5 * time.Minute, + Timeout: defaultGetterReadTimeout, }, &s3.Getter{ - Timeout: 5 * time.Minute, + Timeout: defaultGetterReadTimeout, }, }, }