FD_CLOEXEC support

* 001-fail2ban-server-socket-close-on-exec-no-leak.diff

Add code that marks server and client sockets with FD_CLOEXEC flags.
Avoid leaking file descriptors to processes spawned when handling
fail2ban actions (ex: iptables).

Unix sockets managed by fail2ban-server don't need to be passed to any
child process. Fail2ban already uses the FD_CLOEXEC flags in the filter
code.

This patch also avoids giving iptables access to fail2ban UNIX socket in
a SELinux environment (A sane SELinux policy should trigger an audit
event because "iptables" will be given read/write access to the fail2ban
control socket).

Some random references related to this bug:
 http://sourceforge.net/tracker/?func=detail&atid=689044&aid=2086568&group_id=121032
 http://www.redhat.com/archives/fedora-selinux-list/2009-June/msg00124.html
 http://forums.fedoraforum.org/showthread.php?t=234230

 * 002-fail2ban-filters-close-on-exec-typo-fix.diff

There is a typo in the fail2ban server/filter.py source code. The
FD_CLOEXEC is correctly set but additional *random* flags are also set.
It has no side-effect as long as the fd doesn't match a valid flag :)
"fcntl.fcntl(fd, fcntl.F_SETFD, fd | fcntl.FD_CLOEXEC)" <== the 3rd
parameter should be flags, not a file descriptor.

 * 003-fail2ban-gamin-socket-close-on-exec-no-leak.diff

Add code that marks the Gamin monitor file descriptor with FD_CLOEXEC
flags. Avoid leaking file descriptors to processes spawned when handling
fail2ban actions (ex: iptables).

---

File descriptors in action process before patches:
dr-x------ 2 root root  0 .
dr-xr-xr-x 8 root root  0 ..
lr-x------ 1 root root 64 0 -> /dev/null        <== OK
l-wx------ 1 root root 64 1 -> /tmp/test.log    <== used by test action
lrwx------ 1 root root 64 2 -> /dev/null        <== OK
lrwx------ 1 root root 64 3 -> socket:[116361]  <== NOK (fail2ban.sock leak)
lr-x------ 1 root root 64 4 -> /proc/20090/fd   <== used by test action
l-wx------ 1 root root 64 5 -> /var/log/fail2ban.log <== OK
lrwx------ 1 root root 64 6 -> socket:[115608]  <== NOK (gamin sock leak)

File descriptors in action process after patches:
dr-x------ 2 root root  0 .
dr-xr-xr-x 8 root root  0 ..
lr-x------ 1 root root 64 0 -> /dev/null        <== OK
l-wx------ 1 root root 64 1 -> /tmp/test.log    <== used by test action
lrwx------ 1 root root 64 2 -> /dev/null        <== OK
lr-x------ 1 root root 64 3 -> /proc/18284/fd   <== used by test action
l-wx------ 1 root root 64 5 -> /var/log/fail2ban.log <== OK
pull/167/head
Nicolas Collignon 2013-04-02 19:11:59 +02:00
parent 74e76e068c
commit 39667ff6f7
3 changed files with 21 additions and 3 deletions

View File

@ -29,7 +29,7 @@ __license__ = "GPL"
from pickle import dumps, loads, HIGHEST_PROTOCOL
from common import helpers
import asyncore, asynchat, socket, os, logging, sys, traceback
import asyncore, asynchat, socket, os, logging, sys, traceback, fcntl
# Gets the instance of the logger.
logSys = logging.getLogger("fail2ban.server")
@ -107,6 +107,7 @@ class AsyncServer(asyncore.dispatcher):
except TypeError:
logSys.warning("Type error")
return
AsyncServer.__markCloseOnExec(conn)
# Creates an instance of the handler class to handle the
# request/response on the incoming connection.
RequestHandler(conn, self.__transmitter)
@ -134,6 +135,7 @@ class AsyncServer(asyncore.dispatcher):
self.bind(sock)
except Exception:
raise AsyncServerException("Unable to bind socket %s" % self.__sock)
AsyncServer.__markCloseOnExec(self.socket)
self.listen(1)
# Sets the init flag.
self.__init = True
@ -159,6 +161,18 @@ class AsyncServer(asyncore.dispatcher):
os.remove(self.__sock)
logSys.debug("Socket shutdown")
##
# Marks socket as close-on-exec to avoid leaking file descriptors when
# running actions involving command execution.
# @param sock: socket file.
#@staticmethod
def __markCloseOnExec(sock):
fd = sock.fileno()
flags = fcntl.fcntl(fd, fcntl.F_GETFD)
fcntl.fcntl(fd, fcntl.F_SETFD, flags|fcntl.FD_CLOEXEC)
__markCloseOnExec = staticmethod(__markCloseOnExec)
##
# AsyncServerException is used to wrap communication exceptions.

View File

@ -554,7 +554,8 @@ class FileContainer:
self.__handler = open(self.__filename)
# Set the file descriptor to be FD_CLOEXEC
fd = self.__handler.fileno()
fcntl.fcntl(fd, fcntl.F_SETFD, fd | fcntl.FD_CLOEXEC)
flags = fcntl.fcntl(fd, fcntl.F_GETFD)
fcntl.fcntl(fd, fcntl.F_SETFD, flags | fcntl.FD_CLOEXEC)
firstLine = self.__handler.readline()
# Computes the MD5 of the first line.
myHash = md5sum(firstLine).digest()

View File

@ -27,7 +27,7 @@ from failmanager import FailManagerEmpty
from filter import FileFilter
from mytime import MyTime
import time, logging, gamin
import time, logging, gamin, fcntl
# Gets the instance of the logger.
logSys = logging.getLogger("fail2ban.filter")
@ -52,6 +52,9 @@ class FilterGamin(FileFilter):
self.__modified = False
# Gamin monitor
self.monitor = gamin.WatchMonitor()
fd = self.monitor.get_fd()
flags = fcntl.fcntl(fd, fcntl.F_GETFD)
fcntl.fcntl(fd, fcntl.F_SETFD, flags|fcntl.FD_CLOEXEC)
logSys.debug("Created FilterGamin")