SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #11004 from keszybz/remove-fixme-comment

Browse Source 
socket-util: drop obsolete FIXME comment and add test
This commit is contained in:
Lennart Poettering
2018-12-01 00:01:11 +01:00
committed by GitHub
parent 59ea6e57a5 15dca3711d
commit f8ee4eaa48
4 changed files with 190 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/socket-label.c
+1 -1
View File
@@ -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;
src/basic/socket-util.c
+52 -36
View File
@@ -15,6 +15,7 @@
#include <unistd.h>
#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,42 +650,50 @@ 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("<unnamed>");
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;
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;
src/basic/socket-util.h
+1 -1
View File
@@ -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);
src/test/test-socket-util.c
+136 -40
View File
@@ -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"
@@ -20,6 +21,8 @@
#include "tests.h"
static void test_ifname_valid(void) {
log_info("/* %s */", __func__);
assert(ifname_valid("foo"));
assert(ifname_valid("eth0"));
@@ -42,61 +45,121 @@ 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, const char *expected) {
SocketAddress a;
_cleanup_free_ char *out = NULL;
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);
if (r >= 0)
assert_se(socket_address_print(&a, &out) >= 0);
assert_se(socket_address_parse(&a, "65535") >= 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));
}
}
#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__);
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, "[::]: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. */
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, 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);
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, "[::]: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("/", 0, AF_UNIX, NULL);
test_socket_address_parse_one("@abstract", 0, AF_UNIX, NULL);
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);
{
char aaa[SUN_PATH_LEN + 1] = "@";
assert_se(socket_address_parse(&a, "[::1]:8888") >= 0);
assert_se(a.sockaddr.sa.sa_family == AF_INET6);
memset(aaa + 1, 'a', SUN_PATH_LEN - 1);
char_array_0(aaa);
assert_se(socket_address_parse(&a, "192.168.1.254:8888") >= 0);
assert_se(a.sockaddr.sa.sa_family == AF_INET);
test_socket_address_parse_one(aaa, -EINVAL, 0, NULL);
assert_se(socket_address_parse(&a, "/foo/bar") >= 0);
assert_se(a.sockaddr.sa.sa_family == AF_UNIX);
aaa[SUN_PATH_LEN - 1] = '\0';
test_socket_address_parse_one(aaa, 0, AF_UNIX, NULL);
}
assert_se(socket_address_parse(&a, "@abstract") >= 0);
assert_se(a.sockaddr.sa.sa_family == AF_UNIX);
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);
test_socket_address_parse_one("vsock:2x:1234", -EINVAL, 0, NULL);
test_socket_address_parse_one("vsock:2", -EINVAL, 0, NULL);
}
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);
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, "<unnamed>");
test_socket_print_unix_one("\0", 1, "<unnamed>");
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;
log_info("/* %s */", __func__);
assert_se(socket_address_parse_netlink(&a, "junk") < 0);
assert_se(socket_address_parse_netlink(&a, "") < 0);
@@ -120,8 +183,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 +235,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 +256,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 +267,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 +276,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 +300,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 +342,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 +362,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 +375,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 +401,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 +418,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 +462,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 +475,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 +495,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 +513,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 +583,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 +632,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 +685,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 +723,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 +758,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,12 +794,12 @@ static void test_send_emptydata(void) {
}
int main(int argc, char *argv[]) {
test_setup_logging(LOG_DEBUG);
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();