From: flu0r1ne Date: Fri, 29 Sep 2023 21:28:19 +0000 (-0500) Subject: Fix interface binding by retaining CAP_NET_RAW X-Git-Tag: v0.96~21^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=04d5abe3b5d9975a4e74f15ef86eae3b355ae729;p=thirdparty%2Fmtr.git Fix interface binding by retaining CAP_NET_RAW This commit addresses an issue where mtr would fail with EPERM because setting the SO_BINDTODEVICE socket option requires the CAP_NET_RAW capability. Changes: - Refactor the code to abstract setting privileged socket options. This includes a common interface for setting capabilities depending on the platform (with or without LIBCAP). - Replace direct setsockopt calls with the new abstracted function for setting both SO_MARK and SO_BINDTODEVICE. - Update capability management in `drop_excess_capabilities` to retain CAP_NET_RAW when needed. --- diff --git a/packet/construct_unix.c b/packet/construct_unix.c index a9d6d91..8df9997 100644 --- a/packet/construct_unix.c +++ b/packet/construct_unix.c @@ -18,6 +18,8 @@ #include "construct_unix.h" +#include "utils.h" + #include #include #include @@ -282,25 +284,47 @@ int construct_udp6_packet( return 0; } -#define SET_MARK_N_ADDED_CAPS 1 +/* + This defines a common interface which elevates privileges on + platforms with LIBCAP and acts as a NOOP on platforms without + it. +*/ +#ifdef HAVE_LIBCAP + +typedef cap_value_t mayadd_cap_value_t; +#define MAYADD_CAP_NET_RAW CAP_NET_RAW +#define MAYADD_CAP_NET_ADMIN CAP_NET_ADMIN +#define MAYADD_UNUSED + +#else /* ifdef HAVE_LIBCAP */ + +typedef int mayadd_cap_value_t; +#define MAYADD_CAP_NET_RAW ((mayadd_cap_value_t) 0) +#define MAYADD_CAP_NET_ADMIN ((mayadd_cap_value_t) 0) +#define MAYADD_UNUSED UNUSED + +#endif /* ifdef HAVE_LIBCAP */ + +static +int set_privileged_socket_opt(int socket, int option_name, + void const * option_value, socklen_t option_len, + MAYADD_UNUSED mayadd_cap_value_t required_cap) { -/* Set the socket mark */ -static int set_socket_mark(int socket, unsigned int mark) { int result = -1; // Add CAP_NET_ADMIN to the effective set if libcap is present #ifdef HAVE_LIBCAP - cap_t cap = NULL; - static cap_value_t cap_add[SET_MARK_N_ADDED_CAPS] = { CAP_NET_ADMIN }; + static cap_value_t cap_add[1]; + cap_add[0] = required_cap; // Get the capabilities of the current process - cap = cap_get_proc(); + cap_t cap = cap_get_proc(); if (cap == NULL) { goto cleanup_and_exit; } // Set the required capability flag - if (cap_set_flag(cap, CAP_EFFECTIVE, SET_MARK_N_ADDED_CAPS, cap_add, + if (cap_set_flag(cap, CAP_EFFECTIVE, N_ENTRIES(cap_add), cap_add, CAP_SET)) { goto cleanup_and_exit; } @@ -312,7 +336,7 @@ static int set_socket_mark(int socket, unsigned int mark) { #endif /* ifdef HAVE_LIBPCAP */ // Set the socket mark - if (setsockopt(socket, SOL_SOCKET, SO_MARK, &mark, sizeof(mark))) { + if (setsockopt(socket, SOL_SOCKET, option_name, option_value, option_len)) { goto cleanup_and_exit; } @@ -320,7 +344,7 @@ static int set_socket_mark(int socket, unsigned int mark) { #ifdef HAVE_LIBCAP // Clear the CAP_NET_ADMIN capability flag - if (cap_set_flag(cap, CAP_EFFECTIVE, SET_MARK_N_ADDED_CAPS, cap_add, + if (cap_set_flag(cap, CAP_EFFECTIVE, N_ENTRIES(cap_add), cap_add, CAP_CLEAR)) { goto cleanup_and_exit; } @@ -342,6 +366,22 @@ cleanup_and_exit: return result; } +/* Set the socket mark */ +#ifdef SO_MARK +static +int set_socket_mark(int socket, unsigned int mark) { + return set_privileged_socket_opt(socket, SO_MARK, &mark, sizeof(mark), + MAYADD_CAP_NET_ADMIN); +} +#endif /* ifdef SO_MARK */ + +#ifdef SO_BINDTODEVICE +static +int set_bind_to_device(int socket, char const * device) { + return set_privileged_socket_opt(socket, SO_BINDTODEVICE, device, + strlen(device), MAYADD_CAP_NET_RAW); +} +#endif /* ifdef SO_BINDTODEVICE */ /* Set the socket options for an outgoing stream protocol socket based on @@ -414,8 +454,7 @@ int set_stream_socket_options( #ifdef SO_BINDTODEVICE if (param->local_device) { - if (setsockopt(stream_socket, SOL_SOCKET, - SO_BINDTODEVICE, param->local_device, strlen(param->local_device))) { + if (set_bind_to_device(stream_socket, param->local_device)) { return -1; } } @@ -650,8 +689,7 @@ int construct_ip4_packet( #ifdef SO_BINDTODEVICE if (param->local_device) { - if (setsockopt(send_socket, SOL_SOCKET, - SO_BINDTODEVICE, param->local_device, strlen(param->local_device))) { + if(set_bind_to_device(send_socket, param->local_device)) { return -1; } } diff --git a/packet/packet.c b/packet/packet.c index ba37d25..5f3a39b 100644 --- a/packet/packet.c +++ b/packet/packet.c @@ -33,24 +33,41 @@ #include #endif -#include "wait.h" +#include "utils.h" -#define N_ENTRIES(array) \ - (sizeof((array)) / sizeof(*(array))) +#include "wait.h" #ifdef HAVE_LIBCAP static void drop_excess_capabilities() { - cap_value_t cap_permitted[] = { -#ifdef SO_MARK + /* By default, the root user has all capabilities, which poses a security risk. - Since the socket has already been opened, we only need CAP_NET_ADMIN to set - the fwmark. This capability must remain in the permitted set so that it can - be added to the effective set when needed. + + Some capabilities must be retained in the permitted set so that it can be added + to the effective set when needed. */ - CAP_NET_ADMIN + cap_value_t cap_permitted[] = { +#ifdef SO_MARK + /* + CAP_NET_ADMIN is needed to set the routing mark (SO_MARK) on a socket + */ + CAP_NET_ADMIN, #endif /* ifdef SOMARK */ + +#ifdef SO_BINDTODEVICE + /* + The CAP_NET_RAW capability is necessary for binding to a network device using + the SO_BINDTODEVICE socket option. Although this capability is not needed for + the initial bind operation, it is required when calling setsockopt after data has + been sent. + + Given the current architecture, the socket is re-bound to the device every time + a probe is sent. Therefore, CAP_NET_RAW is required when specifying an interface + using the -I or --interface options. + */ + CAP_NET_RAW, +#endif /* ifdef SO_BINDTODEVICE */ }; cap_t current_cap = cap_get_proc(); diff --git a/packet/utils.h b/packet/utils.h new file mode 100644 index 0000000..6e5ca8c --- /dev/null +++ b/packet/utils.h @@ -0,0 +1,14 @@ +#ifndef _UTILS_H +#define _UTILS_H + +// Fend off -Wunused-parameter +#if defined(__GNUC__) +# define UNUSED __attribute__((__unused__)) +#else +# define UNUSED +#endif + +// Number of entries in a fixed-length array +#define N_ENTRIES(x) (sizeof(x) / sizeof(*x)) + +#endif