From 041103a6884f0ab9214b2456162fd90c74465974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 30 Nov 2018 14:23:19 +0100 Subject: [PATCH 1/4] test-socket-util: the usual modernization --- src/test/test-socket-util.c | 126 +++++++++++++++++++++++------------- 1 file changed, 81 insertions(+), 45 deletions(-) diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 0de29c2235..a38811b2db 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -20,6 +20,8 @@ #include "tests.h" static void test_ifname_valid(void) { + log_info("/* %s */", __func__); + assert(ifname_valid("foo")); assert(ifname_valid("eth0")); @@ -42,61 +44,63 @@ static void test_ifname_valid(void) { assert(!ifname_valid("xxxxxxxxxxxxxxxx")); } -static void test_socket_address_parse(void) { +static void test_socket_address_parse_one(const char *in, int ret, int family) { SocketAddress a; + int r; - assert_se(socket_address_parse(&a, "junk") < 0); - assert_se(socket_address_parse(&a, "192.168.1.1") < 0); - assert_se(socket_address_parse(&a, ".168.1.1") < 0); - assert_se(socket_address_parse(&a, "989.168.1.1") < 0); - assert_se(socket_address_parse(&a, "192.168.1.1:65536") < 0); - assert_se(socket_address_parse(&a, "192.168.1.1:0") < 0); - assert_se(socket_address_parse(&a, "0") < 0); - assert_se(socket_address_parse(&a, "65536") < 0); + r = socket_address_parse(&a, in); + log_info("\"%s\" → %s", in, r >= 0 ? "✓" : "✗"); + assert_se(r == ret); + if (r >= 0) + assert_se(a.sockaddr.sa.sa_family == family); +} - assert_se(socket_address_parse(&a, "65535") >= 0); +static void test_socket_address_parse(void) { + log_info("/* %s */", __func__); + + test_socket_address_parse_one("junk", -EINVAL, 0); + test_socket_address_parse_one("192.168.1.1", -EINVAL, 0); + test_socket_address_parse_one(".168.1.1", -EINVAL, 0); + test_socket_address_parse_one("989.168.1.1", -EINVAL, 0); + test_socket_address_parse_one("192.168.1.1:65536", -ERANGE, 0); + test_socket_address_parse_one("192.168.1.1:0", -EINVAL, 0); + test_socket_address_parse_one("0", -EINVAL, 0); + test_socket_address_parse_one("65536", -ERANGE, 0); + + const int default_family = socket_ipv6_is_supported() ? AF_INET6 : AF_INET; + + test_socket_address_parse_one("65535", 0, default_family); /* The checks below will pass even if ipv6 is disabled in * kernel. The underlying glibc's inet_pton() is just a string * parser and doesn't make any syscalls. */ - assert_se(socket_address_parse(&a, "[::1]") < 0); - assert_se(socket_address_parse(&a, "[::1]8888") < 0); - assert_se(socket_address_parse(&a, "::1") < 0); - assert_se(socket_address_parse(&a, "[::1]:0") < 0); - assert_se(socket_address_parse(&a, "[::1]:65536") < 0); - assert_se(socket_address_parse(&a, "[a:b:1]:8888") < 0); + test_socket_address_parse_one("[::1]", -EINVAL, 0); + test_socket_address_parse_one("[::1]8888", -EINVAL, 0); + test_socket_address_parse_one("::1", -EINVAL, 0); + test_socket_address_parse_one("[::1]:0", -EINVAL, 0); + test_socket_address_parse_one("[::1]:65536", -ERANGE, 0); + test_socket_address_parse_one("[a:b:1]:8888", -EINVAL, 0); - assert_se(socket_address_parse(&a, "8888") >= 0); - assert_se(a.sockaddr.sa.sa_family == (socket_ipv6_is_supported() ? AF_INET6 : AF_INET)); + test_socket_address_parse_one("8888", 0, default_family); + test_socket_address_parse_one("[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888", 0, AF_INET6); + test_socket_address_parse_one("[::1]:8888", 0, AF_INET6); + test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET); + test_socket_address_parse_one("/foo/bar", 0, AF_UNIX); + test_socket_address_parse_one("@abstract", 0, AF_UNIX); - assert_se(socket_address_parse(&a, "[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_INET6); - - assert_se(socket_address_parse(&a, "[::1]:8888") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_INET6); - - assert_se(socket_address_parse(&a, "192.168.1.254:8888") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_INET); - - assert_se(socket_address_parse(&a, "/foo/bar") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_UNIX); - - assert_se(socket_address_parse(&a, "@abstract") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_UNIX); - - assert_se(socket_address_parse(&a, "vsock::1234") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_VSOCK); - assert_se(socket_address_parse(&a, "vsock:2:1234") >= 0); - assert_se(a.sockaddr.sa.sa_family == AF_VSOCK); - assert_se(socket_address_parse(&a, "vsock:2:1234x") < 0); - assert_se(socket_address_parse(&a, "vsock:2x:1234") < 0); - assert_se(socket_address_parse(&a, "vsock:2") < 0); + test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK); + test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK); + test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0); + test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0); + test_socket_address_parse_one("vsock:2", -EINVAL, 0); } static void test_socket_address_parse_netlink(void) { SocketAddress a; + log_info("/* %s */", __func__); + assert_se(socket_address_parse_netlink(&a, "junk") < 0); assert_se(socket_address_parse_netlink(&a, "") < 0); @@ -120,8 +124,9 @@ static void test_socket_address_parse_netlink(void) { } static void test_socket_address_equal(void) { - SocketAddress a; - SocketAddress b; + SocketAddress a, b; + + log_info("/* %s */", __func__); assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); assert_se(socket_address_parse(&b, "192.168.1.1:888") >= 0); @@ -171,6 +176,8 @@ static void test_socket_address_equal(void) { static void test_socket_address_get_path(void) { SocketAddress a; + log_info("/* %s */", __func__); + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); assert_se(!socket_address_get_path(&a)); @@ -190,6 +197,8 @@ static void test_socket_address_get_path(void) { static void test_socket_address_is(void) { SocketAddress a; + log_info("/* %s */", __func__); + assert_se(socket_address_parse(&a, "192.168.1.1:8888") >= 0); assert_se(socket_address_is(&a, "192.168.1.1:8888", SOCK_STREAM)); assert_se(!socket_address_is(&a, "route", SOCK_STREAM)); @@ -199,6 +208,8 @@ static void test_socket_address_is(void) { static void test_socket_address_is_netlink(void) { SocketAddress a; + log_info("/* %s */", __func__); + assert_se(socket_address_parse_netlink(&a, "route 10") >= 0); assert_se(socket_address_is_netlink(&a, "route 10")); assert_se(!socket_address_is_netlink(&a, "192.168.1.1:8888")); @@ -206,9 +217,10 @@ static void test_socket_address_is_netlink(void) { } static void test_in_addr_is_null(void) { - union in_addr_union i = {}; + log_info("/* %s */", __func__); + assert_se(in_addr_is_null(AF_INET, &i) == true); assert_se(in_addr_is_null(AF_INET6, &i) == true); @@ -229,6 +241,7 @@ static void test_in_addr_prefix_intersect_one(unsigned f, const char *a, unsigne } static void test_in_addr_prefix_intersect(void) { + log_info("/* %s */", __func__); test_in_addr_prefix_intersect_one(AF_INET, "255.255.255.255", 32, "255.255.255.254", 32, 0); test_in_addr_prefix_intersect_one(AF_INET, "255.255.255.255", 0, "255.255.255.255", 32, 1); @@ -270,6 +283,7 @@ static void test_in_addr_prefix_next_one(unsigned f, const char *before, unsigne } static void test_in_addr_prefix_next(void) { + log_info("/* %s */", __func__); test_in_addr_prefix_next_one(AF_INET, "192.168.0.0", 24, "192.168.1.0"); test_in_addr_prefix_next_one(AF_INET, "192.168.0.0", 16, "192.169.0.0"); @@ -289,7 +303,6 @@ static void test_in_addr_prefix_next(void) { test_in_addr_prefix_next_one(AF_INET6, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", 128, NULL); test_in_addr_prefix_next_one(AF_INET6, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ff00", 120, NULL); - } static void test_in_addr_to_string_one(int f, const char *addr) { @@ -303,6 +316,8 @@ static void test_in_addr_to_string_one(int f, const char *addr) { } static void test_in_addr_to_string(void) { + log_info("/* %s */", __func__); + test_in_addr_to_string_one(AF_INET, "192.168.0.1"); test_in_addr_to_string_one(AF_INET, "10.11.12.13"); test_in_addr_to_string_one(AF_INET6, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); @@ -327,6 +342,8 @@ static void test_in_addr_ifindex_to_string_one(int f, const char *a, int ifindex } static void test_in_addr_ifindex_to_string(void) { + log_info("/* %s */", __func__); + test_in_addr_ifindex_to_string_one(AF_INET, "192.168.0.1", 7, "192.168.0.1"); test_in_addr_ifindex_to_string_one(AF_INET, "10.11.12.13", 9, "10.11.12.13"); test_in_addr_ifindex_to_string_one(AF_INET6, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff", 10, "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"); @@ -342,6 +359,7 @@ static void test_in_addr_ifindex_from_string_auto(void) { int family, ifindex; union in_addr_union ua; + log_info("/* %s */", __func__); /* Most in_addr_ifindex_from_string_auto() invocations have already been tested above, but let's test some more */ assert_se(in_addr_ifindex_from_string_auto("fe80::17", &family, &ua, &ifindex) >= 0); @@ -385,6 +403,9 @@ static void test_sockaddr_equal(void) { .vm.svm_port = 0, .vm.svm_cid = VMADDR_CID_ANY, }; + + log_info("/* %s */", __func__); + assert_se(sockaddr_equal(&a, &a)); assert_se(sockaddr_equal(&a, &b)); assert_se(sockaddr_equal(&d, &d)); @@ -395,6 +416,8 @@ static void test_sockaddr_equal(void) { } static void test_sockaddr_un_len(void) { + log_info("/* %s */", __func__); + static const struct sockaddr_un fs = { .sun_family = AF_UNIX, .sun_path = "/foo/bar/waldo", @@ -413,6 +436,8 @@ static void test_in_addr_is_multicast(void) { union in_addr_union a, b; int f; + log_info("/* %s */", __func__); + assert_se(in_addr_from_string_auto("192.168.3.11", &f, &a) >= 0); assert_se(in_addr_is_multicast(f, &a) == 0); @@ -429,6 +454,8 @@ static void test_in_addr_is_multicast(void) { static void test_getpeercred_getpeergroups(void) { int r; + log_info("/* %s */", __func__); + r = safe_fork("(getpeercred)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); assert_se(r >= 0); @@ -497,6 +524,8 @@ static void test_passfd_read(void) { _cleanup_close_pair_ int pair[2] = { -1, -1 }; int r; + log_info("/* %s */", __func__); + assert_se(socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) >= 0); r = safe_fork("(passfd_read)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); @@ -544,6 +573,8 @@ static void test_passfd_contents_read(void) { static const char wire_contents[] = "test contents on the wire"; int r; + log_info("/* %s */", __func__); + assert_se(socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) >= 0); r = safe_fork("(passfd_contents_read)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); @@ -595,6 +626,8 @@ static void test_receive_nopassfd(void) { static const char wire_contents[] = "no fd passed here"; int r; + log_info("/* %s */", __func__); + assert_se(socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) >= 0); r = safe_fork("(receive_nopassfd)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); @@ -631,6 +664,8 @@ static void test_send_nodata_nofd(void) { _cleanup_close_pair_ int pair[2] = { -1, -1 }; int r; + log_info("/* %s */", __func__); + assert_se(socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) >= 0); r = safe_fork("(send_nodata_nofd)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); @@ -664,6 +699,8 @@ static void test_send_emptydata(void) { _cleanup_close_pair_ int pair[2] = { -1, -1 }; int r; + log_info("/* %s */", __func__); + assert_se(socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) >= 0); r = safe_fork("(send_emptydata)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); @@ -698,7 +735,6 @@ static void test_send_emptydata(void) { } int main(int argc, char *argv[]) { - test_setup_logging(LOG_DEBUG); test_ifname_valid(); From 39e73acb9b34c14c0c6448e3d53c0ecc96051715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 30 Nov 2018 15:13:32 +0100 Subject: [PATCH 2/4] test-socket-util: add test for socket_address_print --- src/test/test-socket-util.c | 66 +++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index a38811b2db..24dd701808 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -44,56 +44,64 @@ static void test_ifname_valid(void) { assert(!ifname_valid("xxxxxxxxxxxxxxxx")); } -static void test_socket_address_parse_one(const char *in, int ret, int family) { +static void test_socket_address_parse_one(const char *in, int ret, int family, const char *expected) { SocketAddress a; + _cleanup_free_ char *out = NULL; int r; r = socket_address_parse(&a, in); - log_info("\"%s\" → %s", in, r >= 0 ? "✓" : "✗"); - assert_se(r == ret); if (r >= 0) + assert_se(socket_address_print(&a, &out) >= 0); + + log_info("\"%s\" → %s → \"%s\" (expect \"%s\")", in, + r >= 0 ? "✓" : "✗", empty_to_dash(out), r >= 0 ? expected ?: in : "-"); + assert_se(r == ret); + if (r >= 0) { assert_se(a.sockaddr.sa.sa_family == family); + assert_se(streq(out, expected ?: in)); + } } static void test_socket_address_parse(void) { log_info("/* %s */", __func__); - test_socket_address_parse_one("junk", -EINVAL, 0); - test_socket_address_parse_one("192.168.1.1", -EINVAL, 0); - test_socket_address_parse_one(".168.1.1", -EINVAL, 0); - test_socket_address_parse_one("989.168.1.1", -EINVAL, 0); - test_socket_address_parse_one("192.168.1.1:65536", -ERANGE, 0); - test_socket_address_parse_one("192.168.1.1:0", -EINVAL, 0); - test_socket_address_parse_one("0", -EINVAL, 0); - test_socket_address_parse_one("65536", -ERANGE, 0); + test_socket_address_parse_one("junk", -EINVAL, 0, NULL); + test_socket_address_parse_one("192.168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one(".168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one("989.168.1.1", -EINVAL, 0, NULL); + test_socket_address_parse_one("192.168.1.1:65536", -ERANGE, 0, NULL); + test_socket_address_parse_one("192.168.1.1:0", -EINVAL, 0, NULL); + test_socket_address_parse_one("0", -EINVAL, 0, NULL); + test_socket_address_parse_one("65536", -ERANGE, 0, NULL); const int default_family = socket_ipv6_is_supported() ? AF_INET6 : AF_INET; - test_socket_address_parse_one("65535", 0, default_family); + test_socket_address_parse_one("65535", 0, default_family, "[::]:65535"); /* The checks below will pass even if ipv6 is disabled in * kernel. The underlying glibc's inet_pton() is just a string * parser and doesn't make any syscalls. */ - test_socket_address_parse_one("[::1]", -EINVAL, 0); - test_socket_address_parse_one("[::1]8888", -EINVAL, 0); - test_socket_address_parse_one("::1", -EINVAL, 0); - test_socket_address_parse_one("[::1]:0", -EINVAL, 0); - test_socket_address_parse_one("[::1]:65536", -ERANGE, 0); - test_socket_address_parse_one("[a:b:1]:8888", -EINVAL, 0); + test_socket_address_parse_one("[::1]", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]8888", -EINVAL, 0, NULL); + test_socket_address_parse_one("::1", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]:0", -EINVAL, 0, NULL); + test_socket_address_parse_one("[::1]:65536", -ERANGE, 0, NULL); + test_socket_address_parse_one("[a:b:1]:8888", -EINVAL, 0, NULL); - test_socket_address_parse_one("8888", 0, default_family); - test_socket_address_parse_one("[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888", 0, AF_INET6); - test_socket_address_parse_one("[::1]:8888", 0, AF_INET6); - test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET); - test_socket_address_parse_one("/foo/bar", 0, AF_UNIX); - test_socket_address_parse_one("@abstract", 0, AF_UNIX); + test_socket_address_parse_one("8888", 0, default_family, "[::]:8888"); + test_socket_address_parse_one("[2001:0db8:0000:85a3:0000:0000:ac1f:8001]:8888", 0, AF_INET6, + "[2001:db8:0:85a3::ac1f:8001]:8888"); + test_socket_address_parse_one("[::1]:8888", 0, AF_INET6, NULL); + test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET, NULL); + test_socket_address_parse_one("/foo/bar", 0, AF_UNIX, NULL); + test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); - test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK); - test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK); - test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0); - test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0); - test_socket_address_parse_one("vsock:2", -EINVAL, 0); + test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); + test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, "vsock:4294967295:1234"); + test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); + test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0, NULL); + test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); } static void test_socket_address_parse_netlink(void) { From 3a484991535187a1a301eb5df2053a88bc87e2fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 30 Nov 2018 15:17:25 +0100 Subject: [PATCH 3/4] shared/socket-util: do not print empty CID That's just ugly. --- src/basic/socket-util.c | 12 ++++++------ src/test/test-socket-util.c | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index a156566511..d94bac4287 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -672,12 +672,12 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ break; case AF_VSOCK: - if (include_port) - r = asprintf(&p, - "vsock:%u:%u", - sa->vm.svm_cid, - sa->vm.svm_port); - else + if (include_port) { + if (sa->vm.svm_cid == VMADDR_CID_ANY) + r = asprintf(&p, "vsock::%u", sa->vm.svm_port); + else + r = asprintf(&p, "vsock:%u:%u", sa->vm.svm_cid, sa->vm.svm_port); + } else r = asprintf(&p, "vsock:%u", sa->vm.svm_cid); if (r < 0) return -ENOMEM; diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 24dd701808..687caf83be 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -98,7 +98,7 @@ static void test_socket_address_parse(void) { test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); - test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, "vsock:4294967295:1234"); + test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, NULL); test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0, NULL); test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); From 15dca3711da63ac5fcfa01187a3b964d7042b99c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 30 Nov 2018 11:54:42 +0100 Subject: [PATCH 4/4] basic/socket-util: use c-escaping to print unprintable socket paths We are pretty careful to reject abstract sockets that are too long to fit in the address structure as a NUL-terminated string. And since we parse sockets as strings, it is not possible to embed a NUL in the the address either. But we might receive an external socket (abstract or not), and we want to be able to print its address in all cases. We would call socket_address_verify() and refuse to print various sockets that the kernel considers legit. Let's do the strict verification only in case of socket addresses we parse and open ourselves, and do less strict verification when printing addresses of existing sockets, and use c-escaping to print embedded NULs and such. More tests are added. This should make LGTM happier because on FIXME comment is removed. --- src/basic/socket-label.c | 2 +- src/basic/socket-util.c | 76 ++++++++++++++++++++++--------------- src/basic/socket-util.h | 2 +- src/test/test-socket-util.c | 52 +++++++++++++++++++++++++ 4 files changed, 100 insertions(+), 32 deletions(-) diff --git a/src/basic/socket-label.c b/src/basic/socket-label.c index 470bde5145..4ed19cd937 100644 --- a/src/basic/socket-label.c +++ b/src/basic/socket-label.c @@ -39,7 +39,7 @@ int socket_address_listen( assert(a); - r = socket_address_verify(a); + r = socket_address_verify(a, true); if (r < 0) return r; diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index d94bac4287..9bae1552fc 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -15,6 +15,7 @@ #include #include "alloc-util.h" +#include "escape.h" #include "fd-util.h" #include "fileio.h" #include "format-util.h" @@ -111,7 +112,7 @@ int socket_address_parse(SocketAddress *a, const char *s) { size_t l; l = strlen(s+1); - if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminate sockets here + if (l >= sizeof(a->sockaddr.un.sun_path) - 1) /* Note that we refuse non-NUL-terminated sockets here * when parsing, even though abstract namespace sockets * explicitly allow embedded NUL bytes and don't consider * them special. But it's simply annoying to debug such @@ -261,9 +262,12 @@ int socket_address_parse_netlink(SocketAddress *a, const char *s) { return 0; } -int socket_address_verify(const SocketAddress *a) { +int socket_address_verify(const SocketAddress *a, bool strict) { assert(a); + /* With 'strict' we enforce additional sanity constraints which are not set by the standard, + * but should only apply to sockets we create ourselves. */ + switch (socket_address_family(a)) { case AF_INET: @@ -293,19 +297,20 @@ int socket_address_verify(const SocketAddress *a) { case AF_UNIX: if (a->size < offsetof(struct sockaddr_un, sun_path)) return -EINVAL; - if (a->size > sizeof(struct sockaddr_un)+1) /* Allow one extra byte, since getsockname() on Linux will - * append a NUL byte if we have path sockets that are above - * sun_path' full size */ + if (a->size > sizeof(struct sockaddr_un) + !strict) + /* If !strict, allow one extra byte, since getsockname() on Linux will append + * a NUL byte if we have path sockets that are above sun_path's full size. */ return -EINVAL; if (a->size > offsetof(struct sockaddr_un, sun_path) && - a->sockaddr.un.sun_path[0] != 0) { /* Only validate file system sockets here */ - + a->sockaddr.un.sun_path[0] != 0 && + strict) { + /* Only validate file system sockets here, and only in strict mode */ const char *e; e = memchr(a->sockaddr.un.sun_path, 0, sizeof(a->sockaddr.un.sun_path)); if (e) { - /* If there's an embedded NUL byte, make sure the size of the socket addresses matches it */ + /* If there's an embedded NUL byte, make sure the size of the socket address matches it */ if (a->size != offsetof(struct sockaddr_un, sun_path) + (e - a->sockaddr.un.sun_path) + 1) return -EINVAL; } else { @@ -353,7 +358,10 @@ int socket_address_print(const SocketAddress *a, char **ret) { assert(a); assert(ret); - r = socket_address_verify(a); + r = socket_address_verify(a, false); /* We do non-strict validation, because we want to be + * able to pretty-print any socket the kernel considers + * valid. We still need to do validation to know if we + * can meaningfully print the address. */ if (r < 0) return r; @@ -386,8 +394,8 @@ bool socket_address_equal(const SocketAddress *a, const SocketAddress *b) { assert(b); /* Invalid addresses are unequal to all */ - if (socket_address_verify(a) < 0 || - socket_address_verify(b) < 0) + if (socket_address_verify(a, false) < 0 || + socket_address_verify(b, false) < 0) return false; if (a->type != b->type) @@ -642,31 +650,39 @@ int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool translate_ } case AF_UNIX: - if (salen <= offsetof(struct sockaddr_un, sun_path)) { + if (salen <= offsetof(struct sockaddr_un, sun_path) || + (sa->un.sun_path[0] == 0 && salen == offsetof(struct sockaddr_un, sun_path) + 1)) { + /* The name must have at least one character (and the leading NUL does not count) */ + p = strdup(""); if (!p) return -ENOMEM; - } else if (sa->un.sun_path[0] == 0) { - /* abstract */ - - /* FIXME: We assume we can print the - * socket path here and that it hasn't - * more than one NUL byte. That is - * actually an invalid assumption */ - - p = new(char, sizeof(sa->un.sun_path)+1); - if (!p) - return -ENOMEM; - - p[0] = '@'; - memcpy(p+1, sa->un.sun_path+1, sizeof(sa->un.sun_path)-1); - p[sizeof(sa->un.sun_path)] = 0; - } else { - p = strndup(sa->un.sun_path, sizeof(sa->un.sun_path)); + size_t path_len = salen - offsetof(struct sockaddr_un, sun_path); + + if (sa->un.sun_path[0] == 0) { + /* Abstract socket. When parsing address information from, we + * explicitly reject overly long paths and paths with embedded NULs. + * But we might get such a socket from the outside. Let's return + * something meaningful and printable in this case. */ + + _cleanup_free_ char *e = NULL; + + e = cescape_length(sa->un.sun_path + 1, path_len - 1); + if (!e) + return -ENOMEM; + + p = strjoin("@", e); + } else { + if (sa->un.sun_path[path_len - 1] == '\0') + /* We expect a terminating NUL and don't print it */ + path_len --; + + p = cescape_length(sa->un.sun_path, path_len); + } if (!p) - return -ENOMEM; + return -ENOMEM; } break; diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 8090e21657..37b1bca81a 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -70,7 +70,7 @@ int socket_address_parse(SocketAddress *a, const char *s); int socket_address_parse_and_warn(SocketAddress *a, const char *s); int socket_address_parse_netlink(SocketAddress *a, const char *s); int socket_address_print(const SocketAddress *a, char **p); -int socket_address_verify(const SocketAddress *a) _pure_; +int socket_address_verify(const SocketAddress *a, bool strict) _pure_; int sockaddr_un_unlink(const struct sockaddr_un *sa); diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 687caf83be..349d866442 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -6,6 +6,7 @@ #include "alloc-util.h" #include "async.h" +#include "escape.h" #include "exit-status.h" #include "fd-util.h" #include "fileio.h" @@ -62,6 +63,9 @@ static void test_socket_address_parse_one(const char *in, int ret, int family, c } } +#define SUN_PATH_LEN (sizeof(((struct sockaddr_un){}).sun_path)) +assert_cc(sizeof(((struct sockaddr_un){}).sun_path) == 108); + static void test_socket_address_parse(void) { log_info("/* %s */", __func__); @@ -95,8 +99,21 @@ static void test_socket_address_parse(void) { test_socket_address_parse_one("[::1]:8888", 0, AF_INET6, NULL); test_socket_address_parse_one("192.168.1.254:8888", 0, AF_INET, NULL); test_socket_address_parse_one("/foo/bar", 0, AF_UNIX, NULL); + test_socket_address_parse_one("/", 0, AF_UNIX, NULL); test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL); + { + char aaa[SUN_PATH_LEN + 1] = "@"; + + memset(aaa + 1, 'a', SUN_PATH_LEN - 1); + char_array_0(aaa); + + test_socket_address_parse_one(aaa, -EINVAL, 0, NULL); + + aaa[SUN_PATH_LEN - 1] = '\0'; + test_socket_address_parse_one(aaa, 0, AF_UNIX, NULL); + } + test_socket_address_parse_one("vsock:2:1234", 0, AF_VSOCK, NULL); test_socket_address_parse_one("vsock::1234", 0, AF_VSOCK, NULL); test_socket_address_parse_one("vsock:2:1234x", -EINVAL, 0, NULL); @@ -104,6 +121,40 @@ static void test_socket_address_parse(void) { test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL); } +static void test_socket_print_unix_one(const char *in, size_t len_in, const char *expected) { + _cleanup_free_ char *out = NULL, *c = NULL; + + SocketAddress a = { .sockaddr = { .un = { .sun_family = AF_UNIX } }, + .size = offsetof(struct sockaddr_un, sun_path) + len_in, + .type = SOCK_STREAM, + }; + memcpy(a.sockaddr.un.sun_path, in, len_in); + + assert_se(socket_address_print(&a, &out) >= 0); + assert_se(c = cescape(in)); + log_info("\"%s\" → \"%s\" (expect \"%s\")", in, out, expected); + assert_se(streq(out, expected)); +} + +static void test_socket_print_unix(void) { + log_info("/* %s */", __func__); + + /* Some additional tests for abstract addresses which we don't parse */ + + test_socket_print_unix_one("\0\0\0\0", 4, "@\\000\\000\\000"); + test_socket_print_unix_one("@abs", 5, "@abs"); + test_socket_print_unix_one("\n", 2, "\\n"); + test_socket_print_unix_one("", 1, ""); + test_socket_print_unix_one("\0", 1, ""); + test_socket_print_unix_one("\0_________________________there's 108 characters in this string_____________________________________________", 108, + "@_________________________there\\'s 108 characters in this string_____________________________________________"); + test_socket_print_unix_one("////////////////////////////////////////////////////////////////////////////////////////////////////////////", 108, + "////////////////////////////////////////////////////////////////////////////////////////////////////////////"); + test_socket_print_unix_one("////////////////////////////////////////////////////////////////////////////////////////////////////////////", 109, + "////////////////////////////////////////////////////////////////////////////////////////////////////////////"); + test_socket_print_unix_one("\0\a\b\n\255", 6, "@\\a\\b\\n\\255\\000"); +} + static void test_socket_address_parse_netlink(void) { SocketAddress a; @@ -748,6 +799,7 @@ int main(int argc, char *argv[]) { test_ifname_valid(); test_socket_address_parse(); + test_socket_print_unix(); test_socket_address_parse_netlink(); test_socket_address_equal(); test_socket_address_get_path();