diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 4073ebc..9adadbd 100644 --- a/.gitlab-ci.yml +++ b/.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 diff --git a/README.md b/README.md new file mode 100644 index 0000000..9866aab --- /dev/null +++ b/README.md @@ -0,0 +1,3 @@ +[![pipeline status](https://gitlab.niet.verweg.com/ruben/jail2ban-pf/badges/main/pipeline.svg)](https://gitlab.niet.verweg.com/ruben/jail2ban-pf/-/commits/main) +[![coverage report](https://gitlab.niet.verweg.com/ruben/jail2ban-pf/badges/main/coverage.svg)](https://gitlab.niet.verweg.com/ruben/jail2ban-pf/-/commits/main) + diff --git a/jail2ban/__init__.py b/jail2ban/__init__.py index 9d43d7f..889963d 100644 --- a/jail2ban/__init__.py +++ b/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) + # load the instance config, if it exists, when not testing + app.config.from_pyfile('config.py', 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 @@ -80,7 +77,7 @@ def create_app(config=None): ]) res = pfctl_cfg_write(f'f2b-jail/{remote_user}', b'\n'.join(cfg) + b'\n') - elif request.method == 'DELETE': + else: # 'DELETE': cfg = [cfg_line for cfg_line in cfg if cfg_line.find(bytes(f'', 'ascii')) == -1] res = pfctl_cfg_write(f'f2b-jail/{remote_user}', @@ -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 @@ -114,7 +110,7 @@ def create_app(config=None): table=f'f2b-{name}', operation='add', value=str(ip)) - elif request.method == 'DELETE': + else: # 'DELETE': app.logger.info(f'Remove {ip} from f2b-{name}' f' in anchor f2b-jail/{remote_user}') res = pfctl_table_op(f'f2b-jail/{remote_user}', @@ -135,6 +131,22 @@ 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 + + @app.errorhandler(FileNotFoundError) + def filenotfound_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') diff --git a/jail2ban/auth.py b/jail2ban/auth.py index d9867bf..8b4b1b4 100644 --- a/jail2ban/auth.py +++ b/jail2ban/auth.py @@ -1,18 +1,15 @@ -from flask import current_app, g -import os +from flask import current_app def get_users(): - if 'users' not in g: - users = {} - authfile = current_app.config['AUTHFILE'] + users = {} + authfile = current_app.config['AUTHFILE'] - current_app.logger.debug('Reading %s for users', authfile) + current_app.logger.debug('Reading %s for users', authfile) - with current_app.open_resource(os.path.join("..", - authfile)) as f: - for entry in f: - users.update({tuple(entry.decode('ascii').strip().split(':', 1))}) - g.users = users - current_app.logger.debug(g.users) - return g.users + with current_app.open_resource(authfile) as f: + for entry in f: + users.update({ + tuple(entry.decode('ascii').strip().split(':', 1))}) + current_app.logger.debug(users) + return users diff --git a/jail2ban/pfctl.py b/jail2ban/pfctl.py index 4a079e7..0e062c5 100644 --- a/jail2ban/pfctl.py +++ b/jail2ban/pfctl.py @@ -9,12 +9,10 @@ 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() + logging.info('Result: %s', res) + return res.stdout.splitlines() def pfctl_cfg_write(anchor, cfg): @@ -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 + logging.info('Result: %s', res) + return res.stdout.splitlines() 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() + logging.debug(res) + return res.stdout.splitlines() diff --git a/tests/conftest.py b/tests/conftest.py index 2ebdf68..4adf233 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import pytest +import base64 from jail2ban import create_app @@ -8,7 +9,7 @@ def app(): app.config.update({ "TESTING": True, "SECRET_KEY": 'Testing', - "AUTHFILE": 'tests/users-test.txt' + "AUTHFILE": '../tests/users-test.txt' }) # other setup can go here @@ -26,3 +27,8 @@ def client(app): @pytest.fixture() def runner(app): return app.test_cli_runner() + + +@pytest.fixture() +def valid_credentials(): + return base64.b64encode(b"test.example.com:testpassword").decode("utf-8") diff --git a/tests/test_ban.py b/tests/test_ban.py new file mode 100644 index 0000000..a2553f2 --- /dev/null +++ b/tests/test_ban.py @@ -0,0 +1,102 @@ +from types import SimpleNamespace + + +def test_ban_ipv6(client, mocker, valid_credentials): + 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) + + 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, valid_credentials): + 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) + + 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, valid_credentials): + 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) + + 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, valid_credentials): + 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) + + 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, valid_credentials): + 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) + + 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' diff --git a/tests/test_flush.py b/tests/test_flush.py new file mode 100644 index 0000000..9ac64ba --- /dev/null +++ b/tests/test_flush.py @@ -0,0 +1,77 @@ +from types import SimpleNamespace +from subprocess import CalledProcessError + + +def test_flush(client, mocker, valid_credentials): + 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) + + name = 'sshd' + response = client.get(f"/flush/{name}", + headers={"Authorization": + "Basic " + valid_credentials}) + + assert response.json['operation'] == 'flush' + + +def test_flush_nonexistent(client, mocker, valid_credentials): + + cmd = ['/usr/local/bin/sudo', + '/sbin/pfctl', '-a', 'some/anchor', + '-t', 'nonexistent', '-T', 'flush'] + + side_effect = CalledProcessError(255, cmd, output=b'', + stderr=b'pfctl: Table does not exist') + + mocker.patch('jail2ban.pfctl.run', + side_effect=side_effect) + + name = 'nonexistent' + response = client.get(f"/flush/{name}", + headers={"Authorization": + "Basic " + valid_credentials}) + + assert 'error' in response.json + + +def test_wrong_method(client, mocker, valid_credentials): + + cmd = ['/usr/local/bin/sudo', + '/sbin/pfctl', '-a', 'some/anchor', + '-t', 'nonexistent', '-T', 'flush'] + + side_effect = CalledProcessError(255, cmd, output=b'', + stderr=b'pfctl: Table does not exist') + + mocker.patch('jail2ban.pfctl.run', + side_effect=side_effect) + + name = 'nonexistent' + response = client.put(f"/flush/{name}", + headers={"Authorization": + "Basic " + valid_credentials}) + + assert response.status_code == 405 + + +def test_filenotfound(app, mocker, valid_credentials): + + app.config.update({ + "AUTHFILE": '../tests/nonexistent-users-test.txt' + }) + + client = app.test_client() + + name = 'nonexistent' + response = client.get(f"/flush/{name}", + headers={"Authorization": + "Basic " + valid_credentials}) + + assert response.status_code == 500 diff --git a/tests/test_register.py b/tests/test_register.py index b061ad6..9312d09 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -1,5 +1,4 @@ -import base64 -from types import SimpleNamespace +from subprocess import CompletedProcess pfctl_stdout_lines = b''' block drop quick proto tcp from to any port = submission @@ -7,31 +6,108 @@ block drop quick proto tcp from to any port = smtps block drop quick proto tcp from to any port = smtp block drop quick proto tcp from to any port = ssh block drop quick proto tcp from to any -''' +'''.strip() + b'\n' + +pfctl_stdout_lines_scratch = b'table persist counters\n' \ + b'block quick proto tcp from ' \ + b' to any port ' \ + b'{pop3,pop3s,imap,imaps,submission,465,sieve}\n' -def test_request_unauth(client): - json_payload = {"port": "any port {pop3,pop3s,imap,imaps,submission,465,sieve}", "name": "dovecot", "protocol": "tcp"} +def test_register_unauth(client): + 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_unregister_valid(client, mocker, valid_credentials): 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) + 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_register_valid(client, mocker, valid_credentials): + def noop(): + pass + run_res = CompletedProcess(args=['true'], returncode=0) + run_res.stdout = pfctl_stdout_lines + run_res.check_returncode = noop + + pfctl_run = mocker.patch('jail2ban.pfctl.run', return_value=run_res) + + json_payload = {"port": + "any port {pop3,pop3s,imap,imaps,submission,465,sieve}", + "name": "dovecot", "protocol": "tcp"} - 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.put("/register", json=json_payload, - headers={"Authorization": "Basic " + valid_credentials}) + headers={"Authorization": + "Basic " + valid_credentials}) + + pfctl_run_input_arg = pfctl_run.call_args_list[1][1]['input'] + for existing_line in pfctl_stdout_lines.splitlines(): + assert existing_line in pfctl_run_input_arg.splitlines() + + assert response.json['action'] == 'start' - assert response.json['remote_user'] == 'test.example.com' +def test_register_valid_from_scratch(client, mocker, valid_credentials): + def noop(): + pass + run_res = CompletedProcess(args=['true'], returncode=0) + run_res.stdout = b'' + run_res.check_returncode = noop + + pfctl_run = mocker.patch('jail2ban.pfctl.run', return_value=run_res) + + 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}) + + pfctl_run_input_arg = pfctl_run.call_args_list[1][1]['input'] + assert pfctl_run_input_arg == pfctl_stdout_lines_scratch + assert response.json['action'] == 'start' + + +def test_register_invalid(client, mocker, valid_credentials): + 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) + + 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'