Add exception handler for when pfctl operations fail

.gitlab-ci.yml

@@ -4,12 +4,14 @@ run tests:
   script:
     - pip install pytest pytest-cov pytest-mock pytest-flask
     - pip install Flask-HTTPAuth
-    - coverage run -m pytest
+    - coverage run -m pytest --junitxml=report.xml
     - coverage report
     - coverage xml
   coverage: '/^TOTAL.+?(\d+\%)$/'
   artifacts:
+    when: always
     reports:
       cobertura: coverage.xml
+      junit: report.xml
   tags:
     - docker

jail2ban/__init__.py

@@ -5,6 +5,7 @@ from ipaddress import ip_address
 import re
 from jail2ban.pfctl import pfctl_table_op, pfctl_cfg_read, pfctl_cfg_write
 from jail2ban.auth import get_users
+from subprocess import CalledProcessError
 
 
 auth = HTTPBasicAuth()
@@ -26,15 +27,11 @@ def untaint(pattern, string):
         raise ValueError(f'"{string}" is tainted')
 
 
-def create_app(config=None):
+def create_app():
     app = Flask(__name__, instance_relative_config=True)
 
-    if config is None:
     # load the instance config, if it exists, when not testing
     app.config.from_pyfile('config.py', silent=True)
-    else:
-        # load the test config if passed in
-        app.config.from_pyfile(config, silent=True)
 
     @auth.verify_password
     def verify_password(username, password):
@@ -58,7 +55,7 @@ def create_app(config=None):
         return jsonify({'anchor': f'f2b-jail/{remote_user}',
                         'table': f'f2b-{name}',
                         'operation': 'flush',
-                        'result': res})
+                        'result': [x.decode('ascii') for x in res]})
 
     @app.route("/register", methods=['PUT', 'DELETE'])
     @auth.login_required
@@ -92,12 +89,11 @@ def create_app(config=None):
                            table=f'f2b-{name}',
                            operation='kill')
         app.logger.info(f'pfctl -a f2b-jail/{remote_user} -f-')
-        return jsonify({'remote_user': remote_user, 'data': data})
         return jsonify({'anchor': f'f2b-jail/{remote_user}',
                         'table': f'f2b-{name}',
                         'action': 'start' if request.method == 'PUT'
                         else 'stop',
-                        'result': res})
+                        'result': [x.decode('ascii') for x in res]})
 
     @app.route("/ban", methods=['PUT', 'DELETE'])
     @auth.login_required
@@ -135,6 +131,14 @@ def create_app(config=None):
         app.logger.fatal(error)
         return jsonify({'error': str(error)}), 500
 
+    @app.errorhandler(CalledProcessError)
+    def subprocess_err(error):
+        '''
+        Show a json parsable error if the value is illegal
+        '''
+        app.logger.fatal(error)
+        return jsonify({'error': str(error)}), 500
+
     @auth.error_handler
     def auth_error():
         app.logger.error('Access Denied')

jail2ban/pfctl.py

@@ -9,11 +9,9 @@ def pfctl_cfg_read(anchor):
     cmd = [_SUDO, _PFCTL, '-a', anchor, '-sr']
     logging.info('Running %s', cmd)
 
-    res = run(cmd, capture_output=True)
+    res = run(cmd, capture_output=True, check=True)
 
-    if res and res.stdout:
     logging.info('Result: %s', res)
-        res.check_returncode()
     return res.stdout.splitlines()
 
 
@@ -24,12 +22,11 @@ def pfctl_cfg_write(anchor, cfg):
 
     res = run(cmd,
               input=cfg,
+              check=True,
               capture_output=True)
 
-    if res:
     logging.info('Result: %s', res)
-        res.check_returncode()
+    return res.stdout.splitlines()
-        return res
 
 
 def pfctl_table_op(anchor, **kwargs):
@@ -40,9 +37,9 @@ def pfctl_table_op(anchor, **kwargs):
 
     logging.info('Running %s', cmd)
 
-    res = run([x for x in cmd if x is not None], capture_output=True)
+    res = run([x for x in cmd if x is not None],
+              capture_output=True,
+              check=True)
 
-    if res:
     logging.debug(res)
-        res.check_returncode()
     return res.stdout.splitlines()

tests/test_ban.py

@@ -0,0 +1,101 @@
+import base64
+from types import SimpleNamespace
+
+
+def test_ban_ipv6(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses added.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"name": "sshd", "ip": "2001:db8::abad:cafe"}
+    response = client.put("/ban",
+                          json=json_payload,
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['operation'] == 'add'
+
+
+def test_ban_ipv4(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses added.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"name": "sshd", "ip": "192.0.2.42"}
+    response = client.put("/ban",
+                          json=json_payload,
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['operation'] == 'add'
+
+def test_ban_invalid(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses added.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"name": "sshd", "ip": "not:an::addr:ess"}
+    response = client.put("/ban",
+                          json=json_payload,
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['error'] == "'not:an::addr:ess' does not appear to be an IPv4 or IPv6 address"
+
+
+def test_unban_ipv6(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses deleted.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"name": "sshd", "ip": "2001:db8::abad:cafe"}
+    response = client.delete("/ban",
+                             json=json_payload,
+                             headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['operation'] == 'delete'
+
+
+def test_unban_ipv4(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses deleted.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"name": "sshd", "ip": "192.0.2.42"}
+    response = client.delete("/ban",
+                          json=json_payload,
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['operation'] == 'delete'

tests/test_flush.py

@@ -0,0 +1,38 @@
+import base64
+from types import SimpleNamespace
+from subprocess import CalledProcessError
+
+
+def test_flush(client, mocker):
+    def noop():
+        pass
+    run_res = SimpleNamespace()
+    run_res.stdout = b''
+    run_res.stderr = b'1/1 addresses deleted.\n'
+    run_res.returncode = 0
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    name = 'sshd'
+    response = client.get(f"/flush/{name}",
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['operation'] == 'flush'
+
+
+def test_flush_nonexistent(client, mocker):
+
+    cmd = ['/usr/local/bin/sudo', '/sbin/pfctl', '-a', 'some/anchor', '-t', 'nonexistent', '-T', 'flush']
+
+    mocker.patch('jail2ban.pfctl.run',
+                 side_effect=CalledProcessError(255, cmd, output=b'',
+                                                stderr=b'pfctl: Table does not exist'))
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    name = 'nonexistent'
+    response = client.get(f"/flush/{name}",
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert 'error' in response.json

tests/test_register.py

@@ -1,5 +1,5 @@
 import base64
-from types import SimpleNamespace
+from subprocess import CompletedProcess
 
 pfctl_stdout_lines = b'''
 block drop quick proto tcp from <f2b-sendmail-auth> to any port = submission
@@ -10,28 +10,73 @@ block drop quick proto tcp from <f2b-recidive> to any
 '''
 
 
-def test_request_unauth(client):
+def test_register_unauth(client):
-    json_payload = {"port": "any port {pop3,pop3s,imap,imaps,submission,465,sieve}", "name": "dovecot", "protocol": "tcp"}
+    json_payload = {"port":
+                    "any port {pop3,pop3s,imap,imaps,submission,465,sieve}",
+                    "name": "dovecot", "protocol": "tcp"}
     response = client.put("/register", json=json_payload)
 
     assert response.json['error'] == 'Access Denied'
 
 
-def test_request_example(client, mocker):
+def test_register_valid(client, mocker):
     def noop():
         pass
-    run_res = SimpleNamespace()
+    run_res = CompletedProcess(args=['true'], returncode=0)
     run_res.stdout = pfctl_stdout_lines
     run_res.check_returncode = noop
 
     mocker.patch('jail2ban.pfctl.run', return_value=run_res)
 
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"port":
+                    "any port {pop3,pop3s,imap,imaps,submission,465,sieve}",
+                    "name": "dovecot", "protocol": "tcp"}
+
+    response = client.delete("/register",
+                             json=json_payload,
+                             headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['action'] == 'stop'
+
+
+def test_unregister_valid(client, mocker):
+    def noop():
+        pass
+    run_res = CompletedProcess(args=['true'], returncode=0)
+    run_res.stdout = pfctl_stdout_lines
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
 
     valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
-    json_payload = {"port": "any port {pop3,pop3s,imap,imaps,submission,465,sieve}", "name": "dovecot", "protocol": "tcp"}
+    json_payload = {"port":
+                    "any port {pop3,pop3s,imap,imaps,submission,465,sieve}",
+                    "name": "dovecot", "protocol": "tcp"}
+
     response = client.put("/register",
                           json=json_payload,
                           headers={"Authorization": "Basic " + valid_credentials})
 
+    assert response.json['action'] == 'start'
 
-    assert response.json['remote_user'] == 'test.example.com'
+
+def test_register_invalid(client, mocker):
+    def noop():
+        pass
+    run_res = CompletedProcess(args=['true'], returncode=0)
+    run_res.stdout = pfctl_stdout_lines
+    run_res.check_returncode = noop
+
+    mocker.patch('jail2ban.pfctl.run', return_value=run_res)
+
+    valid_credentials = base64.b64encode(b"test.example.com:testpassword").decode("utf-8")
+    json_payload = {"port":
+                    "not a pf statement",
+                    "name": "dovecot", "protocol": "tcp"}
+
+    response = client.put("/register",
+                          json=json_payload,
+                          headers={"Authorization": "Basic " + valid_credentials})
+
+    assert response.json['error'] == '"not a pf statement" is tainted'