Merge djmitche/build-fwunit:bug1118451 (PR #31)

docs/diffing.rst

@@ -0,0 +1,12 @@
+Diffing
+=======
+
+Two compare two rulesets, use ``fwunit-diff``.
+For example:
+
+.. code-block:: none
+
+    $ fwunit-diff yesterday.json my-network
+    + ssh IPSet([IP('172.16.3.0/24')]) -> IPSet([IP('10.90.110.0/23')])
+
+The two sources for comparison can be the names of sources defined in ``fwunit.yaml``, or filenames (e.g., to backup copies).

docs/implementation.rst

@@ -60,6 +60,8 @@ With the config object in hand, call :py:func:`~fwunit.analysis.sources.load_sou
     :rtype: :py:class:`~fwunit.analysis.sources.Source`
 
     Load the ruleset for the given source.
+    The ``source`` parameter can be a source name from the configuration, or a filename.
+
     Rulesets are cached globally to the process.
 
 Using Source Objects
@@ -69,6 +71,12 @@ Using Source Objects
 
     The data from a particular source in ``fwunit.yaml``, along with some analysis methods.
 
+    .. py:method:: rulesForApp(app):
+        :param app: application name
+        :returns: list of rules
+
+        Get the rules for the given app, or if no such app is known, for ``@@other``.
+
     .. py:method:: rulesDeny(src, dst, apps)
 
         :param src: source IPs

docs/index.rst

@@ -24,5 +24,6 @@ Contents
    processing
    policy_types
    querying
+   diffing
    testing
    implementation

example/fwunit.yaml

@@ -9,3 +9,10 @@ my-network:
     application-map:
         22/tcp: ssh
         8140/tcp: puppet
+
+other-network:
+    type: srx
+    output: other-network.json
+    firewall: fw.exmaple.com
+    ssh_username: fwunit
+    ssh_password: sekr!t

example/other-network.json

@@ -0,0 +1 @@
+{"rules": [{"src": ["172.16.1.0/24", "172.16.2.0/24", "172.16.3.0/24"], "dst": ["0.0.0.0/1", "128.0.0.0/3", "160.0.0.0/5", "168.0.0.0/6", "172.0.0.0/12", "172.16.0.0/24", "172.16.3.0/24", "172.16.4.0/22", "172.16.8.0/21", "172.16.16.0/20", "172.16.32.0/19", "172.16.64.0/18", "172.16.128.0/17", "172.17.0.0/16", "172.18.0.0/15", "172.20.0.0/14", "172.24.0.0/13", "172.32.0.0/11", "172.64.0.0/10", "172.128.0.0/9", "173.0.0.0/8", "174.0.0.0/7", "176.0.0.0/4", "192.0.0.0/2"], "app": "*/any", "name": "admin/out+puppetmaster/out+unoccupied/out+worker/out"}, {"src": ["172.16.1.0/24", "172.16.2.0/24"], "dst": ["0.0.0.0/1", "128.0.0.0/3", "160.0.0.0/5", "168.0.0.0/6", "172.0.0.0/12", "172.16.0.0/24", "172.16.3.0/24", "172.16.4.0/22", "172.16.8.0/21", "172.16.16.0/20", "172.16.32.0/19", "172.16.64.0/18", "172.16.128.0/17", "172.17.0.0/16", "172.18.0.0/15", "172.20.0.0/14", "172.24.0.0/13", "172.32.0.0/11", "172.64.0.0/10", "172.128.0.0/9", "173.0.0.0/8", "174.0.0.0/7", "176.0.0.0/4", "192.0.0.0/2"], "app": "puppet", "name": "admin/out+puppetmaster/out+unoccupied/out+worker/out"}, {"src": ["172.16.0.0/16"], "dst": ["172.16.1.220"], "app": "puppet", "name": "admin/out+puppetmaster/in+puppetmaster/out+unoccupied/out+worker/out"}, {"src": ["172.16.1.30"], "dst": ["0.0.0.0/1", "128.0.0.0/3", "160.0.0.0/5", "168.0.0.0/6", "172.0.0.0/12", "172.16.0.0/24", "172.16.1.220", "172.16.2.0/23", "172.16.4.0/22", "172.16.8.0/21", "172.16.16.0/20", "172.16.32.0/19", "172.16.64.0/18", "172.16.128.0/17", "172.17.0.0/16", "172.18.0.0/15", "172.20.0.0/14", "172.24.0.0/13", "172.32.0.0/11", "172.64.0.0/10", "172.128.0.0/9", "173.0.0.0/8", "174.0.0.0/7", "176.0.0.0/4", "192.0.0.0/2"], "app": "ssh", "name": "admin/out+puppetmaster/in+worker/in"}, {"src": ["172.16.1.0/28", "172.16.1.16/29", "172.16.1.24/30", "172.16.1.28/31", "172.16.1.31", "172.16.1.32/27", "172.16.1.64/26", "172.16.1.128/25", "172.16.2.0/24"], "dst": ["0.0.0.0/1", "128.0.0.0/3", "160.0.0.0/5", "168.0.0.0/6", "172.0.0.0/12", "172.16.0.0/24", "172.16.3.0/24", "172.16.4.0/22", "172.16.8.0/21", "172.16.16.0/20", "172.16.32.0/19", "172.16.64.0/18", "172.16.128.0/17", "172.17.0.0/16", "172.18.0.0/15", "172.20.0.0/14", "172.24.0.0/13", "172.32.0.0/11", "172.64.0.0/10", "172.128.0.0/9", "173.0.0.0/8", "174.0.0.0/7", "176.0.0.0/4", "192.0.0.0/2"], "app": "ssh", "name": "puppetmaster/out+unoccupied/out+worker/out"}, {"src": ["5.5.5.5"], "dst": ["172.16.1.30"], "app": "ssh", "name": "admin/in"}, {"src": ["172.16.1.30", "172.16.1.220", "172.16.2.0/24"], "dst": ["0.0.0.0/1", "128.0.0.0/3", "160.0.0.0/5", "168.0.0.0/6", "172.0.0.0/12", "172.16.0.0/24", "172.16.3.0/24", "172.16.4.0/22", "172.16.8.0/21", "172.16.16.0/20", "172.16.32.0/19", "172.16.64.0/18", "172.16.128.0/17", "172.17.0.0/16", "172.18.0.0/15", "172.20.0.0/14", "172.24.0.0/13", "172.32.0.0/11", "172.64.0.0/10", "172.128.0.0/9", "173.0.0.0/8", "174.0.0.0/7", "176.0.0.0/4", "192.0.0.0/2"], "app": "@@other", "name": "admin/out+puppetmaster/out+worker/out"}]}

fwunit/analysis/sources.py

@@ -4,6 +4,7 @@
 
 import itertools
 import json
+import os.path
 import logging
 
 from blessings import Terminal
@@ -24,8 +25,7 @@ class Source(object):
     def __init__(self, filename):
         self.rules = types.from_jsonable(json.load(open(filename))['rules'])
 
-    def _rules_for_app(self, app):
-        # get the rules for the given app, or if no such app is known, for @@other
+    def rulesForApp(self, app):
         try:
             return self.rules[app]
         except KeyError:
@@ -39,7 +39,7 @@ def rulesDeny(self, src, dst, apps):
         apps = apps if not isinstance(apps, basestring) else [apps]
         for app in apps:
             log.info("checking application %r" % app)
-            for rule in self._rules_for_app(app):
+            for rule in self.rulesForApp(app):
                 src_matches = (rule.src & src)
                 if not src_matches:
                     continue
@@ -65,7 +65,7 @@ def rulesPermit(self, src, dst, apps):
         apps = apps if not isinstance(apps, basestring) else [apps]
         for app in apps:
             log.info("checking application %r" % app)
-            for rule in self._rules_for_app(app):
+            for rule in self.rulesForApp(app):
                 if (rule.src & src) and (rule.dst & dst):
                     log.info("matched policy {t.cyan}{name}{t.normal}\n{t.yellow}{src}{t.normal} "
                             "-> {t.magenta}{dst}{t.normal}".format(
@@ -105,7 +105,7 @@ def sourcesFor(self, dst, app, ignore_sources=None):
         dst = _ipset(dst)
         log.info("sourcesFor(%r, %r, ignore_sources=%r)" % (dst, app, ignore_sources))
         rv = IPSet()
-        for rule in self._rules_for_app(app):
+        for rule in self.rulesForApp(app):
             if rule.dst & dst:
                 src = rule.src
                 if ignore_sources:
@@ -122,9 +122,16 @@ def sourcesFor(self, dst, app, ignore_sources=None):
 
 def load_source(cfg, source):
     """Load the named source.  Sources are cached, so multiple calls with the same name
-    will not repeatedly re-load the data from disk."""
+    will not repeatedly re-load the data from disk.  The source can name a source from
+    the configuration, or a filename."""
     if source not in _cache:
-        _cache[source] = Source(cfg[source]['output'])
+        if source in cfg:
+            filename = cfg[source]['output']
+        elif os.path.exists(source):
+            filename = source
+        else:
+            raise KeyError("unknown source {}".format(source))
+        _cache[source] = Source(filename)
     return _cache[source]
 
 

fwunit/diff.py

@@ -0,0 +1,39 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+from fwunit.ip import IPPairs
+from fwunit.analysis import sources
+from blessings import Terminal
+
+terminal = Terminal()
+
+
+def app_diff(app, left, right):
+    left_pairs = IPPairs(*[(r.src, r.dst) for r in left])
+    right_pairs = IPPairs(*[(r.src, r.dst) for r in right])
+    added = right_pairs - left_pairs
+    removed = left_pairs - right_pairs
+    for s, d in removed:
+        yield ("-", app, s, d)
+    for s, d in added:
+        yield ("+", app, s, d)
+
+def make_diff(left, right):
+    for app in sorted(set(left.rules) | set(right.rules)):
+        left_rules = left.rulesForApp(app)
+        right_rules = right.rulesForApp(app)
+        for diff in app_diff(app, left_rules, right_rules):
+            yield diff
+
+def show_diff(cfg, left_name, right_name):
+    prefixes = {
+        '+': '{t.green}+{t.normal}'.format(t=terminal),
+        '-': '{t.red}-{t.normal}'.format(t=terminal),
+    }
+    for symbol, app, src, dst in make_diff(
+        sources.load_source(cfg, left_name),
+        sources.load_source(cfg, right_name)):
+        print "{pfx} {t.bold_cyan}{app}{t.normal} {t.yellow}{src}{t.normal} " \
+              "-> {t.magenta}{dst}{t.normal}".format(
+                t=terminal, pfx=prefixes[symbol], app=app, src=src, dst=dst)

fwunit/ip.py

@@ -106,15 +106,22 @@ def __repr__(self):
     def __sub__(self, other):
         new_pairs = []
         empty = lambda pair: len(pair[0]) == 0 or len(pair[1]) == 0
-        for sa, da in self._pairs:
+        # the approach here is to successively break pairs in self down where they overlap
+        # other, keeping only pairs that are completely disjoint with other.
+        pairs_to_consider = self._pairs[:]
+        while pairs_to_consider:
+            sa, da = pairs_to_consider.pop(0)
             for sb, db in other._pairs:
                 # eliminate non-overlap
                 if sa.isdisjoint(sb) or da.isdisjoint(db):
-                    new_pairs.append((sa, da))
                     continue
                 for pair in (sa & sb, da - db), (sa - sb, da - db), (sa - sb, da & db):
                     if not empty(pair):
-                        new_pairs.append(pair)
+                        pairs_to_consider.append(pair)
+                break
+            else:
+                # no pairs in `other` overlapped sa/da, so we can keep it
+                new_pairs.append((sa, da))
         return IPPairs(*new_pairs)
 
     def _optimize(self):

fwunit/scripts.py

@@ -6,8 +6,7 @@
 import logging
 import sys
 import textwrap
-import os
-import os.path
+from fwunit import diff as diff_module
 from fwunit import log
 from fwunit import types
 from fwunit.analysis import config
@@ -112,3 +111,19 @@ def query():
         parser.error("No subcomand given")
 
     args._func(args, cfg)
+
+def diff():
+    description = textwrap.dedent("""Print differences between two rule sets (sources)""")
+    parser = argparse.ArgumentParser(description=description)
+    parser.add_argument('--config', '-c',
+        help="YAML configuration file", dest='config_file', type=str, default='fwunit.yaml')
+    parser.add_argument('--verbose', action='store_true')
+    parser.add_argument('--quiet', action='store_true')
+    parser.add_argument('left', help='left source')
+    parser.add_argument('right', help='right source')
+
+    args, cfg = _setup(parser)
+    if not args.verbose:
+        logging.getLogger('').setLevel(logging.CRITICAL)
+
+    diff_module.show_diff(cfg, args.left, args.right)

fwunit/test/unit/test_diff.py

@@ -0,0 +1,150 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import mock
+import re
+
+from nose.tools import eq_
+from fwunit.types import Rule
+from fwunit.analysis import sources
+from fwunit.test.util import ipset
+from fwunit import diff
+
+TEN = ipset('10.10.0.0/16')
+TEN_0 = ipset('10.10.0.0/17')
+TEN_128 = ipset('10.10.128.0/17')
+TWENTY = ipset('10.20.0.0/16')
+TWENTY_0 = ipset('10.20.0.0/17')
+TWENTY_128 = ipset('10.20.128.0/17')
+
+LEFT = {
+    'http': [
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.19.0/24'), app='http', name='web access'),
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.20.0/25'), app='http', name='stage web access'),
+    ],
+    'https': [
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.19.0/24'), app='https', name='https web access'),
+    ],
+    'db': [
+        Rule(src=ipset('10.20.19.0/24'), dst=ipset('10.20.30.15', '10.20.30.19'), app='db', name='db'),
+        Rule(src=ipset('10.20.20.0/25'), dst=ipset('10.20.30.16'), app='db', name='stage db'),
+    ],
+    'ssh': [
+        Rule(src=ipset('10.2.1.1'),
+                dst=ipset('10.20.19.0/24', '10.20.20.0/25', '10.20.30.15', '10.20.30.16', '10.20.30.19'),
+                app='ssh', name='ssh'),
+    ]
+}
+RIGHT = {
+    'http': [
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.19.0/24'), app='http', name='web access'),
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.20.0/24'), app='http', name='stage web access'),
+    ],
+    'https': [
+        Rule(src=ipset('0.0.0.0/0'), dst=ipset('10.20.19.0/24'), app='https', name='https web access'),
+    ],
+    'db': [
+        Rule(src=ipset('10.20.19.0/24'), dst=ipset('10.20.30.15', '10.20.30.19', '10.20.30.28'),
+                app='db', name='db'),
+        Rule(src=ipset('10.20.20.0/24'), dst=ipset('10.20.30.16'), app='db', name='stage db'),
+    ],
+    'ssh': [
+        Rule(src=ipset('10.2.1.1'),
+                dst=ipset('10.20.20.0/24', '10.20.30.16'),
+                app='ssh', name='ssh'),
+    ]
+}
+class FakeSource(sources.Source):
+
+    def __init__(self, rules):
+        self.rules = rules
+
+
+def test_app_diff_add():
+    l = []
+    r = [Rule(src=TEN, dst=TWENTY, app='http', name='10->20')]
+    eq_(list(diff.app_diff('http', l, r)),
+        [('+', 'http', TEN, TWENTY)])
+
+
+def test_app_diff_remove():
+    l = [Rule(src=TEN, dst=TWENTY, app='http', name='10->20')]
+    r = []
+    eq_(list(diff.app_diff('http', l, r)),
+        [('-', 'http', TEN, TWENTY)])
+
+
+def test_app_diff_replace():
+    l = [Rule(src=TEN, dst=TWENTY, app='http', name='10->20')]
+    r = [Rule(src=TWENTY, dst=TEN, app='http', name='20->10')]
+    eq_(list(diff.app_diff('http', l, r)),
+        [('-', 'http', TEN, TWENTY),
+         ('+', 'http', TWENTY, TEN)])
+
+
+def test_app_diff_expanded():
+    l = [Rule(src=TEN_128, dst=TWENTY, app='http', name='10.128->20')]
+    r = [Rule(src=TEN,     dst=TWENTY, app='http', name='10->20')]
+    eq_(list(diff.app_diff('http', l, r)),
+        [('+', 'http', TEN_0, TWENTY)])
+
+
+def test_app_diff_expanded_dest():
+    l = [Rule(src=TEN, dst=TWENTY_0, app='http', name='10.128->20')]
+    r = [Rule(src=TEN, dst=TWENTY,   app='http', name='10->20')]
+    eq_(list(diff.app_diff('http', l, r)),
+        [('+', 'http', TEN, TWENTY_128)])
+
+
+def test_app_diff_shrunk():
+    l = [Rule(src=TEN,   dst=TWENTY, app='http', name='10->20')]
+    r = [Rule(src=TEN_0, dst=TWENTY, app='http', name='10.0/17->20')]
+    eq_(list(diff.app_diff('http', l, r)),
+        [('-', 'http', TEN_128, TWENTY)])
+
+
+def test_app_diff_reorg_rules():
+    # two different ways to represent the same flows..
+    l = [
+        Rule(src=TEN_0,   dst=TWENTY_0,   app='http', name='10->20'),
+        Rule(src=TEN,     dst=TWENTY_128, app='http', name='10->20'),
+    ]
+    r = [
+        Rule(src=TEN_0,   dst=TWENTY,     app='http', name='10->20'),
+        Rule(src=TEN_128, dst=TWENTY_128, app='http', name='10->20'),
+    ]
+    eq_(list(diff.app_diff('http', l, r)), [])
+
+def test_make_diff():
+    eq_(sorted(diff.make_diff(FakeSource(LEFT), FakeSource(RIGHT))), sorted([
+        # expand stage from /25 to /24
+        ('+', 'db', ipset('10.20.20.128/25'), ipset('10.20.30.16')),
+        ('+', 'http', ipset('0.0.0.0/0'), ipset('10.20.20.128/25')),
+        ('+', 'ssh', ipset('10.2.1.1'), ipset('10.20.20.128/25')),
+        # add production db server
+        ('+', 'db', ipset('10.20.19.0/24'), ipset('10.20.30.28')),
+        # remove ssh access to production
+        ('-', 'ssh', ipset('10.2.1.1'), ipset('10.20.19.0/24', '10.20.30.15', '10.20.30.19')),
+    ]))
+
+def test_show_diff():
+    with mock.patch('fwunit.analysis.sources.load_source') as load_source, \
+         mock.patch('sys.stdout') as stdout:
+        def fake_load_source(cfg, name):
+            if name == 'left':
+                return FakeSource(LEFT)
+            else:
+                return FakeSource(RIGHT)
+        load_source.side_effect = fake_load_source
+        written = []
+        def fake_write(data):
+            written.append(data)
+        stdout.write.side_effect = fake_write
+        diff.show_diff(None, 'left', 'right')
+        written = ''.join(written)
+        # rather than assert on exactly what's written, which may change as presentaiton improves,
+        # just assert that we wrote lines starting with + or -, after stripping escape codes
+        for line in filter(None, written.split('\n')):
+            line = re.sub('\x1b[^m]+m', '', line)
+            assert line.startswith('+') or line.startswith('-'), line

fwunit/test/unit/test_ip.py

@@ -82,3 +82,13 @@ def test_ippairs():
         IPPairs((ten, ten - ten26), (ten - ten26, ten26)))
         # TODO: equivalent to, but doesn't compare equal to:
         #  IPPairs((ten - ten26, ten), (ten26, ten - ten26)))
+
+def test_ippairs_sub_empty():
+    ten = IPSet([IP('10.0.0.0/8')])
+    twenty = IPSet([IP('20.0.0.0/8')])
+    for pairs in [
+            IPPairs(),
+            IPPairs((ten, twenty)),
+            IPPairs((ten, twenty), (twenty, ten)),
+        ]:
+        eq_(pairs - IPPairs(), pairs)

setup.py

@@ -44,6 +44,7 @@
         "console_scripts": [
             'fwunit = fwunit.scripts:main',
             'fwunit-query = fwunit.scripts:query',
+            'fwunit-diff = fwunit.scripts:diff',
         ],
         "fwunit.types": [
             'srx = fwunit.srx.scripts:run [srx]',