Skip to content

Commit

Permalink
Additional bucket name validation for S3 bucket regex (#20887) (#21199)
Browse files Browse the repository at this point in the history
## 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 43354ff)

Co-authored-by: Ravi Naik <ravinaik1312@gmail.com>
  • Loading branch information
kvch and ravinaik1312 committed Sep 21, 2020
1 parent 95e7e11 commit c925f58
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,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

Expand Down
6 changes: 6 additions & 0 deletions x-pack/functionbeat/provider/aws/aws/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
30 changes: 30 additions & 0 deletions x-pack/functionbeat/provider/aws/aws/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit c925f58

Please sign in to comment.