From cecc96109eed572471168a70bd148403b86e5c55 Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 5 Jul 2021 17:26:10 +0200 Subject: [PATCH 01/10] remove getuid == 0 check While the software needs "root, permissions getuid is not a good way to check for the necessary permissions: 1. uid might be different from the effective uid - which is actually used for permissions 2. The binary might have all necessary capabilities set (via setcap) in this case the UID does not matter at all - the necessary permissions are set already. --- src/bbl.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/bbl.c b/src/bbl.c index cdfea461..78295344 100644 --- a/src/bbl.c +++ b/src/bbl.c @@ -645,20 +645,16 @@ main (int argc, char *argv[]) interactive = true; break; case 'S': - ctx->ctrl_socket_path = optarg; + ctx->ctrl_socket_path = optarg; break; case 'f': - ctx->config.interface_lock_force = true; + ctx->config.interface_lock_force = true; break; default: bbl_print_usage(); exit(1); } } - if (geteuid() != 0) { - fprintf(stderr, "Error: Must be run with root privileges\n"); - exit(1); - } if(!config_file) { fprintf(stderr, "Error: No configuration specified (-C / --config )\n"); From c64ba87ef494a7d613b42058ceab625c69e5beae Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 5 Jul 2021 17:29:24 +0200 Subject: [PATCH 02/10] replace tabs with spaces --- src/bbl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bbl.c b/src/bbl.c index 78295344..b083da46 100644 --- a/src/bbl.c +++ b/src/bbl.c @@ -301,7 +301,7 @@ bbl_add_interface (bbl_ctx_s *ctx, char *interface_name) * Timer to compute periodic rates. */ timer_add_periodic(&ctx->timer_root, &interface->rate_job, "Rate Computation", 1, 0, interface, - &bbl_compute_interface_rate_job); + &bbl_compute_interface_rate_job); return interface; } @@ -606,10 +606,10 @@ main (int argc, char *argv[]) bbl_print_usage(); exit(0); case 'P': - ctx->pcap.filename = optarg; + ctx->pcap.filename = optarg; break; case 'J': - ctx->config.json_report_filename = optarg; + ctx->config.json_report_filename = optarg; break; case 'C': config_file = optarg; @@ -618,7 +618,7 @@ main (int argc, char *argv[]) log_enable(optarg); break; case 'L': - g_log_file = optarg; + g_log_file = optarg; break; case 'u': username = optarg; From 961d9081736352084ccab028f6371af3f2c2a1b1 Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 5 Jul 2021 17:49:03 +0200 Subject: [PATCH 03/10] explicit error message for permission denied when opening sockets --- src/bbl_io.c | 4 ++++ src/bbl_stream.c | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/bbl_io.c b/src/bbl_io.c index bf6840f3..58e4ac93 100644 --- a/src/bbl_io.c +++ b/src/bbl_io.c @@ -385,6 +385,10 @@ bbl_io_add_interface(bbl_ctx_s *ctx, bbl_interface_s *interface) { */ interface->io.fd_tx = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK, 0); if (interface->io.fd_tx == -1) { + if (errno == EPERM) { + LOG(ERROR, "socket() for interface %s Permission denied: Are you root?\n", interface->name); + return false; + } LOG(ERROR, "socket() TX error %s (%d) for interface %s\n", strerror(errno), errno, interface->name); return false; } diff --git a/src/bbl_stream.c b/src/bbl_stream.c index 34b82c2d..372d15c9 100644 --- a/src/bbl_stream.c +++ b/src/bbl_stream.c @@ -565,6 +565,10 @@ bbl_stream_tx_thread (void *thread_data) { /* Open new TX socket for thread. */ fd_tx = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK, 0); if (fd_tx == -1) { + if (errno == EPERM) { + LOG(ERROR, "socket() for interface %s Permission denied: Are you root?\n", interface->name); + return NULL; + } LOG(ERROR, "socket() TX error %s (%d) for interface %s\n", strerror(errno), errno, interface->name); return NULL; } From 2be10d5114503a2955e2e426bae52143d4958715 Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 5 Jul 2021 17:50:03 +0200 Subject: [PATCH 04/10] convert tabs to spaces and remove trailing spaces --- src/bbl_io.c | 10 +++++----- src/bbl_stream.c | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/bbl_io.c b/src/bbl_io.c index 58e4ac93..03c5ab14 100644 --- a/src/bbl_io.c +++ b/src/bbl_io.c @@ -62,8 +62,8 @@ bbl_io_packet_mmap_rx_job (timer_s *timer) { /* Dump the packet into pcap file. */ if (ctx->pcap.write_buf) { - pcapng_push_packet_header(ctx, &interface->rx_timestamp, eth_start, eth_len, - interface->pcap_index, PCAPNG_EPB_FLAGS_INBOUND); + pcapng_push_packet_header(ctx, &interface->rx_timestamp, eth_start, eth_len, + interface->pcap_index, PCAPNG_EPB_FLAGS_INBOUND); } decode_result = decode_ethernet(eth_start, eth_len, interface->ctx->sp_rx, SCRATCHPAD_LEN, ð); @@ -128,7 +128,7 @@ bbl_io_raw_rx_job (timer_s *timer) { while (true) { recv_result = recvfrom(interface->io.fd_rx, interface->io.rx_buf, IO_BUFFER_LEN, 0, &saddr , (socklen_t*)&saddr_size); - if(recv_result < 14 || recv_result > IO_BUFFER_LEN) { + if(recv_result < 14 || recv_result > IO_BUFFER_LEN) { break; } interface->stats.packets_rx++; @@ -136,8 +136,8 @@ bbl_io_raw_rx_job (timer_s *timer) { /* Dump the packet into pcap file. */ if (ctx->pcap.write_buf) { - pcapng_push_packet_header(ctx, &interface->rx_timestamp, interface->io.rx_buf, interface->io.rx_len, - interface->pcap_index, PCAPNG_EPB_FLAGS_INBOUND); + pcapng_push_packet_header(ctx, &interface->rx_timestamp, interface->io.rx_buf, interface->io.rx_len, + interface->pcap_index, PCAPNG_EPB_FLAGS_INBOUND); } decode_result = decode_ethernet(interface->io.rx_buf, interface->io.rx_len, interface->ctx->sp_rx, SCRATCHPAD_LEN, ð); diff --git a/src/bbl_stream.c b/src/bbl_stream.c index 372d15c9..8bf37166 100644 --- a/src/bbl_stream.c +++ b/src/bbl_stream.c @@ -34,14 +34,14 @@ bbl_stream_can_send(bbl_stream *stream) { } break; case STREAM_IPV6: - if(session->ip6cp_state == BBL_PPP_OPENED && - session->icmpv6_ra_received && + if(session->ip6cp_state == BBL_PPP_OPENED && + session->icmpv6_ra_received && *(uint64_t*)session->ipv6_address) { return true; } break; case STREAM_IPV6PD: - if(session->ip6cp_state == BBL_PPP_OPENED && + if(session->ip6cp_state == BBL_PPP_OPENED && session->icmpv6_ra_received && *(uint64_t*)session->delegated_ipv6_address && session->dhcpv6_state >= BBL_DHCP_BOUND) { @@ -59,7 +59,7 @@ bbl_stream_can_send(bbl_stream *stream) { } break; case STREAM_IPV6: - if(*(uint64_t*)session->ipv6_address && + if(*(uint64_t*)session->ipv6_address && session->icmpv6_ra_received) { return true; } From 03e35c3a2aa4e18d5e03d49346603119ff0bb909 Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Fri, 9 Jul 2021 14:15:43 +0200 Subject: [PATCH 05/10] fix: ifreq struct must be passed as pointer to ioctl --- src/bbl_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bbl_io.c b/src/bbl_io.c index 03c5ab14..273dae9b 100644 --- a/src/bbl_io.c +++ b/src/bbl_io.c @@ -438,7 +438,7 @@ bbl_io_add_interface(bbl_ctx_s *ctx, bbl_interface_s *interface) { } ifr.ifr_flags |= IFF_PROMISC; - if (ioctl(interface->io.fd_rx, SIOCSIFFLAGS, ifr) == -1){ + if (ioctl(interface->io.fd_rx, SIOCSIFFLAGS, &ifr) == -1){ LOG(ERROR, "Setting socket flags error %s (%d) when setting promiscuous mode for interface %s\n", strerror(errno), errno, interface->name); return false; From 9dc88dac3150fa8d4cef05c077dcf234438aa48b Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Fri, 9 Jul 2021 14:20:06 +0200 Subject: [PATCH 06/10] cmake: use string append instead of set construct --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a2ac1dd9..87fec812 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -39,8 +39,8 @@ set(CURSES_NEED_NCURSES TRUE) include(FindCurses) target_link_libraries(bngblaster ${CURSES_LIBRARIES} crypto jansson ${libdict} m) -SET(PLATFORM_SPECIFIC_LIBS "-lpthread") -SET(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pthread") +set(PLATFORM_SPECIFIC_LIBS "-lpthread") +string(APPEND CMAKE_C_FLAGS "-pthread") # add experimental netmap support From 018d089c0364fcede49d9b9b7e25cf07c14491dc Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Fri, 9 Jul 2021 14:43:24 +0200 Subject: [PATCH 07/10] use a race free way to set promiscuous mode for interfaces --- src/bbl_io.c | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/bbl_io.c b/src/bbl_io.c index 273dae9b..c5a34076 100644 --- a/src/bbl_io.c +++ b/src/bbl_io.c @@ -353,6 +353,36 @@ bbl_io_send (bbl_interface_s *interface, uint8_t *packet, uint16_t packet_len) { return result; } +/* Taken and adapted from +* https://stackoverflow.com/questions/41678219/how-to-properly-put-network-interface-into-promiscuous-mode-on-linux +* +* This prevents the ioctl get flags / set flags race condition +*/ +int +set_promisc(const char *ifname) { + struct packet_mreq mreq = {0}; + int sfd; + + /* This socket is only opened, but not closed. Closing the socket would reset + * its flags - effectively removing the just added promisc mode. + * We want to keep the interface in promisc mode until the end of the program. + */ + if ((sfd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL))) == -1) { + LOG(ERROR, "unable to open control socket for promisc activation\n"); + return -1; + } + + mreq.mr_ifindex = if_nametoindex(ifname); + mreq.mr_type = PACKET_MR_PROMISC; + + if (mreq.mr_ifindex == 0) { + LOG(ERROR, "unable to get interface index for %s\n", ifname); + return -1; + } + + return setsockopt(sfd, SOL_PACKET, PACKET_ADD_MEMBERSHIP, &mreq, sizeof(mreq)); +} + /** * bbl_io_add_interface * @@ -364,7 +394,6 @@ bbl_io_add_interface(bbl_ctx_s *ctx, bbl_interface_s *interface) { size_t ring_size; char timer_name[32]; - struct ifreq ifr; int version = TPACKET_V2; int qdisc_bypass = 1; int slots = ctx->config.io_slots; @@ -429,18 +458,9 @@ bbl_io_add_interface(bbl_ctx_s *ctx, bbl_interface_s *interface) { } /* Set the interface to promiscuous mode. Only for the RX FD. */ - memset(&ifr, 0, sizeof(ifr)); - snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", interface->name); - if (ioctl(interface->io.fd_rx, SIOCGIFFLAGS, &ifr) == -1) { - LOG(ERROR, "Getting socket flags error %s (%d) when setting promiscuous mode for interface %s\n", - strerror(errno), errno, interface->name); - return false; - } - ifr.ifr_flags |= IFF_PROMISC; - if (ioctl(interface->io.fd_rx, SIOCSIFFLAGS, &ifr) == -1){ - LOG(ERROR, "Setting socket flags error %s (%d) when setting promiscuous mode for interface %s\n", - strerror(errno), errno, interface->name); + if (set_promisc(interface->name) != 0) { + LOG(ERROR, "Failed to put interface %s in promiscuous mode\n", interface->name); return false; } From 4ae2ab301b7ad71c639c8d47042416c9e3e464bf Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 12 Jul 2021 10:24:09 +0200 Subject: [PATCH 08/10] nix: support disabling tests --- flake.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.nix b/flake.nix index f3a1e712..fc9c8199 100644 --- a/flake.nix +++ b/flake.nix @@ -37,14 +37,14 @@ nativeBuildInputs = buildTools; }; - bngblaster = pkgs.stdenv.mkDerivation { + bngblaster = pkgs.stdenv.mkDerivation rec { pname = "bngblaster"; version = "0.52"; src = lib.cleanSource ./.; doCheck = true; - cmakeFlags = - [ "-DCMAKE_BUILD_TYPE=Release" "-DBNGBLASTER_TESTS=ON" ]; + cmakeFlags = [ "-DCMAKE_BUILD_TYPE=Release" ] + ++ (if doCheck then [ "-DBNGBLASTER_TESTS=ON" ] else [ ]); checkInputs = [ pkgs.cmocka pkgs.libpcap ]; nativeBuildInputs = buildTools; From 8de180132c6a2bcd8d3314e525534a1172f96b2e Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 12 Jul 2021 10:43:25 +0200 Subject: [PATCH 09/10] Improve markdown compliance and add cli tags to codeblocks --- docs/install.md | 53 +++++++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/docs/install.md b/docs/install.md index 5c9393f9..9714db0d 100644 --- a/docs/install.md +++ b/docs/install.md @@ -3,27 +3,29 @@ ## Install Ubuntu Install dependencies: -``` + +```cli sudo apt install -y libssl1.1 libncurses5 libjansson4 ``` Download and install debian package: -https://github.com/rtbrick/bngblaster/releases +[https://github.com/rtbrick/bngblaster/releases](https://github.com/rtbrick/bngblaster/releases) -``` +```cli sudo dpkg -i ``` -This command installs the BNG Blaster to `/usr/sbin/bngblaster`. +This command installs the BNG Blaster to `/usr/sbin/bngblaster`. ## Build from Sources ### Dependencies -The BNG Blaster has dependencies to the RtBrick libdict fork -(https://github.com/rtbrick/libdict) and the following standard +The BNG Blaster has dependencies to the RtBrick [libdict +fork](https://github.com/rtbrick/libdict) and the following standard dependencies: -``` + +```cli sudo apt install -y cmake \ libcunit1-dev \ libncurses5-dev \ @@ -35,7 +37,8 @@ sudo apt install -y cmake \ Per default cmake (`cmake .`) will build the BNG Blaster as release version with optimization and without debug symbols. -``` + +```cli mkdir build cd build cmake -DCMAKE_BUILD_TYPE=Release .. @@ -44,7 +47,8 @@ make all Alternative it is also possible to build a debug version for detailed troubleshooting using gdb. -``` + +```cli mkdir build cd build cmake -DCMAKE_BUILD_TYPE=Debug .. @@ -56,12 +60,14 @@ package by just executing `cpack` from build directory. It is also recommended to provide the GIT commit details to be included in the manually build version as shown below: -``` -cmake -DGIT_REF=`git rev-parse --abbrev-ref HEAD` -DGIT_SHA=`git rev-parse HEAD` .. + +```cli +cmake -DGIT_REF=`git rev-parse --abbrev-ref HEAD` -DGIT_SHA=`git rev-parse HEAD` . ``` *Example:* -``` + +```cli $ bngblaster -v GIT: REF: dev @@ -72,36 +78,41 @@ IO Modes: packet_mmap_raw (default), packet_mmap, raw ### Install Then BNG Blaster can be installed using make install target. -``` + +```cli sudo make install ``` -This command installs the BNG Blaster to `/usr/local/sbin/bngblaster`. +This command installs the BNG Blaster to `/usr/local/sbin/bngblaster`. -An existing version installed from debian package in `/usr/sbin` is +An existing version installed from debian package in `/usr/sbin` is not automatically replaced or removed here and should be deleted manually before install. Otherwise it might be possible that two versions remain -in parallel. -``` +in parallel. + +```cli sudo rm /usr/sbin/bngblaster ``` ### Build and Run Unit Tests Building and running unit tests requires CMocka to be installed: -``` + +```cli sudo apt install libcmocka-dev ``` The option `BNGBLASTER_TESTS` enables to build unit tests. -``` + +```cli cmake -DCMAKE_BUILD_TYPE=Debug -DBNGBLASTER_TESTS=ON . make all make test ``` -*Example* -``` +*Example:* + +```cli $ make test Running tests... Test project From f3e6813209864034dfdf4918c476f53565bdbd96 Mon Sep 17 00:00:00 2001 From: Istvan Ruzman Date: Mon, 12 Jul 2021 10:53:07 +0200 Subject: [PATCH 10/10] document how to run bngblaster as non-root with capabilities --- docs/install.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/install.md b/docs/install.md index 9714db0d..78eb253f 100644 --- a/docs/install.md +++ b/docs/install.md @@ -123,3 +123,26 @@ Test project Total Test time (real) = 0.00 sec ``` + +### Running bngblaster + +bngblaster needs permissions to send raw packets and change network interface +settings. The easiest way to run bngblaster is either as the root user or with +sudo: + +```cli +# As root +bngblaster -C config.json -I + +# As a normal user: +sudo bngblaster -C config.json -I +``` + +A third option is to set capabilities on the binary with e.g. `setcap`: + +```cli +sudo setcap cap_net_raw,cap_net_admin,cap_dac_read_search+eip /path/to/bngblaster + +# As either root or a normal user: +bngblaster -C config.json -I +```