Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

May Update: S3 Rules #53

Merged
merged 4 commits into from
May 14, 2020
Merged

Conversation

jacknagz
Copy link
Contributor

Background

A general audit and refresh of our S3 access log rules

Changes

  • Disable MFA delete policy by default
  • Tune S3 access log rules, disable some by default, update severities and test cases

Testing

Locally:

[INFO]: Testing analysis packs in aws_s3_rules/

AWS.S3.ServerAccess.Error
	[PASS] Access Error
	[PASS] Internal Server Error

AWS.S3.ServerAccess.IPWhitelist
	[PASS] Access From Approved IP
	[PASS] Access From Unapproved IP

AWS.S3.ServerAccess.Insecure
	[PASS] Secure Access to S3 Bucket
	[PASS] Insecure Access to S3 Bucket

AWS.S3.ServerAccess.Unauthenticated
	[PASS] Authenticated Access
	[PASS] Unauthenticated Access

AWS.S3.ServerAccess.UnknownRequester
	[PASS] Expected Access
	[PASS] Unexpected Access

# IP addresses (in CIDR notation) indicating approved IP ranges for accessing S3 buckets
IP_WHITELIST = {
BUCKET_NAMES = {
# Example bucket names go here
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this comment say something like "restricted bucket names go here" or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, adding

ip_network('10.0.0.0/8'),
}


def rule(event):
# Only evaluate if the remoteIP field is present
if BUCKET_NAMES:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the intent that if BUCKET_NAMES is empty, it applies to all buckets? Perhaps a comment should clarify that under the BUCKET_NAMES declaration at the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention is that if it's empty, we just apply to all buckets

@jacknagz jacknagz merged commit 93116a1 into master May 14, 2020
@jacknagz jacknagz deleted the jacknaglieri-update-s3-rules-policies branch May 14, 2020 16:14
melenevskyi pushed a commit that referenced this pull request Dec 12, 2023
* adding special handling of SchemaWrongKeyError exceptions

* format update; added unit test to check new method

* fixing bad merge
egibs pushed a commit that referenced this pull request Jan 30, 2024
)

Co-authored-by: Oleh Melenevskyi <767472+melenevskyi@users.noreply.github.com>
risto-liftoff added a commit to risto-liftoff/panther-analysis that referenced this pull request Feb 29, 2024
sync this fork to panther-labs/panther-analysis v3.16.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants