Puppet's use of tempfiles for capturing use of subprocess I/O

[Stephen Smalley]

On Fri, 2008-09-12 at 09:24 -0400, Sean E. Millichamp wrote:
> I know this is long, please be patient as I detail the situation.  There
> is a lot of Puppet-stuff initially to frame the question, but I promise
> there is an SELinux question at the end :)
> 
> A common use-case in Puppet is for it to manage your system services,
> restarting services as needed.  I noticed that when Puppet did this I
> got SELinux violations.  Since we are trying to embrace SELinux (and
> noisy logs don't help in that goal) I dug a bit deeper.
> 
> It turns out that Puppet creates a temp file in /tmp and sets the file
> descriptor for that tempfile to the stdout/stderr of the process before
> it exec()s (say) "/etc/init.d/setroubleshoot" (I've seen this happen
> with a number of different services).
> 
> audit log messages: 
> type=AVC msg=audit(1220897810.383:141): avc:  denied  { read write } for  pid=3452 comm="setroubleshootd" path="/tmp/puppet.3059.7" dev=md3 ino=6036 scontext=root:system_r:setroubleshootd_t:s0 tcontext=root:object_r:tmp_t:s0 tclass=file
> type=AVC msg=audit(1220897810.383:141): avc:  denied  { read write } for  pid=3452 comm="setroubleshootd" path="/tmp/puppet.3059.7" dev=md3 ino=6036 scontext=root:system_r:setroubleshootd_t:s0 tcontext=root:object_r:tmp_t:s0 tclass=file
> type=AVC msg=audit(1220897810.383:141): avc:  denied  { read write } for  pid=3452 comm="setroubleshootd" path="/tmp/puppet.3059.7" dev=md3 ino=6036 scontext=root:system_r:setroubleshootd_t:s0 tcontext=root:object_r:tmp_t:s0 tclass=file
> 
> Now, it seems that the domain that the init scripts transition to
> (rightly) doesn't have access to the tmp_t domain of Puppet's temporary
> file.  It seems that the two results of this are a) audit log noise and
> b) If Puppet were to want to use the output it captures then there
> wouldn't be any for confined services.
> 
> I created and submitted a patch to use Unix pipes instead of a temporary
> file for capturing the output - figuring that this was the only way sure
> to be SELinux-safe.  (See my bug report at
> http://projects.reductivelabs.com/issues/show/1563 for a link to the
> original bug, the patch, and more details.) They told me that Puppet
> used to use pipes about a year ago but that there were occasionally
> weird hanging problems where Puppet would block on IO reads forever so
> the temporary file method was adopted and they didn't want to just go
> back to a situation where Puppet might end up hanging forever on an IO
> read.  Fair enough, I can't object to that. 
> 
> I downloaded Debian Etch and successfully reproduced the originally
> reported problem with the pipes method.  Bottom line is that during the
> package install a process is started, daemonizes, but not correctly and
> never detaches from/closes stdout/stderr, causing the pipe to not close
> and flush, resulting in Puppet blocking forever on the IO read.
> 
> I have now worked out another patch to Puppet which uses non-blocking
> I/O.  This works but causes problems where the package doesn't finish
> installing properly because Puppet can't know when to finish trying to
> read from the pipe and if it closes the pipe before the package is
> finished installing then the install doesn't complete.  At this point I
> would say this is squarely a problem with the package containing the
> poorly written daemon BUT, before I make that case on the bug report
> with my new patch I want to know:
> 
> Is there a clean way of doing this using temporary files that will be
> safe for all SELinux domain transition possibilities?  Perhaps a label I
> could apply to the temporary file after creation but before the
> fork()/exec() that would be permissible in any SELinux context current
> or future?  Or some other deep Unix magic I don't know about?  I suspect
> the answer is "no", but I figure I had to ask the experts before
> declaring there was no other way in the Puppet bug report.
> 
> Thanks for sticking through reading all of this :)

puppet should run in its own domain, and the files created for output
should have their own distinct type devoted to this purpose, so that you
don't open up access to other files in /tmp unwittingly.  That can be
done via policy rules for all files created by puppet in /tmp or via
explicit calls to setfscreatecon(3) or setfilecon(3) by puppet for only
the specific output files.

With a recent kernel and a policy that enables the open_perms capability
(which I believe will be used in Fedora 10, but isn't on presently in
rawhide AFAICS), you can allow domains to inherit and use an open file
descriptor provided by the caller without allowing them to directly open
the file.  Then policy could allow all of the service domains to inherit
and use the open file descriptors to the output files while still
preventing them from opening any other output file created in /tmp by
puppet.  That is done by way of introducing an "open" permission check
on direct opens of files separate from the existing "read" and "write"
checks applied on any access of the file.
 
-- 
Stephen Smalley
National Security Agency