Add firewall rule violations to database (#762)

configs/forseti_conf.yaml.in

@@ -193,6 +193,15 @@ notifier:
                 # gcs_path should begin with "gs://"
                 gcs_path: gs://{SCANNER_BUCKET}/scanner_violations
 
+        - resource: firewall_rule_violations
+          should_notify: false
+          pipelines:
+            # Upload violations to GCS.
+            - name: gcs_violations_pipeline
+              configuration:
+                # gcs_path should begin with "gs://"
+                gcs_path: gs://{SCANNER_BUCKET}/scanner_violations
+
         - resource: forwarding_rule_violations
           should_notify: true
           pipelines:

google/cloud/security/common/data_access/dao.py

@@ -274,7 +274,7 @@ def execute_sql_with_fetch(self, resource_name, sql, values):
             values (tuple): Tuple of string for sql placeholder values.
 
         Returns:
-            list: A list of tuples representing rows of sql query result.
+            list: A list of dict representing rows of sql query result.
 
         Raises:
             MySQLError: When an error has occured while executing the query.

google/cloud/security/common/data_access/sql_queries/create_tables.py

@@ -530,10 +530,18 @@
         `resource_id` varchar(255) NOT NULL,
         `rule_name` varchar(255) DEFAULT NULL,
         `rule_index` int DEFAULT NULL,
-        `violation_type` enum('UNSPECIFIED','ADDED','REMOVED',
-                              'BIGQUERY_VIOLATION', 'BUCKET_VIOLATION',
-                              'IAP_VIOLATION', 'CLOUD_SQL_VIOLATION',
-                              'GROUP_VIOLATION', 'FORWARDING_RULE_VIOLATION',
+        `violation_type` enum('UNSPECIFIED',
+                              'ADDED','REMOVED',
+                              'BIGQUERY_VIOLATION',
+                              'BUCKET_VIOLATION',
+                              'CLOUD_SQL_VIOLATION',
+                              'FIREWALL_BLACKLIST_VIOLATION',
+                              'FIREWALL_MATCHES_VIOLATION',
+                              'FIREWALL_REQUIRED_VIOLATION',
+                              'FIREWALL_WHITELIST_VIOLATION',
+                              'FORWARDING_RULE_VIOLATION',
+                              'GROUP_VIOLATION',
+                              'IAP_VIOLATION',
                               'INSTANCE_NETWORK_INTERFACE_VIOLATION') NOT NULL,
         `violation_data` json DEFAULT NULL,
         PRIMARY KEY (`id`)

google/cloud/security/common/data_access/sql_queries/select_data.py

@@ -166,44 +166,13 @@
     WHERE project_number = %s;
 """
 
-# TODO: reduce these sql to a generic statement
-SELECT_POLICY_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type in ('ADDED', 'REMOVED');
-"""
-
-SELECT_IAP_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'IAP_VIOLATION';
-"""
-
-SELECT_BIGQUERY_ACL_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'BIGQUERY_VIOLATION';
-"""
-
-SELECT_BUCKETS_ACL_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'BUCKET_VIOLATION';
-"""
-SELECT_CLOUDSQL_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'CLOUD_SQL_VIOLATION';
-"""
-
-SELECT_FORWARDING_RULE_VIOLATION = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'FORWARDING_RULE_VIOLATION';
-"""
-
-SELECT_GROUPS_VIOLATIONS = """
-    SELECT * FROM violations_{0}
-    WHERE violation_type = 'GROUP_VIOLATION';
+SELECT_ALL_VIOLATIONS = """
+    SELECT * FROM violations_{0};
 """
 
-SELECT_INSTANCE_NETWORK_INTERFACE_VIOLATIONS = """
+SELECT_VIOLATIONS_BY_TYPE = """
     SELECT * FROM violations_{0}
-    WHERE violation_type = 'INSTANCE_NETWORK_INTERFACE_VIOLATION';
+    WHERE violation_type = %s;
 """
 
 BACKEND_SERVICES = """

google/cloud/security/common/data_access/violation_dao.py

@@ -14,14 +14,21 @@
 
 """Provides the data access object (DAO) for Organizations."""
 
+import json
+
+from collections import defaultdict
 from collections import namedtuple
+
 import MySQLdb
 
 from google.cloud.security.common.data_access import dao
 from google.cloud.security.common.data_access import errors as db_errors
 from google.cloud.security.common.data_access import violation_map as vm
+from google.cloud.security.common.data_access.sql_queries import load_data
+from google.cloud.security.common.data_access.sql_queries import select_data
 from google.cloud.security.common.util import log_util
 
+
 LOGGER = log_util.get_logger(__name__)
 
 
@@ -34,13 +41,12 @@ class ViolationDao(dao.Dao):
     frozen_violation_attribute_list = frozenset(violation_attribute_list)
     Violation = namedtuple('Violation', frozen_violation_attribute_list)
 
-    def insert_violations(self, violations, resource_name,
+    def insert_violations(self, violations,
                           snapshot_timestamp=None):
         """Import violations into database.
 
         Args:
             violations (iterator): An iterator of RuleViolations.
-            resource_name (str): String that defines a resource.
             snapshot_timestamp (str): The snapshot timestamp to associate
                 these violations with.
 
@@ -53,6 +59,8 @@ def insert_violations(self, violations, resource_name,
             MySQLError: is raised when the snapshot table can not be created.
         """
 
+        resource_name = 'violations'
+
         try:
             # Make sure to have a reasonable timestamp to use.
             if not snapshot_timestamp:
@@ -89,7 +97,7 @@ def insert_violations(self, violations, resource_name,
                 try:
                     self.execute_sql_with_commit(
                         resource_name,
-                        vm.VIOLATION_INSERT_MAP[resource_name](snapshot_table),
+                        load_data.INSERT_VIOLATION.format(snapshot_table),
                         formatted_violation)
                     inserted_rows += 1
                 except MySQLdb.Error, e:
@@ -99,19 +107,28 @@ def insert_violations(self, violations, resource_name,
 
         return (inserted_rows, violation_errors)
 
-    def get_all_violations(self, timestamp, resource_name):
+    def get_all_violations(self, timestamp, violation_type=None):
         """Get all the violations.
 
         Args:
             timestamp (str): The timestamp of the snapshot.
-            resource_name (str): String that defines a resource.
+            violation_type (str): The violation type.
 
         Returns:
-            tuple: A tuple of the violations as dict.
+            list: A list of dict of the violations data.
         """
-        violations_sql = vm.VIOLATION_SELECT_MAP[resource_name](timestamp)
+        if not violation_type:
+            resource_name = 'all_violations'
+            query = select_data.SELECT_ALL_VIOLATIONS
+            params = ()
+        else:
+            resource_name = violation_type
+            query = select_data.SELECT_VIOLATIONS_BY_TYPE
+            params = (violation_type,)
+
+        violations_sql = query.format(timestamp)
         rows = self.execute_sql_with_fetch(
-            resource_name, violations_sql, ())
+            resource_name, violations_sql, params)
         return rows
 
 
@@ -128,3 +145,29 @@ def _format_violation(violation, resource_name):
     """
     formatted_output = vm.VIOLATION_MAP[resource_name](violation)
     return formatted_output
+
+
+def map_by_resource(violation_rows):
+    """Create a map of violation types to violations of that resource.
+
+    Args:
+        violation_rows (list): A list of dict of violation data.
+
+    Returns:
+        dict: A dict of violation types mapped to the list of corresponding
+            violation types, i.e. { resource => [violation_data...] }.
+    """
+    v_by_type = defaultdict(list)
+
+    for v_data in violation_rows:
+        try:
+            v_data['violation_data'] = json.loads(v_data['violation_data'])
+        except ValueError:
+            LOGGER.warn('Invalid violation data, unable to parse json for %s',
+                        v_data['violation_data'])
+
+        v_resource = vm.VIOLATION_RESOURCES.get(v_data['violation_type'])
+        if v_resource:
+            v_by_type[v_resource].append(v_data)
+
+    return dict(v_by_type)

google/cloud/security/common/data_access/violation_map.py

@@ -14,11 +14,8 @@
 
 """Provides violations map"""
 
-# pylint: disable=line-too-long
 from google.cloud.security.common.data_access import violation_format as vf
-from google.cloud.security.common.data_access.sql_queries import load_data
-from google.cloud.security.common.data_access.sql_queries import select_data as sd
-# pylint: enable=line-too-long
+
 
 VIOLATION_MAP = {
     'violations': vf.format_violation,
@@ -27,24 +24,19 @@
     'groups_violations': vf.format_groups_violation,
 }
 
-# TODO: Now that all violations are going into the same table, a map is not
-# need anymore.
-VIOLATION_INSERT_MAP = {
-    'violations': load_data.INSERT_VIOLATION.format,
-    'bigquery_acl_violations': load_data.INSERT_VIOLATION.format,
-    'buckets_acl_violations': load_data.INSERT_VIOLATION.format,
-    'cloudsql_acl_violations': load_data.INSERT_VIOLATION.format,
-    'groups_violations': load_data.INSERT_VIOLATION.format
-}
-
-VIOLATION_SELECT_MAP = {
-    'bigquery_acl_violations': sd.SELECT_BIGQUERY_ACL_VIOLATIONS.format,
-    'buckets_acl_violations': sd.SELECT_BUCKETS_ACL_VIOLATIONS.format,
-    'cloudsql_acl_violations': sd.SELECT_CLOUDSQL_VIOLATIONS.format,
-    'forwarding_rule_violations': sd.SELECT_FORWARDING_RULE_VIOLATION.format,
-    'groups_violations': sd.SELECT_GROUPS_VIOLATIONS.format,
-    'policy_violations': sd.SELECT_POLICY_VIOLATIONS.format,
-    'iap_violations': sd.SELECT_IAP_VIOLATIONS.format,
-    'instance_network_interface_violations': (
-        sd.SELECT_INSTANCE_NETWORK_INTERFACE_VIOLATIONS.format),
+VIOLATION_RESOURCES = {
+    'ADDED': 'policy_violations',
+    'REMOVED': 'policy_violations',
+    'BIGQUERY_VIOLATION': 'bigquery_acl_violations',
+    'BUCKET_VIOLATION': 'buckets_acl_violations',
+    'CLOUD_SQL_VIOLATION': 'cloudsql_acl_violations',
+    'FORWARDING_RULE_VIOLATION': 'forwarding_rule_violations',
+    'FIREWALL_BLACKLIST_VIOLATION': 'firewall_rule_violations',
+    'FIREWALL_MATCHES_VIOLATION': 'firewall_rule_violations',
+    'FIREWALL_REQUIRED_VIOLATION': 'firewall_rule_violations',
+    'FIREWALL_WHITELIST_VIOLATION': 'firewall_rule_violations',
+    'GROUP_VIOLATION': 'groups_violations',
+    'IAP_VIOLATION': 'iap_violations',
+    'INSTANCE_NETWORK_INTERFACE_VIOLATION': (
+        'instance_network_interface_violations'),
 }

google/cloud/security/notifier/notifier.py

@@ -37,7 +37,6 @@
 from google.cloud.security.notifier.pipelines.base_notification_pipeline import BaseNotificationPipeline
 from google.cloud.security.notifier.pipelines import email_inventory_snapshot_summary_pipeline as inv_summary
 from google.cloud.security.notifier.pipelines import email_scanner_summary_pipeline as scanner_summary
-from google.cloud.security.scanner.scanners.scanners_map import SCANNER_VIOLATION_MAP
 # pylint: enable=line-too-long
 
 
@@ -181,15 +180,14 @@ def main(_):
     # get violations
     v_dao = violation_dao.ViolationDao(global_configs)
     violations = {}
-    for mapped_scanner_violation in SCANNER_VIOLATION_MAP:
-        try:
-            violations[mapped_scanner_violation] = v_dao.get_all_violations(
-                timestamp, mapped_scanner_violation)
-        except db_errors.MySQLError, e:
-            # even if an error is raised we still want to continue execution
-            # this is because if we don't have violations the Mysql table
-            # is not present and an error is thrown
-            LOGGER.error('get_all_violations error: %s', e.message)
+    try:
+        violations = violation_dao.map_by_resource(
+            v_dao.get_all_violations(timestamp))
+    except db_errors.MySQLError, e:
+        # even if an error is raised we still want to continue execution
+        # this is because if we don't have violations the Mysql table
+        # is not present and an error is thrown
+        LOGGER.error('get_all_violations error: %s', e.message)
 
     for retrieved_v in violations:
         LOGGER.info('retrieved %d violations for resource \'%s\'',
@@ -199,8 +197,8 @@ def main(_):
     pipelines = []
     for resource in notifier_configs['resources']:
         if violations.get(resource['resource']) is None:
-            LOGGER.error('The resource name \'%s\' is invalid, skipping',
-                         resource['resource'])
+            LOGGER.warn('The resource name \'%s\' has no violations, '
+                        'skipping', resource['resource'])
             continue
         if not violations[resource['resource']]:
             LOGGER.debug('No violations for: %s', resource['resource'])

google/cloud/security/scanner/scanners/base_scanner.py

@@ -54,26 +54,24 @@ def run(self):
         """Runs the pipeline."""
         pass
 
-    def _output_results_to_db(self, resource_name, violations):
+    def _output_results_to_db(self, violations):
         """Output scanner results to DB.
 
         Args:
-            resource_name (str): Resource name.
             violations (list): A list of violations.
 
         Returns:
             list: Violations that encountered an error during insert.
         """
-        resource_name = 'violations'
         (inserted_row_count, violation_errors) = (0, [])
         try:
             vdao = violation_dao.ViolationDao(self.global_configs)
             (inserted_row_count, violation_errors) = vdao.insert_violations(
                 violations,
-                resource_name=resource_name,
                 snapshot_timestamp=self.snapshot_timestamp)
         except db_errors.MySQLError as err:
-            LOGGER.error('Error importing violations to database: %s', err)
+            LOGGER.error('Error importing violations to database: %s\n%s',
+                         err, violations)
 
         # TODO: figure out what to do with the errors. For now, just log it.
         LOGGER.debug('Inserted %s rows with %s errors',

google/cloud/security/scanner/scanners/bigquery_scanner.py

@@ -82,10 +82,8 @@ def _output_results(self, all_violations):
         Args:
             all_violations (list): A list of BigQuery violations.
         """
-        resource_name = 'violations'
-
         all_violations = self._flatten_violations(all_violations)
-        self._output_results_to_db(resource_name, all_violations)
+        self._output_results_to_db(all_violations)
 
     def _find_violations(self, bigquery_data):
         """Find violations in the policies.

google/cloud/security/scanner/scanners/bucket_rules_scanner.py

@@ -80,10 +80,8 @@ def _output_results(self, all_violations):
         Args:
             all_violations (list): All violations
         """
-        resource_name = 'violations'
-
         all_violations = self._flatten_violations(all_violations)
-        self._output_results_to_db(resource_name, all_violations)
+        self._output_results_to_db(all_violations)
 
     def _find_violations(self, bucket_data):
         """Find violations in the policies.

google/cloud/security/scanner/scanners/cloudsql_rules_scanner.py

@@ -80,10 +80,8 @@ def _output_results(self, all_violations):
         Args:
             all_violations (list): A list of violations.
         """
-        resource_name = 'violations'
-
         all_violations = self._flatten_violations(all_violations)
-        self._output_results_to_db(resource_name, all_violations)
+        self._output_results_to_db(all_violations)
 
     def _find_violations(self, cloudsql_data):
         """Find violations in the policies.

google/cloud/security/scanner/scanners/forwarding_rule_scanner.py

@@ -87,10 +87,8 @@ def _output_results(self, all_violations):
         Args:
             all_violations (list): All violations
         """
-        resource_name = 'violations'
-
         all_violations = self._flatten_violations(all_violations)
-        self._output_results_to_db(resource_name, all_violations)
+        self._output_results_to_db(all_violations)
 
     def _retrieve(self):
         """Runs the data collection.

google/cloud/security/scanner/scanners/fw_rules_scanner.py

@@ -96,8 +96,7 @@ def _output_results(self, all_violations, resource_counts):
         rule_indices = self.rules_engine.rule_book.rule_indices
         all_violations = list(self._flatten_violations(all_violations,
                                                        rule_indices))
-        violation_errors = self._output_results_to_db(resource_name,
-                                                      all_violations)
+        violation_errors = self._output_results_to_db(all_violations)
 
         # Write the CSV for all the violations.
         # TODO: Move this into the base class? The IAP scanner version of this