From 43354ffb18707e4b59a02f3fc5a6871b0b33703b Mon Sep 17 00:00:00 2001
From: Ravi Naik <ravinaik1312@gmail.com>
Date: Mon, 21 Sep 2020 05:57:28 -0700
Subject: [PATCH] Additional bucket name validation for S3 bucket regex
 (#20887)

## What does this PR do?

This is a bug fix when functionbeat errors out with a blanket error message. According to me, this error can be mitigated pretty early by validating the S3 bucket name by creating the regex patterns from the rules for bucket naming.
https://docs.aws.amazon.com/AmazonS3/latest/dev/BucketRestrictions.html#bucketnamingrules

## Why is it important?

This will restrict users from shooting themselves in their foot when they pass an incorrect bucket name in the functionbeat configuration.

## Related issues

Closes #17572
---
 CHANGELOG.next.asciidoc                       |  1 +
 .../functionbeat/provider/aws/aws/config.go   |  6 ++++
 .../provider/aws/aws/config_test.go           | 30 +++++++++++++++++++
 3 files changed, 37 insertions(+)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 6a938dc8ba9f..2e89512f7223 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -364,6 +364,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix timeout option of GCP functions. {issue}16282[16282] {pull}16287[16287]
 - Do not need Google credentials if not required for the operation. {issue}17329[17329] {pull}21072[21072]
 - Fix dependency issues of GCP functions. {issue}20830[20830] {pull}21070[21070]
+- Fix catchall bucket config errors by adding more validation. {issue}17572[16282] {pull}20887[16287]
 
 ==== Added
 
diff --git a/x-pack/functionbeat/provider/aws/aws/config.go b/x-pack/functionbeat/provider/aws/aws/config.go
index 604035522b5d..932b8a1bc521 100644
--- a/x-pack/functionbeat/provider/aws/aws/config.go
+++ b/x-pack/functionbeat/provider/aws/aws/config.go
@@ -153,6 +153,12 @@ func (b *bucket) Unpack(s string) error {
 		return fmt.Errorf("bucket name '%s' is too short, name need to be at least %d chars long", s, min)
 	}
 
+	const bucketNamePattern = "^[a-z0-9][a-z0-9.\\-]{1,61}[a-z0-9]$"
+	var bucketRE = regexp.MustCompile(bucketNamePattern)
+	if !bucketRE.MatchString(s) {
+		return fmt.Errorf("invalid bucket name: '%s', bucket name must match pattern: '%s'", s, bucketNamePattern)
+	}
+
 	*b = bucket(s)
 	return nil
 }
diff --git a/x-pack/functionbeat/provider/aws/aws/config_test.go b/x-pack/functionbeat/provider/aws/aws/config_test.go
index ac8e325804ef..ef1045f188ed 100644
--- a/x-pack/functionbeat/provider/aws/aws/config_test.go
+++ b/x-pack/functionbeat/provider/aws/aws/config_test.go
@@ -66,6 +66,36 @@ func TestBucket(t *testing.T) {
 		err := b.Unpack("he")
 		assert.Error(t, err)
 	})
+
+	t.Run("bucket regex pattern, disallows semi-colon", func(t *testing.T) {
+		b := bucket("")
+		err := b.Unpack("asdfdaf;dfadsfadsf")
+		assert.Error(t, err)
+	})
+
+	t.Run("bucket regex pattern, disallows slash", func(t *testing.T) {
+		b := bucket("")
+		err := b.Unpack("asdfdaf/dfadsfadsf")
+		assert.Error(t, err)
+	})
+
+	t.Run("bucket regex pattern, allows dots", func(t *testing.T) {
+		b := bucket("")
+		err := b.Unpack("this.is.a.bucket")
+		if !assert.NoError(t, err) {
+			return
+		}
+		assert.Equal(t, bucket("this.is.a.bucket"), b)
+	})
+
+	t.Run("bucket regex pattern, allows hyphens", func(t *testing.T) {
+		b := bucket("")
+		err := b.Unpack("this-is-a-bucket")
+		if !assert.NoError(t, err) {
+			return
+		}
+		assert.Equal(t, bucket("this-is-a-bucket"), b)
+	})
 }
 
 func TestNormalize(t *testing.T) {