Bug 467381 - aiccu leaves file descriptor open on exec(), selinux reports problem
aiccu leaves file descriptor open on exec(), selinux reports problem
Status: CLOSED NEXTRELEASE
Product: Fedora EPEL
Classification: Fedora
Component: aiccu (Show other bugs)
el5
x86_64 Linux
medium Severity high
: ---
: ---
Assigned To: Matt Domsch
Fedora Extras Quality Assurance
https://www.sixxs.net/forum/?msg=setu...
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-17 04:51 EDT by Hugo van der Kooij
Modified: 2009-01-07 04:27 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-11-02 01:00:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
aiccu-close-before-system.patch (2.84 KB, patch)
2008-10-18 00:28 EDT, Matt Domsch
no flags Details | Diff

  None (edit)
Description Hugo van der Kooij 2008-10-17 04:51:16 EDT
Description of problem:
This aiccu package does not contain a selinux policy to make it owrk if selinux is enabled.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1. install Centos 5.2 X86_64 with selinux
2. install aiccu package
3. Try to setup tunnel to SIXXS
  
Actual results:
No tunnel

Expected results:
Working tunnel

Additional info:

Oct 17 10:20:40 aragorn kernel: tun: Universal TUN/TAP device driver, 1.6
Oct 17 10:20:40 aragorn kernel: tun: (C) 1999-2004 Max Krasnyansky <maxk@qualcomm.com>
Oct 17 10:20:41 aragorn aiccu: Succesfully retrieved tunnel information for T17469
Oct 17 10:20:41 aragorn aiccu: AICCU running as PID 30008
Oct 17 10:20:41 aragorn kernel: sixxs: Disabled Privacy Extensions
Oct 17 10:20:41 aragorn aiccu: [AYIYA-start] : Anything in Anything (draft-02)
Oct 17 10:20:41 aragorn aiccu: [AYIYA-tun->tundev] : (Socket to TUN) started
Oct 17 10:20:43 aragorn setroubleshoot: SELinux is preventing ip (ifconfig_t) "read write" to socket (initrc_t). For complete SELinux messages. run sealert -l 915fccce-d55c-42e0-9aa6-cd2975ce48e0

And the full report is:

Summary:

SELinux is preventing ip (ifconfig_t) "read write" to socket (initrc_t).

Detailed Description:

SELinux denied access requested by ip. It is not expected that this access is
required by ip and this access may signal an intrusion attempt. It is also
possible that the specific version or configuration of the application is
causing it to require additional access.

Allowing Access:

You can generate a local policy module to allow this access - see FAQ
(http://fedora.redhat.com/docs/selinux-faq-fc5/#id2961385) Or you can disable
SELinux protection altogether. Disabling SELinux protection is not recommended.
Please file a bug report (http://bugzilla.redhat.com/bugzilla/enter_bug.cgi)
against this package.

Additional Information:

Source Context user_u:system_r:ifconfig_t
Target Context user_u:system_r:initrc_t
Target Objects socket [ udp_socket ]
Source ip
Source Path /sbin/ip
Port <Unknown>
Host aragorn.hugo.vanderkooij.org
Source RPM Packages iproute-2.6.18-7.el5
Target RPM Packages
Policy RPM selinux-policy-2.4.6-137.1.el5
Selinux Enabled True
Policy Type targeted
MLS Enabled True
Enforcing Mode Enforcing
Plugin Name catchall
Host Name aragorn.hugo.vanderkooij.org
Platform Linux aragorn.hugo.vanderkooij.org 2.6.18-92.el5
#1 SMP Tue Jun 10 18:51:06 EDT 2008 x86_64 x86_64
Alert Count 5
First Seen Fri Oct 17 10:20:41 2008
Last Seen Fri Oct 17 10:20:41 2008
Local ID 915fccce-d55c-42e0-9aa6-cd2975ce48e0
Line Numbers

Raw Audit Messages

host=aragorn.hugo.vanderkooij.org type=AVC msg=audit(1224231641.611:3009): avc: denied { read write } for pid=30018 comm="ip" path="socket:[7701766]" dev=sockfs ino=7701766 scontext=user_u:system_r:ifconfig_t:s0 tcontext=user_u:system_r:initrc_t:s0 tclass=udp_socket

host=aragorn.hugo.vanderkooij.org type=SYSCALL msg=audit(1224231641.611:3009): arch=c000003e syscall=59 success=yes exit=0 a0=1fa79440 a1=1fa79b40 a2=1fa78300 a3=3 items=0 ppid=30008 pid=30018 auid=500 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=476 comm="ip" exe="/sbin/ip" subj=user_u:system_r:ifconfig_t:s0 key=(null)
Comment 1 Hugo van der Kooij 2008-10-17 04:52:14 EDT
The generated local.te file reads as:

module local 1.0;

require {
        type ifconfig_t;
        type initrc_tmp_t;
        type rhgb_t;
        type initrc_t;
        class udp_socket { read write };
        class dir search;
}

#============= ifconfig_t ==============
allow ifconfig_t initrc_t:udp_socket { read write };

#============= rhgb_t ==============
allow rhgb_t initrc_tmp_t:dir search;
Comment 2 Hugo van der Kooij 2008-10-17 04:54:28 EDT
This local policy works but may contain unneeded bits.
Comment 3 Matt Domsch 2008-10-17 08:06:58 EDT
Dan, what's the best approach here?  aiccu is in EPEL, but not RHEL.

Thanks,
Matt
Comment 4 Daniel Walsh 2008-10-17 08:39:41 EDT
This is a leaked file descriptor, in aiccu, You need to close all your open file descriptors before execing other applications.

fcntl(fd, F_SETFD, FD_CLOEXEC)

This is not an SELinux bug.
Comment 5 Matt Domsch 2008-10-17 11:16:39 EDT
-----Original Message-----
From: Jeroen Massar [mailto:jeroen@sixxs.net] 
Sent: Friday, October 17, 2008 8:52 AM
To: Domsch, Matt
Cc: info@sixxs.net
Subject: Re: FW: [Bug 467381] aiccu package does not contain required selinux policy

Matt_Domsch@Dell.com wrote:
> FYI, most likely the logfile descriptor is being left open.

We don't close them indeed, and I am fairly sure that a lot of other programs don't do that either. It is not a 'leak' though as the called program terminates almost directly anyway.

I've added a note for the next version which we are working on, thus it will be included there, it is a bit stalled though because of non-existing connectivity, but with some luck that might get arranged soon and then a new AICCU will arise.

Greets,
 Jeroen
Comment 6 Daniel Walsh 2008-10-17 15:22:10 EDT
Getting a aiccu policy into F10 would be great.

Yes I know a lot of programs do not close file descriptors on exec, the problem happens when you exec a confined domain, selinux will see the descriptor, close it and report it as a potential problem.
Comment 7 Matt Domsch 2008-10-17 20:02:01 EDT
if I can patch aiccu to to the fcntl(), I will.  I need to get a new tunnel from sixxs.net as my current tunnels point as POPs which are offline.  Then I can test.

--- aiccu.orig/common/tun.c     2008-10-17 18:43:13.000000000 -0500
+++ aiccu/common/tun.c  2008-10-17 18:52:24.000000000 -0500
@@ -696,6 +696,8 @@ bool tun_start(struct tun_reader *tun)

        /* Create a new tap device */
        tun_fd = open("/dev/net/tun", O_RDWR);
+       if (tun_fd >= 0)
+               fcntl(tun_fd, F_SETFD, FD_CLOEXEC);
        if (tun_fd == -1)
        {
                tun_log(LOG_ERR, "start", "Couldn't open device %s: %s (%d)\n", "/dev/net/tun", strerror(errno), errno);
@@ -725,6 +727,8 @@ bool tun_start(struct tun_reader *tun)
        tun_log(LOG_DEBUG, "start", "Trying Configured TUN/TAP interface %s...\n", g_aiccu->ipv6_interface);
        snprintf(buf, sizeof(buf), "/dev/%s", g_aiccu->ipv6_interface);
        tun_fd = open(buf, O_RDWR);
+       if (tun_fd >= 0)
+               fcntl(tun_fd, F_SETFD, FD_CLOEXEC);
        if (tun_fd < 0)
        {
                /* Fall back to trying all /dev/tun* devices */
@@ -735,6 +739,7 @@ bool tun_start(struct tun_reader *tun)
                        tun_fd = open(buf, O_RDWR);
                        if (tun_fd >= 0)
                        {
+                               fcntl(tun_fd, F_SETFD, FD_CLOEXEC);
                                /* Copy over the name of the interface so that configging goes okay */
                                if (g_aiccu->ipv6_interface) free(g_aiccu->ipv6_interface);
                                snprintf(buf, sizeof(buf), "tun%u", i);
Comment 8 Matt Domsch 2008-10-17 23:36:11 EDT
this patch won't work.  This is going to be a pain to fix.

There are 2 cases of forks().
1) this thing daemonizes itself.  So we need the logfile and tunnel fds left open in the daemon child.  So we can't FD_CLOEXEC them.

2) a pile of calls to aiccu_exec() which calls system() to invoke scripts like /sbin/ip, /sbin/ifconfig, /sbin/route, and the like.  For these, we would like all the file descriptors closed before invoking system().  We don't want them closed in the parent (these are called before it daemonizes).

So to fix it right, aiccu_exec() is going to have to fork from the parent, close all file descriptors in the child, then invoke system().
Comment 9 Matt Domsch 2008-10-18 00:28:10 EDT
Created attachment 320736 [details]
aiccu-close-before-system.patch

possible patch. This makes startup much slower, as it repeatedly closes file descriptors.  Might not need to close nearly this many (~4k per invocation), if I knew how many fds could possibly be open (probably at most ten).
Comment 10 Daniel Walsh 2008-10-20 10:02:54 EDT
If you closeonexec the file descriptors when you create them, they will stay open over the fork and both processes will be able to use them.  They will not be open when you exec or system programs,  Which I think is what you want.
Comment 11 Ulrich Drepper 2008-10-20 10:32:40 EDT
(In reply to comment #9)
> Created an attachment (id=320736) [details]
> aiccu-close-before-system.patch
> 
> possible patch. This makes startup much slower, as it repeatedly closes file
> descriptors.  Might not need to close nearly this many (~4k per invocation), if
> I knew how many fds could possibly be open (probably at most ten).

People have to stop spreading horrible code like this.  If you have to loop then do this:

void
closeall (void)
{
  DIR *dir = opendir ("/proc/self/fd");
  if (dir != NULL)
    {
      struct dirent *d;
      while ((d = readdir (dir)) != NULL)
	{
	  if (d->d_name[0] == '.')
	    continue;
	  char *cp;
	  long int n = strtol (d->d_name, &cp, 10);
	  if (*cp != '\0' || n <= 2 || n == dirfd (dir))
	    continue;
	  close (n);
	}
      return;
    }
  int nmax = sysconf (_SC_OPEN_MAX);
  for (int n = 3; n < nmax; ++n)
    close (n);
}
Comment 12 Matt Domsch 2008-10-20 13:17:58 EDT
Dan, is there a way to see which file descriptors are being leaked during the system() calls?  selinux reports the leaks, but AFAICT, not which ones.  I've got a simple patch that does FD_CLOEXEC which I'll try out, but want to be sure I'm catching all fds.  You're right, I was confusing fork() with fork()+exec().
Comment 13 Matt Domsch 2008-10-20 13:22:38 EDT
http://domsch.com/linux/fedora/aiccu-2007.01.15-5.fc10.src.rpm
has my patch which does FD_CLOEXEC wherever I caught them.

Hugo, can you build and try this on your system, to see that selinux no longer reports problems, or if so, the messages?

Thanks,
Matt
Comment 14 Daniel Walsh 2008-10-20 13:35:27 EDT
Sorry can't do it.  SELinux is only going to report the leaks that the new confined domain is not allowed access to.
Comment 15 Matt Domsch 2008-10-29 12:00:34 EDT
http://koji.fedoraproject.org/koji/taskinfo?taskID=910811
building now, using FD_CLOEXEC.

Hugo, can you download this build and try it, to see that it resolves your issue?

Thanks,
Matt
Comment 16 Matt Domsch 2008-11-02 01:00:36 EST
This no longer fails for me on rawhide.

I've built this for Fedora 8, 9, rawhide, and EL-5 (branches with selinux).
Comment 17 Fedora Update System 2008-11-02 08:46:41 EST
aiccu-2007.01.15-5.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc9
Comment 18 Fedora Update System 2008-11-02 08:47:10 EST
aiccu-2007.01.15-5.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc8
Comment 19 Fedora Update System 2008-11-26 01:15:56 EST
aiccu-2007.01.15-5.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Fedora Update System 2008-11-26 01:20:50 EST
aiccu-2007.01.15-5.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 David Moore 2008-12-14 14:20:08 EST
Any reason this hasn't been pushed to Fedora 10?
Comment 22 Fedora Update System 2008-12-14 17:05:36 EST
aiccu-2007.01.15-5.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/aiccu-2007.01.15-5.fc10
Comment 23 Matt Domsch 2008-12-14 17:06:47 EST
David, thanks for the reminder.  I had fixed it after F10 package freeze, but forgot to issue the update.  I have done so now, it should be in testing with the next push.
Comment 24 Fedora Update System 2009-01-07 04:27:25 EST
aiccu-2007.01.15-5.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.