Skip to content

Commit

Permalink
Fix missing serialisation for from-{unix,abstract}
Browse files Browse the repository at this point in the history
When adding support for transforming paths/names from existing Unix
domain sockets in e332bf5, I forgot to
actually add the new fields to the serialisation methods.

The effect of this is that when using from-*, we suddenly match *all*
the sockets, regardless of whether it's an existing Unix domain socket
or not.

Our test case didn't catch that because it ran in isolation with only
one rule, so I modified the test case to add an unrelated socket that
should not be picked up by ip2unix.

Signed-off-by: aszlig <[email protected]>
  • Loading branch information
aszlig committed Aug 18, 2023
1 parent 5a26d97 commit 87c06e9
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 12 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog], and this project adheres to
[Semantic Versioning].

## [Unreleased]

### Fixed

- Missing serialisation for `from-unix` and `from-abstract`.

## [2.2.0] - 2023-08-16

### Fixed
Expand Down Expand Up @@ -128,6 +134,7 @@ The format is based on [Keep a Changelog], and this project adheres to
- The initial release, which evolved from an early prototype specific to a
certain use case into a more generic command line tool.

[Unreleased]: https://github.com/nixcloud/ip2unix/compare/v2.2.0...HEAD
[2.2.0]: https://github.com/nixcloud/ip2unix/compare/v2.1.4...v2.2.0
[2.1.4]: https://github.com/nixcloud/ip2unix/compare/v2.1.3...v2.1.4
[2.1.3]: https://github.com/nixcloud/ip2unix/compare/v2.1.2...v2.1.3
Expand Down
9 changes: 9 additions & 0 deletions src/serial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ void serialise(const Rule &rule, std::ostream &out)
serialise(rule.matches.address, out);
serialise(rule.matches.port, out);
serialise(rule.matches.port_end, out);
#ifdef ABSTRACT_SUPPORT
serialise(rule.matches.from_abstract, out);
#endif
serialise(rule.matches.from_unix, out);

serialise(rule.action.socket_path, out);
#ifdef SYSTEMD_SUPPORT
serialise(rule.action.socket_activation, out);
Expand All @@ -232,6 +237,10 @@ MaybeError deserialise(std::istream &in, Rule::Matches *out)
DESERIALISE_OR_ERR(address);
DESERIALISE_OR_ERR(port);
DESERIALISE_OR_ERR(port_end);
#ifdef ABSTRACT_SUPPORT
DESERIALISE_OR_ERR(from_abstract);
#endif
DESERIALISE_OR_ERR(from_unix);
return std::nullopt;
}

Expand Down
52 changes: 40 additions & 12 deletions tests/test_from_unix.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
TESTPROG = r'''
import socket
with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
sock.connect('{}')
sock.sendall(b'unrelated')
with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
sock.connect('{}')
sock.sendall(b'hello')
Expand All @@ -16,35 +20,59 @@


def test_from_unix(tmpdir):
unrelated_sockfile = str(tmpdir.join('unrelated.sock'))
sockfile = str(tmpdir.join('foo.sock'))

with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
testprog = TESTPROG.format(unrelated_sockfile, '/foo/bar/xyz')

with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as unrelated_sock, \
socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
unrelated_sock.bind(unrelated_sockfile)
unrelated_sock.listen(10)

sock.bind(sockfile)
sock.listen(10)

cmd = [IP2UNIX]
cmd += ['-r', f'from-unix=/foo/ba[q-s]/xyz,path={sockfile}']
cmd += [sys.executable, '-c', TESTPROG.format('/foo/bar/xyz')]
cmd += [sys.executable, '-c', testprog]

with subprocess.Popen(cmd, stdout=subprocess.PIPE) as client:
with unrelated_sock.accept()[0] as unrelated_conn:
assert unrelated_conn.recv(9) == b'unrelated'

with subprocess.Popen(cmd, stdout=subprocess.PIPE), \
sock.accept()[0] as conn:
assert conn.recv(5) == b'hello'
conn.sendall(b'world')
with sock.accept()[0] as conn:
assert conn.recv(5) == b'hello'
conn.sendall(b'world')

assert client.wait() == 0


@abstract_sockets_only
def test_from_abstract(tmpdir):
unrelated_sockfile = str(tmpdir.join('unrelated.sock'))
name = uuid4().hex

with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
testprog = TESTPROG.format(unrelated_sockfile, '\\0foobar')

with socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as unrelated_sock, \
socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) as sock:
unrelated_sock.bind(unrelated_sockfile)
unrelated_sock.listen(10)

sock.bind('\0' + name)
sock.listen(10)

cmd = [IP2UNIX]
cmd += ['-r', f'from-abstract=foob[a-c]r,abstract={name}']
cmd += [sys.executable, '-c', TESTPROG.format('\\0foobar')]
cmd += [sys.executable, '-c', testprog]

with subprocess.Popen(cmd, stdout=subprocess.PIPE) as client:
with unrelated_sock.accept()[0] as unrelated_conn:
assert unrelated_conn.recv(9) == b'unrelated'

with sock.accept()[0] as conn:
assert conn.recv(5) == b'hello'
conn.sendall(b'world')

with subprocess.Popen(cmd, stdout=subprocess.PIPE), \
sock.accept()[0] as conn:
assert conn.recv(5) == b'hello'
conn.sendall(b'world')
assert client.wait() == 0
15 changes: 15 additions & 0 deletions tests/unit/serial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ static std::vector<SocketPath::Type> socketpathtypes = {
SocketPath::Type::FILESYSTEM
};

static std::vector<std::optional<std::string>> simple_strings = {
std::nullopt,
"something",
};

static std::vector<std::optional<std::string>> strings = {
std::nullopt,
"",
Expand Down Expand Up @@ -152,6 +157,11 @@ static unsigned long test_rule(unsigned long seed)
rule.matches.address = CHOOSE(strings);
rule.matches.port = CHOOSE(ports);
rule.matches.port_end = CHOOSE(ports);
#ifdef ABSTRACT_SUPPORT
rule.matches.from_abstract = CHOOSE(simple_strings);
#endif
rule.matches.from_unix = CHOOSE(simple_strings);

#ifdef SYSTEMD_SUPPORT
rule.action.socket_activation = CHOOSE(bools);
rule.action.fd_name = CHOOSE(strings);
Expand All @@ -178,6 +188,11 @@ static unsigned long test_rule(unsigned long seed)
ASSERT_RULEVAL(matches.address);
ASSERT_RULEVAL(matches.port);
ASSERT_RULEVAL(matches.port_end);
#ifdef ABSTRACT_SUPPORT
ASSERT_RULEVAL(matches.from_abstract);
#endif
ASSERT_RULEVAL(matches.from_unix);

#ifdef SYSTEMD_SUPPORT
ASSERT_RULEVAL(action.socket_activation);
ASSERT_RULEVAL(action.fd_name);
Expand Down

0 comments on commit 87c06e9

Please sign in to comment.