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()