From 02664c33c574ae75d06dda48c4ef57c5ed32c2df 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

(cherry picked from commit 43354ffb18707e4b59a02f3fc5a6871b0b33703b)
---
 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 8ab50e22981..be16635cff8 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -414,6 +414,7 @@ field. You can revert this change by configuring tags for the module and omittin
 - 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 604035522b5..932b8a1bc52 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 ac8e325804e..ef1045f188e 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) {