]> git.ipfire.org Git - thirdparty/mtr.git/commitdiff
Fix interface binding by retaining CAP_NET_RAW
authorflu0r1ne <flu0r1ne@flu0r1ne.net>
Fri, 29 Sep 2023 21:28:19 +0000 (16:28 -0500)
committerflu0r1ne <flu0r1ne@flu0r1ne.net>
Fri, 29 Sep 2023 23:42:16 +0000 (18:42 -0500)
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.

packet/construct_unix.c
packet/packet.c
packet/utils.h [new file with mode: 0644]

index a9d6d91fdb23da9e3deb55c0e8d2c4f0b820f149..8df9997a212a9addea38fa9a6e3b687fb44bbfa9 100644 (file)
@@ -18,6 +18,8 @@
 
 #include "construct_unix.h"
 
+#include "utils.h"
+
 #include <errno.h>
 #include <stdio.h>
 #include <string.h>
@@ -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;
         }
     }
index ba37d25ce3e1d8784e61a4f4a37b6e8e8fbea485..5f3a39b47b24b5f02277d272b07e1625ffa8de4f 100644 (file)
 #include <sys/capability.h>
 #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 (file)
index 0000000..6e5ca8c
--- /dev/null
@@ -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