AWS Rule Tuning #66
AWS Rule Tuning #66
Conversation
| elif event['eventName'] == 'StopLogging': | ||
| action = 'stopped' | ||
| return 'CloudTrail [{}] in account [{}] has been {}'.format( | ||
| trail_arn, lookup_aws_account_name(event['recipientAccountId']), action) |
There was a problem hiding this comment.
If neither condition is true, you will get a runtime exception:
UnboundLocalError: local variable 'action' referenced before assignment
I know this may not be possible right now with the rule, but still better to have a default value rather than risk an exception. Set action = '(unknown)' or some other default before the if statement on line 21
|
|
||
| def rule(event): | ||
| # Filter events | ||
| if event['errorCode'] != 'AccessDenied': |
There was a problem hiding this comment.
Is errorCode always defined? if event.get('errorCode') != 'AccessDenied' to be safe
There was a problem hiding this comment.
errorCode is not a required field, should be a safe lookup.
| @@ -258,6 +259,26 @@ def aws_strip_role_session_id(user_identity_arn): | |||
| return user_identity_arn | |||
|
|
|||
|
|
|||
| def check_threshold(key: str, | |||
There was a problem hiding this comment.
"Check threshold" makes it sound read-only, but you're changing the counter value. let's make that clear somehow. Maybe increment_and_threshold or increment_and_check
|
|
||
|
|
||
| def pattern_match(string_to_match: str, pattern: str): | ||
| return fnmatch(string_to_match, pattern) |
There was a problem hiding this comment.
Not sure if from panther_oss_helpers import pattern_match is any better than from fnmatch import fnmatch
I'd vote to remove this one
Co-authored-by: Austin Byers <austin.byers@runpanther.io>
| def rule(event): | ||
| return (event['eventName'] == 'ConsoleLogin' and | ||
| event['userIdentity'].get('type') == 'IAMUser' and | ||
| event.get('responseElements', {}).get('ConsoleLogin') == 'Failure') | ||
|
|
||
|
|
||
| def dedup(event): | ||
| return event['userIdentity'].get('arn') | ||
| return event['recipientAccountId'] |
There was a problem hiding this comment.
This is not a required field, this should be a safe lookup with .get
| def rule(event): | ||
| return (event['eventName'] == 'ConsoleLogin' and | ||
| event['userIdentity'].get('type') == 'Root' and | ||
| event.get('responseElements', {}).get('ConsoleLogin') == 'Success') | ||
|
|
||
|
|
||
| def dedup(event): | ||
| return event.get('sourceIPAddress') | ||
| return '{ip}-{account}'.format(ip=event['sourceIPAddress'], | ||
| account=event['recipientAccountId']) |
There was a problem hiding this comment.
Same comment about recipientAccountId.
|
|
||
| def rule(event): | ||
| # Filter events | ||
| if event['errorCode'] != 'AccessDenied': |
There was a problem hiding this comment.
errorCode is not a required field, should be a safe lookup.
* add contains function * add test & fix bug
* Checking log keypath existence when using Expressions * BugFix - GCP serviceusage.apiKeys.create Privilege Escalation
…1089) * BugFix - GCP serviceusage.apiKeys.create Privilege Escalation (#66) * Checking log keypath existence when using Expressions * BugFix - GCP serviceusage.apiKeys.create Privilege Escalation * Appease the linter --------- Co-authored-by: akozlovets098 <95437895+akozlovets098@users.noreply.github.com>
Background
General cleanup, adding deduplication, add better titles, add some helpers.
Changes
Testing