Merge bf5d3d8 into c9e772e

.travis.yml

@@ -41,6 +41,7 @@ script:
   - coverage run -a -m py.test security_monkey/tests/core || exit 1
   - coverage run -a -m py.test security_monkey/tests/views || exit 1
   - coverage run -a -m py.test security_monkey/tests/interface || exit 1
+  - coverage run -a -m py.test security_monkey/tests/utilities || exit 1
 
 after_success:
   - coveralls

docs/configuration.rst

@@ -159,6 +159,7 @@ SM-ReadOnly
                     "s3:getbucketversioning",
                     "s3:getbucketwebsite",
                     "s3:getlifecycleconfiguration",
+                    "s3:listbucket",
                     "s3:listallmybuckets",
                     "s3:getreplicationconfiguration",
                     "s3:getanalyticsconfiguration",

docs/quickstart.rst

@@ -183,6 +183,7 @@ Paste in this JSON with the name "SecurityMonkeyReadOnly":
                     "s3:getbucketversioning",
                     "s3:getbucketwebsite",
                     "s3:getlifecycleconfiguration",
+                    "s3:listbucket",
                     "s3:listallmybuckets",
                     "s3:getreplicationconfiguration",
                     "s3:getanalyticsconfiguration",

manage.py

@@ -15,7 +15,9 @@
 import sys
 
 from flask.ext.script import Manager, Command, Option, prompt_pass
-from security_monkey.datastore import clear_old_exceptions, store_exception
+
+from security_monkey.common.s3_canonical import get_canonical_ids, fetch_id
+from security_monkey.datastore import clear_old_exceptions, store_exception, AccountType
 
 from security_monkey import app, db
 from security_monkey.common.route53 import Route53Service
@@ -34,6 +36,7 @@
 
 try:
     from gunicorn.app.base import Application
+
     GUNICORN = True
 except ImportError:
     # Gunicorn does not yet support Windows.
@@ -42,7 +45,6 @@
     print('Could not import gunicorn, skipping.')
     GUNICORN = False
 
-
 manager = Manager(app)
 migrate = Migrate(app, db)
 manager.add_command('db', MigrateCommand)
@@ -52,6 +54,7 @@
 find_modules('auditors')
 load_plugins('security_monkey.plugins')
 
+
 @manager.command
 def drop_db():
     """ Drops the database. """
@@ -184,34 +187,35 @@ def amazon_accounts():
         store_exception("manager-amazon-accounts", None, e)
 
 
-@manager.option('-u', '--number', dest='number', type=unicode, required=True)
-@manager.option('-a', '--active', dest='active', type=bool, default=True)
-@manager.option('-t', '--thirdparty', dest='third_party', type=bool, default=False)
-@manager.option('-n', '--name', dest='name', type=unicode, required=True)
-@manager.option('-s', '--s3name', dest='s3_name', type=unicode, default=u'')
-@manager.option('-o', '--notes', dest='notes', type=unicode, default=u'')
-@manager.option('-y', '--type', dest='account_type', type=unicode, default=u'AWS')
-@manager.option('-r', '--rolename', dest='role_name', type=unicode, default=u'SecurityMonkey')
-@manager.option('-f', '--force', dest='force', help='Override existing accounts', action='store_true')
-def add_account(number, third_party, name, s3_name, active, notes, account_type, role_name, force):
-    from security_monkey.account_manager import account_registry
-    account_manager = account_registry.get(account_type)()
-    account = account_manager.lookup_account_by_identifier(number)
-    if account:
-        from security_monkey.common.audit_issue_cleanup import clean_account_issues
-        clean_account_issues(account)
-
-        if force:
-            account_manager.update(account.id, account_type, name, active,
-                    third_party, notes, number,
-                    custom_fields={ 's3_name': s3_name, 'role_name': role_name })
-        else:
-            app.logger.info('Account with id {} already exists'.format(number))
-    else:
-        account_manager.create(account_type, name, active, third_party, notes, number,
-                    custom_fields={ 's3_name': s3_name, 'role_name': role_name })
-
-    db.session.close()
+# DEPRECATED:
+# @manager.option('-u', '--number', dest='number', type=unicode, required=True)
+# @manager.option('-a', '--active', dest='active', type=bool, default=True)
+# @manager.option('-t', '--thirdparty', dest='third_party', type=bool, default=False)
+# @manager.option('-n', '--name', dest='name', type=unicode, required=True)
+# @manager.option('-s', '--s3name', dest='s3_name', type=unicode, default=u'')
+# @manager.option('-o', '--notes', dest='notes', type=unicode, default=u'')
+# @manager.option('-y', '--type', dest='account_type', type=unicode, default=u'AWS')
+# @manager.option('-r', '--rolename', dest='role_name', type=unicode, default=u'SecurityMonkey')
+# @manager.option('-f', '--force', dest='force', help='Override existing accounts', action='store_true')
+# def add_account(number, third_party, name, s3_name, active, notes, account_type, role_name, force):
+#     from security_monkey.account_manager import account_registry
+#     account_manager = account_registry.get(account_type)()
+#     account = account_manager.lookup_account_by_identifier(number)
+#     if account:
+#         from security_monkey.common.audit_issue_cleanup import clean_account_issues
+#         clean_account_issues(account)
+#
+#         if force:
+#             account_manager.update(account.id, account_type, name, active,
+#                     third_party, notes, number,
+#                     custom_fields={ 's3_name': s3_name, 'role_name': role_name })
+#         else:
+#             app.logger.info('Account with id {} already exists'.format(number))
+#     else:
+#         account_manager.create(account_type, name, active, third_party, notes, number,
+#                     custom_fields={ 's3_name': s3_name, 'role_name': role_name })
+#
+#     db.session.close()
 
 
 @manager.command
@@ -350,6 +354,7 @@ def add_override_score(tech_name, method, auditor, score, disabled, pattern_scor
     db.session.commit()
     db.session.close()
 
+
 @manager.option('-f', '--file_name', dest='file_name', type=str, required=True)
 @manager.option('-m', '--mappings', dest='field_mappings', type=str, required=False)
 def add_override_scores(file_name, field_mappings):
@@ -490,7 +495,7 @@ def _parse_tech_names(tech_str):
 
 def _parse_accounts(account_str, active=True):
     if account_str == 'all':
-        accounts = Account.query.filter(Account.third_party==False).filter(Account.active==active).all()
+        accounts = Account.query.filter(Account.third_party == False).filter(Account.active == active).all()
         accounts = [account.name for account in accounts]
         return accounts
     else:
@@ -508,7 +513,7 @@ def delete_account(name):
 # We are locking down the allowed intervals here to 15 minutes, 1 hour, 12 hours, 24
 # hours or one week because too many different intervals could result in too many
 # scheduler threads, impacting performance.
-@manager.option('-i', '--interval', dest='interval', type=int, default=60, choices= [15, 60, 720, 1440, 10080])
+@manager.option('-i', '--interval', dest='interval', type=int, default=60, choices=[15, 60, 720, 1440, 10080])
 def add_watcher_config(tech_name, disabled, interval):
     from security_monkey.datastore import WatcherConfig
     from security_monkey.watcher import watcher_registry
@@ -532,6 +537,22 @@ def add_watcher_config(tech_name, disabled, interval):
     db.session.close()
 
 
+@manager.option("--override", dest="override", type=bool, default=True)
+def fetch_aws_canonical_ids(override):
+    """
+    Adds S3 canonical IDs in for all AWS accounts in SM.
+    """
+    app.logger.info("[ ] Fetching S3 canonical IDs for all AWS accounts being monitored by Security Monkey.")
+
+    # Get all the active AWS accounts:
+    accounts = Account.query.filter(Account.active == True) \
+        .join(AccountType).filter(AccountType.name == "AWS").all()  # noqa
+
+    get_canonical_ids(accounts, override=override)
+
+    app.logger.info("[@] Completed canonical ID fetching.")
+
+
 @manager.command
 def clean_stale_issues():
     """
@@ -585,7 +606,6 @@ def load(self):
 
 
 class AddAccount(Command):
-
     def __init__(self, account_manager, *args, **kwargs):
         super(AddAccount, self).__init__(*args, **kwargs)
         self._account_manager = account_manager
@@ -598,6 +618,7 @@ def get_options(self):
             Option('--active', action='store_true'),
             Option('--notes', type=unicode),
             Option('--id', dest='identifier', type=unicode, required=True),
+            Option('--update-existing', action="store_true")
         ]
         for cf in self._account_manager.custom_field_configs:
             options.append(Option('--%s' % cf.name, dest=cf.name, type=str))
@@ -609,15 +630,26 @@ def handle(self, app, *args, **kwargs):
         thirdparty = kwargs.pop('thirdparty', False)
         notes = kwargs.pop('notes', u'')
         identifier = kwargs.pop('identifier')
-        self._account_manager.create(
+        update = kwargs.pop('update_existing', False)
+        if update:
+            result = self._account_manager.update(
+                self._account_manager.account_type, name, active, thirdparty, notes, identifier,
+                custom_fields=kwargs
+            )
+        else:
+            result = self._account_manager.create(
                 self._account_manager.account_type,
                 name, active, thirdparty, notes, identifier,
                 custom_fields=kwargs)
         db.session.close()
 
+        if not result:
+            return -1
+
 
 if __name__ == "__main__":
     from security_monkey.account_manager import account_registry
+
     for name, account_manager in account_registry.items():
         manager.add_command("add_account_%s" % name.lower(), AddAccount(account_manager()))
     manager.add_command("run_api_server", APIServer())

migrations/versions/908b0085d28d_.py

@@ -61,13 +61,7 @@ def upgrade():
                 session.commit()
                 print("[-] Deleted plaintext password from user: {}'s account".format(user.email))
 
-            else:
-                print("[:D] User: {} has bcrypted password -- so all good!.".format(user.email))
-
-        else:
-            print("[:D] User: {} does not appear to be using username/password login, so all good!".format(user.email))
-
-    print("[@] Completed check.")
+    print("[@] Completed plaintext password check.")
 
 
 def downgrade():

migrations/versions/b8ccf5b8089b_.py

@@ -0,0 +1,55 @@
+"""Fetch the S3 Canonical IDs for all active AWS accounts.
+
+Revision ID: b8ccf5b8089b
+Revises: 908b0085d28d
+Create Date: 2017-03-23 11:00:43.792538
+Author: Mike Grima <mgrima@netflix.com>
+
+"""
+
+# revision identifiers, used by Alembic.
+import sqlalchemy as sa
+
+from alembic import op
+from sqlalchemy.ext.declarative import declarative_base
+from sqlalchemy.orm import sessionmaker
+
+from manage import fetch_aws_canonical_ids
+
+Session = sessionmaker()
+Base = declarative_base()
+
+revision = 'b8ccf5b8089b'
+down_revision = '908b0085d28d'
+
+class Account(Base):
+    """
+    Meant to model AWS accounts.
+    """
+    __tablename__ = "account"
+    id = sa.Column(sa.Integer, primary_key=True)
+    active = sa.Column(sa.Boolean())
+    third_party = sa.Column(sa.Boolean())
+    name = sa.Column(sa.String(32), index=True, unique=True)
+    notes = sa.Column(sa.String(256))
+    identifier = sa.Column(sa.String(256))  # Unique id of the account, the number for AWS.
+    account_type_id = sa.Column(sa.Integer, sa.ForeignKey("account_type.id"), nullable=False)
+    unique_const = sa.UniqueConstraint('account_type_id', 'identifier')
+
+
+def upgrade():
+    print("[-->] Adding canonical IDs to all AWS accounts that are active...")
+    bind = op.get_bind()
+    session = Session(bind=bind)
+
+    # If there are currently no accounts, then skip... (avoids alembic issues...)
+    accounts = session.query(Account).all()
+    if len(accounts) > 0:
+        fetch_aws_canonical_ids(True)
+
+    print("[@] Completed adding canonical IDs to all active AWS accounts...")
+
+
+def downgrade():
+    # No need to go back...
+    pass

security_monkey/account_manager.py

@@ -34,7 +34,7 @@
 
 class AccountManagerType(type):
     """
-    Generates a global account regstry as AccountManager derived classes
+    Generates a global account registry as AccountManager derived classes
     are loaded
     """
     def __init__(cls, name, bases, attrs):
@@ -68,22 +68,22 @@ class AccountManager(object):
     identifier_label = None
     identifier_tool_tip = None
 
-    def update(self, account_id, account_type, name, active, third_party, notes,
-               identifier, custom_fields=None):
+    def update(self, account_type, name, active, third_party, notes, identifier, custom_fields=None):
         """
         Updates an existing account in the database.
         """
         account_type_result = _get_or_create_account_type(account_type)
-        query = Account.query.filter(Account.id == account_id)
-        if query.count():
-            account = query.first()
-        else:
-            app.logger.info(
-                'Account with id {} does not exist exists'.format(account_id))
+        account = Account.query.filter(Account.name == name, Account.account_type_id == account_type_result.id).first()
+        if not account:
+            app.logger.error(
+                'Account with name {} does not exist'.format(name))
             return None
 
-        account = self._populate_account(account, account_type_result.id, name,
-                                         active, third_party, notes, identifier, custom_fields)
+        account.active = active
+        account.notes = notes
+        account.active = active
+        account.third_party = third_party
+        self._update_custom_fields(account, custom_fields)
 
         db.session.add(account)
         db.session.commit()
@@ -98,6 +98,14 @@ def create(self, account_type, name, active, third_party, notes, identifier,
         Creates an account in the database.
         """
         account_type_result = _get_or_create_account_type(account_type)
+        account = Account.query.filter(Account.name == name, Account.account_type_id == account_type_result.id).first()
+
+        # Make sure the account doesn't already exist:
+        if account:
+            app.logger.error(
+                'Account with name {} already exists!'.format(name))
+            return None
+
         account = Account()
         account = self._populate_account(account, account_type_result.id, name,
                                          active, third_party, notes, identifier, custom_fields)
@@ -135,6 +143,12 @@ def _populate_account(self, account, account_type_id, name, active, third_party,
         account.active = active
         account.third_party = third_party
         account.account_type_id = account_type_id
+
+        self._update_custom_fields(account, custom_fields)
+
+        return account
+
+    def _update_custom_fields(self, account, custom_fields):
         if account.custom_fields is None:
             account.custom_fields = []
 
@@ -152,8 +166,6 @@ def _populate_account(self, account, account_type_id, name, active, third_party,
                         name=field_name, value=custom_fields.get(field_name))
                     account.custom_fields.append(new_value)
 
-        return account
-
     def is_compatible_with_account_type(self, account_type):
         if self.account_type == account_type or account_type in self.compatable_account_types:
             return True

security_monkey/account_managers/aws_account.py

@@ -23,21 +23,27 @@
 
 """
 from security_monkey.account_manager import AccountManager, CustomFieldConfig
-from security_monkey.datastore import Account
 
 
 class AWSAccountManager(AccountManager):
     account_type = 'AWS'
     identifier_label = 'Number'
     identifier_tool_tip = 'Enter the AWS account number, if you have it. (12 digits)'
-    s3_name_label = ('The S3 Name is the way AWS presents the account in an ACL policy.  '
-                     'This is often times the first part of the email address that was used '
+    s3_name_label = ('[DEPRECATED -- use canonical id] The S3 Name is the way AWS presents the account '
+                     'in an ACL policy.  This is often times the first part of the email address that was used '
                      'to create the Amazon account.  (myaccount@example.com may be represented '
                      'as myaccount\).  If you see S3 issues appear for unknown cross account '
                      'access, you may need to update the S3 Name.')
+    s3_canonical_id = ('The Canonical ID is the way AWS presents the account in an ACL policy.  '
+                       'It is a unique set of characters that is tied to an AWS account.  '
+                       'If you see S3 issues appear for unknown cross account '
+                       'access, you may need to update the canonical ID.  A manager.py command has been '
+                       'included that can fetch this for you automatically (fetch_aws_canonical_ids), since it '
+                       'requires a \'list_buckets\' API call against AWS to obtain.')
     role_name_label = ("Optional custom role name, otherwise the default 'SecurityMonkey' is used. "
                        "When deploying roles via CloudFormation, this is the Physical ID of the generated IAM::ROLE.")
     custom_field_configs = [
+        CustomFieldConfig('canonical_id', "Canonical ID", True, s3_canonical_id),
         CustomFieldConfig('s3_name', 'S3 Name', True, s3_name_label),
         CustomFieldConfig('role_name', 'Role Name', True, role_name_label)
     ]