From 250da0ab49d9f4a15a024dcd550537c3f187ad8e Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 13 Aug 2018 17:01:13 -0700 Subject: [PATCH 1/3] fix security hole with ami filter --- builder/amazon/common/run_config.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/builder/amazon/common/run_config.go b/builder/amazon/common/run_config.go index 9467d42bd..dbafd0f38 100644 --- a/builder/amazon/common/run_config.go +++ b/builder/amazon/common/run_config.go @@ -25,6 +25,10 @@ func (d *AmiFilterOptions) Empty() bool { return len(d.Owners) == 0 && len(d.Filters) == 0 } +func (d *AmiFilterOptions) NoOwner() bool { + return len(d.Owners) == 0 +} + // RunConfig contains configuration for running an instance from a source // AMI and details on how to access that launched image. type RunConfig struct { @@ -101,6 +105,10 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { errs = append(errs, fmt.Errorf("A source_ami or source_ami_filter must be specified")) } + if c.SourceAmi == "" && c.SourceAmiFilter.NoOwner() { + errs = append(errs, fmt.Errorf("For security reasons, your source AMI filter must declare an owner.")) + } + if c.InstanceType == "" { errs = append(errs, fmt.Errorf("An instance_type must be specified")) } From 71cad4f2a91b67dd150264cc13f674035016e7fb Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 14 Aug 2018 12:30:05 -0700 Subject: [PATCH 2/3] fix tests --- builder/amazon/common/run_config_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/builder/amazon/common/run_config_test.go b/builder/amazon/common/run_config_test.go index fde9ef760..f43b9f12c 100644 --- a/builder/amazon/common/run_config_test.go +++ b/builder/amazon/common/run_config_test.go @@ -55,18 +55,28 @@ func TestRunConfigPrepare_InstanceType(t *testing.T) { func TestRunConfigPrepare_SourceAmi(t *testing.T) { c := testConfig() c.SourceAmi = "" - if err := c.Prepare(nil); len(err) != 1 { + if err := c.Prepare(nil); len(err) != 2 { t.Fatalf("Should error if a source_ami (or source_ami_filter) is not specified") } } func TestRunConfigPrepare_SourceAmiFilterBlank(t *testing.T) { c := testConfigFilter() - if err := c.Prepare(nil); len(err) != 1 { + if err := c.Prepare(nil); len(err) != 2 { t.Fatalf("Should error if source_ami_filter is empty or not specified (and source_ami is not specified)") } } +func TestRunConfigPrepare_SourceAmiFilterOwnersBlank(t *testing.T) { + c := testConfigFilter() + filter_key := "name" + filter_value := "foo" + c.SourceAmiFilter = AmiFilterOptions{Filters: map[*string]*string{&filter_key: &filter_value}} + if err := c.Prepare(nil); len(err) != 1 { + t.Fatalf("Should error if Owners is not specified)") + } +} + func TestRunConfigPrepare_SourceAmiFilterGood(t *testing.T) { c := testConfigFilter() owner := "123" From d57b599f3c83721750bc78ca58f862ffb840bf0a Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 14 Aug 2018 12:33:09 -0700 Subject: [PATCH 3/3] update docs --- website/source/docs/builders/amazon-chroot.html.md | 2 +- website/source/docs/builders/amazon-ebs.html.md | 2 +- website/source/docs/builders/amazon-ebssurrogate.html.md | 2 +- website/source/docs/builders/amazon-ebsvolume.html.md | 2 +- website/source/docs/builders/amazon-instance.html.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/website/source/docs/builders/amazon-chroot.html.md b/website/source/docs/builders/amazon-chroot.html.md index 116fd9ae5..82d88a053 100644 --- a/website/source/docs/builders/amazon-chroot.html.md +++ b/website/source/docs/builders/amazon-chroot.html.md @@ -300,7 +300,7 @@ each category, the available configuration keys are alphabetized. is valid. - `owners` (array of strings) - This scopes the AMIs to certain Amazon account IDs. - This is helpful to limit the AMIs to a trusted third party, or to your own account. + This is a required option, necessary to limit the AMIs your account or a trusted third party. - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build. diff --git a/website/source/docs/builders/amazon-ebs.html.md b/website/source/docs/builders/amazon-ebs.html.md index 944691794..f238d5190 100644 --- a/website/source/docs/builders/amazon-ebs.html.md +++ b/website/source/docs/builders/amazon-ebs.html.md @@ -319,7 +319,7 @@ builder. is valid. - `owners` (array of strings) - This scopes the AMIs to certain Amazon account IDs. - This is helpful to limit the AMIs to a trusted third party, or to your own account. + This is a required option, necessary to limit the AMIs your account or a trusted third party. - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build. diff --git a/website/source/docs/builders/amazon-ebssurrogate.html.md b/website/source/docs/builders/amazon-ebssurrogate.html.md index 881cce45e..d2672be0d 100644 --- a/website/source/docs/builders/amazon-ebssurrogate.html.md +++ b/website/source/docs/builders/amazon-ebssurrogate.html.md @@ -312,7 +312,7 @@ builder. is valid. - `owners` (array of strings) - This scopes the AMIs to certain Amazon account IDs. - This is helpful to limit the AMIs to a trusted third party, or to your own account. + This is a required option, necessary to limit the AMIs your account or a trusted third party. - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build. diff --git a/website/source/docs/builders/amazon-ebsvolume.html.md b/website/source/docs/builders/amazon-ebsvolume.html.md index a3ff74785..26306a1ef 100644 --- a/website/source/docs/builders/amazon-ebsvolume.html.md +++ b/website/source/docs/builders/amazon-ebsvolume.html.md @@ -240,7 +240,7 @@ builder. is valid. - `owners` (array of strings) - This scopes the AMIs to certain Amazon account IDs. - This is helpful to limit the AMIs to a trusted third party, or to your own account. + This is a required option, necessary to limit the AMIs your account or a trusted third party. - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build. diff --git a/website/source/docs/builders/amazon-instance.html.md b/website/source/docs/builders/amazon-instance.html.md index 00700dfb5..fa0a8d3e0 100644 --- a/website/source/docs/builders/amazon-instance.html.md +++ b/website/source/docs/builders/amazon-instance.html.md @@ -312,7 +312,7 @@ builder. is valid. - `owners` (array of strings) - This scopes the AMIs to certain Amazon account IDs. - This is helpful to limit the AMIs to a trusted third party, or to your own account. + This is a required option, necessary to limit the AMIs your account or a trusted third party. - `most_recent` (boolean) - Selects the newest created image when true. This is most useful for selecting a daily distro build.