From c24e1bccbaf26e7ae4be02a7ae175bc4b86bd212 Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Thu, 1 Aug 2024 16:52:12 +0200 Subject: [PATCH] Switch patch suggestion to upstreamed patch --- ...alling-SO_BINDTODEVICE-if-not-needed.patch | 67 ------------------- ...ling-SO_BINDTODEVICE-if-not-needed.repro.c | 53 --------------- ...d-re-binding-accepted-sockets-to-VRF.patch | 38 +++++++++++ 3 files changed, 38 insertions(+), 120 deletions(-) delete mode 100644 patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.patch delete mode 100644 patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.repro.c create mode 100644 patches/0001-IO-Avoid-re-binding-accepted-sockets-to-VRF.patch diff --git a/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.patch b/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.patch deleted file mode 100644 index e84a69e..0000000 --- a/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.patch +++ /dev/null @@ -1,67 +0,0 @@ -From b84129c8b9cc1bc42119c1fc398183a3925125bd Mon Sep 17 00:00:00 2001 -From: Christian Svensson -Date: Sat, 27 Jul 2024 11:13:06 +0200 -Subject: [PATCH] IO: Avoid calling SO_BINDTODEVICE if not needed - -Since Linux 5.7 (see linux/c427bfec18f21) non-root users are allowed to -bind a socket using SO_BINDTODEVICE as long as the socket is not already bound. - -When using BGP with VRFs, BIRD correctly binds the listening socket to -the VRF but also re-binds the accept()'d socket to the same VRF. -This is not needed as the interface bind is inherited in this case, and -indeed this redundant bind causes an -EPERM if BIRD is running as non-root -making BIRD close the connection and reject the peer. - -We change the behaviour of the generic sk_setup to first query the socket -and see if the socket is already correctly bound, and call -setsockopt(SO_BINDTODEVICE) iff it is truly needed. In addition, -since the getsockopt(SO_BINDTODEVICE) was implemented in Linux 3.8 or -otherwise might be blocked in existing installations, we quietly fall -back to the previous behavior if the getsockopt call fails. - -Test case: - Run BIRD as a non-root user (and no extra capabilities) using passive - BGP inside a VRF. Before the patch observe the error: - " SOCK: Incoming connection: SO_BINDTODEVICE: Operation not permitted" - - protocol bgp AS1234_1 { - [..] - vrf "VrfTest"; - passive on; - } - - After the patch this works as expected. - -Signed-off-by: Christian Svensson ---- - sysdep/unix/io.c | 14 +++++++++++--- - 1 file changed, 11 insertions(+), 3 deletions(-) - -diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c -index ba2e1661..c329512f 100644 ---- a/sysdep/unix/io.c -+++ b/sysdep/unix/io.c -@@ -977,9 +977,17 @@ sk_setup(sock *s) - This is Linux-specific, but so is SO_BINDTODEVICE. */ - #ifdef SO_BINDTODEVICE - struct ifreq ifr = {}; -- strcpy(ifr.ifr_name, s->vrf->name); -- if (setsockopt(s->fd, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr)) < 0) -- ERR("SO_BINDTODEVICE"); -+ /* Re-binding a socket to another (or the same interface) is a high-privileged -+ action. Avoid doing that if the socket is already correctly bound. */ -+ socklen_t len = sizeof(ifr.ifr_name); -+ /* getsockopt(SO_BINDTODEVICE) was implemented in Linux 3.8, if the call -+ fails ignore the error and just call setsockopt(SO_BINDTODEVICE). */ -+ if (getsockopt(s->fd, SOL_SOCKET, SO_BINDTODEVICE, ifr.ifr_name, &len) < 0 || -+ strcmp(ifr.ifr_name, s->vrf->name) != 0) { -+ strcpy(ifr.ifr_name, s->vrf->name); -+ if (setsockopt(s->fd, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr)) < 0) -+ ERR("SO_BINDTODEVICE"); -+ } - #endif - } - --- -2.45.2 - diff --git a/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.repro.c b/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.repro.c deleted file mode 100644 index 64c018c..0000000 --- a/patches/0001-IO-Avoid-calling-SO_BINDTODEVICE-if-not-needed.repro.c +++ /dev/null @@ -1,53 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include -#include -#include - -int main(int argc, const char* argv[]) { - int sockfd; - struct ifreq ifr; - if (argc < 2) { - printf("Usage: %s interface\n", argv[0]); - exit(EXIT_FAILURE); - } - - if ((sockfd = socket(AF_INET, SOCK_STREAM, 0)) < 0) { - perror("socket"); - exit(EXIT_FAILURE); - } - - memset(&ifr, 0, sizeof(ifr)); - strncpy(ifr.ifr_name, argv[1], IFNAMSIZ - 1); - - // Bind the socket to the interface - if (setsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE, (void *)&ifr, sizeof(ifr)) < 0) { - perror("setsockopt SO_BINDTODEVICE"); - close(sockfd); - exit(EXIT_FAILURE); - } - - printf("Socket successfully bound to interface \"%s\"\n", ifr.ifr_name); - - // Bind the socket to the interface - if (setsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE, (void *)&ifr, sizeof(ifr)) < 0) { - perror("2nd try, setsockopt SO_BINDTODEVICE"); - } else { - printf("2nd try, socket successfully bound to interface \"%s\"\n", ifr.ifr_name); - } - - memset(&ifr, 0, sizeof(ifr)); - - socklen_t len = sizeof(ifr.ifr_name); - if (getsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE, ifr.ifr_name, &len) < 0) { - perror("getsockopt SO_BINDTODEVICE"); - exit(EXIT_FAILURE); - } - - printf("Socket is currently bound to interface \"%s\"\n", ifr.ifr_name); - return 0; -} diff --git a/patches/0001-IO-Avoid-re-binding-accepted-sockets-to-VRF.patch b/patches/0001-IO-Avoid-re-binding-accepted-sockets-to-VRF.patch new file mode 100644 index 0000000..741237e --- /dev/null +++ b/patches/0001-IO-Avoid-re-binding-accepted-sockets-to-VRF.patch @@ -0,0 +1,38 @@ +From df22b3140cad6bd8742dce16e6a1b342d4a83f6d Mon Sep 17 00:00:00 2001 +From: Ondrej Zajicek +Date: Tue, 30 Jul 2024 16:33:51 +0200 +Subject: [PATCH] IO: Avoid re-binding accepted sockets to VRF + +When VRFs are used, BIRD correctly binds listening (and connecting) +sockets to their VRFs but also re-binds accepted sockets to the same VRF. +This is not needed as the interface bind is inherited in this case, and +indeed this redundant bind causes an -EPERM if BIRD is running as +non-root making BIRD close the connection and reject the peer. + +Thanks to Christian Svensson for the original patch and Alexander Zubkov +for suggestions. +--- + sysdep/unix/io.c | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c +index ba2e1661..987c7a6b 100644 +--- a/sysdep/unix/io.c ++++ b/sysdep/unix/io.c +@@ -971,10 +971,11 @@ sk_setup(sock *s) + } + #endif + +- if (s->vrf && !s->iface) ++ if (s->vrf && !s->iface && (s->type != SK_TCP)) + { + /* Bind socket to associated VRF interface. +- This is Linux-specific, but so is SO_BINDTODEVICE. */ ++ This is Linux-specific, but so is SO_BINDTODEVICE. ++ For accepted TCP sockets it is inherited from the listening one. */ + #ifdef SO_BINDTODEVICE + struct ifreq ifr = {}; + strcpy(ifr.ifr_name, s->vrf->name); +-- +2.44.1 +