Merge pull request #624 from airbnb/jacknaglieri-req-subkeys-fix

Required Sub-key Bug Fix, Community Rules Update, and More
airbnb · Mar 8, 2018 · e8151bf · e8151bf
2 parents 3c70334 + 793d3c6
commit e8151bf
Show file tree

Hide file tree

Showing 29 changed files with 147 additions and 165 deletions.
diff --git a/conf/outputs.json b/conf/outputs.json
@@ -2,8 +2,11 @@
   "aws-lambda": {
     "sample-lambda": "function-name:qualifier"
   },
+  "aws-firehose": {
+    "alerts": "<prefix>_streamalert_alert_delivery"
+  },
   "aws-s3": {
-    "sample-bucket": "sample.bucket.name"
+    "bucket": "aws-s3-bucket"
   },
   "komand": [
     "sample-integration"

diff --git a/docs/source/rules.rst b/docs/source/rules.rst
@@ -129,9 +129,11 @@ An alert can be sent to multiple destinations.
 req_subkeys
 ~~~~~~~~~~~
 
-``req_subkeys`` is an optional argument which defines required sub-keys that must exist in the incoming record in order for it to be evaluated.
+``req_subkeys`` is an optional argument which defines sub-keys that must exist in the incoming record in order for it to be evaluated.
 
-This feature should be avoided, but it is useful if you defined a loose schema to trade flexibility for safety; see `Schemas <conf-schemas.html#json-example-osquery>`_.
+Each defined sub-key must have a non zero value as well in order for the rule to evaluate the log.
+
+This feature should be used if you have logs with a loose schema defined in order to avoid ``KeyError`` in rules.
 
 Examples:
 
@@ -143,15 +145,11 @@ Examples:
   @rule(logs=['osquery:differential'],
         outputs=['pagerduty', 'aws-s3'],
         req_subkeys={'columns':['address', 'hostnames']})
-        ...
-
-  # The 'columns' key must contain
-  # sub-keys of 'port' and 'protocol'
+        def osquery_host_check(rec):
+          # If all logs did not have the 'address' sub-key, this rule would
+          # throw a KeyError.  Using req_subkeys avoids this.
+          return rec['columns']['address'] == '127.0.0.1'
 
-  @rule(logs=['osquery:differential'],
-        outputs=['pagerduty', 'aws-s3'],
-        req_subkeys={'columns':['port', 'protocol']})
-        ...
 
 context
 ~~~~~~~~~~~

diff --git a/helpers/base.py b/helpers/base.py
@@ -15,6 +15,7 @@
 """
 from fnmatch import fnmatch
 import logging
+import json
 import time
 
 from netaddr import IPAddress, IPNetwork
@@ -217,3 +218,17 @@ def data_has_value_from_substring_list(data, needle_list):
         return False
 
     return any(needle in data for needle in needle_list)
+
+def safe_json_loads(data):
+    """Safely load a JSON string into a dictionary
+
+    Args:
+        data (str): A JSON string
+
+    Returns:
+        dict: The loaded JSON string or empty dict
+    """
+    try:
+        return json.loads(data)
+    except ValueError:
+        return {}
diff --git a/rules/community/cloudtrail/cloudtrail_critical_api_calls.py b/rules/community/cloudtrail/cloudtrail_critical_api_calls.py
@@ -6,11 +6,9 @@
 disable = StreamRules.disable()
 
 
-@rule(logs=['cloudtrail:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+@rule(
+    logs=['cloudtrail:events'],
+    outputs=['aws-firehose:alerts'])
 def cloudtrail_critical_api_calls(rec):
     """
     author:           airbnb_csirt

diff --git a/rules/community/cloudtrail/cloudtrail_mfa_policy_abuse_attempt.py b/rules/community/cloudtrail/cloudtrail_mfa_policy_abuse_attempt.py
@@ -6,11 +6,9 @@
 disable = StreamRules.disable()
 
 
-@rule(logs=['cloudtrail:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+@rule(
+    logs=['cloudtrail:events'],
+    outputs=['aws-firehose:alerts'])
 def cloudtrail_mfa_policy_abuse_attempt(rec):
     """
     author:           Scott Piper of Summit Route in collaboration with Duo Security
@@ -70,8 +68,8 @@ def cloudtrail_mfa_policy_abuse_attempt(rec):
     # - 'AccessDenied'
     # - 'EntityAlreadyExists': Can't create another MFA device with the same name.
     # - 'LimitExceeded': Can't enable a second MFA device for the same user.
-    if ('errorCode' in rec and
-            in_set(rec['eventName'], {'CreateVirtualMFADevice', 'EnableMFADevice'})):
+    if ('errorCode' in rec
+            and in_set(rec['eventName'], {'CreateVirtualMFADevice', 'EnableMFADevice'})):
         return True
 
     return False
diff --git a/rules/community/cloudtrail/cloudtrail_network_acl_ingress_anywhere.py b/rules/community/cloudtrail/cloudtrail_network_acl_ingress_anywhere.py
@@ -3,12 +3,13 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['cloudwatch:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'],
-      req_subkeys={'detail': ['eventName', 'requestParameters']})
+
+@rule(
+    logs=['cloudwatch:events'],
+    outputs=['aws-firehose:alerts'],
+    req_subkeys={
+        'detail': ['eventName', 'requestParameters']
+    })
 def cloudtrail_network_acl_ingress_anywhere(rec):
     """
     author:         @mimeframe
@@ -22,8 +23,6 @@ def cloudtrail_network_acl_ingress_anywhere(rec):
 
     req_params = rec['detail']['requestParameters']
 
-    return (
-        req_params['cidrBlock'] == '0.0.0.0/0' and
-        req_params['ruleAction'] == 'allow' and
-        req_params['egress'] is False
-    )
+    return (req_params['cidrBlock'] == '0.0.0.0/0'
+            and req_params['ruleAction'] == 'allow'
+            and req_params['egress'] is False)
diff --git a/rules/community/cloudtrail/cloudtrail_put_bucket_acl.py b/rules/community/cloudtrail/cloudtrail_put_bucket_acl.py
@@ -4,12 +4,12 @@
 rule = StreamRules.rule
 
 
-@rule(logs=['cloudwatch:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'],
-      req_subkeys={'detail': ['requestParameters', 'eventName']})
+@rule(
+    logs=['cloudwatch:events'],
+    outputs=['aws-firehose:alerts'],
+    req_subkeys={
+        'detail': ['requestParameters', 'eventName']
+    })
 def cloudtrail_put_bucket_acl(rec):
     """
     author:       airbnb_csirt

diff --git a/rules/community/cloudtrail/cloudtrail_put_object_acl_public.py b/rules/community/cloudtrail/cloudtrail_put_object_acl_public.py
@@ -4,11 +4,13 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['cloudwatch:events'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'],
-      req_subkeys={'detail': ['requestParameters']})
+
+@rule(
+    logs=['cloudwatch:events'],
+    outputs=['aws-firehose:alerts'],
+    req_subkeys={
+        'detail': ['requestParameters']
+    })
 def cloudtrail_put_object_acl_public(rec):
     """
     author:         @mimeframe
@@ -25,16 +27,13 @@ def cloudtrail_put_object_acl_public(rec):
     }
 
     # s3 buckets that are expected to have public objects
-    public_buckets = {
-        'example-bucket-to-ignore'
-    }
+    public_buckets = {'example-bucket-to-ignore'}
 
     request_params = rec['detail']['requestParameters']
     return (
         rec['detail']['eventName'] == 'PutObjectAcl' and
         # note: substring is used because it can exist as:
         # "http://acs.amazonaws.com/groups/global/AllUsers" or
         # "uri=http://acs.amazonaws.com/groups/global/AllUsers"
-        data_has_value_from_substring_list(request_params, public_acls) and
-        not in_set(request_params.get('bucketName'), public_buckets)
-    )
+        data_has_value_from_substring_list(request_params, public_acls)
+        and not in_set(request_params.get('bucketName'), public_buckets))
diff --git a/rules/community/cloudtrail/cloudtrail_root_account_usage.py b/rules/community/cloudtrail/cloudtrail_root_account_usage.py
@@ -4,12 +4,12 @@
 rule = StreamRules.rule
 
 
-@rule(logs=['cloudwatch:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'],
-      req_subkeys={'detail': ['userIdentity', 'eventType']})
+@rule(
+    logs=['cloudwatch:events'],
+    outputs=['aws-firehose:alerts'],
+    req_subkeys={
+        'detail': ['userIdentity', 'eventType']
+    })
 def cloudtrail_root_account_usage(rec):
     """
     author:           airbnb_csirt
@@ -22,8 +22,6 @@ def cloudtrail_root_account_usage(rec):
                       (b) ping the individual to determine if intentional and/or legitimate
     """
     # reference_1 contains details on logic below
-    return (
-        rec['detail']['userIdentity']['type'] == 'Root' and
-        rec['detail']['userIdentity'].get('invokedBy') is None and
-        rec['detail']['eventType'] != 'AwsServiceEvent'
-    )
+    return (rec['detail']['userIdentity']['type'] == 'Root'
+            and rec['detail']['userIdentity'].get('invokedBy') is None
+            and rec['detail']['eventType'] != 'AwsServiceEvent')
diff --git a/rules/community/cloudtrail/cloudtrail_security_group_ingress_anywhere.py b/rules/community/cloudtrail/cloudtrail_security_group_ingress_anywhere.py
@@ -4,12 +4,13 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['cloudwatch:events'],
-      matchers=[],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'],
-      req_subkeys={'detail': ['eventName', 'requestParameters']})
+
+@rule(
+    logs=['cloudwatch:events'],
+    outputs=['aws-firehose:alerts'],
+    req_subkeys={
+        'detail': ['eventName', 'requestParameters']
+    })
 def cloudtrail_security_group_ingress_anywhere(rec):
     """
     author:         @mimeframe, @ryandeivert

diff --git a/rules/community/duo_administrator/duo_bypass_code_create_non_auto_generated.py b/rules/community/duo_administrator/duo_bypass_code_create_non_auto_generated.py
@@ -1,20 +1,16 @@
 """Alert when a DUO bypass code is artisanly crafted and not auto-generated."""
-import json
+from helpers.base import safe_json_loads
 from stream_alert.rule_processor.rules_engine import StreamRules
 
 rule = StreamRules.rule
 
-@rule(logs=['duo:administrator'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['duo:administrator'], outputs=['aws-firehose:alerts'])
 def duo_bypass_code_create_non_auto_generated(rec):
     """
     author:       @mimeframe
     description:  Alert when a DUO bypass code is artisanly crafted and not auto-generated.
     reference:    https://duo.com/docs/administration-users#generating-a-bypass-code
     """
-    return (
-        rec['action'] == 'bypass_create' and
-        json.loads(rec['description']).get('auto_generated') is False
-    )
+    return (rec['action'] == 'bypass_create'
+            and safe_json_loads(rec['description']).get('auto_generated') is False)
diff --git a/rules/community/duo_administrator/duo_bypass_code_create_non_expiring.py b/rules/community/duo_administrator/duo_bypass_code_create_non_expiring.py
@@ -1,20 +1,16 @@
 """Alert when a DUO bypass code is created that is non-expiring."""
-import json
+from helpers.base import safe_json_loads
 from stream_alert.rule_processor.rules_engine import StreamRules
 
 rule = StreamRules.rule
 
-@rule(logs=['duo:administrator'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['duo:administrator'], outputs=['aws-firehose:alerts'])
 def duo_bypass_code_create_non_expiring(rec):
     """
     author:       @mimeframe
     description:  Alert when a DUO bypass code is created that is non-expiring.
     reference:    https://duo.com/docs/administration-users#generating-a-bypass-code
     """
-    return (
-        rec['action'] == 'bypass_create' and
-        json.loads(rec['description']).get('valid_secs') is None
-    )
+    return (rec['action'] == 'bypass_create'
+            and safe_json_loads(rec['description']).get('valid_secs') is None)
diff --git a/rules/community/duo_administrator/duo_bypass_code_create_unlimited_use.py b/rules/community/duo_administrator/duo_bypass_code_create_unlimited_use.py
@@ -1,20 +1,16 @@
 """Alert when a DUO bypass code is created that has unlimited use."""
-import json
+from helpers.base import safe_json_loads
 from stream_alert.rule_processor.rules_engine import StreamRules
 
 rule = StreamRules.rule
 
-@rule(logs=['duo:administrator'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['duo:administrator'], outputs=['aws-firehose:alerts'])
 def duo_bypass_code_create_unlimited_use(rec):
     """
     author:       @mimeframe
     description:  Alert when a DUO bypass code is created that has unlimited use.
     reference:    https://duo.com/docs/administration-users#generating-a-bypass-code
     """
-    return (
-        rec['action'] == 'bypass_create' and
-        json.loads(rec['description']).get('remaining_uses') is None
-    )
+    return (rec['action'] == 'bypass_create'
+            and safe_json_loads(rec['description']).get('remaining_uses') is None)
diff --git a/rules/community/duo_authentication/duo_anonymous_ip_failure.py b/rules/community/duo_authentication/duo_anonymous_ip_failure.py
@@ -3,10 +3,8 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['duo:authentication'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['duo:authentication'], outputs=['aws-firehose:alerts'])
 def duo_anonymous_ip_failure(rec):
     """
     author:       airbnb_csirt

diff --git a/rules/community/duo_authentication/duo_fraud.py b/rules/community/duo_authentication/duo_fraud.py
@@ -3,10 +3,8 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['duo:authentication'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['duo:authentication'], outputs=['aws-firehose:alerts'])
 def duo_fraud(rec):
     """
     author:       airbnb_csirt

diff --git a/rules/community/github/github_disable_dismiss_stale_pull_request_approvals.py b/rules/community/github/github_disable_dismiss_stale_pull_request_approvals.py
@@ -3,10 +3,8 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['ghe:general'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['ghe:general'], outputs=['aws-firehose:alerts'])
 def github_disable_dismiss_stale_pull_request_approvals(rec):
     """
     author:       @mimeframe

diff --git a/rules/community/github/github_disable_protect_this_branch.py b/rules/community/github/github_disable_protect_this_branch.py
@@ -3,10 +3,8 @@
 
 rule = StreamRules.rule
 
-@rule(logs=['ghe:general'],
-      outputs=['aws-s3:sample-bucket',
-               'pagerduty:sample-integration',
-               'slack:sample-channel'])
+
+@rule(logs=['ghe:general'], outputs=['aws-firehose:alerts'])
 def github_disable_protect_this_branch(rec):
     """
     author:       @mimeframe