diff --git a/mdp_client.c b/mdp_client.c index 1f7cb3f4..d3143dce 100644 --- a/mdp_client.c +++ b/mdp_client.c @@ -41,7 +41,7 @@ int overlay_mdp_send(overlay_mdp_frame *mdp, int flags, int timeout_ms) /* Construct name of socket to send to. */ struct sockaddr_un addr; socklen_t addrlen; - if (socket_setname(&addr, &addrlen, "mdp.socket") == -1) + if (make_local_sockaddr(&addr, &addrlen, "mdp.socket") == -1) return -1; // Send to that socket set_nonblock(mdp_client_socket); @@ -95,7 +95,7 @@ int overlay_mdp_client_init() uint32_t random_value; if (urandombytes((unsigned char *)&random_value, sizeof random_value) == -1) return WHY("urandombytes() failed"); - if (socket_setname(&addr, &addrlen, "mdp.client.%u.%08lx.socket", getpid(), (unsigned long)random_value) == -1) + if (make_local_sockaddr(&addr, &addrlen, "mdp.client.%u.%08lx.socket", getpid(), (unsigned long)random_value) == -1) return -1; if ((mdp_client_socket = esocket(AF_UNIX, SOCK_DGRAM, 0)) == -1) return -1; @@ -156,7 +156,7 @@ int overlay_mdp_recv(overlay_mdp_frame *mdp, int port, int *ttl) /* Construct name of socket to receive from. */ struct sockaddr_un mdp_addr; socklen_t mdp_addrlen; - if (socket_setname(&mdp_addr, &mdp_addrlen, "mdp.socket") == -1) + if (make_local_sockaddr(&mdp_addr, &mdp_addrlen, "mdp.socket") == -1) return -1; /* Check if reply available */ @@ -175,7 +175,14 @@ int overlay_mdp_recv(overlay_mdp_frame *mdp, int port, int *ttl) if (recvaddrlen > sizeof recvaddr) return WHY("reply did not come from server: address overrun"); - if (cmp_sockaddr((struct sockaddr *)&recvaddr, recvaddrlen, (struct sockaddr *)&mdp_addr, mdp_addrlen) != 0) + // Compare the address of the sender with the address of our server, to ensure they are the same. + // If the comparison fails, then try using realpath(3) on the sender address and compare again. + if ( cmp_sockaddr((struct sockaddr *)&recvaddr, recvaddrlen, (struct sockaddr *)&mdp_addr, mdp_addrlen) != 0 + && ( recvaddr.sun_family != AF_UNIX + || real_sockaddr(&recvaddr, recvaddrlen, &recvaddr, &recvaddrlen) <= 0 + || cmp_sockaddr((struct sockaddr *)&recvaddr, recvaddrlen, (struct sockaddr *)&mdp_addr, mdp_addrlen) != 0 + ) + ) return WHYF("reply did not come from server: %s", alloca_sockaddr(&recvaddr, recvaddrlen)); // silently drop incoming packets for the wrong port number diff --git a/monitor-client.c b/monitor-client.c index 3e752df2..01f5d1a1 100644 --- a/monitor-client.c +++ b/monitor-client.c @@ -74,7 +74,7 @@ int monitor_client_open(struct monitor_state **res) return WHYF_perror("socket(AF_UNIX, SOCK_STREAM, 0)"); struct sockaddr_un addr; socklen_t addrlen; - if (socket_setname(&addr, &addrlen, "%s", config.monitor.socket) == -1) + if (make_local_sockaddr(&addr, &addrlen, "%s", config.monitor.socket) == -1) return -1; INFOF("Attempting to connect to %s", alloca_sockaddr(&addr, addrlen)); if (socket_connect(fd, (struct sockaddr*)&addr, addrlen) == -1) { diff --git a/monitor.c b/monitor.c index 3a4929cf..1248525e 100644 --- a/monitor.c +++ b/monitor.c @@ -82,7 +82,7 @@ int monitor_setup_sockets() goto error; struct sockaddr_un addr; socklen_t addrlen; - if (socket_setname(&addr, &addrlen, "%s", config.monitor.socket) == -1) + if (make_local_sockaddr(&addr, &addrlen, "%s", config.monitor.socket) == -1) goto error; if (socket_bind(sock, (struct sockaddr*)&addr, addrlen) == -1) goto error; diff --git a/overlay_mdp.c b/overlay_mdp.c index b78bbacc..bfd9bc3f 100644 --- a/overlay_mdp.c +++ b/overlay_mdp.c @@ -69,7 +69,7 @@ int overlay_mdp_setup_sockets() overlay_mdp_clean_socket_files(); if (mdp_sock.poll.fd == -1) { - if (socket_setname(&addr, &addrlen, "mdp.socket") == -1) + if (make_local_sockaddr(&addr, &addrlen, "mdp.socket") == -1) return -1; if ((mdp_sock.poll.fd = esocket(AF_UNIX, SOCK_DGRAM, 0)) == -1) return -1; diff --git a/serval.h b/serval.h index 63b3cc9b..6046ce25 100644 --- a/serval.h +++ b/serval.h @@ -171,23 +171,24 @@ void serval_setinstancepath(const char *instancepath); /* Basic socket operations. */ -int _esocket(struct __sourceloc, int domain, int type, int protocol); -int _socket_setname(struct __sourceloc, struct sockaddr_un *sockname, socklen_t *addrlen, const char *fmt, ...) +int _make_local_sockaddr(struct __sourceloc, struct sockaddr_un *sockname, socklen_t *addrlen, const char *fmt, ...) __attribute__((format(printf, 4, 5))); +int _esocket(struct __sourceloc, int domain, int type, int protocol); int _socket_bind(struct __sourceloc, int sock, const struct sockaddr *addr, socklen_t addrlen); int _socket_connect(struct __sourceloc, int sock, const struct sockaddr *addr, socklen_t addrlen); int _socket_listen(struct __sourceloc, int sock, int backlog); int _socket_set_reuseaddr(struct __sourceloc, int sock, int reuseP); int _socket_set_rcvbufsize(struct __sourceloc, int sock, unsigned buffer_size); +#define make_local_sockaddr(sockname, addrlenp, fmt,...) _make_local_sockaddr(__WHENCE__, (sockname), (addrlenp), (fmt), ##__VA_ARGS__) #define esocket(domain, type, protocol) _esocket(__WHENCE__, (domain), (type), (protocol)) -#define socket_setname(sockname, addrlenp, fmt,...) _socket_setname(__WHENCE__, (sockname), (addrlenp), (fmt), ##__VA_ARGS__) #define socket_bind(sock, addr, addrlen) _socket_bind(__WHENCE__, (sock), (addr), (addrlen)) #define socket_connect(sock, addr, addrlen) _socket_connect(__WHENCE__, (sock), (addr), (addrlen)) #define socket_listen(sock, backlog) _socket_listen(__WHENCE__, (sock), (backlog)) #define socket_set_reuseaddr(sock, reuseP) _socket_set_reuseaddr(__WHENCE__, (sock), (reuseP)) #define socket_set_rcvbufsize(sock, buffer_size) _socket_set_rcvbufsize(__WHENCE__, (sock), (buffer_size)) +int real_sockaddr(const struct sockaddr_un *src_addr, socklen_t src_addrlen, struct sockaddr_un *dst_addr, socklen_t *dst_addrlen); int cmp_sockaddr(const struct sockaddr *, socklen_t, const struct sockaddr *, socklen_t); #define SERVER_CONFIG_RELOAD_INTERVAL_MS 1000 diff --git a/socket.c b/socket.c index 1f558b35..7e90392c 100644 --- a/socket.c +++ b/socket.c @@ -17,31 +17,39 @@ along with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +#include +#include + #include "serval.h" #include "conf.h" #include "log.h" #include "strbuf_helpers.h" -/* Under Linux, create a socket name in the abstract namespace. This permits us to use local - * sockets on Android despite its lack of a shared writeable directory on a UFS partition. +/* Form the name of an AF_UNIX (local) socket in the instance directory as an absolute path. + * Under Linux, this will create a socket name in the abstract namespace. This permits us to use + * local sockets on Android despite its lack of a shared writeable directory on a UFS partition. * - * On non-Linux systems, create a conventional named local socket in the $SERVALINSTANCE_PATH - * directory. + * The absolute file name is resolved to its real path using realpath(3), to ensure that name + * comparisons of addresses returned by recvmsg(2) can reliably be used on systems where the + * instance path may have a symbolic link in it. + * + * Returns -1 if the path name overruns the size of a sockaddr_un structure, or if realpath(3) fails + * with an error. The contents of *addr and *addrlen are undefined in this case. * * @author Andrew Bettison * @author Daniel O'Connor */ -int _socket_setname(struct __sourceloc __whence, struct sockaddr_un *addr, socklen_t *addrlen, const char *fmt, ...) +int _make_local_sockaddr(struct __sourceloc __whence, struct sockaddr_un *addr, socklen_t *addrlen, const char *fmt, ...) { bzero(addr, sizeof(*addr)); - addr->sun_family = AF_UNIX; va_list ap; va_start(ap, fmt); int r = vformf_serval_instance_path(__WHENCE__, addr->sun_path, sizeof addr->sun_path, fmt, ap); va_end(ap); - if (r == -1) - return WHY("local socket name overflow"); - *addrlen = sizeof(addr->sun_family) + strlen(addr->sun_path) + 1; + if (!r) + return WHY("socket name overflow"); + if (real_sockaddr(addr, strlen(addr->sun_path) + 1, addr, addrlen) == -1) + return -1; #ifdef USE_ABSTRACT_NAMESPACE // For the abstract name we use the absolute path name with the initial '/' replaced by the // leading nul. This ensures that different instances of the Serval daemon have different socket @@ -49,13 +57,52 @@ int _socket_setname(struct __sourceloc __whence, struct sockaddr_un *addr, sockl addr->sun_path[0] = '\0'; // mark as Linux abstract socket --*addrlen; // do not count trailing nul in abstract socket name #endif // USE_ABSTRACT_NAMESPACE + addr->sun_family = AF_UNIX; return 0; } -/* Compare any two struct sockaddr. Return -1, 0 or 1. Cope with invalid and truncated sockaddr - * structures. Uses inode number comparison to resolve symbolic links in AF_UNIX path names so is - * not suitable for sorting, because comparison results are inconsistent (eg, if A is a symlink to - * C, then A == C but A < B and B < C). +/* Converts an AF_UNIX local socket file name to contain a real path name using realpath(3), leaves + * all other socket types intact, including abstract local socket names. Returns -1 in case of an + * error from realpath(3) or a buffer overflow, without modifying *dst_addr or *dst_addrlen. + * Returns 1 if the path is changed and puts the modified path in *dst_addr and *dst_addrlen. + * Returns 0 if not the path is not changed and copies from *src_addr to *dst_addr, src_addrlen to + * *dst_addrlen. + * + * Can safely be used to perform an in-place conversion by using src_addr == dst_addr and + * dst_addrlen == &src_addrlen. + * + * @author Andrew Bettison + */ +int real_sockaddr(const struct sockaddr_un *src_addr, socklen_t src_addrlen, struct sockaddr_un *dst_addr, socklen_t *dst_addrlen) +{ + int src_path_len = src_path_len = src_addrlen - sizeof src_addr->sun_family; + if ( src_addrlen >= sizeof src_addr->sun_family + 1 + && src_addr->sun_family == AF_UNIX + && src_addr->sun_path[0] + && src_addr->sun_path[src_path_len - 1] == '\0' + ) { + char real_path[PATH_MAX]; + size_t real_path_len; + if (realpath(src_addr->sun_path, real_path) == NULL) + return WHYF_perror("realpath(%s)", alloca_str_toprint(src_addr->sun_path)); + else if ((real_path_len = strlen(real_path) + 1) > sizeof dst_addr->sun_path) + return WHYF("sockaddr overrun: realpath(%s) returned %s", alloca_str_toprint(src_addr->sun_path), alloca_str_toprint(real_path)); + else if ( real_path_len != src_path_len + || memcmp(real_path, src_addr->sun_path, src_path_len) != 0 + ) { + memcpy(dst_addr->sun_path, real_path, real_path_len); + *dst_addrlen = real_path_len - sizeof dst_addr->sun_family; + return 1; + } + } + if (dst_addr != src_addr) + memcpy(dst_addr, src_addr, src_addrlen); + *dst_addrlen = src_addrlen; + return 0; +} + +/* Compare any two struct sockaddr. Return -1, 0 or 1. Copes with invalid and truncated sockaddr + * structures. * * @author Andrew Bettison */ @@ -81,41 +128,22 @@ int cmp_sockaddr(const struct sockaddr *addrA, socklen_t addrlenA, const struct case AF_UNIX: { unsigned pathlenA = addrlenA - sizeof ((const struct sockaddr_un *)addrA)->sun_family; unsigned pathlenB = addrlenB - sizeof ((const struct sockaddr_un *)addrB)->sun_family; + int c; if ( pathlenA > 1 && pathlenB > 1 && ((const struct sockaddr_un *)addrA)->sun_path[0] == '\0' && ((const struct sockaddr_un *)addrB)->sun_path[0] == '\0' ) { // Both abstract sockets - just compare names, nul bytes are not terminators. - int c = memcmp(&((const struct sockaddr_un *)addrA)->sun_path[1], - &((const struct sockaddr_un *)addrB)->sun_path[1], - (pathlenA < pathlenB ? pathlenA : pathlenB) - 1); - if (c == 0) - c = pathlenA < pathlenB ? -1 : pathlenA > pathlenB ? 1 : 0; - return c; - } - // Either or both are named local file sockets. If the file names are identical up to the - // first nul, then the addresses are equal. Otherwise, if both are nul terminated file names - // (not abstract) then compare for equality by using the inode numbers to factor out symbolic - // links. Otherwise, simply compare the nul-terminated names (abstract names start with a nul - // so will always collate ahead of non-empty file names). - int c = strncmp(((const struct sockaddr_un *)addrA)->sun_path, - ((const struct sockaddr_un *)addrB)->sun_path, - (pathlenA < pathlenB ? pathlenA : pathlenB)); - if (c == 0 && pathlenA == pathlenB) - return 0; - if ( pathlenA && pathlenB - && ((const struct sockaddr_un *)addrA)->sun_path[0] - && ((const struct sockaddr_un *)addrB)->sun_path[0] - && ((const struct sockaddr_un *)addrA)->sun_path[pathlenA - 1] == '\0' - && ((const struct sockaddr_un *)addrB)->sun_path[pathlenB - 1] == '\0' - ) { - struct stat statA, statB; - if ( stat(((const struct sockaddr_un *)addrA)->sun_path, &statA) == 0 - && stat(((const struct sockaddr_un *)addrB)->sun_path, &statB) == 0 - && statA.st_dev == statB.st_dev - && statA.st_ino == statB.st_ino - ) - return 0; + c = memcmp(&((const struct sockaddr_un *)addrA)->sun_path[1], + &((const struct sockaddr_un *)addrB)->sun_path[1], + (pathlenA < pathlenB ? pathlenA : pathlenB) - 1); + } else { + // Either or both are named local file sockets. If the file names are identical up to the + // first nul, then the addresses are equal. This collates abstract socket names, whose first + // character is a nul, ahead of all non-empty file socket names. + c = strncmp(((const struct sockaddr_un *)addrA)->sun_path, + ((const struct sockaddr_un *)addrB)->sun_path, + (pathlenA < pathlenB ? pathlenA : pathlenB)); } if (c == 0) c = pathlenA < pathlenB ? -1 : pathlenA > pathlenB ? 1 : 0; @@ -123,7 +151,7 @@ int cmp_sockaddr(const struct sockaddr *addrA, socklen_t addrlenA, const struct } break; } - // Fall back to comparing the data bytes. + // Fall back to comparing raw data bytes. int c = memcmp(addrA->sa_data, addrB->sa_data, (addrlenA < addrlenB ? addrlenA : addrlenB) - sizeof addrA->sa_family); if (c == 0) c = addrlenA < addrlenB ? -1 : addrlenA > addrlenB ? 1 : 0;