Issue #20: Improve socket name handling

The recently added cmp_sockaddr() function does not call stat(2) any
more to compare local AF_UNIX socket address paths, so not it is stable
enough to use for ordering sockaddr structs.

New function: real_sockaddr() converts the file path of a local AF_UNIX
file socket using realpath(3).  The MDP client uses it on the sender
address of every MDP reply packet it receives to ensure that symlinks in
the instance path do not cause MDP client failures.

Rename recently added socket_setname() function: make_local_sockaddr().
This commit is contained in:
Andrew Bettison 2013-09-20 14:07:19 +09:30
parent 17347dbe3f
commit cc96e08e9d
6 changed files with 90 additions and 54 deletions

View File

@ -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

View File

@ -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) {

View File

@ -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;

View File

@ -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;

View File

@ -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

116
socket.c
View File

@ -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 <limits.h>
#include <stdlib.h>
#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 <andrew@servalproject.com>
* @author Daniel O'Connor <daniel@servalproject.com>
*/
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 <andrew@servalproject.com>
*/
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 <andrew@servalproject.com>
*/
@ -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;