From c6c89760eb7b0cb2fc1b63e50cfebb4a87ea36d1 Mon Sep 17 00:00:00 2001 From: aszlig Date: Fri, 10 Jul 2020 00:07:48 +0200 Subject: [PATCH] Remove support for reading YAML files We already introduced deprecation warnings back in 49b889b03eb5e39f4fc348966a989567d58, telling people that we're going to remove YAML in version 3.0. With this, the removal is essentially done and we now no longer depend on yaml-cpp, nor do we need pkg-config or anything like that, because we essentially have zero (external) library dependencies except libc and libdl. This of course also greatly simplifies our parsing code and also gets rid of a few tests that now became obsolete. Signed-off-by: aszlig --- .github/workflows/main.yml | 2 +- README.adoc | 9 +- default.nix | 3 +- meson.build | 1 - release.nix | 3 +- src/ip2unix.cc | 70 +-------------- src/preload.cc | 5 -- src/rules.hh | 2 - src/rules/parse.cc | 173 ------------------------------------- tests/helper.py | 24 ++++- tests/test_connections.py | 16 ++-- tests/test_fdleak.py | 2 +- tests/test_program_args.py | 58 ++++++++----- tests/test_rule_file.py | 172 +++++++++++++++--------------------- tests/test_run_direct.py | 30 ------- tests/test_sockopt_fail.py | 2 +- 16 files changed, 147 insertions(+), 425 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 739ddfe..71c851c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -9,7 +9,7 @@ jobs: - uses: actions/setup-python@v1 with: python-version: '3.x' - - run: sudo apt install libyaml-cpp-dev asciidoctor libsystemd-dev + - run: sudo apt install asciidoctor libsystemd-dev - run: pip install meson ninja pytest - run: meson setup build - run: meson test -C build -v diff --git a/README.adoc b/README.adoc index d83aee9..6c430f1 100644 --- a/README.adoc +++ b/README.adoc @@ -156,9 +156,6 @@ endif::[] * https://mesonbuild.com/[Meson], at least version 0.46.0. * https://ninja-build.org/[Ninja], at least version 1.5. -* https://github.com/jbeder/yaml-cpp[yaml-cpp], at least version 0.5.0 - Requirement will be removed in *ip2unix* version 3, since the YAML rule file - format is deprecated. * {cpp} compiler supporting {cpp}17 (https://gcc.gnu.org/[GNU G++] version 7.0 onwards). * https://www.python.org/[Python] 3, at least version 3.6 is needed for running @@ -223,7 +220,7 @@ To install the required dependencies: [source,sh-session] --------------------------------------------------------------------- -$ sudo apt install meson g++ libyaml-cpp-dev pkg-config +$ sudo apt install meson g++ --------------------------------------------------------------------- If you want to have the manpage: @@ -246,7 +243,7 @@ To install the required dependencies: [source,sh-session] --------------------------------------------------------------------- -$ sudo yum install meson gcc-c++ yaml-cpp-devel +$ sudo yum install meson gcc-c++ --------------------------------------------------------------------- If you want to have the manpage: @@ -269,7 +266,7 @@ To install the required dependencies: [source,sh-session] --------------------------------------------------------------------- -$ sudo pacman -S yaml-cpp meson gcc pkg-config +$ sudo pacman -S meson gcc --------------------------------------------------------------------- If you want to have the manpage: diff --git a/default.nix b/default.nix index 6f67a9b..e1f1ce6 100644 --- a/default.nix +++ b/default.nix @@ -29,12 +29,11 @@ pkgs.stdenv.mkDerivation rec { }; nativeBuildInputs = [ - pkgs.meson pkgs.ninja pkgs.pkgconfig pkgs.asciidoc pkgs.libxslt.bin + pkgs.meson pkgs.ninja pkgs.asciidoc pkgs.libxslt.bin pkgs.docbook_xml_dtd_45 pkgs.docbook_xsl pkgs.libxml2.bin pkgs.docbook5 pkgs.python3Packages.pytest pkgs.python3Packages.pytest-timeout pkgs.systemd ]; - buildInputs = [ pkgs.libyamlcpp ]; doCheck = true; diff --git a/meson.build b/meson.build index 5043341..822c5de 100644 --- a/meson.build +++ b/meson.build @@ -33,7 +33,6 @@ lib_cflags = [] lib_ldflags = [] deps = [ - dependency('yaml-cpp', version: '>=0.5.0'), cc.find_library('dl') ] diff --git a/release.nix b/release.nix index b91f7c0..5013de3 100644 --- a/release.nix +++ b/release.nix @@ -17,9 +17,8 @@ let mesonFlags = [ "-Dtest-timeout=3600" ] ++ attrs.mesonFlags or []; - nativeBuildInputs = [ pkgs.meson pkgs.ninja pkgs.pkgconfig ] + nativeBuildInputs = [ pkgs.meson pkgs.ninja ] ++ attrs.nativeBuildInputs or []; - buildInputs = [ pkgs.libyamlcpp ] ++ attrs.buildInputs or []; doCheck = attrs.doCheck or true; diff --git a/src/ip2unix.cc b/src/ip2unix.cc index 2d7ef19..4aa82bc 100644 --- a/src/ip2unix.cc +++ b/src/ip2unix.cc @@ -97,31 +97,7 @@ static void print_version(void) stdout); } -static void warn_deprecated_rules_file_long_opt(void) -{ - fputs("The use of the --rules-file option is deprecated and the option" - " will be removed in ip2unix version 3.0. Furthermore, YAML will no" - " longer be supported for rule specification and the new --file" - " option simply takes a file with a list of newline-separated rules" - " as specified via the -r/--rule option.", stderr); -} - -static void warn_deprecated_yaml_data(void) -{ - fputs("The use of -F/--rules-data option is deprecated and it will be" - " removed in ip2unix version 3. Please use the -r/--rule option" - " instead.\n", stderr); -} - -static void warn_deprecated_yaml_file(std::string &filename) -{ - fprintf(stderr, "The rule file '%s' contains YAML data, which will no" - " longer be supported in ip2unix version 3. Please use a list of" - " newline-separated rules as specified via the -r/--rule option.", - filename.c_str()); -} - -static bool push_rule_args_from_file(std::string &filename, +static bool push_rule_args_from_file(std::string filename, std::vector &rule_args) { std::ifstream input(filename); @@ -164,10 +140,6 @@ int main(int argc, char *argv[]) bool show_rules = false; unsigned int verbosity = 0; - // TODO: Remove in version 3.0. - bool show_warn_deprecated_rules_file_long_opt = false; - bool show_warn_deprecated_yaml_data = false; - static struct option lopts[] = { {"help", no_argument, nullptr, 'h'}, {"version", no_argument, nullptr, 'V'}, @@ -176,11 +148,6 @@ int main(int argc, char *argv[]) {"rule", required_argument, nullptr, 'r'}, {"file", required_argument, nullptr, 'f'}, {"verbose", no_argument, nullptr, 'v'}, - - // TODO: Remove in version 3.0. - {"rules-file", required_argument, nullptr, 'y'}, - {"rules-data", required_argument, nullptr, 'F'}, - {nullptr, 0, nullptr, 0} }; @@ -188,7 +155,7 @@ int main(int argc, char *argv[]) std::optional ruledata = std::nullopt; std::vector rule_args; - while ((c = getopt_long(argc, argv, "+hcpr:f:F:v", + while ((c = getopt_long(argc, argv, "+hcpr:f:v", lopts, nullptr)) != -1) { switch (c) { case 'h': @@ -211,28 +178,11 @@ int main(int argc, char *argv[]) rule_args.push_back(optarg); break; - case 'y': - show_warn_deprecated_rules_file_long_opt = true; - /* fallthrough */ case 'f': - rulefile = std::string(optarg); - if (is_yaml_rule_file(*rulefile)) - warn_deprecated_yaml_file(*rulefile); - else if (push_rule_args_from_file(*rulefile, rule_args)) - // XXX: This is to make sure that when we use the new rule - // file format we can use multiple -f options. - // TODO: Remove this in version 3.0 when dropping YAML - // support. - rulefile = std::nullopt; - else + if (!push_rule_args_from_file(std::string(optarg), rule_args)) return EXIT_FAILURE; break; - case 'F': - show_warn_deprecated_yaml_data = true; - ruledata = std::string(optarg); - break; - case 'v': verbosity++; break; @@ -244,12 +194,6 @@ int main(int argc, char *argv[]) } } - // TODO: Remove in version 3.0. - if (show_warn_deprecated_rules_file_long_opt) - warn_deprecated_rules_file_long_opt(); - if (show_warn_deprecated_yaml_data) - warn_deprecated_yaml_data(); - if (!rule_args.empty() && (rulefile || ruledata)) { fprintf(stderr, "%s: Can't specify both direct rules and a rule" " file.\n\n", self); @@ -275,14 +219,6 @@ int main(int argc, char *argv[]) else return EXIT_FAILURE; } - } else if (rulefile) { - auto result = parse_rules(rulefile.value(), true); - if (!result) return EXIT_FAILURE; - rules = result.value(); - } else if (ruledata) { - auto result = parse_rules(ruledata.value(), false); - if (!result) return EXIT_FAILURE; - rules = result.value(); } else { fprintf(stderr, "%s: You need to either specify a rule file with '-f'" " or directly specify rules via '-r'.\n\n", self); diff --git a/src/preload.cc b/src/preload.cc index 75a83cf..6a5dd9a 100644 --- a/src/preload.cc +++ b/src/preload.cc @@ -45,11 +45,6 @@ static void init_rules(void) LOG(FATAL) << "Unable to decode __IP2UNIX_RULES: " << *err; _exit(EXIT_FAILURE); } - } else if ((rule_source = getenv("IP2UNIX_RULE_FILE")) != nullptr) { - std::cerr << "The use of the IP2UNIX_RULE_FILE environment" - " variable is deprecated and will be removed in" - " ip2unix version 3.0." << std::endl; - rules = parse_rules(std::string(rule_source), true); } else { LOG(FATAL) << "Unable to find __IP2UNIX_RULES!"; _exit(EXIT_FAILURE); diff --git a/src/rules.hh b/src/rules.hh index c7358d8..62408ca 100644 --- a/src/rules.hh +++ b/src/rules.hh @@ -33,8 +33,6 @@ struct Rule { bool ignore = false; }; -bool is_yaml_rule_file(std::string); -std::optional> parse_rules(std::string, bool); std::optional parse_rule_arg(size_t, const std::string&); void print_rules(std::vector&, std::ostream&); diff --git a/src/rules/parse.cc b/src/rules/parse.cc index 3c8bf99..87ffedf 100644 --- a/src/rules/parse.cc +++ b/src/rules/parse.cc @@ -9,24 +9,10 @@ #include #include -#include - #include "../rules.hh" #include "errno_list.hh" -static const std::string describe_nodetype(const YAML::Node &node) -{ - switch (node.Type()) { - case YAML::NodeType::Undefined: return "undefined"; - case YAML::NodeType::Null: return "null"; - case YAML::NodeType::Scalar: return "a scalar"; - case YAML::NodeType::Sequence: return "a sequence"; - case YAML::NodeType::Map: return "a map"; - } - return "an unknown type"; -} - static std::optional validate_rule(Rule &rule) { if (rule.address) { @@ -141,165 +127,6 @@ static std::optional parse_errno(const std::string &str) return name2errno(str); } -#define RULE_ERROR(msg) \ - std::cerr << file << ":rule #" << pos + 1 << ": " << msg << std::endl - -#define RULE_CONVERT(target, key, type, tname) \ - try { \ - target = value.as(); \ - } catch (const YAML::BadConversion &e) { \ - RULE_ERROR("The \"" key "\" option needs to be a " tname "."); \ - return std::nullopt; \ - } - -static std::optional parse_rule(const std::string &file, int pos, - const YAML::Node &doc) -{ - Rule rule; - - for (const auto &foo : doc) { - std::string key = foo.first.as(); - YAML::Node value = foo.second; - if (key == "direction") { - std::string val; - RULE_CONVERT(val, "direction", std::string, "string"); - if (val == "outgoing") { - rule.direction = RuleDir::OUTGOING; - } else if (val == "incoming") { - rule.direction = RuleDir::INCOMING; - } else { - RULE_ERROR("Invalid direction \"" << val << "\"."); - return std::nullopt; - } - } else if (key == "type") { - std::string val; - RULE_CONVERT(val, "type", std::string, "string"); - if (val == "tcp") { - rule.type = SocketType::TCP; - } else if (val == "udp") { - rule.type = SocketType::UDP; - } else { - RULE_ERROR("Invalid type \"" << val << "\"."); - return std::nullopt; - } - } else if (key == "address") { - RULE_CONVERT(rule.address, "address", std::string, "string"); - } else if (key == "port") { - std::string val; - RULE_CONVERT(val, "port", std::string, "16 bit unsigned int"); - std::optional port = string2port(val); - if (port) { - rule.port = port.value(); - } else { - RULE_ERROR("Port number is not a 16 bit unsigned int."); - return std::nullopt; - } - } else if (key == "portEnd") { - std::string val; - RULE_CONVERT(val, "portEnd", std::string, "16 bit unsigned int"); - std::optional portend = string2port(val); - if (portend) { - rule.port_end = portend.value(); - } else { - RULE_ERROR("Port range end number is not a " - "16 bit unsigned int."); - return std::nullopt; - } -#ifdef SYSTEMD_SUPPORT - } else if (key == "socketActivation") { - RULE_CONVERT(rule.socket_activation, "socketActivation", bool, - "bool"); - } else if (key == "fdName") { - RULE_CONVERT(rule.fd_name, "fdName", std::string, "string"); -#endif - } else if (key == "reject") { - RULE_CONVERT(rule.reject, "reject", bool, "bool"); - } else if (key == "rejectError") { - std::string val; - RULE_CONVERT(val, "rejectError", std::string, "string"); - std::optional rej_errno = parse_errno(val); - if (rej_errno) { - rule.reject_errno = rej_errno; - } else { - RULE_ERROR("Invalid reject error code \"" << val << "\"."); - return std::nullopt; - } - } else if (key == "blackhole") { - RULE_CONVERT(rule.blackhole, "blackhole", bool, "bool"); - } else if (key == "ignore") { - RULE_CONVERT(rule.ignore, "ignore", bool, "bool"); - } else if (key == "socketPath") { - RULE_CONVERT(rule.socket_path, "socketPath", std::string, - "string"); - } else { - RULE_ERROR("Invalid key \"" << key << "\"."); - return std::nullopt; - } - } - - std::optional errmsg = validate_rule(rule); - if (errmsg) { - RULE_ERROR(errmsg.value()); - return std::nullopt; - } - - return rule; -} - -bool is_yaml_rule_file(std::string filename) -{ - YAML::Node doc; - - try { - doc = YAML::LoadFile(filename); - } catch (const YAML::ParserException &e) { - return false; - } catch (const YAML::BadFile &e) { - // If the file can't be opened, let's assume it's YAML for now, since - // we're going to eventually throw an error anyway. - return true; - } - - return doc.IsSequence(); -} - -std::optional> - parse_rules(std::string content, bool content_is_filename) -{ - YAML::Node doc; - std::string file = content_is_filename ? content : ""; - - try { - if (content_is_filename) - doc = YAML::LoadFile(file); - else - doc = YAML::Load(content); - } catch (const YAML::ParserException &e) { - std::cerr << file << ": " << e.msg << std::endl; - return std::nullopt; - } catch (const YAML::BadFile &e) { - std::cerr << "Unable to open file \"" << file << "\"." << std::endl; - return std::nullopt; - } - - if (!doc.IsSequence()) { - std::cerr << file << ": Root node needs to be a sequence but it's " - << describe_nodetype(doc) << " instead." << std::endl; - return std::nullopt; - } - - std::vector result; - - int pos = 0; - for (const YAML::Node &node : doc) { - std::optional rule = parse_rule(file, pos++, node); - if (!rule) return std::nullopt; - result.push_back(rule.value()); - } - - return result; -} - static void print_arg_error(size_t rulepos, const std::string &arg, size_t pos, size_t len, const std::string &msg) { diff --git a/tests/helper.py b/tests/helper.py index b4d3c3c..55903e4 100644 --- a/tests/helper.py +++ b/tests/helper.py @@ -1,4 +1,3 @@ -import json import subprocess from contextlib import contextmanager @@ -7,8 +6,24 @@ from conftest import IP2UNIX, LIBIP2UNIX, SYSTEMD_SUPPORT, SYSTEMD_SA_PATH __all__ = ['IP2UNIX', 'LIBIP2UNIX', 'SYSTEMD_SUPPORT', 'SYSTEMD_SA_PATH', - 'ip2unix', 'systemd_only', 'non_systemd_only', - 'systemd_sa_helper_only'] + 'dict_to_rule', 'dict_to_rule_args', 'ip2unix', 'systemd_only', + 'non_systemd_only', 'systemd_sa_helper_only'] + + +def dict_to_rule(rule): + items = [] + for key, value in rule.items(): + if key in ['dir', 'type']: + items.append(value) + elif value is True: + items.append(key) + else: + items.append(f'{key}={value}') + return ','.join(items) + + +def dict_to_rule_args(rules): + return [arg for rule in rules for arg in ['-r', dict_to_rule(rule)]] @contextmanager @@ -17,7 +32,8 @@ def ip2unix(rules, childargs, *args, **kwargs): cmdargs = [] if ip2unix_args is None else ip2unix_args pre_cmd = kwargs.pop('pre_cmd', None) pre = [] if pre_cmd is None else pre_cmd - full_args = pre + [IP2UNIX, '-F', json.dumps(rules)] + cmdargs + childargs + rule_args = dict_to_rule_args(rules) + full_args = pre + [IP2UNIX] + rule_args + cmdargs + childargs yield subprocess.Popen(full_args, *args, **kwargs) diff --git a/tests/test_connections.py b/tests/test_connections.py index f7778f6..358f465 100644 --- a/tests/test_connections.py +++ b/tests/test_connections.py @@ -55,14 +55,14 @@ def run_server(self, rules, *args, **kwargs): proc.stdin.write(b'\n') def assert_connection(self, crule, srule, cargs, sargs, pre_cmd_srv=None): - client_rule = {'direction': 'outgoing', 'socketPath': self.sockpath} + client_rule = {'dir': 'out', 'path': self.sockpath} client_rule.update(crule) - if 'socketActivation' in srule: + if 'systemd' in srule: sync = False server_rule = dict(srule) else: sync = True - server_rule = {'socketPath': self.sockpath} + server_rule = {'path': self.sockpath} server_rule.update(srule) with self.run_server([server_rule], *sargs, pre_cmd=pre_cmd_srv, sync=sync): @@ -93,9 +93,9 @@ def test_several_threaded(self): def test_path_placeholders(self): args = ['127.0.0.1', 111] - srule = {'socketPath': os.path.join(self.tmpdir, '%a-%t-%p.sock')} + srule = {'path': os.path.join(self.tmpdir, '%a-%t-%p.sock')} clipath = '127.0.0.1-' + self.SOTYPE + '-111.sock' - crule = {'socketPath': os.path.join(self.tmpdir, clipath)} + crule = {'path': os.path.join(self.tmpdir, clipath)} self.assert_connection(crule, srule, args, args) def test_nomatch(self): @@ -105,7 +105,7 @@ def test_nomatch(self): @helper.systemd_sa_helper_only def test_socket_activation(self): - srule = {'socketActivation': True} + srule = {'systemd': True} args = ['-c', 10, '4.3.2.1', 321] pre_cmd = [helper.SYSTEMD_SA_PATH, '-l', self.sockpath] if self.SOTYPE == 'udp': @@ -114,7 +114,7 @@ def test_socket_activation(self): @helper.systemd_sa_helper_only def test_socket_activation_threaded(self): - srule = {'socketActivation': True} + srule = {'systemd': True} args = ['-m', 'threading', '-p', 10, '-c', 20, '4.3.2.1', 321] pre_cmd = [helper.SYSTEMD_SA_PATH, '-l', self.sockpath] if self.SOTYPE == 'udp': @@ -123,7 +123,7 @@ def test_socket_activation_threaded(self): @helper.systemd_sa_helper_only def test_socket_activation_with_fdname(self): - srule = {'socketActivation': True, 'fdName': 'foo', 'port': 333} + srule = {'systemd': 'foo', 'port': 333} args = ['-c', 10, '4.3.2.1', 333] extrasock = os.path.join(self.tmpdir, 'extra.sock') pre_cmd = [helper.SYSTEMD_SA_PATH, '-l', extrasock] diff --git a/tests/test_fdleak.py b/tests/test_fdleak.py index 716d7c8..3a27fb8 100644 --- a/tests/test_fdleak.py +++ b/tests/test_fdleak.py @@ -23,7 +23,7 @@ @pytest.mark.skipif(not os.path.exists('/proc/self/fd'), reason='requires procfs') def test_fdleak(): - rules = [{'socketPath': '/dev/null/not/existing', 'direction': 'outgoing'}] + rules = [{'path': '/dev/null/not/existing', 'dir': 'out'}] cmd = [sys.executable, '-c', TESTPROG] with helper.ip2unix(rules, cmd, stdout=subprocess.PIPE) as proc: stdout = proc.communicate()[0] diff --git a/tests/test_program_args.py b/tests/test_program_args.py index b793e25..df37be8 100644 --- a/tests/test_program_args.py +++ b/tests/test_program_args.py @@ -1,7 +1,8 @@ -import json import sys import subprocess +from tempfile import NamedTemporaryFile + from helper import IP2UNIX, LIBIP2UNIX @@ -24,29 +25,44 @@ def test_unknown_arg(): assert b'invalid option' in check_error([IP2UNIX, '-X']) -def test_rules_and_rulefile(): - stderr = check_error([IP2UNIX, '-r', 'path=/foo', '-f', '/nonexistent']) - assert b"Can't specify both" in stderr - - -def test_rulefile_and_ruledata(): - stderr = check_error([IP2UNIX, '-f', '/nonexistent', '-F', '{}']) - assert b"rule file path and inline rules at the same time" in stderr +def test_rule_args_and_file(): + with NamedTemporaryFile('w') as rf: + rf.write('in,port=1234,path=/foo\nout,path=/bar\n') + rf.flush() + cmd = [IP2UNIX, '-c', '-p', '-r', 'in,udp,addr=127.0.0.1,ignore', + '-f', rf.name, '-r', 'out,addr=0.0.0.0,reject'] + result = subprocess.run(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) + + expected = b'Rule #1:\n' \ + b' Direction: incoming\n' \ + b' IP Type: UDP\n' \ + b' Address: 127.0.0.1\n' \ + b' Port: \n' \ + b' Don\'t handle this socket.\n' \ + b'Rule #2:\n' \ + b' Direction: incoming\n' \ + b' IP Type: TCP and UDP\n' \ + b' Address: \n' \ + b' Port: 1234\n' \ + b' Socket path: /foo\n' \ + b'Rule #3:\n' \ + b' Direction: outgoing\n' \ + b' IP Type: TCP and UDP\n' \ + b' Address: \n' \ + b' Port: \n' \ + b' Socket path: /bar\n' \ + b'Rule #4:\n' \ + b' Direction: outgoing\n' \ + b' IP Type: TCP and UDP\n' \ + b' Address: 0.0.0.0\n' \ + b' Port: \n' \ + b' Reject connect() and bind() calls.\n' + + assert result.stdout == expected def test_rule_longopts(tmpdir): - rulesfile = str(tmpdir.join('rules.yml')) - rulesdata = json.dumps([{'socketPath': '/test'}]) - open(rulesfile, 'w').write(rulesdata) - for deprecated_cmd in [ - [IP2UNIX, '-cp', '--rules-file', rulesfile], - [IP2UNIX, '-cp', '--rules-data', rulesdata], - ]: - stdout = subprocess.check_output(deprecated_cmd, - stderr=subprocess.STDOUT) - assert b"is deprecated" in stdout - assert b"path: /test\n" in stdout - stdout = subprocess.check_output([IP2UNIX, '-cp', '--rule', 'path=/test']) assert b"path: /test\n" in stdout diff --git a/tests/test_rule_file.py b/tests/test_rule_file.py index 0ad3049..b8ff643 100644 --- a/tests/test_rule_file.py +++ b/tests/test_rule_file.py @@ -1,103 +1,81 @@ -import json import subprocess import sys import unittest from tempfile import NamedTemporaryFile -from helper import IP2UNIX, systemd_only, non_systemd_only +from helper import IP2UNIX, dict_to_rule, systemd_only, non_systemd_only class RuleFileTest(unittest.TestCase): def assert_good_rules(self, rules): - cmd = [IP2UNIX, '-c', '-F', json.dumps(rules)] - result = subprocess.run(cmd, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT) - self.assertTrue(result.stdout.startswith( - b'The use of -F/--rules-data option is deprecated and it will be' - b' removed in ip2unix version 3. Please use the -r/--rule option' - b' instead.\n' - )) + with NamedTemporaryFile('w') as rf: + for rule in rules: + rf.write(dict_to_rule(rule) + '\n') + rf.flush() + + cmd = [IP2UNIX, '-c', '-f', rf.name] + result = subprocess.run(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) msg = 'Rules {!r} do not validate: {}'.format(rules, result.stdout) self.assertEqual(result.returncode, 0, msg) def assert_bad_rules(self, rules): - cmd = [IP2UNIX, '-c', '-F', json.dumps(rules)] - result = subprocess.run(cmd, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT) - self.assertTrue(result.stdout.startswith( - b'The use of -F/--rules-data option is deprecated and it will be' - b' removed in ip2unix version 3. Please use the -r/--rule option' - b' instead.\n' - )) + with NamedTemporaryFile('w') as rf: + for rule in rules: + rf.write(dict_to_rule(rule) + '\n') + rf.flush() + + cmd = [IP2UNIX, '-c', '-f', rf.name] + result = subprocess.run(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) msg = 'Rules {!r} should not be valid.'.format(rules) self.assertNotEqual(result.returncode, 0, msg) - def test_no_array(self): - self.assert_bad_rules({'rule1': {'socketPath': '/foo'}}) - self.assert_bad_rules({'rule2': {}}) - self.assert_bad_rules({}) - def test_empty(self): - self.assert_good_rules([]) + self.assert_bad_rules([]) def test_complete_rules(self): self.assert_good_rules([ - {'direction': 'outgoing', - 'type': 'udp', - 'socketPath': '/tmp/foo'}, - {'direction': 'incoming', - 'address': '::', - 'socketPath': '/tmp/bar'} + {'dir': 'out', 'type': 'udp', 'path': '/tmp/foo'}, + {'dir': 'in', 'addr': '::', 'path': '/tmp/bar'} ]) def test_unknown_rule_attrs(self): self.assert_bad_rules([{'foo': 1}]) - self.assert_bad_rules([{'socketpath': 'xxx'}]) + self.assert_bad_rules([{'sockpath': 'xxx'}]) def test_wrong_rule_types(self): - self.assert_bad_rules([{'type': 'nope', 'socketPath': '/tmp/foo'}]) - self.assert_bad_rules([{'direction': 'out', 'socketPath': '/tmp/foo'}]) - self.assert_bad_rules([{'socketPath': 1234}]) + self.assert_bad_rules([{'type': 'nope', 'path': '/tmp/foo'}]) + self.assert_bad_rules([{'dir': 'outgoing', 'path': '/tmp/foo'}]) + self.assert_bad_rules([{'path': True}]) def test_no_socket_path(self): - self.assert_bad_rules([{'address': '1.2.3.4'}]) - - def test_relative_socket_path(self): - self.assert_bad_rules([{'socketPath': 'aaa/bbb'}]) - self.assert_bad_rules([{'socketPath': 'bbb'}]) - - def test_absolute_socket_path(self): - self.assert_good_rules([{'socketPath': '/xxx'}]) + self.assert_bad_rules([{'addr': '1.2.3.4'}]) def test_invalid_enums(self): - self.assert_bad_rules([{'socketPath': '/bbb', 'direction': 111}]) - self.assert_bad_rules([{'socketPath': '/bbb', 'direction': False}]) - self.assert_bad_rules([{'socketPath': '/bbb', 'type': 234}]) - self.assert_bad_rules([{'socketPath': '/bbb', 'type': True}]) + self.assert_bad_rules([{'path': '/bbb', 'dir': '111'}]) + self.assert_bad_rules([{'path': '/bbb', 'dir': ''}]) + self.assert_bad_rules([{'path': '/bbb', 'type': '234'}]) + self.assert_bad_rules([{'path': '/bbb', 'type': 'true'}]) def test_invalid_port_type(self): - self.assert_bad_rules([{'socketPath': '/aaa', 'port': 'foo'}]) - self.assert_bad_rules([{'socketPath': '/aaa', 'port': True}]) - self.assert_bad_rules([{'socketPath': '/aaa', 'port': -1}]) - self.assert_bad_rules([{'socketPath': '/aaa', 'port': 65536}]) + self.assert_bad_rules([{'path': '/aaa', 'port': 'foo'}]) + self.assert_bad_rules([{'path': '/aaa', 'port': 'True'}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '-1'}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '65536'}]) def test_port_range(self): - self.assert_good_rules([{'socketPath': '/aaa', 'port': 123, - 'portEnd': 124}]) - self.assert_good_rules([{'socketPath': '/aaa', 'port': 1000, - 'portEnd': 65535}]) + self.assert_good_rules([{'path': '/aaa', 'port': '123-124'}]) + self.assert_good_rules([{'path': '/aaa', 'port': '1000-65535'}]) def test_invalid_port_range(self): - self.assert_bad_rules([{'socketPath': '/aaa', 'port': 123, - 'portEnd': 10}]) - self.assert_bad_rules([{'socketPath': '/aaa', 'port': 123, - 'portEnd': 123}]) - self.assert_bad_rules([{'socketPath': '/aaa', 'port': 123, - 'portEnd': 65536}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '123-10'}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '123-123'}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '123-65536'}]) def test_missing_start_port_in_range(self): - self.assert_bad_rules([{'socketPath': '/aaa', 'portEnd': 123}]) + self.assert_bad_rules([{'path': '/aaa', 'port': '-123'}]) def test_valid_address(self): valid_addrs = [ @@ -106,7 +84,7 @@ def test_valid_address(self): '7::', '::2:1', '::17', 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff' ] for addr in valid_addrs: - self.assert_good_rules([{'socketPath': '/foo', 'address': addr}]) + self.assert_good_rules([{'path': '/foo', 'addr': addr}]) def test_invalid_addrss(self): invalid_addrs = [ @@ -115,36 +93,36 @@ def test_invalid_addrss(self): '8:7:6:5:4:3:2:1::', '::8:7:6:5:4:3:2:1', 'f:f11::01100:2' ] for addr in invalid_addrs: - self.assert_bad_rules([{'socketPath': '/foo', 'address': addr}]) + self.assert_bad_rules([{'path': '/foo', 'addr': addr}]) def test_valid_reject(self): for val in ["EBADF", "EINTR", "enomem", "EnOMeM", 13, 12]: - self.assert_good_rules([{'reject': True, 'rejectError': val}]) + self.assert_good_rules([{'reject': val}]) def test_invalid_reject(self): for val in ["EBAAAADF", "", "XXX", "vvv", -10]: - self.assert_bad_rules([{'reject': True, 'rejectError': val}]) + self.assert_bad_rules([{'reject': val}]) def test_reject_with_sockpath(self): - self.assert_bad_rules([{'socketPath': '/foo', 'reject': True}]) + self.assert_bad_rules([{'path': '/foo', 'reject': True}]) def test_blackhole_with_reject(self): - self.assert_bad_rules([{'direction': 'incoming', 'reject': True, + self.assert_bad_rules([{'dir': 'in', 'reject': True, 'blackhole': True}]) def test_blackhole_outgoing(self): self.assert_bad_rules([{'blackhole': True}]) - self.assert_bad_rules([{'direction': 'outgoing', 'blackhole': True}]) + self.assert_bad_rules([{'dir': 'out', 'blackhole': True}]) def test_blackhole_with_sockpath(self): - self.assert_bad_rules([{'direction': 'incoming', 'socketPath': '/foo', + self.assert_bad_rules([{'dir': 'in', 'path': '/foo', 'blackhole': True}]) def test_blackhole_all(self): - self.assert_good_rules([{'direction': 'incoming', 'blackhole': True}]) + self.assert_good_rules([{'dir': 'in', 'blackhole': True}]) def test_ignore_with_sockpath(self): - self.assert_bad_rules([{'socketPath': '/foo', 'ignore': True}]) + self.assert_bad_rules([{'path': '/foo', 'ignore': True}]) def test_ignore_with_reject(self): self.assert_bad_rules([{'reject': True, 'ignore': True}]) @@ -158,58 +136,50 @@ def test_ignore_with_systemd(self): @systemd_only def test_contradicting_systemd(self): - self.assert_bad_rules([{'socketPath': '/foo', + self.assert_bad_rules([{'path': '/foo', 'socketActivation': True}]) @systemd_only def test_socket_fdname(self): - self.assert_good_rules([{'socketActivation': True, 'fdName': 'foo'}]) + self.assert_good_rules([{'systemd': 'foo'}]) @non_systemd_only def test_no_systemd_options(self): - self.assert_bad_rules([{'socketActivation': True}]) - self.assert_bad_rules([{'socketActivation': True, 'fdName': 'foo'}]) + self.assert_bad_rules([{'systemd': True}]) + self.assert_bad_rules([{'systemd': 'foo'}]) def test_print_rules_check_stdout(self): rules = [ - {'direction': 'outgoing', - 'type': 'tcp', - 'socketPath': '/foo'}, - {'address': '0.0.0.0', - 'socketPath': '/bar'} + {'dir': 'out', 'type': 'tcp', 'path': '/foo'}, + {'addr': '0.0.0.0', 'path': '/bar'} ] - cmd = [IP2UNIX, '-cp', '-F', json.dumps(rules)] - result = subprocess.run(cmd, stderr=subprocess.PIPE, - stdout=subprocess.PIPE) + with NamedTemporaryFile('w') as rf: + for rule in rules: + rf.write(dict_to_rule(rule) + '\n') + rf.flush() + + cmd = [IP2UNIX, '-cp', '-f', rf.name] + result = subprocess.run(cmd, stderr=subprocess.PIPE, + stdout=subprocess.PIPE) self.assertEqual(result.returncode, 0) - self.assertEqual( - result.stderr, - b'The use of -F/--rules-data option is deprecated and it will be' - b' removed in ip2unix version 3. Please use the -r/--rule option' - b' instead.\n' - ) self.assertNotEqual(result.stdout, b'') self.assertGreater(len(result.stdout), 0) self.assertIn(b'IP Type', result.stdout) def test_print_rules_stderr(self): - rules = [{'socketPath': '/xxx'}] - cmd = [IP2UNIX, '-p', '-F', json.dumps(rules), - sys.executable, '-c', ''] - result = subprocess.run(cmd, stderr=subprocess.PIPE, - stdout=subprocess.PIPE) + with NamedTemporaryFile('w') as rf: + rf.write('path=/xxx\n') + rf.flush() + cmd = [IP2UNIX, '-p', '-f', rf.name, sys.executable, '-c', ''] + result = subprocess.run(cmd, stderr=subprocess.PIPE, + stdout=subprocess.PIPE) self.assertEqual(result.returncode, 0) self.assertEqual(result.stdout, b'') - self.assertTrue(result.stderr.startswith( - b'The use of -F/--rules-data option is deprecated and it will be' - b' removed in ip2unix version 3. Please use the -r/--rule option' - b' instead.\n' - )) - self.assertRegex(result.stderr[134:], b'^Rule #1.*') + self.assertRegex(result.stderr, b'^Rule #1.*') self.assertGreater(len(result.stderr), 0) self.assertIn(b'IP Type', result.stderr) - def test_new_style_rule_files(self): + def test_weirdly_formatted(self): with NamedTemporaryFile('w') as rf1, NamedTemporaryFile('w') as rf2: rf1.write('in,port=1234,path=/foo\n') rf1.flush() diff --git a/tests/test_run_direct.py b/tests/test_run_direct.py index a5be199..cd7f381 100644 --- a/tests/test_run_direct.py +++ b/tests/test_run_direct.py @@ -1,5 +1,3 @@ -import json -import socket import subprocess import sys @@ -16,34 +14,6 @@ ''' -def test_run_direct(tmpdir): - sockfile = tmpdir.join('foo.sock') - rules = [{'direction': 'outgoing', 'socketPath': str(sockfile)}] - rulefile = tmpdir.join('rules.json') - rulefile.write(json.dumps(rules)) - cmd = [sys.executable, '-c', TESTPROG] - - env = { - 'LD_PRELOAD': helper.LIBIP2UNIX, - 'IP2UNIX_RULE_FILE': str(rulefile), - } - - with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock: - sock.bind(str(sockfile)) - sock.listen(10) - - with subprocess.Popen(cmd, env=env, stderr=subprocess.PIPE) as proc, \ - sock.accept()[0] as conn: - data = conn.recv(6) - assert data == b'foobar' - conn.sendall(b'barfoo') - expected = b'The use of the IP2UNIX_RULE_FILE environment' \ - b' variable is deprecated and will be removed in' \ - b' ip2unix version 3.0.\n' - assert proc.stderr.read() == expected - assert proc.wait() == 0 - - def test_run_direct_fail(): cmd = [sys.executable, '-c', TESTPROG] env = {'LD_PRELOAD': helper.LIBIP2UNIX} diff --git a/tests/test_sockopt_fail.py b/tests/test_sockopt_fail.py index aeb717e..d3c9bf9 100644 --- a/tests/test_sockopt_fail.py +++ b/tests/test_sockopt_fail.py @@ -19,7 +19,7 @@ def test_setsockopt_fail(tmpdir): - rules = [{'socketPath': str(tmpdir.join('foo.sock'))}] + rules = [{'path': str(tmpdir.join('foo.sock'))}] cmd = [sys.executable, '-c', TESTPROG] with helper.ip2unix(rules, cmd) as proc: assert proc.wait() == 0