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

[Sean E. Millichamp]

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 :)

Sean

[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

[Sean E. Millichamp]

On Fri, 2008-09-12 at 09:43 -0400, Stephen Smalley wrote:

>  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. 
Hi Stephen, thanks for your reply.

Well, as I understand it, putting Puppet in its own domain and labeling
the /tmp files so Puppet can only read them and not other files in /tmp
would certainly be a good thing, but doesn't address my problem.  I'm
just starting to spend time interacting with SELinux so if I am
completely misunderstanding something please be patient.

My problem (in this case) isn't that I want to confine Puppet (that is a
different project for a different day - maybe), it is that those /tmp
files Puppet creates and attaches to arbitrary process STDOUT/STDERR
streams have to be writable by any process in any domain.  Any
service/command you would run on the command line should be available to
an admin via Puppet, but in this case instead of sending their output to
a tty they are sending it to a file.

Basically, I want to be able to do this:
 - create the temporary file
 - chcon the temporary file to allow_all_domains_to_write_to_me_t
 - attach the files to stdout/stderr and exec whatever the command is
 - regardless of any policy on the command, it should be able to write
to allow_all_domains_to_write_to_me_t

I know that having a context like "allow_all_domains_to_write_to_me_t"
is probably against the spirit of SELinux, but if such a file context
exists it would solve my problem.

>  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. 
This sounds like exactly what I need, except unfortunately I need
something that will work on existing and older distributions.  Is there
anyway I can simulate that behavior now with existing SELinux
implementations?

Sean

[Stephen Smalley]

On Fri, 2008-09-12 at 11:58 -0400, Sean E. Millichamp wrote:
>  On Fri, 2008-09-12 at 09:43 -0400, Stephen Smalley wrote:
>  
>  > 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.
>  
>  Hi Stephen, thanks for your reply.
>  
>  Well, as I understand it, putting Puppet in its own domain and labeling
>  the /tmp files so Puppet can only read them and not other files in /tmp
>  would certainly be a good thing, but doesn't address my problem. 
That isn't what I meant.  I said to put puppet in its domain so that the
policy rules can define a type for files it creates in /tmp that are
different than the type used by any other process, and then we can allow
all service domains to read that new type created only by puppet w/o
exposing the temporary files of any other process to such access.  See
the difference?  What domain does puppet run in presently, initrc_t?

>    I'm
>  just starting to spend time interacting with SELinux so if I am
>  completely misunderstanding something please be patient.
>  
>  My problem (in this case) isn't that I want to confine Puppet (that is a
>  different project for a different day - maybe), it is that those /tmp
>  files Puppet creates and attaches to arbitrary process STDOUT/STDERR
>  streams have to be writable by any process in any domain. 
Precisely - which means they need their own type.  And the easiest way
to ensure that goal is to put puppet into its own domain and define a
file type transition from that domain on tmp_t:dir such that any /tmp
files created by puppet get that type automatically.

>    Any
>  service/command you would run on the command line should be available to
>  an admin via Puppet, but in this case instead of sending their output to
>  a tty they are sending it to a file.
>  
>  Basically, I want to be able to do this:
>   - create the temporary file
>   - chcon the temporary file to allow_all_domains_to_write_to_me_t 
This step becomes unnecessary if we put puppet into its own domain and
define a file type transition to a new type, say puppet_tmp_t when
creating files in /tmp, and then the puppet policy can say "allow domain
puppet_tmp_t:file { read write getattr append };"

>  This sounds like exactly what I need, except unfortunately I need
>  something that will work on existing and older distributions.  Is there
>  anyway I can simulate that behavior now with existing SELinux
>  implementations? 
The approach above will work for existing distributions but will allow
the service domains to potentially open other files created by puppet
in /tmp as well (but not open arbitrary /tmp files created by other
processes).  Then in newer distributions where the new open permission
is enabled in policy, the service domains will not be able to open other
files created by puppet in /tmp other than the one handed to them due to
the checking of the new open permission.

-- 
Stephen Smalley
National Security Agency

[Sean E. Millichamp]

On Fri, 2008-09-12 at 13:33 -0400, Stephen Smalley wrote:

>  That isn't what I meant.  I said to put puppet in its domain so
> that the
>  policy rules can define a type for files it creates in /tmp that are
>  different than the type used by any other process, and then we can allow
>  all service domains to read that new type created only by puppet w/o
>  exposing the temporary files of any other process to such access.  See
>  the difference?  What domain does puppet run in presently, initrc_t? 
Ah, okay.  Now I get it.  I didn't realize/understand that putting it in
its own domain would provide a route to do that.  Puppet runs in
initrc_t if started via /etc/init.d/puppet and in unconfined_t if run as
puppetd from the command line (which I frequently do for testing new
configs).

>  This step becomes unnecessary if we put puppet into its own domain
> and
>  define a file type transition to a new type, say puppet_tmp_t when
>  creating files in /tmp, and then the puppet policy can say "allow domain
>  puppet_tmp_t:file { read write getattr append };" 
Okay, I think it starting to make sense to me now.

Between your explanation and Dan's sample policy and explanation I think
I am starting to understand what is needed.

So, to clarify, if I create the new puppet domain definition and policy
correctly I theoretically won't even need to modify a line of Puppet
code itself?  It seems I have some more learning to do :)

I think I am going to try this approach and see if I can come up with a
policy that will cover a domain transition and the required labeling.

>  The approach above will work for existing distributions but will
> allow
>  the service domains to potentially open other files created by puppet
>  in /tmp as well (but not open arbitrary /tmp files created by other
>  processes).  Then in newer distributions where the new open permission
>  is enabled in policy, the service domains will not be able to open
>  other files created by puppet in /tmp other than the one handed to
>  them due to the checking of the new open permission. 
Good point.

Thanks!

Sean

[Stephen Smalley]

On Fri, 2008-09-12 at 14:16 -0400, Sean E. Millichamp wrote:
>  Between your explanation and Dan's sample policy and explanation
> I think
>  I am starting to understand what is needed.
>  
>  So, to clarify, if I create the new puppet domain definition and policy
>  correctly I theoretically won't even need to modify a line of Puppet
>  code itself?  It seems I have some more learning to do :) 
Yes.  Something along the lines of:
policy_module(puppet, 1.0)
type puppet_t;
type puppet_exec_t;
domain_type(puppet_t)
init_daemon_domain(puppet_t, puppet_exec_t)
role system_r types puppet_t;
type puppet_tmp_t;
files_tmp_file(puppet_tmp_t)
files_tmp_filetrans(puppet_t, puppet_tmp_t, file)

should get you started.  And if your goal is to leave puppet completely
unrestricted, you can always add a:
optional_policy(`
	unconfined_domain(puppet_t)
')
to leave it unrestricted in its own actions by SELinux.

>  I think I am going to try this approach and see if I can come up with
> a
>  policy that will cover a domain transition and the required labeling. 
-- 
Stephen Smalley
National Security Agency

[Daniel J Walsh]

Sean E. Millichamp wrote:
>  On Fri, 2008-09-12 at 13:33 -0400, Stephen Smalley wrote:
>  
> > That isn't what I meant.  I said to put puppet in its domain so that the
> > policy rules can define a type for files it creates in /tmp that are
> > different than the type used by any other process, and then we can allow
> > all service domains to read that new type created only by puppet w/o
> > exposing the temporary files of any other process to such access.  See
> > the difference?  What domain does puppet run in presently, initrc_t?
>  
>  Ah, okay.  Now I get it.  I didn't realize/understand that putting it in
>  its own domain would provide a route to do that.  Puppet runs in
>  initrc_t if started via /etc/init.d/puppet and in unconfined_t if run as
>  puppetd from the command line (which I frequently do for testing new
>  configs).
>  
> > This step becomes unnecessary if we put puppet into its own domain and
> > define a file type transition to a new type, say puppet_tmp_t when
> > creating files in /tmp, and then the puppet policy can say "allow domain
> > puppet_tmp_t:file { read write getattr append };"
>  
>  Okay, I think it starting to make sense to me now.
>  
>  Between your explanation and Dan's sample policy and explanation I think
>  I am starting to understand what is needed.
>  
>  So, to clarify, if I create the new puppet domain definition and policy
>  correctly I theoretically won't even need to modify a line of Puppet
>  code itself?  It seems I have some more learning to do :)
>  
>  I think I am going to try this approach and see if I can come up with a
>  policy that will cover a domain transition and the required labeling.
>  
> > The approach above will work for existing distributions but will allow
> > the service domains to potentially open other files created by puppet
> > in /tmp as well (but not open arbitrary /tmp files created by other
> > processes).  Then in newer distributions where the new open permission
> > is enabled in policy, the service domains will not be able to open
> > other files created by puppet in /tmp other than the one handed to
> > them due to the checking of the new open permission.
>  
>  Good point.
>  
>  Thanks!
>  
>  Sean
>   
Well puppet has problems when it installs files, that have been reported
upstream.  Basically it needs to ask the system what the label of a file
it puts on disk and make sure it is correct.  I wrote ruby bindings
during the summer for this purpose and hopefully an updated version of
puppet will be available soon.  Currently Fedora Infrastructure team is
using restorecond to try to maintain the labels of files provided via
puppet.

[Daniel J Walsh]

Sean E. Millichamp wrote:
>  On Fri, 2008-09-12 at 09:43 -0400, Stephen Smalley wrote:
>  
> > 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.
>  
>  Hi Stephen, thanks for your reply.
>  
>  Well, as I understand it, putting Puppet in its own domain and labeling
>  the /tmp files so Puppet can only read them and not other files in /tmp
>  would certainly be a good thing, but doesn't address my problem.  I'm
>  just starting to spend time interacting with SELinux so if I am
>  completely misunderstanding something please be patient.
>  
>  My problem (in this case) isn't that I want to confine Puppet (that is a
>  different project for a different day - maybe), it is that those /tmp
>  files Puppet creates and attaches to arbitrary process STDOUT/STDERR
>  streams have to be writable by any process in any domain.  Any
>  service/command you would run on the command line should be available to
>  an admin via Puppet, but in this case instead of sending their output to
>  a tty they are sending it to a file.
>  
>  Basically, I want to be able to do this:
>   - create the temporary file
>   - chcon the temporary file to allow_all_domains_to_write_to_me_t
>   - attach the files to stdout/stderr and exec whatever the command is
>   - regardless of any policy on the command, it should be able to write
>  to allow_all_domains_to_write_to_me_t
>  
>  I know that having a context like "allow_all_domains_to_write_to_me_t"
>  is probably against the spirit of SELinux, but if such a file context
>  exists it would solve my problem.
>  
> > 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.
>  
>  This sounds like exactly what I need, except unfortunately I need
>  something that will work on existing and older distributions.  Is there
>  anyway I can simulate that behavior now with existing SELinux
>  implementations?
>  
>  Sean
>  
>   
Right and you would

allow domain puppet_tmp_t:file rw_file_perms;

Which would allow every process on the system to read/write these files.

Of course I would suggest that you not use /tmp for this activity since
/tmp is really a USER resource and not a System resource.  You should
never create files by privileged processes in /tmp/ they should be
created in /var/run/puppet or /var/log/puppet.

http://danwalsh.livejournal.com/11467.html



You can generate a policy

# cat puppetout.te

policy_module(puppetout, 1.0)

gen_require(`
	attribute domain;
')

type puppet_log_t;
files_type(puppet_log_t)

allow domain puppet_log_t:file rw_file_perms;


# make -f /usr/share/selinux/devel/Makefile
# semodule -i puppet.pp
# touch /var/run/puppet.log
# chcon -t puppet_log_t /var/log/puppet.log

Go to town.

[Sean E. Millichamp]

On Fri, 2008-09-12 at 13:35 -0400, Daniel J Walsh wrote:

>  Of course I would suggest that you not use /tmp for this activity
> since
>  /tmp is really a USER resource and not a System resource.  You should
>  never create files by privileged processes in /tmp/ they should be
>  created in /var/run/puppet or /var/log/puppet.
>  
>  http://danwalsh.livejournal.com/11467.html 
Hi Dan,

Thanks for chiming in and providing the example policy.

I have been so focused on the file labeling and errors I hadn't even
stopped to consider the location :).  Puppet currently uses the Ruby
Tempfile class without specifying a tmpdir and defaults to /tmp as the
Ruby built-in default.  I might take a stab at adding a configuration
setting for that and defaulting it someplace else.

Excellent idea, thanks!

Sean