From 2bf7306bb9eccaf1959dcf0387fbf9bffce6211c Mon Sep 17 00:00:00 2001 From: "Nicholas J. Kain" Date: Fri, 20 Jul 2012 18:48:26 -0400 Subject: [PATCH] Add some more syscalls to the ndhc permit filter. Netlink sockets were broken before because of too-strict filters. Move setup_signals under the seccomp filter to give it more testing coverage. Make the UDP datagram length check much more strict. If the read buffer does not match up with the header lengths exactly, it is discarded. Print a warning to syslog/stdout when ifchd execute_buffer() returns an error. Fix a regression introduced in ifchd that would cause the epoll handler to spin when a client connection closed. --- ifchd/CMakeLists.txt | 5 +---- ifchd/ifchd-defines.h | 2 +- ifchd/ifchd.c | 13 +++++++++---- ndhc/dhcp.c | 6 ++++-- ndhc/ndhc.c | 10 +++++++++- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/ifchd/CMakeLists.txt b/ifchd/CMakeLists.txt index f015b65..60d2018 100644 --- a/ifchd/CMakeLists.txt +++ b/ifchd/CMakeLists.txt @@ -2,10 +2,7 @@ project (ifchd) cmake_minimum_required (VERSION 2.6) -set(IFCHD_SRCS - ifchd.c - linux.c - ) +file(GLOB IFCHD_SRCS "*.c") add_executable(ifchd ${IFCHD_SRCS}) target_link_libraries(ifchd ncmlib) diff --git a/ifchd/ifchd-defines.h b/ifchd/ifchd-defines.h index a62045c..078ba38 100644 --- a/ifchd/ifchd-defines.h +++ b/ifchd/ifchd-defines.h @@ -5,7 +5,7 @@ #define PID_FILE_DEFAULT "/var/run/ifchd.pid" #define IFCHD_VERSION "0.9" -#define MAX_BUF 1024 +#define MAX_BUF 384 #define SOCK_QUEUE 2 #define CONN_TIMEOUT 60 #define MAX_IFACES 10 diff --git a/ifchd/ifchd.c b/ifchd/ifchd.c index b4fe466..7b8808d 100644 --- a/ifchd/ifchd.c +++ b/ifchd/ifchd.c @@ -664,15 +664,20 @@ static void process_client_fd(int fd) memset(buf, '\0', sizeof buf); r = safe_read(cl->fd, buf, sizeof buf - 1); - if (r == 0) - return; - else if (r < 0) { + if (r == 0) { + // Remote end hung up. + goto fail; + } else if (r < 0) { + if (errno == EAGAIN || errno == EWOULDBLOCK) + return; log_line("error reading from client fd: %s", strerror(errno)); goto fail; } - if (execute_buffer(cl, buf) == -1) + if (execute_buffer(cl, buf) == -1) { + log_line("execute_buffer was passed invalid commands"); goto fail; + } return; fail: ifchd_client_wipe(cl); diff --git a/ndhc/dhcp.c b/ndhc/dhcp.c index 3b2ee27..cc060d8 100644 --- a/ndhc/dhcp.c +++ b/ndhc/dhcp.c @@ -283,8 +283,10 @@ static int get_raw_packet(struct client_state_t *cs, struct dhcpmsg *payload) return -1; } - if (inc > ntohs(packet.ip.tot_len)) - log_line("Discarded extra bytes after reading a single UDP datagram."); + if (inc != ntohs(packet.ip.tot_len)) { + log_line("UDP length does not match header length fields."); + return -2; + } if (!cs->using_dhcp_bpf && !get_raw_packet_validate_bpf(&packet)) return -2; diff --git a/ndhc/ndhc.c b/ndhc/ndhc.c index 0c35e04..e10875c 100644 --- a/ndhc/ndhc.c +++ b/ndhc/ndhc.c @@ -125,12 +125,19 @@ static int enforce_seccomp(void) ALLOW_SYSCALL(read), ALLOW_SYSCALL(write), ALLOW_SYSCALL(close), + ALLOW_SYSCALL(recvmsg), ALLOW_SYSCALL(socket), ALLOW_SYSCALL(setsockopt), ALLOW_SYSCALL(fcntl), ALLOW_SYSCALL(bind), ALLOW_SYSCALL(open), ALLOW_SYSCALL(connect), + ALLOW_SYSCALL(getsockname), + + // These are for 'write_leasefile()' + ALLOW_SYSCALL(ftruncate), + ALLOW_SYSCALL(lseek), + ALLOW_SYSCALL(fsync), // These are for 'background()' ALLOW_SYSCALL(socketpair), @@ -230,11 +237,12 @@ static void do_work(void) cs.epollFd = epoll_create1(0); if (cs.epollFd == -1) suicide("epoll_create1 failed"); - setup_signals(&cs); if (enforce_seccomp()) log_line("seccomp filter cannot be installed"); + setup_signals(&cs); + epoll_add(&cs, cs.nlFd); set_listen_raw(&cs); nowts = curms();