Edgewall Software
Modify

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#10991 closed defect (fixed)

Avoid log file descriptor leakage

Reported by: bunk@… Owned by: Remy Blank
Priority: normal Milestone: 1.0.1
Component: general Version:
Severity: normal Keywords: selinux
Cc: bunk@… Branch:
Release Notes:

Close file descriptors when shelling out to sendmail to avoid file descriptor leakage.

API Changes:
Internal Changes:

Description

When trac is configured to log to a file and sends mail notifications via sendmail, SELinux complains:

type=AVC msg=audit(1356034258.828:30164):
 avc:  denied  { append } for  pid=28546 comm="sendmail"
 path="/tmp/trac.log" dev=sda1 ino=673386
 scontext=system_u:system_r:system_mail_t:s0
 tcontext=unconfined_u:object_r:httpd_tmp_t:s0 tclass=file

Attached patch against the current trunk revision r11179 sets the FD_CLOEXEC flag on the log file descriptor to avoid leaking it to forked children including sendmail.

More info about fd leaks and SELinux: http://danwalsh.livejournal.com/53603.html

Attachments (1)

log.py.diff (819 bytes ) - added by anonymous 8 years ago.

Download all attachments as: .zip

Change History (10)

by anonymous, 8 years ago

Attachment: log.py.diff added

comment:1 by Remy Blank, 8 years ago

Interesting, thanks for the patch. I have a few questions.

  • Wouldn't the same be necessary for all open file descriptors, for example the database file for SQLite?
  • Could we solve this in a more generic way by setting close_fds=True when using the subprocess module (on non-Windows only)?
  • Why does sendmail try to write to a file descriptor it didn't open in the first place?

comment:2 by anonymous, 8 years ago

Good questions.

comment:3 by anonymous, 8 years ago

Ping (sorry to bug you, but this is an easy to fix issue, so if you don't have further questions just apply one or both ways to fix it)

in reply to:  1 comment:4 by Peter Suter, 8 years ago

Replying to rblank:

  • Could we solve this in a more generic way by setting close_fds=True when using the subprocess module (on non-Windows only)?

Looks like we already do that for all other calls to subprocess.call and subprocess.Popen:

Some history digging suggests this was an oversight, not a deliberate decision:

  • [5951] Added close_fds=True parameters (in functional testing sandbox)
  • [6001] Added close_fds=close_fds Windows compat flag
  • [7935] Introduced SendmailEmailSender (in trunk)
  • [8027] Moved flag to trac.util.compat
  • [8031] Added some more usages (still in testing sandbox)
  • [8131] Merged flag from testing sandbox to trunk
Last edited 8 years ago by Peter Suter (previous) (diff)

comment:5 by Remy Blank, 8 years ago

Milestone: 1.0.1
Owner: set to Remy Blank
Status: newassigned

Thanks for checking! So this is a strong indicator that the close_fds solution is the right one.

comment:6 by Remy Blank, 8 years ago

Resolution: fixed
Status: assignedclosed

Added close_fds when calling sendmail in [11496].

comment:7 by anonymous, 8 years ago

Great! Thanks for fixing :)

comment:8 by Peter Suter, 8 years ago

OT: Coincidentally today pep:0433 was posted, proposing some easier way to suppress file descriptor leaks. (For Python 3.4)

comment:9 by Ryan J Ollos <ryan.j.ollos@…>, 7 years ago

Release Notes: modified (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.