Skip to content

Commit

Permalink
Cleanup unneeded ResourceType/LogType fields in policy/rule tests (#34)
Browse files Browse the repository at this point in the history
The ResourceType (in policy tests) and LogType (in rule tests) fields are not
necessary since they can be inferred from the policy/rule under test.

The tool has been changed to successfully parse older policy/rule tests that
include these fields.
However, this is a backwards-incompatible change for the analysis-api and
the fields must be present in the tests when deploying them to older Panther versions.

See https://github.com/panther-labs/panther/issues/610 for more context.
  • Loading branch information
giorgosp authored Jul 8, 2020
1 parent c44ed8b commit ad1393c
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 27 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Name to describe our first test.
ResourceType: Resource.Type.Here
ResourceType: Resource.Type.Here # Not needed in Panther versions >= 1.6.0
ExpectedResult: true/false
Resource:
Key: Values
Expand Down Expand Up @@ -288,7 +288,7 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Name to describe our first test.
LogType: Log.Type.Here
LogType: Log.Type.Here # Not needed in Panther versions >= 1.6.0
ExpectedResult: true/false
Resource:
Key: Values
Expand Down
13 changes: 7 additions & 6 deletions panther_analysis_tool/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,12 @@

class TestCase():

def __init__(self, data: Dict[str, Any], schema: str) -> None:
def __init__(self, data: Dict[str, Any]) -> None:
"""
Args:
data (Dict[str, Any]): An AWS Resource representation or Log event to test the policy or rule against respectively.
"""
self._data = data
self.schema = schema

def __getitem__(self, arg: str) -> Any:
return self._data.get(arg, None)
Expand Down Expand Up @@ -402,9 +405,7 @@ def run_tests(analysis: Dict[str, Any], analysis_funcs: Dict[str, Any],

for unit_test in analysis['Tests']:
try:
test_case = TestCase(
unit_test.get('Resource') or unit_test['Log'],
unit_test.get('ResourceType') or unit_test['LogType'])
test_case = TestCase(unit_test.get('Resource') or unit_test['Log'])
result = analysis_funcs['run'](test_case)
except KeyError as err:
logging.warning('KeyError: {%s}', err)
Expand Down Expand Up @@ -434,7 +435,7 @@ def setup_parser() -> argparse.ArgumentParser:
prog='panther_analysis_tool')
parser.add_argument('--version',
action='version',
version='panther_analysis_tool 0.3.2')
version='panther_analysis_tool 0.3.3')
subparsers = parser.add_subparsers()

test_parser = subparsers.add_parser(
Expand Down
10 changes: 6 additions & 4 deletions panther_analysis_tool/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,13 @@
},
Optional('Tests'): [{
'Name': str,
'ResourceType': str,
Optional('ResourceType'):
str, # Not needed anymore, optional for backwards compatibility
'ExpectedResult': bool,
'Resource': object,
}],
},
ignore_extra_keys=False)
ignore_extra_keys=False) # Prevent user typos on optional fields

RULE_SCHEMA = Schema(
{
Expand Down Expand Up @@ -102,9 +103,10 @@
},
Optional('Tests'): [{
'Name': str,
'LogType': str,
Optional('LogType'):
str, # Not needed anymore, optional for backwards compatibility
'ExpectedResult': bool,
'Log': object,
}],
},
ignore_extra_keys=False)
ignore_extra_keys=False) # Prevent user typos on optional fields
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
setup(
name='panther_analysis_tool',
packages=['panther_analysis_tool'],
version='0.3.2',
version='0.3.3',
license='apache-2.0',
description=
'Panther command line interface for writing, testing, and packaging policies/rules.',
Expand Down
1 change: 0 additions & 1 deletion tests/fixtures/aws_globals.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ Tests:
-
Name: Dummy Test
ExpectedResult: true
ResourceType: AWS.Dummy.Type
Resource: {}
3 changes: 0 additions & 3 deletions tests/fixtures/example_malformed_policy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Root MFA not enabled triggers a violation.
ResourceType: AWS.IAM.RootUser.Snapshot
ExpectedResult: false
Resource:
Arn: arn:aws:iam::123456789012:user/root
Expand All @@ -26,7 +25,6 @@ Tests:
UserName: root
-
Name: User MFA not enabled triggers a violation.
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: false
Resource:
{
Expand All @@ -40,7 +38,6 @@ Tests:
}
-
Name: User with no password enabled does not trigger a policy violation.
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: true
DOG: lol
Resource:
Expand Down
3 changes: 0 additions & 3 deletions tests/fixtures/example_policy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Suppressions:
Tests:
-
Name: Root MFA not enabled triggers a violation.
ResourceType: AWS.IAM.RootUser.Snapshot
ExpectedResult: false
Resource:
Arn: arn:aws:iam::123456789012:user/root
Expand All @@ -33,7 +32,6 @@ Tests:
UserName: root
-
Name: User MFA not enabled triggers a violation.
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: false
Resource:
{
Expand All @@ -47,7 +45,6 @@ Tests:
}
-
Name: User MFA enabled.
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: false
Resource:
{
Expand Down
1 change: 0 additions & 1 deletion tests/fixtures/example_policy_import.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ ResourceTypes:
Tests:
-
Name: Always Returns True
ResourceType: AWS.Import.Test
ExpectedResult: true
Resource:
Key: value
2 changes: 0 additions & 2 deletions tests/fixtures/valid_analysis/policies/example_policy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Root MFA not enabled fails compliance
ResourceType: AWS.IAM.RootUser.Snapshot
ExpectedResult: false
Resource:
Arn: arn:aws:iam::123456789012:user/root
Expand All @@ -35,7 +34,6 @@ Tests:
UserName: root
-
Name: User MFA not enabled fails compliance
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: false
Resource:
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Root MFA not enabled fails compliance
ResourceType: AWS.IAM.RootUser.Snapshot
ExpectedResult: false
Resource:
Arn: arn:aws:iam::123456789012:user/root
Expand All @@ -30,7 +29,6 @@ Tests:
UserName: root
-
Name: User MFA not enabled fails compliance
ResourceType: AWS.IAM.User.Snapshot
ExpectedResult: false
Resource:
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
AnalysisType: policy
Filename: example_policy.py
DisplayName: MFA Is Enabled For User
Description: MFA is a security best practice that adds an extra layer of protection for your AWS account logins.
Severity: High
PolicyID: AWS.IAM.MFAEnabled Extra Fields
Enabled: true
ResourceTypes:
- AWS.IAM.RootUser.Snapshot
- AWS.IAM.User.Snapshot
Tags:
- AWS Managed Rules - Security, Identity & Compliance
- AWS
- CIS
- SOC2
Runbook: >
Find out who disabled MFA on the account.
Reference: https://www.link-to-info.io
Suppressions:
- aws:resource:1
- aws:.*:other-resource
Tests:
-
Name: Root MFA not enabled triggers a violation.
ExpectedResult: false
ResourceType: AWS.IAM.User.Snapshot (extraneous field)
Resource:
Arn: arn:aws:iam::123456789012:user/root
CreateDate: 2019-01-01T00:00:00Z
CredentialReport:
MfaActive: false
PasswordEnabled: true
UserName: root
2 changes: 0 additions & 2 deletions tests/fixtures/valid_analysis/rules/example_rule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ Reference: https://www.link-to-info.io
Tests:
-
Name: Root MFA not enabled fails compliance
LogType: AWS.CloudTrail
ExpectedResult: false
Log:
Arn: arn:aws:iam::123456789012:user/root
Expand All @@ -29,7 +28,6 @@ Tests:
UserName: root
-
Name: User MFA not enabled fails compliance
LogType: AWS.CloudTrail
ExpectedResult: false
Log:
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
AnalysisType: rule
Filename: example_rule.py
DisplayName: MFA Rule
Description: MFA is a security best practice that adds an extra layer of protection for your AWS account logins.
Severity: High
RuleID: AWS.CloudTrail.MFAEnabled Extra Fields
Enabled: true
LogTypes:
- AWS.CloudTrail
Tags:
- AWS Managed Rules - Security, Identity & Compliance
- AWS
- CIS
- SOC2
Runbook: >
Find out who disabled MFA on the account.
Reference: https://www.link-to-info.io
Tests:
-
Name: Root MFA not enabled fails compliance
LogType: AWS.CloudTrail (extraneous Field)
ExpectedResult: false
Log:
Arn: arn:aws:iam::123456789012:user/root
CreateDate: 2019-01-01T00:00:00Z
CredentialReport:
MfaActive: false
PasswordEnabled: true
UserName: root
-
Name: User MFA not enabled fails compliance
LogType: AWS.CloudTrail (extraneous Field)
ExpectedResult: false
Log:
{
"Arn": "arn:aws:iam::123456789012:user/test",
"CreateDate": "2019-01-01T00:00:00",
"CredentialReport": {
"MfaActive": false,
"PasswordEnabled": true
},
"UserName": "test"
}

0 comments on commit ad1393c

Please sign in to comment.