feat: Add s3 rules for CIS AWS 1.4 (#905)

* feat: Add s3 rules for CIS AWS 1.4

* add missing docs

* fix linting

* fix bad merge

* add missing docs

* fix last broken test

avd_docs/aws/iam/AVD-AWS-0165/docs.md

@@ -0,0 +1,15 @@
+
+
+Hardware MFA adds an extra layer of protection on top of a user name and password. With MFA enabled, when a user signs in to an AWS website, they're prompted for their user name and password and for an authentication code from their AWS MFA device.
+
+
+### Impact
+Compromise of the root account compromises the entire AWS account and all resources within it.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_mfa_enable_physical.html
+
+

avd_docs/aws/iam/AVD-AWS-0166/docs.md

@@ -0,0 +1,15 @@
+
+
+Disabling or removing unnecessary credentials will reduce the window of opportunity for credentials associated with a compromised or abandoned account to be used.
+
+
+### Impact
+Leaving unused credentials active widens the scope for compromise.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://console.aws.amazon.com/iam/
+
+

avd_docs/aws/iam/AVD-AWS-0167/docs.md

@@ -0,0 +1,15 @@
+
+
+Multiple active access keys widens the scope for compromise.
+
+
+### Impact
+Widened scope for compromise.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://console.aws.amazon.com/iam/
+
+

avd_docs/aws/iam/AVD-AWS-0168/docs.md

@@ -0,0 +1,18 @@
+
+
+Removing expired SSL/TLS certificates eliminates the risk that an invalid certificate will be
+deployed accidentally to a resource such as AWS Elastic Load Balancer (ELB), which can
+damage the credibility of the application/website behind the ELB. As a best practice, it is
+recommended to delete expired certificates.
+
+
+### Impact
+Risk of misconfiguration and damage to credibility
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://console.aws.amazon.com/iam/
+
+

avd_docs/aws/iam/AVD-AWS-0169/docs.md

@@ -0,0 +1,16 @@
+
+
+By implementing least privilege for access control, an IAM Role will require an appropriate
+IAM Policy to allow Support Center Access in order to manage Incidents with AWS Support.
+
+
+### Impact
+Incident management is not possible without a support role.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://console.aws.amazon.com/iam/
+
+

avd_docs/aws/s3/AVD-AWS-0170/docs.md

@@ -0,0 +1,15 @@
+
+
+Adding MFA delete to an S3 bucket, requires additional authentication when you change the version state of your bucket or you delete an object version, adding another layer of security in the event your security credentials are compromised or unauthorized access is obtained.
+
+
+### Impact
+Lessened protection against accidental/malicious deletion of data
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://docs.aws.amazon.com/AmazonS3/latest/userguide/MultiFactorAuthenticationDelete.html
+
+

avd_docs/aws/s3/AVD-AWS-0171/docs.md

@@ -0,0 +1,15 @@
+
+
+Enabling object-level logging will help you meet data compliance requirements within your organization, perform comprehensive security analysis, monitor specific patterns of user behavior in your AWS account or take immediate actions on any object-level API activity within your S3 Buckets using Amazon CloudWatch Events.
+
+
+### Impact
+Difficult/impossible to audit bucket object/data changes.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://docs.aws.amazon.com/AmazonS3/latest/userguide/enable-cloudtrail-logging-for-s3.html
+
+

avd_docs/aws/s3/AVD-AWS-0172/docs.md

@@ -0,0 +1,15 @@
+
+
+Enabling object-level logging will help you meet data compliance requirements within your organization, perform comprehensive security analysis, monitor specific patterns of user behavior in your AWS account or take immediate actions on any object-level API activity within your S3 Buckets using Amazon CloudWatch Events.
+
+
+### Impact
+Difficult/impossible to audit bucket object/data changes.
+
+<!-- DO NOT CHANGE -->
+{{ remediationActions }}
+
+### Links
+- https://docs.aws.amazon.com/AmazonS3/latest/userguide/enable-cloudtrail-logging-for-s3.html
+
+

internal/adapters/cloud/aws/cloudtrail/adapt.go

@@ -107,6 +107,39 @@ func (a *adapter) adaptTrail(info types.TrailInfo) (*cloudtrail.Trail, error) {
 		isLogging = defsecTypes.Bool(*status.IsLogging, metadata)
 	}
 
+	var eventSelectors []cloudtrail.EventSelector
+	if response.Trail.HasCustomEventSelectors != nil && *response.Trail.HasCustomEventSelectors {
+		output, err := a.client.GetEventSelectors(a.Context(), &api.GetEventSelectorsInput{
+			TrailName: info.Name,
+		})
+		if err != nil {
+			return nil, err
+		}
+		for _, eventSelector := range output.EventSelectors {
+			var resources []cloudtrail.DataResource
+			for _, dataResource := range eventSelector.DataResources {
+				typ := defsecTypes.StringDefault("", metadata)
+				if dataResource.Type != nil {
+					typ = defsecTypes.String(*dataResource.Type, metadata)
+				}
+				var values defsecTypes.StringValueList
+				for _, value := range dataResource.Values {
+					values = append(values, defsecTypes.String(value, metadata))
+				}
+				resources = append(resources, cloudtrail.DataResource{
+					Metadata: metadata,
+					Type:     typ,
+					Values:   values,
+				})
+			}
+			eventSelectors = append(eventSelectors, cloudtrail.EventSelector{
+				Metadata:      metadata,
+				DataResources: resources,
+				ReadWriteType: defsecTypes.String(string(eventSelector.ReadWriteType), metadata),
+			})
+		}
+	}
+
 	return &cloudtrail.Trail{
 		Metadata:                  metadata,
 		Name:                      name,
@@ -116,5 +149,6 @@ func (a *adapter) adaptTrail(info types.TrailInfo) (*cloudtrail.Trail, error) {
 		KMSKeyID:                  defsecTypes.String(kmsKeyId, metadata),
 		IsLogging:                 isLogging,
 		BucketName:                defsecTypes.String(bucketName, metadata),
+		EventSelectors:            eventSelectors,
 	}, nil
 }

internal/adapters/cloud/aws/s3/s3.go

@@ -207,8 +207,9 @@ func (a *adapter) getBucketEncryption(bucketName *string, metadata defsecTypes.M
 
 func (a *adapter) getBucketVersioning(bucketName *string, metadata defsecTypes.Metadata) s3.Versioning {
 	bucketVersioning := s3.Versioning{
-		Metadata: metadata,
-		Enabled:  defsecTypes.BoolDefault(false, metadata),
+		Metadata:  metadata,
+		Enabled:   defsecTypes.BoolDefault(false, metadata),
+		MFADelete: defsecTypes.BoolDefault(false, metadata),
 	}
 
 	versioning, err := a.api.GetBucketVersioning(a.Context(), &s3api.GetBucketVersioningInput{Bucket: bucketName})
@@ -227,6 +228,8 @@ func (a *adapter) getBucketVersioning(bucketName *string, metadata defsecTypes.M
 		bucketVersioning.Enabled = defsecTypes.Bool(true, metadata)
 	}
 
+	bucketVersioning.MFADelete = defsecTypes.Bool(versioning.MFADelete == s3types.MFADeleteStatusEnabled, metadata)
+
 	return bucketVersioning
 }
 

internal/adapters/cloudformation/aws/s3/bucket.go

@@ -23,8 +23,9 @@ func getBuckets(cfFile parser.FileContext) []s3.Bucket {
 			PublicAccessBlock: getPublicAccessBlock(r),
 			Encryption:        getEncryption(r, cfFile),
 			Versioning: s3.Versioning{
-				Metadata: r.Metadata(),
-				Enabled:  hasVersioning(r),
+				Metadata:  r.Metadata(),
+				Enabled:   hasVersioning(r),
+				MFADelete: defsecTypes.BoolUnresolvable(r.Metadata()),
 			},
 			Logging: getLogging(r),
 			ACL:     convertAclValue(r.GetStringProperty("AccessControl", "private")),

internal/adapters/terraform/aws/cloudtrail/adapt.go

@@ -35,6 +35,24 @@ func adaptTrail(resource *terraform.Block) cloudtrail.Trail {
 	KMSKeyIDAttr := resource.GetAttribute("kms_key_id")
 	KMSKeyIDVal := KMSKeyIDAttr.AsStringValueOrDefault("", resource)
 
+	var selectors []cloudtrail.EventSelector
+	for _, selBlock := range resource.GetBlocks("event_selector") {
+		var resources []cloudtrail.DataResource
+		for _, resBlock := range selBlock.GetBlocks("data_resource") {
+			resources = append(resources, cloudtrail.DataResource{
+				Metadata: resBlock.GetMetadata(),
+				Type:     resBlock.GetAttribute("type").AsStringValueOrDefault("", resBlock),
+				Values:   resBlock.GetAttribute("values").AsStringValues(),
+			})
+		}
+		selector := cloudtrail.EventSelector{
+			Metadata:      selBlock.GetMetadata(),
+			DataResources: resources,
+			ReadWriteType: selBlock.GetAttribute("read_write_type").AsStringValueOrDefault("All", selBlock),
+		}
+		selectors = append(selectors, selector)
+	}
+
 	return cloudtrail.Trail{
 		Metadata:                  resource.GetMetadata(),
 		Name:                      nameVal,
@@ -44,5 +62,6 @@ func adaptTrail(resource *terraform.Block) cloudtrail.Trail {
 		CloudWatchLogsLogGroupArn: resource.GetAttribute("cloud_watch_logs_group_arn").AsStringValueOrDefault("", resource),
 		IsLogging:                 resource.GetAttribute("enable_logging").AsBoolValueOrDefault(true, resource),
 		BucketName:                resource.GetAttribute("s3_bucket_name").AsStringValueOrDefault("", resource),
+		EventSelectors:            selectors,
 	}
 }

internal/adapters/terraform/aws/s3/adapt_test.go

@@ -173,6 +173,7 @@ func Test_Adapt(t *testing.T) {
 				bucket = aws_s3_bucket.example.id
 				versioning_configuration {
 				  status = "Enabled"
+                  mfa_delete = "Enabled"
 				}
 			  }
 
@@ -239,8 +240,9 @@ func Test_Adapt(t *testing.T) {
 							KMSKeyId:  defsecTypes.String("string-key", defsecTypes.NewTestMetadata()),
 						},
 						Versioning: s3.Versioning{
-							Metadata: defsecTypes.NewTestMetadata(),
-							Enabled:  defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
+							Metadata:  defsecTypes.NewTestMetadata(),
+							Enabled:   defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
+							MFADelete: defsecTypes.Bool(true, defsecTypes.NewTestMetadata()),
 						},
 						Logging: s3.Logging{
 							Metadata:     defsecTypes.NewTestMetadata(),

internal/adapters/terraform/aws/s3/bucket.go

@@ -81,39 +81,50 @@ func getEncryption(block *terraform.Block, a *adapter) s3.Encryption {
 }
 
 func getVersioning(block *terraform.Block, a *adapter) s3.Versioning {
-	if block.HasChild("versioning") {
-		return s3.Versioning{
-			Metadata: block.GetMetadata(),
-			Enabled:  isVersioned(block),
-		}
+	versioning := s3.Versioning{
+		Metadata:  block.GetMetadata(),
+		Enabled:   defsecTypes.BoolDefault(false, block.GetMetadata()),
+		MFADelete: defsecTypes.BoolDefault(false, block.GetMetadata()),
+	}
+	if vBlock := block.GetBlock("versioning"); vBlock != nil {
+		versioning.Enabled = vBlock.GetAttribute("enabled").AsBoolValueOrDefault(true, vBlock)
+		versioning.MFADelete = vBlock.GetAttribute("mfa_delete").AsBoolValueOrDefault(false, vBlock)
 	}
 	for _, versioningResource := range a.modules.GetResourcesByType("aws_s3_bucket_versioning") {
 		bucketAttr := versioningResource.GetAttribute("bucket")
 		if bucketAttr.IsNotNil() {
 			if bucketAttr.IsString() {
 				actualBucketName := block.GetAttribute("bucket").AsStringValueOrDefault("", block).Value()
 				if bucketAttr.Equals(block.ID()) || bucketAttr.Equals(actualBucketName) {
-					return s3.Versioning{
-						Metadata: versioningResource.GetMetadata(),
-						Enabled:  isVersioned(versioningResource),
-					}
+					return getVersioningFromResource(versioningResource)
 				}
 			}
 			if referencedBlock, err := a.modules.GetReferencedBlock(bucketAttr, versioningResource); err == nil {
 				if referencedBlock.ID() == block.ID() {
-					return s3.Versioning{
-						Metadata: versioningResource.GetMetadata(),
-						Enabled:  isVersioned(versioningResource),
-					}
+					return getVersioningFromResource(versioningResource)
 				}
 			}
 		}
 	}
+	return versioning
+}
 
-	return s3.Versioning{
-		Metadata: block.GetMetadata(),
-		Enabled:  defsecTypes.BoolDefault(false, block.GetMetadata()),
+// from aws_s3_bucket_versioning
+func getVersioningFromResource(block *terraform.Block) s3.Versioning {
+	versioning := s3.Versioning{
+		Metadata:  block.GetMetadata(),
+		Enabled:   defsecTypes.BoolDefault(false, block.GetMetadata()),
+		MFADelete: defsecTypes.BoolDefault(false, block.GetMetadata()),
 	}
+	if config := block.GetBlock("versioning_configuration"); config != nil {
+		if status := config.GetAttribute("status"); status.IsNotNil() {
+			versioning.Enabled = defsecTypes.Bool(status.Equals("Enabled", terraform.IgnoreCase), status.GetMetadata())
+		}
+		if mfa := config.GetAttribute("mfa_delete"); mfa.IsNotNil() {
+			versioning.MFADelete = defsecTypes.Bool(mfa.Equals("Enabled", terraform.IgnoreCase), mfa.GetMetadata())
+		}
+	}
+	return versioning
 }
 
 func getLogging(block *terraform.Block, a *adapter) s3.Logging {
@@ -222,21 +233,3 @@ func hasLogging(b *terraform.Block) defsecTypes.BoolValue {
 	}
 	return defsecTypes.BoolDefault(false, b.GetMetadata())
 }
-
-func isVersioned(b *terraform.Block) defsecTypes.BoolValue {
-	if versioningBlock := b.GetBlock("versioning"); versioningBlock.IsNotNil() {
-		return versioningBlock.GetAttribute("enabled").AsBoolValueOrDefault(true, versioningBlock)
-	}
-	if versioningBlock := b.GetBlock("versioning_configuration"); versioningBlock.IsNotNil() {
-		status := versioningBlock.GetAttribute("status")
-		if status.Equals("Enabled", terraform.IgnoreCase) {
-			return defsecTypes.Bool(true, status.GetMetadata())
-		} else {
-			return defsecTypes.Bool(false, b.GetMetadata())
-		}
-	}
-	return defsecTypes.BoolDefault(
-		false,
-		b.GetMetadata(),
-	)
-}

internal/adapters/terraform/google/iam/project_iam_test.go

@@ -11,33 +11,6 @@ import (
 	"github.com/aquasecurity/defsec/test/testutil"
 )
 
-func Test_AdaptMember(t *testing.T) {
-	t.SkipNow()
-	tests := []struct {
-		name      string
-		terraform string
-		expected  iam.Member
-	}{
-		{
-			name: "basic",
-			terraform: `
-resource "" "example" {
-    
-}
-`,
-			expected: iam.Member{},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			modules := tftestutil.CreateModulesFromSource(t, test.terraform, ".tf")
-			adapted := AdaptMember(modules.GetBlocks()[0], modules)
-			testutil.AssertDefsecEqual(t, test.expected, adapted)
-		})
-	}
-}
-
 func Test_AdaptBinding(t *testing.T) {
 	tests := []struct {
 		name      string