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 a156566511..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,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(""); 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; 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 0de29c2235..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" @@ -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, ""); + 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; + 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();