Skip to content

Commit

Permalink
Adding ResourcePolicyAuditor base class for *much* more intelligent c…
Browse files Browse the repository at this point in the history 
…ross account introspection (Netflix#789) 👀

* Creating a new ResourcePolicyAuditor base class that sqs,sns,kms,elasticsearch,lambda,vpcendpoints,glacier,opsworks,s3 can use. Moving ARN and Policy parsing code into policyuniverse.

* Updating SNS.

* Naively updating s3,sns,kms,es to use new ResourcePolicyAuditor.

* Removing aws:username as a trusted field in condition block and bumping policyuniverse version.

* Updating ResourcePolicyAuditor to look for cross account account-wide access and other small fixes to get the ElasticSearch tests to work.

* Fixing RPA Tests.

* Alerting on ThirdParty and access granted to Root ARNs.

* Unitests for ResourcePolicyAuditor and subclasses need to clear the OBJECT_STORE in the pre_test_setup()

* Adding tests for the SNS Auditor.

* Upping coverage on the S3 Auditor.

* Ignoring service principals. Fixing Item.config hybrid name.

* Fixing the add method to ignore empty userids.

* Going back to using shorter issue text inline with existing issues.

* Removing SQS one-off where ResourcePolicyAuditor wouldn't look for a Policy block.

* Common Issue Category Text. Moving S3 ACL Issues into new category format. Saving actions with resource policy issues.

* Standardizing on Cross Account Root IAM and SNS Subscription wording.

* Fixing bug in sns auditor and fixing tests

* Fixing bug found running in prod.
  • Loading branch information
Patrick Kelley authored Sep 6, 2017
1 parent 2c7e78a commit c83571b
Show file tree
Hide file tree
Showing 21 changed files with 1,655 additions and 936 deletions.
1 change: 0 additions & 1 deletion .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,4 @@ secmonkey.env
*.crt
*.key
postgres-data/

docker-compose.override.yml
24 changes: 7 additions & 17 deletions dart/web/ui.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,35 +47,25 @@
<li class="dropdown">
<a class="dropdown-toggle" data-toggle="dropdown">Internet Accessible <b class="caret"></b></a>
<ul class="dropdown-menu">
<li><a href="#/issues/-/s3/-/-/-/-/True/ACL%20-%20AllUsers%20USED./1/25">S3 - ACL - AllUsers USED</a></li>
<li><a href="#/issues/-/s3/-/-/-/-/True/POLICY%20-%20This%20Policy%20Allows%20Access%20From%20Anyone./1/25">S3 - POLICY - This Policy Allows Access From Anyone</a></li>
<li><a href="#/issues/-/s3/-/-/-/-/True/POLICY%20-%20This%20Policy%20Allows%20Conditional%20Access%20From%20Anyone./1/25">S3 - POLICY - This Policy Allows Conditional Access From Anyone</a></li>
<li><a href="#/issues/-/s3/-/-/-/-/True/ACL%20-%20AuthenticatedUsers%20USED./1/25">S3 - ACL - Authenticated Users</a></li>
<li class="divider"></li>
<li><a href="#/issues/-/sns/-/-/-/-/True/SNS%20Topic%20open%20to%20everyone/1/25">SNS - Open to the World</a></li>
<li><a href="#/issues/-/sqs/-/-/-/-/True/SQS%20Queue%20open%20to%20everyone/1/25">SQS - Open to the World</a></li>
<li><a href="#/issues/-/elasticsearchservice/-/-/-/-/True/ElasticSearch%20Service%20domain%20open%20to%20everyone/1/25">ElasticSearch - Open to the World</a></li>
<li><a href="#/issues/-/kms/-/-/-/-/True/open%20to%20the%20world/1/25">KMS - Open to the World</a></li>
<li><a href="#/issues/-/securitygroup/-/-/-/-/True/Security%20Group%20ingress%20rule%20contains%200.0.0.0%2F0/1/25">Security Group Ingress 0.0.0.0/0</a></li>
<li><a href="#/issues/-/securitygroup/-/-/-/-/True/Security%20Group%20egress%20rule%20contains%200.0.0.0%2F0/1/25">Security Group Egress 0.0.0.0/0</a></li>
<li><a href="#/issues/-/elb/-/-/-/-/True/VPC%20ELB%20is%20Internet%20accessible/1/25">VPC ELB is Internet Accessible</a></li>
<li><a href="#/issues/-/alb/-/-/-/-/True/ALB%20is%20Internet%20accessible/1/25">ALB is Internet Accessible</a></li>
<li class="divider"></li>
<li><a href="#/issues/-/elasticsearchservice%2Ckms%2Clambda%2Cs3%2Csns%2Csqs%2Cvault/-/-/-/-/True/Internet%20Accessible/1/25">via Resource Policy</a></li>
<li><a href="#/issues/-/iamrole/-/-/-/-/True/allows%20assume-role%20from%20anyone/1/25">IAM Role Allows AssumeRole from Anyone</a></li>
</ul>
</li>
<li class="dropdown">
<a class="dropdown-toggle" data-toggle="dropdown">Cross Account <b class="caret"></b></a>
<ul class="dropdown-menu">
<li><a href="#/issues/-/s3/-/-/-/-/True/ACL%20-%20Unknown%20Cross%20Account%20Access./1/25">S3 - ACL - Unknown Cross Account Access</a></li>
<li><a href="#/issues/-/s3/-/-/-/-/True/POLICY%20-%20Unknown%20Cross%20Account%20Access./1/25">S3 - POLICY - Unknown Cross Account Access</a></li>
<li><a href="#/issues/-/s3/-/-/-/-/True/ACL%20-%20AuthenticatedUsers%20USED./1/25">S3 - ACL - Authenticated Users</a></li>
<li class="divider"></li>
<li><a href="#/issues/-/sns/-/-/-/-/True/Unknown%20Cross%20Account%20Access/1/25">SNS - Unknown Cross Account Access</a></li>
<li><a href="#/issues/-/sqs/-/-/-/-/True/Unknown%20Cross%20Account%20Access/1/25">SQS - Unknown Cross Account Access</a></li>
<li><a href="#/issues/-/kms/-/-/-/-/True/Condition%20-%20kms%3ACallerAccount/1/25">KMS - Condition - Cross Account</a></li>
<li><a href="#/issues/-/kms/-/-/-/-/True/Principal%20-%20/1/25">KMS - Principal - Cross Account</a></li>
<li><a href="#/issues/-/elb/-/-/-/-/True/VPC%20ELB%20accessible%20from%20non-private%20CIDR./1/25">VPC ELB accessible from non-private CIDR.</a></li>
<li><a href="#/issues/-/alb/-/-/-/-/True/ALB%20accessible%20from%20non-private%20CIDR./1/25">ALB accessible from non-private CIDR.</a></li>
<li class="divider"></li>
<li><a href="#/issues/-/elasticsearchservice%2Ckms%2Clambda%2Cs3%2Csns%2Csqs%2Cvault/-/-/-/-/True/Unknown%20Access/1/25">Unknown Access via Resource Policy</a></li>
<li><a href="#/issues/-/elasticsearchservice%2Ckms%2Clambda%2Cs3%2Csns%2Csqs%2Cvault/-/-/-/-/True/Friendly%20Cross%20Account/1/25">Friendly Access via Resource Policy</a></li>
<li><a href="#/issues/-/elasticsearchservice%2Ckms%2Clambda%2Cs3%2Csns%2Csqs%2Cvault/-/-/-/-/True/Thirdparty%20Cross%20Account/1/25">Thirdparty Access via Resource Policy</a></li>
<li class="divider"></li>
<li><a href="#/issues/-/iamrole/-/-/-/-/True/IAM%20Role%20allows%20assume-role%20from%20an%20Unknown%20Account/1/25">IAM Role Allows AssumeRole from Unknown Account</a></li>
</ul>
</li>
Expand Down
38 changes: 38 additions & 0 deletions migrations/versions/c9dd06c919ac_.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
"""Add fields to ItemAudit table to support a future revamp of the issue system.
Mostly not using these new fields yet.
Action Instructions will hold information for how the user can fix the issue.
Background Information will hold information on why something is a problem, likely with links to AWS documentation for the user to read more.
Origin will hold the statement causing the issue. Hopefully the UI can use this to highlight the offending part of an item policy.
Origin Summary will hold a summary of the Origin. A JSON Policy statement may be summarized as something like "S3 READ FROM * TO s3:mybucket".
Class UUID will be used so that the text (itemaudit.issue, itemaudit.notes) can be changed in the future without losing justifications.
Revision ID: c9dd06c919ac
Revises: b8ccf5b8089b
Create Date: 2017-09-05 17:21:08.162000
"""

# revision identifiers, used by Alembic.
revision = 'c9dd06c919ac'
down_revision = 'b8ccf5b8089b'

from alembic import op
import sqlalchemy as sa


def upgrade():
op.add_column('itemaudit', sa.Column('action_instructions', sa.Text(), nullable=True))
op.add_column('itemaudit', sa.Column('background_info', sa.Text(), nullable=True))
op.add_column('itemaudit', sa.Column('origin', sa.Text(), nullable=True))
op.add_column('itemaudit', sa.Column('origin_summary', sa.Text(), nullable=True))
op.add_column('itemaudit', sa.Column('class_uuid', sa.VARCHAR(length=32), nullable=True))


def downgrade():
op.drop_column('itemaudit', 'action_instructions')
op.drop_column('itemaudit', 'background_info')
op.drop_column('itemaudit', 'class_uuid')
op.drop_column('itemaudit', 'origin')
op.drop_column('itemaudit', 'origin_summary')
130 changes: 89 additions & 41 deletions security_monkey/auditor.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,61 @@

from sqlalchemy import and_
from collections import defaultdict
import json

auditor_registry = defaultdict(list)


class Categories:
""" Define common issue categories to maintain consistency. """
INTERNET_ACCESSIBLE = 'Internet Accessible'
INTERNET_ACCESSIBLE_NOTES = '{entity} Actions: {actions}'

FRIENDLY_CROSS_ACCOUNT = 'Friendly Cross Account'
FRIENDLY_CROSS_ACCOUNT_NOTES = '{entity} Actions: {actions}'

THIRDPARTY_CROSS_ACCOUNT = 'Thirdparty Cross Account'
THIRDPARTY_CROSS_ACCOUNT_NOTES = '{entity} Actions: {actions}'

UNKNOWN_ACCESS = 'Unknown Access'
UNKNOWN_ACCESS_NOTES = '{entity} Actions: {actions}'

PARSE_ERROR = 'Parse Error'
PARSE_ERROR_NOTES = 'Could not parse {input_type} - {input}'

CROSS_ACCOUNT_ROOT = 'Cross-Account Root IAM'
CROSS_ACCOUNT_ROOT_NOTES = '{entity} Actions: {actions}'

# TODO
# INSECURE_CERTIFICATE = 'Insecure Certificate'
# INSECURE_TLS = 'Insecure TLS'
# OVERLY_BROAD_ACCESS = 'Access Granted Broadly'
# ADMIN_ACCESS = 'Administrator Access'
# SENSITIVE_PERMISSIONS = 'Sensitive Permissions'


class Entity:
""" Entity instances provide a place to map policy elements like s3:my_bucket to the related account. """
def __init__(self, category, value, account_name=None, account_identifier=None):
self.category = category
self.value = value
self.account_name = account_name
self.account_identifier = account_identifier

@staticmethod
def from_tuple(entity_tuple):
return Entity(category=entity_tuple.category, value=entity_tuple.value)

def __str__(self):
strval = ''
if self.account_name or self.account_identifier:
strval = 'Account: [{identifier}/{account_name}] '.format(identifier=self.account_identifier, account_name=self.account_name)
strval += 'Entity: [{category}:{value}]'.format(category=self.category, value=self.value)
return strval

def __repr__(self):
return self.__str__()

class AuditorType(type):
def __init__(cls, name, bases, attrs):
super(AuditorType, cls).__init__(name, bases, attrs)
Expand Down Expand Up @@ -88,7 +139,43 @@ def __init__(self, accounts=None, debug=False):
users = User.query.filter(User.daily_audit_email==True).filter(User.accounts.any(name=account)).all()
self.emails.extend([user.email for user in users])

def add_issue(self, score, issue, item, notes=None):
def record_internet_access(self, item, entity, actions):
tag = Categories.INTERNET_ACCESSIBLE
notes = Categories.INTERNET_ACCESSIBLE_NOTES.format(entity=entity, actions=json.dumps(actions))
action_instructions = "An {singular} ".format(singular=self.i_am_singular)
action_instructions += "with { 'Principal': { 'AWS': '*' } } must also have a strong condition block or it is Internet Accessible. "
self.add_issue(10, tag, item, notes=notes, action_instructions=action_instructions)

def record_friendly_access(self, item, entity, actions):
tag = Categories.FRIENDLY_CROSS_ACCOUNT
notes = Categories.FRIENDLY_CROSS_ACCOUNT_NOTES.format(
entity=entity, actions=json.dumps(actions))
self.add_issue(0, tag, item, notes=notes)

def record_thirdparty_access(self, item, entity, actions):
tag = Categories.THIRDPARTY_CROSS_ACCOUNT
notes = Categories.THIRDPARTY_CROSS_ACCOUNT_NOTES.format(
entity=entity, actions=json.dumps(actions))
self.add_issue(0, tag, item, notes=notes)

def record_unknown_access(self, item, entity, actions):
tag = Categories.UNKNOWN_ACCESS
notes = Categories.UNKNOWN_ACCESS_NOTES.format(
entity=entity, actions=json.dumps(actions))
self.add_issue(10, tag, item, notes=notes)

def record_cross_account_root(self, item, entity, actions):
tag = Categories.CROSS_ACCOUNT_ROOT
notes = Categories.CROSS_ACCOUNT_ROOT_NOTES.format(
entity=entity, actions=json.dumps(actions))
self.add_issue(6, tag, item, notes=notes)

def record_arn_parse_issue(self, item, arn):
tag = Categories.PARSE_ERROR
notes = Categories.PARSE_ERROR_NOTES.format(input_type='ARN', input=arn)
self.add_issue(3, tag, item, notes=notes)

def add_issue(self, score, issue, item, notes=None, action_instructions=None):
"""
Adds a new issue to an item, if not already reported.
:return: The new issue
Expand Down Expand Up @@ -118,6 +205,7 @@ def add_issue(self, score, issue, item, notes=None):
new_issue = datastore.ItemAudit(score=score,
issue=issue,
notes=notes,
action_instructions=action_instructions,
justified=False,
justified_user_id=None,
justified_date=None,
Expand Down Expand Up @@ -389,46 +477,6 @@ def _set_auditor_setting_for_issue(self, issue):

return auditor_setting

def _check_cross_account(self, src_account_number, dest_item, location):
account = Account.query.filter(Account.identifier == src_account_number).first()
account_name = None
if account is not None:
account_name = account.name

src = account_name or src_account_number
dst = dest_item.account

if src == dst:
return None

notes = "SRC [{}] DST [{}]. Location: {}".format(src, dst, location)

if not account_name:
tag = "Unknown Cross Account Access"
self.add_issue(10, tag, dest_item, notes=notes)
elif account_name != dest_item.account and not account.third_party:
tag = "Friendly Cross Account Access"
self.add_issue(0, tag, dest_item, notes=notes)
elif account_name != dest_item.account and account.third_party:
tag = "Friendly Third Party Cross Account Access"
self.add_issue(0, tag, dest_item, notes=notes)

def _check_cross_account_root(self, source_item, dest_arn, actions):
if not actions:
return None

account = Account.query.filter(Account.name == source_item.account).first()
source_item_account_number = account.identifier

if source_item_account_number == dest_arn.account_number:
return None

tag = "Cross-Account Root IAM"
notes = "ALL IAM Roles/users/groups in account {} can perform the following actions:\n"\
.format(dest_arn.account_number)
notes += "{}".format(actions)
self.add_issue(6, tag, source_item, notes=notes)

def get_auditor_support_items(self, auditor_index, account):
for index in self.support_auditor_indexes:
if index == auditor_index:
Expand Down
Loading

0 comments on commit c83571b

Please sign in to comment.