[BUG] Can't turn off S3 data event logging

[iainelder]

Describe the bug

Once S3 data event logging has been enabled using the pEnableS3DataEvents parameter, it cannot be disabled.

The stack for the trail will accept false as a value for the parameter update, but does not translate that to an update on the trail.

To Reproduce

The following is a "pseudorepro". These steps should be reproducible by adapting them to whatever deployment method is used in practice.

Set up all the prerequisites.

• create an S3 bucket for Lambda code
• package the lambda code to the S3 bucket using the package_lambda.sh script
• create the KMS key using the stack template
• create the S3 bucket for CloudTrail logs using the stack template

Create the trail configured to log just management events.

• Use the stack template

• Derive most of the parameter values from the configuration of the prerequisites

• Use the following values to configure trail's event selectors

  pEnableDataEventsOnly: false
  pEnableLambdaDataEvents: false
  pEnableS3DataEvents: false
  

• Wait for the stack creation to complete

• Confirm via the API that the EventSelectors.DataResources list is empty

  $ aws cloudtrail get-event-selectors --trail-name ...
  {
      "TrailARN": "arn:aws:cloudtrail:...",
      "EventSelectors": [
          {
              "ReadWriteType": "All",
              "IncludeManagementEvents": true,
              "DataResources": [],
              "ExcludeManagementEventSources": []
          }
      ]
  }
  

Now update the trail so that it also logs S3 data events.

• Use the stack template

• Update the following parameter:

  pEnableS3DataEvents: true
  

• Wait for the stack update to complete

• Confirm via the API that the EventSelectors.DataResources list has a selector for all S3 objects

  $ aws cloudtrail get-event-selectors --trail-name ...
  {
      "TrailARN": "arn:aws:cloudtrail:eu-west-1:...",
      "EventSelectors": [
          {
              "ReadWriteType": "All",
              "IncludeManagementEvents": true,
              "DataResources": [
                  {
                      "Type": "AWS::S3::Object",
                      "Values": [
                          "arn:aws:s3:::"
                      ]
                  }
              ],
              "ExcludeManagementEventSources": []
          }
      ]
  }
  

• Confirm in CloudTrail history that a PutEventSelectors event is present

Now update the trail again so that it no longer logs S3 data events.

• Use the stack template

• Update the following parameter:

  pEnableS3DataEvents: false
  

• Wait for the stack update to complete

• Confirm via the API that the EventSelectors.DataResources list still has a selector for all S3 objects. The output would be the same as before.

• Confirm in CloudTrail history that a PutEventSelectors event is absent

Expected behavior

After updating the trail so that it no longer logs S3 data events:

• The EventSelectors.DataResources list should be empty again
• There should be a corresponding PutEventSelectors event in CloudTrail history

Deployment Environment

Custom deployment environment using stack sets.

Additional context

The problem seems to be caused by the following branch in the update method of the custom resource lambda.

aws-security-reference-architecture-examples/aws_sra_examples/solutions/cloudtrail/cloudtrail_org/lambda/src/app.py

Lines 243 to 249 in 0eba951

	if event_selectors and event_selectors["DataResources"]:
	CLOUDTRAIL_CLIENT.put_event_selectors(
	TrailName=cloudtrail_name,
	EventSelectors=[event_selectors]
	)
	logger.info("Data Events Updated")

It will call put_event_selectors only when the DataResources list is not empty.

When we attempt to turn off S3 data event logging, the template is updated with these parameter values:

pEnableDataEventsOnly: false
pEnableLambdaDataEvents: false
pEnableS3DataEvents: false

The Lambda receives these input parameters:

ENABLE_DATA_EVENTS_ONLY: false
ENABLE_LAMBDA_DATA_EVENTS: flase
ENABLE_S3_DATA_EVENTS: false

Which causes get_data_event_config to return DataResources as an empty list.

{
    "ReadWriteType": "All",
    "IncludeManagementEvents": True,
    "DataResources": [],
}

And so the branch to call put_event_selectors is not followed, and so the data selector is not removed.

I have fixed the function in my own environment. I could submit a PR with the essential fix if it would help.

[iainelder]

Thanks for the quick response to this.