Adds new test for cross-account roles requiring MFA (#97)

* Initial pass at test for admin roles requiring MFA

* Fixed filename and added new policies needed

* Added disclaimers/notes about potential for false positives/negatives

README.md

@@ -124,10 +124,13 @@ The below policy will allow you to run all AWS tests in pytest-services against
         "iam:GetCredentialReport",
         "iam:GetLoginProfile",
         "iam:ListAttachedGroupPolicies",
+        "iam:ListAttachedRolePolicies",
         "iam:ListAttachedUserPolicies",
         "iam:ListGroupPolicies",
         "iam:ListGroupsForUser",
         "iam:ListMFADevices",
+        "iam:ListRolePolicies",
+        "iam:ListRoles",
         "iam:ListUserPolicies",
         "iam:ListUsers",
         "rds:DescribeDbInstances",
@@ -389,6 +392,18 @@ aws:
 ...
 ```
 
+### Test Accuracy
+
+There are two important things to note about `pytest-services` tests that may be different from your expectations.
+
+First, the focus is on "actionable results". This plays out as an attempt to reduce false
+positives by trying to filter out unused resources. An example of this can be seen by looking at
+any of the security group tests, where we are skipping any security groups that are not attached to a resource.
+
+Second, there are some tests that make naive assumptions instead of trying to capture the complexities
+of the system. The current best example of this is all IAM tests that relate to "admin" users. How we
+are determining what an user or role is an admin is based simply off substring matching on the policies
+attached. This obviously has a high chance of false negatives.
 
 ## Development
 
@@ -401,6 +416,7 @@ aws:
 1. be vendor agnostic e.g. support checks across cloud providers or in hybrid environments or competing services
 1. cache and share responses to reduce third party API usage (i.e. lots of tests check AWS security groups so fetch them once)
 1. provide a way to run a single test or subset of tests
+1. focus on actionable results (see [test accuracy](#test-accuracy) for more information)
 
 ### Non-Goals
 

aws/iam/resources.py

@@ -153,6 +153,53 @@ def iam_mfa_devices(users):
     ]
 
 
+def iam_roles():
+    "http://botocore.readthedocs.io/en/latest/reference/services/iam.html#IAM.Client.list_roles"
+    return botocore_client.get(
+        'iam', 'list_roles', [], {})\
+        .extract_key('Roles')\
+        .flatten()\
+        .values()
+
+
+def iam_all_role_policies(rolename):
+    return [
+        {'PolicyName': policy_name} for policy_name
+        in iam_role_inline_policies(rolename=rolename)
+    ] + iam_role_managed_policies(rolename=rolename)
+
+
+def iam_roles_with_policies():
+    return [
+        {
+            **{'Policies': iam_all_role_policies(rolename=role['RoleName'])},
+            **role,
+        } for role in iam_roles()
+    ]
+
+
+def iam_role_inline_policies(rolename):
+    "http://botocore.readthedocs.io/en/latest/reference/services/iam.html#IAM.Client.list_role_policies"
+    return botocore_client.get(
+        'iam', 'list_role_policies', [], {'RoleName': rolename})\
+        .extract_key('PolicyNames')\
+        .flatten()\
+        .values()
+
+
+def iam_role_managed_policies(rolename):
+    "http://botocore.readthedocs.io/en/latest/reference/services/iam.html#IAM.Client.list_attached_role_policies"
+    return botocore_client.get(
+        'iam', 'list_attached_role_policies', [], {'RoleName': rolename})\
+        .extract_key('AttachedPolicies')\
+        .flatten()\
+        .values()
+
+
+def iam_admin_roles():
+    return [role for role in iam_roles_with_policies() if user_is_admin(role)]
+
+
 def iam_generate_credential_report():
     "http://botocore.readthedocs.io/en/latest/reference/services/iam.html#IAM.Client.generate_credential_report"
     results = botocore_client.get('iam', 'generate_credential_report', [], {}, do_not_cache=True).results

aws/iam/test_iam_admin_user_with_access_keys.py

@@ -11,6 +11,9 @@
 def test_iam_admin_user_with_access_key(iam_admin_user):
     """Test that all "admin" users do not have access keys
     associated to their user.
+
+    Note: Due to the naive mechanism for determing what an "admin" is, this test
+    can easily have both false positives and (more likely) false negatives.
     """
     assert iam_admin_user['CredentialReport']['access_key_1_active'] != 'true', \
         'Access key found for admin user: {}'.format(iam_admin_user['UserName'])

aws/iam/test_iam_admin_user_without_mfa.py

@@ -13,6 +13,9 @@
         ids=lambda login: login['UserName'])
 def test_iam_admin_user_without_mfa(iam_login_profile, iam_user_mfa_devices):
     """Test that all "admin" users with console access also have an MFA device.
+
+    Note: Due to the naive mechanism for determing what an "admin" is, this test
+    can easily have both false positives and (more likely) false negatives.
     """
     if bool(iam_login_profile):
         assert len(iam_user_mfa_devices) > 0, \

aws/iam/test_iam_cross_account_admin_roles_require_mfa.py

@@ -0,0 +1,22 @@
+import pytest
+
+from aws.iam.resources import iam_admin_roles
+
+
+@pytest.mark.iam
+@pytest.mark.parametrize(
+        'iam_admin_role',
+        iam_admin_roles(),
+        ids=lambda role: role['RoleName'])
+def test_iam_cross_account_admin_roles_require_mfa(iam_admin_role):
+    """Test that all IAM Roles that include admin policies and have cross account
+    trust relationships require MFA.
+
+    Note: Due to the naive mechanism for determing what an "admin" is, this test
+    can easily have both false positives and (more likely) false negatives.
+    """
+    for statement in iam_admin_role["AssumeRolePolicyDocument"]["Statement"]:
+        if statement["Action"].startswith("sts") and "AWS" in statement["Principal"]:
+            assert 'Condition' in statement
+            assert 'aws:MultiFactorAuthPresent' in statement['Condition']['Bool']
+            assert statement['Condition']['Bool']['aws:MultiFactorAuthPresent'] == 'true'