Skip to content
This repository has been archived by the owner on Jun 5, 2023. It is now read-only.

Commit

Permalink
Moving rule validation in to a library and improving tests. (#3652)
Browse files Browse the repository at this point in the history
* Adding validators to verify that rule names conform to RFC standards
that are used by GCE firewall validators.

* Fix missing dependency

* Adding a rule validator library for upstream source validation and
converting gce_firewall_enforcer to use that for validation.

* Appending library.

* Fixing some pylint errors and a module path problem.

* Fixing lint errors.

* Fixing style lint errors.

* Excepting legacy lint errors pending refactor.

* Improving callback handling logic.
  • Loading branch information
zorania committed Feb 24, 2020
1 parent 9ffcfc5 commit 9de4392
Show file tree
Hide file tree
Showing 4 changed files with 442 additions and 323 deletions.
135 changes: 135 additions & 0 deletions google/cloud/forseti/common/util/rule_validator.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# Copyright 2017 The Forseti Security Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""Validation routines for GCE firewall rules."""
# pylint: disable=too-many-branches,too-many-return-statements
from __future__ import unicode_literals

import re

# Allowed items in a firewall rule.
ALLOWED_RULE_ITEMS = frozenset(('allowed', 'denied', 'description', 'direction',
'disabled', 'name', 'network', 'priority',
'sourceRanges', 'destinationRanges',
'sourceTags', 'targetTags', 'logConfig'))

# Top level required items for a valid rule.
REQUIRED_RULE_ITEMS = frozenset(('name', 'network'))

# Keys that allow a list but only up to 256 entries.
MAX_256_VALUE_KEYS = frozenset(
['sourceRanges', 'sourceTags', 'targetTags', 'destinationRanges'])

# Name restrictions described at
# https://cloud.google.com/compute/docs/reference/rest/v1/firewalls
VALID_RESOURCE_NAME_RE = re.compile('(?:^[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?$)')


def validate_gcp_rule(rule):
"""Validates that a rule is sound and meets the requirements of GCP.
Validation is based on reference:
https://cloud.google.com/compute/docs/reference/beta/firewalls and
https://cloud.google.com/compute/docs/vpc/firewalls#gcp_firewall_rule_summary_table
Args:
rule (dict): A dict representation of the rule to be validated.
Returns:
err_str: A string describing the reason the rule is invalid. None if
rule passes validation.
"""
# Keys must be from the set of allowed items.
key_set = set(rule)
unknown_keys = key_set - ALLOWED_RULE_ITEMS
if unknown_keys:
# This is probably the result of a API version upgrade that didn't
# properly update this function (or a broken binary).
return ('An unexpected entry exists in a firewall rule dict:'
' "%s".' % ','.join(list(unknown_keys)))

# Check for the presense of all items from REQUIRED_RULE_ITEMS.
missing_keys = REQUIRED_RULE_ITEMS - key_set
if missing_keys:
return ('Rule missing required field(s):"%s".' %
','.join(list(missing_keys)))

# Direction defaults to INGRESS
direction = rule.get('direction', 'INGRESS')

# Direction must be either INGRESS or EGRESS
if direction not in ['INGRESS', 'EGRESS']:
return ('Rule "direction" must be either "INGRESS" or "EGRESS":'
' "%s".' % rule)

# Ingress rules cannot have destinationRanges.
if direction == 'INGRESS' and 'destinationRanges' in rule:
return ('Ingress rules cannot include "destinationRanges": '
'"%s".' % rule)

# Egress rules cannot have sourceRanges or sourceTags.
if direction == 'EGRESS' and key_set.intersection(
['sourceRanges', 'sourceTags']):
return ('Egress rules cannot include "sourceRanges", "sourceTags": '
'"%s".' % rule)

# Keys that allow repeated values have a maximum of 256 values.
for key, values in ((key, rule.get(key, [])) for key in MAX_256_VALUE_KEYS):
if len(values) > 256:
return ('Rule entry "%s" must contain 256 or fewer values:'
' "%s".' % (key, rule))

# Network tags resemble valid domain names.
for tag_type in ['sourceTags', 'targetTags']:
for tag in rule.get(tag_type, []):
if VALID_RESOURCE_NAME_RE.match(tag) is None:
return ('Rule tag does not match valid GCP resource'
' name regex "%s": "%s".' % (
VALID_RESOURCE_NAME_RE.pattern, rule['name']))

# Rules must have either allowed or denied sections, but cannot have both.
if not ('allowed' in rule) ^ ('denied' in rule):
return ('Rule must contain one of "allowed" or "denied" entries:'
' "%s".' % rule)

# IPProtocol must be specified in allowed rules.
for allow in rule.get('allowed', []):
if 'IPProtocol' not in allow:
return (
'Allow rule %s missing required field "IPProtocol": "%s".' % (
rule['name'], allow))

# IPProtocol must be specified in denied rules.
for deny in rule.get('denied', []):
if 'IPProtocol' not in deny:
return (
'Deny rule %s missing required field "IPProtocol": "%s".' % (
rule['name'], deny))

# Priority is optional but must be an integer between 0-65535 inclusive.
if 'priority' in rule:
try:
priority = int(rule['priority'])
except ValueError:
return ('Rule "priority" could not be converted to an'
' integer: "%s".' % rule)
if priority < 0 or priority > 65535:
return 'Rule "priority" out of range 0-65535: "%s".' % rule

# Rule name must look like a domain.
if VALID_RESOURCE_NAME_RE.match(rule['name']) is None:
return ('Rule name does not match valid GCP resource name regex '
'"%s": "%s".' % (VALID_RESOURCE_NAME_RE.pattern, rule['name']))

return None
157 changes: 21 additions & 136 deletions google/cloud/forseti/enforcer/gce_firewall_enforcer.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,18 @@
Simplifies the interface with the compute API for managing firewall policies.
"""
from builtins import object
import hashlib
import http.client
import json
import operator
import re
import socket
import ssl

from future import standard_library
import http.client
import httplib2

from google.cloud.forseti.common.gcp_api import errors as api_errors
from google.cloud.forseti.common.util import logger

standard_library.install_aliases()
from google.cloud.forseti.common.util import rule_validator as rv


# The name of the GCE API.
Expand All @@ -42,28 +38,18 @@
# The version of the GCE API to use.
API_VERSION = 'v1'

LOGGER = logger.get_logger(__name__)
LOGGER = logger.logging

# What transient exceptions should be retried.
RETRY_EXCEPTIONS = (http.client.ResponseNotReady, http.client.IncompleteRead,
httplib2.ServerNotFoundError, socket.error, ssl.SSLError,)

# Allowed items in a firewall rule.
ALLOWED_RULE_ITEMS = frozenset(('allowed', 'denied', 'description', 'direction',
'disabled', 'logConfig', 'name', 'network',
'priority', 'sourceRanges', 'destinationRanges',
'sourceTags', 'targetTags'))

# Maximum time to allow an active API operation to wait for status=Done
OPERATION_TIMEOUT = 120.0

# The number of times to retry an operation if it times out before completion.
OPERATION_RETRY_COUNT = 5

# Name restrictions described at
# https://cloud.google.com/compute/docs/reference/rest/v1/firewalls
VALID_RESOURCE_NAME_RE = re.compile('(?:^[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?$)')


class Error(Exception):
"""Base error class for the module."""
Expand Down Expand Up @@ -273,7 +259,7 @@ def add_rules_from_api(self, compute_client):
# Only include keys in the ALLOWED_RULE_ITEMS set.
scrubbed_rule = dict(
[(k, v) for k, v in list(rule.items()) if k
in ALLOWED_RULE_ITEMS])
in rv.ALLOWED_RULE_ITEMS])
self.add_rule(scrubbed_rule)

def add_rules(self, rules, network_name=None):
Expand Down Expand Up @@ -313,6 +299,7 @@ def add_rule(self, rule, network_name=None):
name.
InvalidFirewallRuleError: One or more rules failed validation.
"""
# pylint: disable=too-many-branches
if not isinstance(rule, dict):
raise InvalidFirewallRuleError(
'Invalid rule type. Found %s expected dict' % type(rule))
Expand Down Expand Up @@ -375,7 +362,19 @@ def add_rule(self, rule, network_name=None):
if 'disabled' not in new_rule:
new_rule['disabled'] = self.DEFAULT_DISABLED

if self._check_rule_before_adding(new_rule):
error = rv.validate_gcp_rule(new_rule)
if error:
raise InvalidFirewallRuleError(error)

if rule['name'] in self.rules:
raise DuplicateFirewallRuleNameError(
'Rule %s already defined in rules: %s' % (
rule['name'], ', '.join(sorted(self.rules.keys()))))

callback_ok = (
self._add_rule_callback(new_rule)
if self._add_rule_callback else True)
if callback_ok:
self.rules[new_rule['name']] = new_rule

def filtered_by_networks(self, networks):
Expand Down Expand Up @@ -447,8 +446,9 @@ def add_rules_from_json(self, json_rules):
elif isinstance(rules, dict):
if 'items' in rules:
for item in rules['items']:
rule = dict([(key, item[key]) for key in ALLOWED_RULE_ITEMS
if key in item])
rule = dict(
[(key, item[key]) for key in rv.ALLOWED_RULE_ITEMS
if key in item])
self.add_rule(rule)

def _order_lists_in_rule(self, unsorted_rule):
Expand Down Expand Up @@ -480,121 +480,6 @@ def _order_lists_in_rule(self, unsorted_rule):
sorted_rule[key] = value
return sorted_rule

# TODO: clean up break up into additional methods
# pylint: disable=too-many-branches
def _check_rule_before_adding(self, rule):
"""Validates that a rule is valid and not a duplicate.
Validation is based on reference:
https://cloud.google.com/compute/docs/reference/beta/firewalls and
https://cloud.google.com/compute/docs/vpc/firewalls#gcp_firewall_rule_summary_table
If add_rule_callback is set, this will also confirm that
add_rule_callback returns True for the rule, otherwise it will not add
the rule.
Args:
rule (dict): The rule to validate.
Returns:
bool: True if rule is valid, False if the add_rule_callback returns
False.
Raises:
DuplicateFirewallRuleNameError: Two or more rules have the same
name.
InvalidFirewallRuleError: One or more rules failed validation.
"""
unknown_keys = set(rule.keys()) - ALLOWED_RULE_ITEMS
if unknown_keys:
# This is probably the result of a API version upgrade that didn't
# properly update this function (or a broken binary).
raise InvalidFirewallRuleError(
'An unexpected entry exists in a firewall rule dict: "%s".' %
','.join(list(unknown_keys)))

for key in ['name', 'network']:
if key not in rule:
raise InvalidFirewallRuleError(
'Rule missing required field "%s": "%s".' % (key, rule))

if 'direction' not in rule or rule['direction'] == 'INGRESS':
if 'destinationRanges' in rule:
raise InvalidFirewallRuleError(
'Ingress rules cannot include "destinationRanges": "%s".'
% rule)

elif rule['direction'] == 'EGRESS':
if 'sourceRanges' in rule or 'sourceTags' in rule:
raise InvalidFirewallRuleError(
'Egress rules cannot include "sourceRanges", "sourceTags":'
'"%s".' % rule)

else:
raise InvalidFirewallRuleError(
'Rule "direction" must be either "INGRESS" or "EGRESS": "%s".'
% rule)

max_256_value_keys = {'sourceRanges', 'sourceTags', 'targetTags',
'destinationRanges'}
for key in max_256_value_keys:
if key in rule and len(rule[key]) > 256:
raise InvalidFirewallRuleError(
'Rule entry "%s" must contain 256 or fewer values: "%s".'
% (key, rule))

if (('allowed' not in rule and 'denied' not in rule) or
('allowed' in rule and 'denied' in rule)):
raise InvalidFirewallRuleError(
'Rule must contain oneof "allowed" or "denied" entries: '
' "%s".' % rule)

if 'allowed' in rule:
for allow in rule['allowed']:
if 'IPProtocol' not in allow:
raise InvalidFirewallRuleError(
'Allow rule in %s missing required field '
'"IPProtocol": "%s".' % (rule['name'], allow))

elif 'denied' in rule:
for deny in rule['denied']:
if 'IPProtocol' not in deny:
raise InvalidFirewallRuleError(
'Deny rule in %s missing required field '
'"IPProtocol": "%s".' % (rule['name'], deny))

if 'priority' in rule:
try:
priority = int(rule['priority'])
except ValueError:
raise InvalidFirewallRuleError(
'Rule "priority" could not be converted to an integer: '
'"%s".' % rule)
if priority < 0 or priority > 65535:
raise InvalidFirewallRuleError(
'Rule "priority" out of range 0-65535: "%s".' % rule)

if len(rule['name']) > 63:
raise InvalidFirewallRuleError(
'Rule name exceeds length limit of 63 chars: "%s".' %
rule['name'])

if VALID_RESOURCE_NAME_RE.match(rule['name']) is None:
raise InvalidFirewallRuleError(
'Rule name does not match valid GCP resource name regex "'
'(?:^[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)": "%s".' % rule['name'])

if rule['name'] in self.rules:
raise DuplicateFirewallRuleNameError(
'Rule %s already defined in rules: %s' %
(rule['name'], ', '.join(sorted(self.rules.keys()))))

if self._add_rule_callback:
if not self._add_rule_callback(rule):
return False

return True
# pylint: enable=too-many-branches


# pylint: disable=too-many-instance-attributes
# TODO: Investigate improving so we can avoid the pylint disable.
Expand Down

0 comments on commit 9de4392

Please sign in to comment.