From 8b8a3d1f3567861eb395a1c2d092787533d188c7 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Sat, 9 May 2026 14:18:31 +0400 Subject: [PATCH 1/2] fix(packet): drop capabilities permanently --- Makefile.am | 2 + packet/construct_unix.c | 100 +++++----------------------------------- packet/packet.c | 70 ++++++---------------------- test/capability-drop.py | 52 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 145 deletions(-) create mode 100755 test/capability-drop.py diff --git a/Makefile.am b/Makefile.am index 20c238eb..f18a7d7a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -7,11 +7,13 @@ EXTRA_DIST = \ sbin_PROGRAMS = mtr mtr-packet TESTS = \ + test/capability-drop.py \ test/cmdparse.py \ test/param.py \ test/probe.py TEST_FILES = \ + test/capability-drop.py \ test/cmdparse.py \ test/mtrpacket.py \ test/param.py \ diff --git a/packet/construct_unix.c b/packet/construct_unix.c index f2f56dc1..ef81a840 100644 --- a/packet/construct_unix.c +++ b/packet/construct_unix.c @@ -34,10 +34,6 @@ #define SOL_IP IPPROTO_IP #endif -#ifdef HAVE_LIBCAP -#include -#endif - #define MIN_UNPRIVILEGED_PORT 1024 #define UDP_PORT_RANGE 65536 @@ -298,99 +294,25 @@ int construct_udp6_packet( return 0; } -/* - 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 - -#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) - -#endif /* ifdef HAVE_LIBCAP */ - -UNUSED static -int set_privileged_socket_opt(int socket, int option_name, - void const * option_value, socklen_t option_len, - UNUSED mayadd_cap_value_t required_cap) { - - int result = -1; - - // Add CAP_NET_ADMIN to the effective set if libcap is present -#ifdef HAVE_LIBCAP - static cap_value_t cap_add[1]; - cap_add[0] = required_cap; - - // Get the capabilities of the current process - 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, N_ENTRIES(cap_add), cap_add, - CAP_SET)) { - goto cleanup_and_exit; - } - - // Apply the modified capabilities to the current process - if (cap_set_proc(cap)) { - goto cleanup_and_exit; - } -#endif /* ifdef HAVE_LIBCAP */ - - // Set the socket mark - int set_sock_err = setsockopt(socket, SOL_SOCKET, option_name, option_value, option_len); - - // Drop CAP_NET_ADMIN from the effective set if libcap is present -#ifdef HAVE_LIBCAP - - // Clear the CAP_NET_ADMIN capability flag - if (cap_set_flag(cap, CAP_EFFECTIVE, N_ENTRIES(cap_add), cap_add, - CAP_CLEAR)) { - goto cleanup_and_exit; - } - - // Apply the modified capabilities to the current process - if (cap_set_proc(cap)) { - goto cleanup_and_exit; - } -#endif /* ifdef HAVE_LIBCAP */ - - if(!set_sock_err) { - result = 0; // Success - } - -#ifdef HAVE_LIBCAP -cleanup_and_exit: - cap_free(cap); -#endif /* ifdef HAVE_LIBCAP */ - - 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); +int set_socket_mark( + int socket, + unsigned int mark) +{ + return setsockopt(socket, SOL_SOCKET, SO_MARK, &mark, sizeof(mark)); } #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); +int set_bind_to_device( + int socket, + char const *device) +{ + return setsockopt(socket, SOL_SOCKET, SO_BINDTODEVICE, device, + strlen(device)); } #endif /* ifdef SO_BINDTODEVICE */ diff --git a/packet/packet.c b/packet/packet.c index 9721b5b0..5050cfb0 100644 --- a/packet/packet.c +++ b/packet/packet.c @@ -42,70 +42,29 @@ #ifdef HAVE_LIBCAP static -void drop_excess_capabilities() { - - /* - By default, the root user has all capabilities, which poses a security risk. - - Some capabilities must be retained in the permitted set so that it can be added - to the effective set when needed. - */ - 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(); +void drop_all_capabilities() +{ cap_t wanted_cap = cap_get_proc(); - if(!current_cap || !wanted_cap) { + if (!wanted_cap) { goto pcap_error; } - // Clear all capabilities from the 'wanted_cap' set - if(cap_clear(wanted_cap)) { + if (cap_clear(wanted_cap)) { goto pcap_error; } - // Retain only the necessary capabilities defined in 'cap_permitted' in the permitted set. - // This approach ensures the principle of least privilege. - // If the user has dropped capabilities, the code assumes those features will not be needed. - for(unsigned i = 0; i < N_ENTRIES(cap_permitted); i++) { - cap_flag_value_t is_set; - - if(cap_get_flag(current_cap, cap_permitted[i], CAP_PERMITTED, &is_set)) { - goto pcap_error; - } - - if(cap_set_flag(wanted_cap, CAP_PERMITTED, 1, &cap_permitted[i], is_set)) { - goto pcap_error; - } - } - - // Update the process's capabilities to match 'wanted_cap' - if(cap_set_proc(wanted_cap)) { + /* + mtr-packet opens any sockets that need elevated privileges before this + point. Do not keep capabilities in the permitted set for later + re-enabling: once privilege is dropped, later packet handling must not be + able to regain it. + */ + if (cap_set_proc(wanted_cap)) { goto pcap_error; } - if(cap_free(current_cap) || cap_free(wanted_cap)) { + if (cap_free(wanted_cap)) { goto pcap_error; } @@ -113,7 +72,6 @@ void drop_excess_capabilities() { pcap_error: - cap_free(current_cap); cap_free(wanted_cap); error(EXIT_FAILURE, errno, "Failed to drop capabilities"); } @@ -134,10 +92,10 @@ int drop_elevated_permissions( } /* - Drop all process capabilities. + Drop all process capabilities permanently. */ #ifdef HAVE_LIBCAP - drop_excess_capabilities(); + drop_all_capabilities(); #endif return 0; diff --git a/test/capability-drop.py b/test/capability-drop.py new file mode 100755 index 00000000..596fc269 --- /dev/null +++ b/test/capability-drop.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +PACKET_FILES = sorted((ROOT / 'packet').glob('*.[ch]')) + +ALLOWED_CAP_CALLS = { + 'cap_clear', + 'cap_free', + 'cap_get_proc', + 'cap_set_proc', + 'cap_t', +} + +C_TOKEN = re.compile(r'\b(?:cap_[a-zA-Z0-9_]+|CAP_[A-Z0-9_]+)\b') + + +def strip_c_comments_and_strings(source): + return re.sub( + r'/\*.*?\*/|//[^\n]*|"(?:\\.|[^"\\])*"|\'(?:\\.|[^\'\\])*\'', + lambda match: '\n' * match.group(0).count('\n'), + source, + flags=re.DOTALL, + ) + + +def main(): + errors = [] + + for path in PACKET_FILES: + source = strip_c_comments_and_strings(path.read_text()) + + for match in C_TOKEN.finditer(source): + token = match.group(0) + + if token.startswith('CAP_'): + errors.append((path, token)) + continue + + if token not in ALLOWED_CAP_CALLS: + errors.append((path, token)) + + if errors: + for path, token in errors: + print(f'{path.relative_to(ROOT)}: disallowed capability token {token}') + raise SystemExit(1) + + +if __name__ == '__main__': + main() From a76de3a4b25b93d2540324b36dd8c16eb8c3a735 Mon Sep 17 00:00:00 2001 From: Darafei Praliaskouski Date: Fri, 8 May 2026 02:12:02 +0400 Subject: [PATCH 2/2] fix(packet): allow privileged local ports --- Makefile.am | 2 ++ man/mtr.8.in | 4 +++ packet/command.c | 6 +---- packet/construct_unix.c | 54 ++++++++++++++++++++++++++++++++++++++--- packet/ports.h | 33 +++++++++++++++++++++++++ packet/probe.h | 1 + packet/probe_unix.c | 2 +- test/probe.py | 5 ++++ ui/cmdpipe.c | 7 ++++++ ui/mtr.c | 6 ++--- ui/mtr.h | 4 +-- 11 files changed, 109 insertions(+), 15 deletions(-) create mode 100644 packet/ports.h diff --git a/Makefile.am b/Makefile.am index f18a7d7a..362efa31 100644 --- a/Makefile.am +++ b/Makefile.am @@ -62,6 +62,7 @@ mtr_SOURCES = ui/mtr.c ui/mtr.h \ ui/select.c ui/select.h \ ui/utils.c ui/utils.h \ packet/cmdparse.c packet/cmdparse.h \ + packet/ports.h \ packet/sockaddr.c packet/sockaddr.h \ ui/mtr-curses.h \ img/mtr_icon.xpm \ @@ -108,6 +109,7 @@ mtr_packet_SOURCES = \ packet/packet.c \ packet/cmdparse.c packet/cmdparse.h \ packet/command.c packet/command.h \ + packet/ports.h \ packet/platform.h \ packet/probe.c packet/probe.h \ packet/utils.h \ diff --git a/man/mtr.8.in b/man/mtr.8.in index 98ea74d2..44adfa87 100644 --- a/man/mtr.8.in +++ b/man/mtr.8.in @@ -479,6 +479,10 @@ rotating privileged ports into the high end of the UDP port range unless .B \-\-port was given; in that case, UDP traces encode the probe sequence number in the source port instead. +Valid port numbers are 1 through 65535. Ports below 1024 can be used +only when the operating system allows the packet helper to bind +privileged local ports; otherwise the probe fails with a permission +error. .TP .B \-Z \fISECONDS\fR, \fB\-\-timeout \fISECONDS The number of seconds to keep probe sockets open before giving up on diff --git a/packet/command.c b/packet/command.c index 663b3476..6985a314 100644 --- a/packet/command.c +++ b/packet/command.c @@ -226,11 +226,7 @@ bool decode_probe_argument( return false; } - /* - Don't allow using a local port which requires - privileged binding. - */ - if (param->local_port < 1024) { + if (!MTR_IS_VALID_PORT(param->local_port)) { param->local_port = 0; return false; } diff --git a/packet/construct_unix.c b/packet/construct_unix.c index ef81a840..d8d78cd3 100644 --- a/packet/construct_unix.c +++ b/packet/construct_unix.c @@ -34,8 +34,7 @@ #define SOL_IP IPPROTO_IP #endif -#define MIN_UNPRIVILEGED_PORT 1024 -#define UDP_PORT_RANGE 65536 +#include "ports.h" /* A source of data for computing a checksum */ struct checksum_source_t { @@ -48,8 +47,8 @@ uint16_t udp_source_port_from_pid(void) { uint16_t port = getpid() & 0xffff; - if (port < MIN_UNPRIVILEGED_PORT) { - port += UDP_PORT_RANGE - MIN_UNPRIVILEGED_PORT; + if (port < MTR_UNPRIVILEGED_PORT_MIN) { + port += MTR_UDP_PORT_RANGE - MTR_UNPRIVILEGED_PORT_MIN; } return port; @@ -99,6 +98,39 @@ void construct_addr_port( *sockaddr_port_offset(addr_with_port) = htons(port); } +static +int check_udp_bind_allowed( + const struct sockaddr_storage *addr, + socklen_t addr_len) +{ + int saved_errno; + int udp_socket; + + udp_socket = socket(addr->ss_family, SOCK_DGRAM, IPPROTO_UDP); + if (udp_socket == -1) { + return -1; + } + + /* + Raw sockets can put any UDP source port in the packet header. For + privileged ports, first prove that this process can bind the same port + after capability dropping, so raw packet construction cannot bypass the + kernel's local-port permission policy. + */ + if (bind(udp_socket, (const struct sockaddr *) addr, addr_len)) { + saved_errno = errno; + close(udp_socket); + errno = saved_errno; + return -1; + } + + if (close(udp_socket)) { + return -1; + } + + return 0; +} + /* Construct an ICMP header for IPv4 */ static int construct_icmp4_packet( @@ -592,6 +624,13 @@ int construct_ip4_packet( (net_state, probe, packet_buffer, packet_size, param)) { return -1; } + + if (net_state->platform.ip4_socket_raw && + MTR_IS_PRIVILEGED_PORT(param->local_port) && + check_udp_bind_allowed(&probe->local_addr, + sizeof(struct sockaddr_in))) { + return -1; + } } else { errno = EINVAL; return -1; @@ -745,6 +784,13 @@ int construct_ip6_packet( (net_state, probe, packet_buffer, packet_size, param)) { return -1; } + + if (net_state->platform.ip6_socket_raw && + MTR_IS_PRIVILEGED_PORT(param->local_port) && + check_udp_bind_allowed(&probe->local_addr, + sizeof(struct sockaddr_in6))) { + return -1; + } } else { errno = EINVAL; return -1; diff --git a/packet/ports.h b/packet/ports.h new file mode 100644 index 00000000..6d9d6932 --- /dev/null +++ b/packet/ports.h @@ -0,0 +1,33 @@ +/* + mtr -- a network diagnostic tool + Copyright (C) 2026 Darafei Praliaskouski + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License version 2 as + published by the Free Software Foundation. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#ifndef PORTS_H +#define PORTS_H + +#define MTR_PORT_MIN 1 +#define MTR_PORT_MAX 65535 +#define MTR_UNPRIVILEGED_PORT_MIN 1024 +#define MTR_UDP_PORT_RANGE 65536 + +#define MTR_IS_VALID_PORT(port) \ + ((port) >= MTR_PORT_MIN && (port) <= MTR_PORT_MAX) + +#define MTR_IS_PRIVILEGED_PORT(port) \ + ((port) >= MTR_PORT_MIN && (port) < MTR_UNPRIVILEGED_PORT_MIN) + +#endif diff --git a/packet/probe.h b/packet/probe.h index d0ab0cfd..6a8b0cab 100644 --- a/packet/probe.h +++ b/packet/probe.h @@ -26,6 +26,7 @@ #include #include +#include "ports.h" #include "portability/queue.h" #ifdef PLATFORM_CYGWIN diff --git a/packet/probe_unix.c b/packet/probe_unix.c index 3d1d26ca..befed0c8 100644 --- a/packet/probe_unix.c +++ b/packet/probe_unix.c @@ -542,7 +542,7 @@ void report_packet_error( printf("%d no-route-network\n", command_token); } else if (errno == EHOSTUNREACH) { printf("%d no-route-host\n", command_token); - } else if (errno == EPERM) { + } else if (errno == EACCES || errno == EPERM) { printf("%d permission-denied\n", command_token); } else if (errno == EADDRINUSE) { printf("%d address-in-use\n", command_token); diff --git a/test/probe.py b/test/probe.py index 3647ab1a..ae9cd171 100755 --- a/test/probe.py +++ b/test/probe.py @@ -334,6 +334,11 @@ def udp_port_test(self, address): # type: (unicode) -> None reply = self.parse_reply() self.assertEqual('reply', reply.command_name) + cmd = '83 send-probe protocol udp local-port 80 ' + address + self.write_command(cmd) + reply = self.parse_reply() + self.assertIn(reply.command_name, ('reply', 'permission-denied')) + def test_udp_v4(self): 'Test IPv4 UDP probes' diff --git a/ui/cmdpipe.c b/ui/cmdpipe.c index 7f18406f..57069f03 100644 --- a/ui/cmdpipe.c +++ b/ui/cmdpipe.c @@ -688,6 +688,13 @@ void handle_reply_errors( if (!strcmp(reply_name, "permission-denied")) { display_close(ctl); + if (ctl->mtrtype == IPPROTO_UDP && + MTR_IS_PRIVILEGED_PORT(ctl->localport)) { + error(EXIT_FAILURE, 0, + "permission denied binding UDP source port %d; " + "the OS did not allow this privileged local port", + ctl->localport); + } error(EXIT_FAILURE, 0, "mtr-packet reported permission denied while sending a probe"); } diff --git a/ui/mtr.c b/ui/mtr.c index de47683f..01045a26 100644 --- a/ui/mtr.c +++ b/ui/mtr.c @@ -187,7 +187,7 @@ static int parse_target_port( { int remoteport = strtoint_or_err(port, "invalid argument"); - if (remoteport < 1 || MaxPort < remoteport) { + if (!MTR_IS_VALID_PORT(remoteport)) { error(EXIT_FAILURE, 0, "Illegal port number: %d", remoteport); } @@ -667,7 +667,7 @@ static void parse_arg( case 'P': ctl->remoteport = strtoint_or_err(optarg, "invalid argument"); - if (ctl->remoteport < 1 || MaxPort < ctl->remoteport) { + if (!MTR_IS_VALID_PORT(ctl->remoteport)) { error(EXIT_FAILURE, 0, "Illegal port number: %d", ctl->remoteport); } @@ -675,7 +675,7 @@ static void parse_arg( case 'L': ctl->localport = strtoint_or_err(optarg, "invalid argument"); - if (ctl->localport < MinPort || MaxPort < ctl->localport) { + if (!MTR_IS_VALID_PORT(ctl->localport)) { error(EXIT_FAILURE, 0, "Illegal port number: %d", ctl->localport); } diff --git a/ui/mtr.h b/ui/mtr.h index 7af3dcc8..8890f090 100644 --- a/ui/mtr.h +++ b/ui/mtr.h @@ -27,6 +27,8 @@ #include #include +#include "packet/ports.h" + #ifdef HAVE_NETINET_IN_H #include #endif @@ -66,8 +68,6 @@ typedef int time_t; #define SAVED_PINGS 400 #define MAX_PATH 128 #define MaxHost 256 -#define MinPort 1024 -#define MaxPort 65535 #define MAXPACKET 65535 /* largest test packet size */ #define MINPACKET 28 /* 20 bytes IP header and 8 bytes ICMP or UDP */ #define MAXLABELS 8 /* http://kb.juniper.net/KB2190 (+ 3 just in case) */