Skip to content

Commit 337cb75

Browse files
authored
fix(cloudformation): support of all SSE algorithms for s3 (aquasecurity#6270)
1 parent 9361cdb commit 337cb75

File tree

2 files changed

+184
-18
lines changed

2 files changed

+184
-18
lines changed

pkg/iac/adapters/cloudformation/aws/s3/bucket.go

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
package s3
22

33
import (
4+
"cmp"
45
"regexp"
6+
"slices"
57
"strings"
68

9+
s3types "github.com/aws/aws-sdk-go-v2/service/s3/types"
10+
711
"github.com/aquasecurity/trivy/pkg/iac/providers/aws/s3"
8-
parser2 "github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser"
12+
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser"
913
iacTypes "github.com/aquasecurity/trivy/pkg/iac/types"
1014
)
1115

1216
var aclConvertRegex = regexp.MustCompile(`[A-Z][^A-Z]*`)
1317

14-
func getBuckets(cfFile parser2.FileContext) []s3.Bucket {
18+
func getBuckets(cfFile parser.FileContext) []s3.Bucket {
1519
var buckets []s3.Bucket
1620
bucketResources := cfFile.GetResourcesByType("AWS::S3::Bucket")
1721

@@ -37,10 +41,15 @@ func getBuckets(cfFile parser2.FileContext) []s3.Bucket {
3741

3842
buckets = append(buckets, s3b)
3943
}
44+
45+
slices.SortFunc(buckets, func(a, b s3.Bucket) int {
46+
return cmp.Compare(a.Name.Value(), b.Name.Value())
47+
})
48+
4049
return buckets
4150
}
4251

43-
func getPublicAccessBlock(r *parser2.Resource) *s3.PublicAccessBlock {
52+
func getPublicAccessBlock(r *parser.Resource) *s3.PublicAccessBlock {
4453
if block := r.GetProperty("PublicAccessBlockConfiguration"); block.IsNil() {
4554
return nil
4655
}
@@ -60,8 +69,7 @@ func convertAclValue(aclValue iacTypes.StringValue) iacTypes.StringValue {
6069
return iacTypes.String(strings.ToLower(strings.Join(matches, "-")), aclValue.GetMetadata())
6170
}
6271

63-
func getLogging(r *parser2.Resource) s3.Logging {
64-
72+
func getLogging(r *parser.Resource) s3.Logging {
6573
logging := s3.Logging{
6674
Metadata: r.Metadata(),
6775
Enabled: iacTypes.BoolDefault(false, r.Metadata()),
@@ -77,7 +85,7 @@ func getLogging(r *parser2.Resource) s3.Logging {
7785
return logging
7886
}
7987

80-
func hasVersioning(r *parser2.Resource) iacTypes.BoolValue {
88+
func hasVersioning(r *parser.Resource) iacTypes.BoolValue {
8189
versioningProp := r.GetProperty("VersioningConfiguration.Status")
8290

8391
if versioningProp.IsNil() {
@@ -92,8 +100,7 @@ func hasVersioning(r *parser2.Resource) iacTypes.BoolValue {
92100
return iacTypes.Bool(versioningEnabled, versioningProp.Metadata())
93101
}
94102

95-
func getEncryption(r *parser2.Resource, _ parser2.FileContext) s3.Encryption {
96-
103+
func getEncryption(r *parser.Resource, _ parser.FileContext) s3.Encryption {
97104
encryption := s3.Encryption{
98105
Metadata: r.Metadata(),
99106
Enabled: iacTypes.BoolDefault(false, r.Metadata()),
@@ -103,23 +110,26 @@ func getEncryption(r *parser2.Resource, _ parser2.FileContext) s3.Encryption {
103110

104111
if encryptProps := r.GetProperty("BucketEncryption.ServerSideEncryptionConfiguration"); encryptProps.IsNotNil() {
105112
for _, rule := range encryptProps.AsList() {
106-
if algo := rule.GetProperty("ServerSideEncryptionByDefault.SSEAlgorithm"); algo.EqualTo("AES256") {
107-
encryption.Enabled = iacTypes.Bool(true, algo.Metadata())
108-
} else if kmsKeyProp := rule.GetProperty("ServerSideEncryptionByDefault.KMSMasterKeyID"); !kmsKeyProp.IsEmpty() && kmsKeyProp.IsString() {
109-
encryption.KMSKeyId = kmsKeyProp.AsStringValue()
113+
algo := rule.GetProperty("ServerSideEncryptionByDefault.SSEAlgorithm")
114+
if algo.IsString() {
115+
algoVal := algo.AsString()
116+
isValidAlgo := slices.Contains(s3types.ServerSideEncryption("").Values(), s3types.ServerSideEncryption(algoVal))
117+
encryption.Enabled = iacTypes.Bool(isValidAlgo, algo.Metadata())
118+
encryption.Algorithm = algo.AsStringValue()
110119
}
111-
if encryption.Enabled.IsFalse() {
112-
encryption.Enabled = rule.GetBoolProperty("BucketKeyEnabled", false)
120+
121+
kmsKeyProp := rule.GetProperty("ServerSideEncryptionByDefault.KMSMasterKeyID")
122+
if !kmsKeyProp.IsEmpty() && kmsKeyProp.IsString() {
123+
encryption.KMSKeyId = kmsKeyProp.AsStringValue()
113124
}
114125
}
115126
}
116127

117128
return encryption
118129
}
119130

120-
func getLifecycle(resource *parser2.Resource) []s3.Rules {
121-
LifecycleProp := resource.GetProperty("LifecycleConfiguration")
122-
RuleProp := LifecycleProp.GetProperty("Rules")
131+
func getLifecycle(resource *parser.Resource) []s3.Rules {
132+
RuleProp := resource.GetProperty("LifecycleConfiguration.Rules")
123133

124134
var rule []s3.Rules
125135

@@ -136,7 +146,7 @@ func getLifecycle(resource *parser2.Resource) []s3.Rules {
136146
return rule
137147
}
138148

139-
func getWebsite(r *parser2.Resource) *s3.Website {
149+
func getWebsite(r *parser.Resource) *s3.Website {
140150
if block := r.GetProperty("WebsiteConfiguration"); block.IsNil() {
141151
return nil
142152
} else {
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package s3
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/aquasecurity/trivy/internal/testutil"
8+
"github.com/aquasecurity/trivy/pkg/iac/providers/aws/s3"
9+
"github.com/aquasecurity/trivy/pkg/iac/scanners/cloudformation/parser"
10+
"github.com/aquasecurity/trivy/pkg/iac/types"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestAdapt(t *testing.T) {
15+
tests := []struct {
16+
name string
17+
source string
18+
expected s3.S3
19+
}{
20+
{
21+
name: "complete s3 bucket",
22+
source: `AWSTemplateFormatVersion: 2010-09-09
23+
Resources:
24+
Key:
25+
Type: "AWS::KMS::Key"
26+
LoggingBucket:
27+
Type: AWS::S3::Bucket
28+
Properties:
29+
BucketName: logging-bucket
30+
Bucket:
31+
Type: AWS::S3::Bucket
32+
Properties:
33+
BucketName: test-bucket
34+
BucketEncryption:
35+
ServerSideEncryptionConfiguration:
36+
- ServerSideEncryptionByDefault:
37+
KMSMasterKeyID:
38+
Fn::GetAtt:
39+
- Key
40+
- Arn
41+
SSEAlgorithm: aws:kms
42+
AccessControl: AwsExecRead
43+
PublicAccessBlockConfiguration:
44+
BlockPublicAcls: true
45+
BlockPublicPolicy: true
46+
IgnorePublicAcls: true
47+
RestrictPublicBuckets: true
48+
LoggingConfiguration:
49+
DestinationBucketName: !Ref LoggingBucket
50+
LogFilePrefix: testing-logs
51+
LifecycleConfiguration:
52+
Rules:
53+
- Id: GlacierRule
54+
Prefix: glacier
55+
Status: Enabled
56+
ExpirationInDays: 365
57+
AccelerateConfiguration:
58+
AccelerationStatus: Enabled
59+
`,
60+
expected: s3.S3{
61+
Buckets: []s3.Bucket{
62+
{
63+
Name: types.String("logging-bucket", types.NewTestMetadata()),
64+
},
65+
{
66+
Name: types.String("test-bucket", types.NewTestMetadata()),
67+
Encryption: s3.Encryption{
68+
Enabled: types.Bool(true, types.NewTestMetadata()),
69+
Algorithm: types.String("aws:kms", types.NewTestMetadata()),
70+
KMSKeyId: types.String("Key", types.NewTestMetadata()),
71+
},
72+
ACL: types.String("aws-exec-read", types.NewTestMetadata()),
73+
PublicAccessBlock: &s3.PublicAccessBlock{
74+
BlockPublicACLs: types.Bool(true, types.NewTestMetadata()),
75+
BlockPublicPolicy: types.Bool(true, types.NewTestMetadata()),
76+
IgnorePublicACLs: types.Bool(true, types.NewTestMetadata()),
77+
RestrictPublicBuckets: types.Bool(true, types.NewTestMetadata()),
78+
},
79+
Logging: s3.Logging{
80+
TargetBucket: types.String("LoggingBucket", types.NewTestMetadata()),
81+
Enabled: types.Bool(true, types.NewTestMetadata()),
82+
},
83+
LifecycleConfiguration: []s3.Rules{
84+
{
85+
Status: types.String("Enabled", types.NewTestMetadata()),
86+
},
87+
},
88+
AccelerateConfigurationStatus: types.String("Enabled", types.NewTestMetadata()),
89+
},
90+
},
91+
},
92+
},
93+
{
94+
name: "empty s3 bucket",
95+
source: `AWSTemplateFormatVersion: 2010-09-09
96+
Resources:
97+
Bucket:
98+
Type: AWS::S3::Bucket
99+
Properties:
100+
BucketName: test-bucket`,
101+
expected: s3.S3{
102+
Buckets: []s3.Bucket{
103+
{
104+
Name: types.String("test-bucket", types.NewTestMetadata()),
105+
Encryption: s3.Encryption{
106+
Enabled: types.BoolDefault(false, types.NewTestMetadata()),
107+
},
108+
},
109+
},
110+
},
111+
},
112+
{
113+
name: "incorrect SSE algorithm",
114+
source: `AWSTemplateFormatVersion: 2010-09-09
115+
Resources:
116+
Bucket:
117+
Type: AWS::S3::Bucket
118+
Properties:
119+
BucketName: test-bucket
120+
BucketEncryption:
121+
ServerSideEncryptionConfiguration:
122+
- ServerSideEncryptionByDefault:
123+
KMSMasterKeyID: alias/my-key
124+
SSEAlgorithm: aes256
125+
`,
126+
expected: s3.S3{
127+
Buckets: []s3.Bucket{
128+
{
129+
Name: types.String("test-bucket", types.NewTestMetadata()),
130+
Encryption: s3.Encryption{
131+
Enabled: types.BoolDefault(false, types.NewTestMetadata()),
132+
KMSKeyId: types.String("alias/my-key", types.NewTestMetadata()),
133+
Algorithm: types.String("aes256", types.NewTestMetadata()),
134+
},
135+
},
136+
},
137+
},
138+
},
139+
}
140+
141+
for _, tt := range tests {
142+
t.Run(tt.name, func(t *testing.T) {
143+
144+
fsys := testutil.CreateFS(t, map[string]string{
145+
"main.yaml": tt.source,
146+
})
147+
148+
fctx, err := parser.New().ParseFile(context.TODO(), fsys, "main.yaml")
149+
require.NoError(t, err)
150+
151+
adapted := Adapt(*fctx)
152+
testutil.AssertDefsecEqual(t, tt.expected, adapted)
153+
})
154+
}
155+
156+
}

0 commit comments

Comments
 (0)