We currently have a problem with cp -a /media/cdrom /etc

We currently have a problem with cp -a /media/cdrom /etc

[Daniel J Walsh]

Currently in policy we do NOT have the following rule

allow iso9660_t fs_t:filesystem associate;

This causes cp -a to blow up when copying a cdrom to ext3.   I notice in 
policy we do allow this for nfs_t and dosfs_t to be assicoated with fs_t.

So this causes two problems, if we use cp -a from nfs_t or dosfs_t we 
end up with files on local disk labeled as nfs_t/dosfs_t when I believe 
we would be better off if they had transitioned.  So I could change 
policy to similarly allow iso9660_t files to be created and fix the cp 
-a problem.

Or I could remove the nfs_t and dosfs_t association and make the cp -a 
problem worse.  Since cp -a gets permission denied it really has no way 
of knowing what the correct behavior should be. 

Maybe a fix would be to allow cp to ask the kernel what to do if it can 
not setfscreatecon a particular context on a file system.

Thoughts?

Dan

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 13:46 -0500, Daniel J Walsh wrote:
> Currently in policy we do NOT have the following rule
> 
> allow iso9660_t fs_t:filesystem associate;
> 
> This causes cp -a to blow up when copying a cdrom to ext3.   I notice in 
> policy we do allow this for nfs_t and dosfs_t to be assicoated with fs_t.
> 
> So this causes two problems, if we use cp -a from nfs_t or dosfs_t we 
> end up with files on local disk labeled as nfs_t/dosfs_t when I believe 
> we would be better off if they had transitioned.  So I could change 
> policy to similarly allow iso9660_t files to be created and fix the cp 
> -a problem.
> 
> Or I could remove the nfs_t and dosfs_t association and make the cp -a 
> problem worse.  Since cp -a gets permission denied it really has no way 
> of knowing what the correct behavior should be. 
> 
> Maybe a fix would be to allow cp to ask the kernel what to do if it can 
> not setfscreatecon a particular context on a file system.
> 
> Thoughts?

First, I seem to recall debates about whether -p (and thus -a) should
automatically enable preservation of SELinux context or whether that
should remain a separate option only.  Pros and cons to both sides.

I agree that allowing the association isn't the right thing to do.  Part
of the problem here is that setfscreatecon() doesn't know where you are
going to put the file, so it cannot report an error at that time, and
the open/mkdir failure could happen for a variety of reasons, not just
this one.

cp could fall back to resetting fscreate to NULL and try again in that
case (as long as the user didn't explicitly request preservation via
--preserve=context).

What does the coreutils maintainer think?

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[James Antill]

[-- Attachment #1: Type: text/plain, Size: 2643 bytes --]

kOn Fri, 2007-01-12 at 13:55 -0500, Stephen Smalley wrote:
> On Fri, 2007-01-12 at 13:46 -0500, Daniel J Walsh wrote:
> > Currently in policy we do NOT have the following rule
> > 
> > allow iso9660_t fs_t:filesystem associate;
> > 
[...]
> > Maybe a fix would be to allow cp to ask the kernel what to do if it can 
> > not setfscreatecon a particular context on a file system.

 Note that copy.c uses fsetfilecon() on the open fd of the new file, but
it saves the context of the old file in setfscreatecon() (and so does
open+getfscreatecon+fsetfilecon). Fixing this to just store in a local
variable would mean we'd see the error at setfilecon() time.

> > Thoughts?
> 
> First, I seem to recall debates about whether -p (and thus -a) should
> automatically enable preservation of SELinux context or whether that
> should remain a separate option only.  Pros and cons to both sides.

 I never saw the original discussion, but I'm 99% sure I complained to
Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
everything, when I was adding the xattr support.
 Note that the changes in question are not really consistent:


               FC-5           FC-6            rawhide          

mode = acls[1] yes            yes             no
-p             trad.          trad.           trad.+acls+xattrs
-a             -dpPR          -dpPRc          -dpPR
--preserve=all trad.+context  trad.+context   trad.+context+acls+xattrs

...and in FC-6 -Z also does the open+getfscreatecon+fsetfilecon dance.


> I agree that allowing the association isn't the right thing to do.  Part
> of the problem here is that setfscreatecon() doesn't know where you are
> going to put the file, so it cannot report an error at that time, and
> the open/mkdir failure could happen for a variety of reasons, not just
> this one.

 So if we moved coreutils to just using fsetfilecon() for -p/-a, could
we do something?

> cp could fall back to resetting fscreate to NULL and try again in that
> case (as long as the user didn't explicitly request preservation via
> --preserve=context).

 I'm not sure that's the right behaviour, I think the behaviour should
be like if you copy an ACL to an FS that doesn't support them (Ie. it
knows what failed, and why ... and so "does the right thing").


[1] This means that preserving the "unix permission mode bits" implies
preserving any ACLs.

-- 
James Antill - <james.antill@redhat.com>
setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, ...);
setsockopt(fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, ...);
setsockopt(fd, SOL_SOCKET,  SO_ATTACH_FILTER, ...);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:
> kOn Fri, 2007-01-12 at 13:55 -0500, Stephen Smalley wrote:
> > On Fri, 2007-01-12 at 13:46 -0500, Daniel J Walsh wrote:
> > > Currently in policy we do NOT have the following rule
> > > 
> > > allow iso9660_t fs_t:filesystem associate;
> > > 
> [...]
> > > Maybe a fix would be to allow cp to ask the kernel what to do if it can 
> > > not setfscreatecon a particular context on a file system.
> 
>  Note that copy.c uses fsetfilecon() on the open fd of the new file, but
> it saves the context of the old file in setfscreatecon() (and so does
> open+getfscreatecon+fsetfilecon). Fixing this to just store in a local
> variable would mean we'd see the error at setfilecon() time.

Hmm...I'm not sure about the precise history of the coreutils selinux
patch, but I had thought it was using setfscreatecon() as much as
possible to ensure that the file was labeled correctly at creation time.
Otherwise, the file can exist in the wrong label temporarily, which can
produce "false" denials (e.g. another process tries to access the file
before it is in the expected label and gets a denial) as well as "false"
grantings (e.g. another process tries to open the file before it is in
the right label and gets a descriptor - which is mitigated in part by
the revalidation of access on read/write/... calls and fd
inheritance/transfer but revocation support is incomplete). Note also
that the permission checks are different for creating vs. relabeling, so
the behavior affects policy.

Also, I'm not sure what you mean by the above, as I would expect that it
would do one of two things for preserving context:
1) getfilecon() on the original followed by setfscreatecon() to create
the copy in the same context immediately, or
2) getfilecon() on the original followed by creation of the new file
followed by setfilecon/fsetfilecon followed by writing data to the new
file.

Then there was the separate case of -Z where a particular context was
specified by the user, in which case you could just setfscreatecon() up
front to that value.

>  I never saw the original discussion, but I'm 99% sure I complained to
> Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
> everything, when I was adding the xattr support.

I seem to recall earlier discussions where people found that
automatically enabling preservation of contexts upon -p and -a produced
a fair amount of breakage in practice, particularly under -strict
policy, because processes could very well be allowed to preserve DAC
information but not alter their own default MAC labeling behaviors.

>  Note that the changes in question are not really consistent:
> 
> 
>                FC-5           FC-6            rawhide          
> 
> mode = acls[1] yes            yes             no
> -p             trad.          trad.           trad.+acls+xattrs
> -a             -dpPR          -dpPRc          -dpPR
> --preserve=all trad.+context  trad.+context   trad.+context+acls+xattrs
> 
> ...and in FC-6 -Z also does the open+getfscreatecon+fsetfilecon dance.

Not sure what that dance is precisely.

> > I agree that allowing the association isn't the right thing to do.  Part
> > of the problem here is that setfscreatecon() doesn't know where you are
> > going to put the file, so it cannot report an error at that time, and
> > the open/mkdir failure could happen for a variety of reasons, not just
> > this one.
> 
>  So if we moved coreutils to just using fsetfilecon() for -p/-a, could
> we do something?

You don't have to do that; it just makes it clearer as to what failed.
But it does have other implications as well, as above.

> > cp could fall back to resetting fscreate to NULL and try again in that
> > case (as long as the user didn't explicitly request preservation via
> > --preserve=context).
> 
>  I'm not sure that's the right behaviour, I think the behaviour should
> be like if you copy an ACL to an FS that doesn't support them (Ie. it
> knows what failed, and why ... and so "does the right thing").

If the user/caller specifies to cp that it wants the security context
preserved, then any failure there should be fatal to the copy.  But if
the user/caller just specified -p or -a, which used to not include
security contexts, then it seems that an inability to preserve the
context shouldn't be fatal to the copy.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 15:24 -0500, Stephen Smalley wrote:
> If the user/caller specifies to cp that it wants the security context
> preserved, then any failure there should be fatal to the copy.  But if
> the user/caller just specified -p or -a, which used to not include
> security contexts, then it seems that an inability to preserve the
> context shouldn't be fatal to the copy.

An example from the MLS/LSPP world:  a Secret process can read an
Unclassified document and copy it to a Secret document, but it cannot
copy it to an Unclassified document (no write down).  But it may very
well wish to preserve DAC information on the copy.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Casey Schaufler]

--- Stephen Smalley <sds@tycho.nsa.gov> wrote:

> An example from the MLS/LSPP world:  a Secret
> process can read an
> Unclassified document and copy it to a Secret
> document, but it cannot
> copy it to an Unclassified document (no write down).
>  But it may very
> well wish to preserve DAC information on the copy.

Well, sort of. That's what actually got
implemented, but if you read the CMW spec
it's clear that what they really wanted was
the copy to remain Unclass until the Secret
user did something to it that would make it
Secret. Any difference in treatment of the
MAC attributes from the DAC atributes is an
artifact of implementation, not an intention
of design. The traditional rule that mv
maintains attributes while cp use what the
current process would do holds with (most)
MAC systems.


Casey Schaufler
casey@schaufler-ca.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Russell Coker]

On Saturday 13 January 2007 09:29, Casey Schaufler <casey@schaufler-ca.com> 
wrote:
> of design. The traditional rule that mv
> maintains attributes while cp use what the
> current process would do holds with (most)
> MAC systems.

Do "most" systems have a copy of MV that preserves MAC attributes (and 
therefore is a privileged process)?  Or are you just referring to the case of 
moving files within a filesystem?

I can imagine the benefits in having mv and cp preserve the context of files, 
and it shouldn't be difficult to implement.

-- 
russell@coker.com.au
http://etbe.blogspot.com/          My Blog

http://www.coker.com.au/sponsorship.html Sponsoring Free Software development

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Casey Schaufler]

--- Russell Coker <russell@coker.com.au> wrote:


> Do "most" systems have a copy of MV that preserves
> MAC attributes (and 
> therefore is a privileged process)?  Or are you just
> referring to the case of 
> moving files within a filesystem?

The "desired" behavior (mv retains, cp resets)
is an artifact of the behavior of the underlying
system calls used to implement the commands,
and nothing more. There has always been tension
around the behavior of mv when two file systems
are involved. Doing a link and an unlink has no
effect on the attribues of the file beyond a
brief increase in the link count, so mv "ought"
not need to worry about mode bits, ACLs, MAC
labels, or much of anything. When mv devolves
into a cp and an rm preserving the illusion of
the same file system behavior is hard,
especially when "the community" can't agree on
what attributes matter.

> I can imagine the benefits in having mv and cp
> preserve the context of files, 
> and it shouldn't be difficult to implement.

Remember the rule: mv acts as if you did rename.
cp acts as if you created a new thing with the
same contents as the old one.

As for what MLS systems have done, mv needs no
attention on the local file system and relies
on cp behavior off file system. cp makes a copy
with the user's attribute settings. Some have
tried to make mv off local attempt to do its
best to retain attributes, but always leave the
failure to do so a possibility.


Casey Schaufler
casey@schaufler-ca.com

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[James Antill]

[-- Attachment #1: Type: text/plain, Size: 3829 bytes --]

On Fri, 2007-01-12 at 15:24 -0500, Stephen Smalley wrote:
> On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:

> Hmm...I'm not sure about the precise history of the coreutils selinux
> patch, but I had thought it was using setfscreatecon() as much as
> possible to ensure that the file was labeled correctly at creation time.
> Otherwise, the file can exist in the wrong label temporarily, which can
> produce "false" denials (e.g. another process tries to access the file
> before it is in the expected label and gets a denial) as well as "false"
> grantings (e.g. another process tries to open the file before it is in
> the right label and gets a descriptor - which is mitigated in part by
> the revalidation of access on read/write/... calls and fd
> inheritance/transfer but revocation support is incomplete). Note also
> that the permission checks are different for creating vs. relabeling, so
> the behavior affects policy.
> Also, I'm not sure what you mean by the above, as I would expect that it
> would do one of two things for preserving context:

 Ok, I can see why we'd use setfscreatecon() but atm. cp does:

newcon = getfilecon("old")
setfscreatecon(newcon);
[...]
fd = open("new");
con = getfscreatecon();
fsetfilecon(fd, con);

...the later of which, I think, does nothing useful atm.

> I seem to recall earlier discussions where people found that
> automatically enabling preservation of contexts upon -p and -a produced
> a fair amount of breakage in practice, particularly under -strict
> policy, because processes could very well be allowed to preserve DAC
> information but not alter their own default MAC labeling behaviors.

 That seems likely, although I think apart from a few corner cases (like
isos and nfs) the labels should be preserved in targeted.

> >  So if we moved coreutils to just using fsetfilecon() for -p/-a, could
> > we do something?
> 
> You don't have to do that; it just makes it clearer as to what failed.
> But it does have other implications as well, as above.

 Right, but if we did this for targeted then I think it's safe to ignore
the EPERM error (maybe with a single warning). This should then do the
right thing all the time for targeted, AIUI.

> > > cp could fall back to resetting fscreate to NULL and try again in that
> > > case (as long as the user didn't explicitly request preservation via
> > > --preserve=context).
> > 
> >  I'm not sure that's the right behaviour, I think the behaviour should
> > be like if you copy an ACL to an FS that doesn't support them (Ie. it
> > knows what failed, and why ... and so "does the right thing").
> 
> If the user/caller specifies to cp that it wants the security context
> preserved, then any failure there should be fatal to the copy.  But if
> the user/caller just specified -p or -a, which used to not include
> security contexts, then it seems that an inability to preserve the
> context shouldn't be fatal to the copy.

 This implies that --preserve=context is "different" from
--preserve=all, -p or -a. I don't think upstream coreutils will accept
that, and it seems pretty magic to me.

 Maybe if we had --preserve=context-open and --preserve=context-post[1]
(usable names pending), we could have -a/-p/--preserve=all use
context-post (and that not error out). But specifying context-open would
error out.
 We could even change -p/-a if is_selinux_mls_enabled() is true?

 Of course, I'm not 100% upstream coreutils will accept that either :)

[1] context-open == use setfscreatecon and context-post = just use
fsetfilecon().

-- 
James Antill - <james.antill@redhat.com>
setsockopt(fd, IPPROTO_TCP, TCP_CONGESTION, ...);
setsockopt(fd, IPPROTO_TCP, TCP_DEFER_ACCEPT, ...);
setsockopt(fd, SOL_SOCKET,  SO_ATTACH_FILTER, ...);


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 16:15 -0500, James Antill wrote:
> On Fri, 2007-01-12 at 15:24 -0500, Stephen Smalley wrote:
> > On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:
> 
> > Hmm...I'm not sure about the precise history of the coreutils selinux
> > patch, but I had thought it was using setfscreatecon() as much as
> > possible to ensure that the file was labeled correctly at creation time.
> > Otherwise, the file can exist in the wrong label temporarily, which can
> > produce "false" denials (e.g. another process tries to access the file
> > before it is in the expected label and gets a denial) as well as "false"
> > grantings (e.g. another process tries to open the file before it is in
> > the right label and gets a descriptor - which is mitigated in part by
> > the revalidation of access on read/write/... calls and fd
> > inheritance/transfer but revocation support is incomplete). Note also
> > that the permission checks are different for creating vs. relabeling, so
> > the behavior affects policy.
> > Also, I'm not sure what you mean by the above, as I would expect that it
> > would do one of two things for preserving context:
> 
>  Ok, I can see why we'd use setfscreatecon() but atm. cp does:
> 
> newcon = getfilecon("old")
> setfscreatecon(newcon);
> [...]
> fd = open("new");
> con = getfscreatecon();
> fsetfilecon(fd, con);
> 
> ...the later of which, I think, does nothing useful atm.

Yes, that appears to be completely redundant.  Please post proposed
changes to the selinux userland functionality to the list for
discussion.  vixie cron selinux support was completely rewritten without
ever being discussed here, yielding broken support, and it looks like
the openssh selinux support has been significantly reworked for LSPP
without ever appearing here.  We aren't gatekeepers for them anymore,
but we can provide useful feedback...

> > I seem to recall earlier discussions where people found that
> > automatically enabling preservation of contexts upon -p and -a produced
> > a fair amount of breakage in practice, particularly under -strict
> > policy, because processes could very well be allowed to preserve DAC
> > information but not alter their own default MAC labeling behaviors.
> 
>  That seems likely, although I think apart from a few corner cases (like
> isos and nfs) the labels should be preserved in targeted.

Then you need graceful fallback if denied so that it still works sanely
under -strict or -mls.

> > >  So if we moved coreutils to just using fsetfilecon() for -p/-a, could
> > > we do something?
> > 
> > You don't have to do that; it just makes it clearer as to what failed.
> > But it does have other implications as well, as above.
> 
>  Right, but if we did this for targeted then I think it's safe to ignore
> the EPERM error (maybe with a single warning). This should then do the
> right thing all the time for targeted, AIUI.

Ignoring it is fine if the user didn't request preservation of contexts
specifically, but if the user did specify that, it should be fatal.

> > If the user/caller specifies to cp that it wants the security context
> > preserved, then any failure there should be fatal to the copy.  But if
> > the user/caller just specified -p or -a, which used to not include
> > security contexts, then it seems that an inability to preserve the
> > context shouldn't be fatal to the copy.
> 
>  This implies that --preserve=context is "different" from
> --preserve=all, -p or -a. I don't think upstream coreutils will accept
> that, and it seems pretty magic to me.

-p is documented as "same as --preserve=mode,ownership,timestamps".
-a is documented as "same as -dpR"
So _not_ preserving context is completely consistent with the
documentation for those options.  --preserve=all might be reasonable to
enable context preservation; I don't know how often it is used.

>  Maybe if we had --preserve=context-open and --preserve=context-post[1]
> (usable names pending), we could have -a/-p/--preserve=all use
> context-post (and that not error out). But specifying context-open would
> error out.
>  We could even change -p/-a if is_selinux_mls_enabled() is true?
> 
>  Of course, I'm not 100% upstream coreutils will accept that either :)
> 
> [1] context-open == use setfscreatecon and context-post = just use
> fsetfilecon().

I'd prefer just a single mechanism and policy-independent behavior.
And _stable_ behavior.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Jim Meyering]

James Antill <james.antill@redhat.com> wrote:
> On Fri, 2007-01-12 at 15:24 -0500, Stephen Smalley wrote:
>> On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:
>
>> Hmm...I'm not sure about the precise history of the coreutils selinux
>> patch, but I had thought it was using setfscreatecon() as much as
>> possible to ensure that the file was labeled correctly at creation time.
>> Otherwise, the file can exist in the wrong label temporarily, which can
>> produce "false" denials (e.g. another process tries to access the file
>> before it is in the expected label and gets a denial) as well as "false"
>> grantings (e.g. another process tries to open the file before it is in
>> the right label and gets a descriptor - which is mitigated in part by
>> the revalidation of access on read/write/... calls and fd
>> inheritance/transfer but revocation support is incomplete). Note also
>> that the permission checks are different for creating vs. relabeling, so
>> the behavior affects policy.
>> Also, I'm not sure what you mean by the above, as I would expect that it
>> would do one of two things for preserving context:
>
>  Ok, I can see why we'd use setfscreatecon() but atm. cp does:
>
> newcon = getfilecon("old")
> setfscreatecon(newcon);
> [...]
> fd = open("new");
> con = getfscreatecon();
> fsetfilecon(fd, con);
>
> ...the later of which, I think, does nothing useful atm.

Isn't that fsetfilecon call useful, since your example must
also handle an existing destination file?
It's easier to see with a little more context, and knowing that
*new_dst is set when the destination file is guaranteed not to exist.
Here's a snippet from coreutils-6.7 + Red Hat's coreutils-selinux.patch,
+ my // comment:

  if ( ! *new_dst)
    {
      // DST_NAME *may* already exist
      dest_desc = open (dst_name, O_WRONLY | O_TRUNC | O_BINARY, dst_mode);

#ifdef WITH_SELINUX
      if (dest_desc >= 0 && selinux_enabled &&
	  (x->preserve_security_context || x->set_security_context))
	{
	  security_context_t con;
	  if(getfscreatecon(&con) == -1)
	    {
	      return_val = false;
	      goto close_src_desc;
	    }

	  if (con)
	    {
	      if(fsetfilecon(dest_desc, con) == -1)
		{
		  return_val = false;
		  freecon(con);
		  goto close_src_desc;
		}
	      freecon(con);
	    }
	}
#endif

This is a good opportunity to ask about an old note I made:
  - is it worthwhile to check for getfscreatecon failure?
    The documentation says it can fail, but not how.
    Even it's truly a "can't happen" condition now, that might change.

I presume the lack of a diagnostic for failed fsetfilecon
is just an oversight.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[James Antill]

[-- Attachment #1: Type: text/plain, Size: 1268 bytes --]

On Sat, 2007-01-13 at 11:05 +0100, Jim Meyering wrote:

> Isn't that fsetfilecon call useful, since your example must
> also handle an existing destination file?

 Ahh, I see. I'd somehow missed the if check before the ifdef, or not
parsed what it meant.

 Note that this does mean that:

% echo me > uid
% sudo touch ouid_r ouid_w
% sudo chmod o+w ouid_w
% cp -a uid ouid_r
cp: cannot create regular file `ouid_r': Permission denied
zsh: 25994 exit 1     cp -a uid ouid_r
% cp -a uid ouid_w
zsh: 25995 exit 1     cp -a uid ouid_w


> This is a good opportunity to ask about an old note I made:
>   - is it worthwhile to check for getfscreatecon failure?
>     The documentation says it can fail, but not how.
>     Even it's truly a "can't happen" condition now, that might change.

 IIRC it:

 open's /proc/self/task/$$/attr/fscreate
 reads the data
 malloc's the correct length for the result and copies.

...either of these three operations can fail.

> I presume the lack of a diagnostic for failed fsetfilecon
> is just an oversight.

 Yeh, probably the test case above wasn't obvious so it was hard to
test.


 So any ideas on how to make cp do the right thing in all cases ?:)

-- 
James Antill <jantill@redhat.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: We currently have a problem with cp -a /media/cdrom /etc

[Jim Meyering]

James Antill <jantill@redhat.com> wrote:
> On Sat, 2007-01-13 at 11:05 +0100, Jim Meyering wrote:
>
>> Isn't that fsetfilecon call useful, since your example must
>> also handle an existing destination file?
>
>  Ahh, I see. I'd somehow missed the if check before the ifdef, or not
> parsed what it meant.
>
>  Note that this does mean that:
>
> % echo me > uid
> % sudo touch ouid_r ouid_w
> % sudo chmod o+w ouid_w
> % cp -a uid ouid_r
> cp: cannot create regular file `ouid_r': Permission denied
> zsh: 25994 exit 1     cp -a uid ouid_r
> % cp -a uid ouid_w
> zsh: 25995 exit 1     cp -a uid ouid_w

Thanks for the example.  I'll make that into a regression test.
I need a lot more of those for the SELinux-related changes.

That demonstrates two problems in the Red Hat version of cp.
Your first copy failed (as expected), because open-unreadable fails,
The second failed (#1: but with no diagnostic!) due to failure
to propagate the security context to the destination.  But imho, #2:
cp -a must not try to apply any security context to the destination.

Over the weekend I nearly completed my own merge, and tried that.
The latter "cp -a" still fails, but here because it fails to propagate
the time stamps to a file not owned by me:

  $ cp -a uid ouid_r
  cp: cannot create regular file `ouid_r': Permission denied
  [Exit 1]
  $ cp -a uid ouid_w
  cp: preserving times for `ouid_w': Operation not permitted
  [Exit 1]

With a little luck, I'll check it in and push today.

>> This is a good opportunity to ask about an old note I made:
>>   - is it worthwhile to check for getfscreatecon failure?
>>     The documentation says it can fail, but not how.
>>     Even it's truly a "can't happen" condition now, that might change.
>
>  IIRC it:
>
>  open's /proc/self/task/$$/attr/fscreate
>  reads the data
>  malloc's the correct length for the result and copies.
>
> ...either of these three operations can fail.

Thanks.  I added code to detect the failures and to give diagnostics.

>  So any ideas on how to make cp do the right thing in all cases ?:)

Can you give more examples of how it doesn't work?

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Mon, 2007-01-15 at 08:54 +0100, Jim Meyering wrote:
> James Antill <jantill@redhat.com> wrote:
> > On Sat, 2007-01-13 at 11:05 +0100, Jim Meyering wrote:
> >
> >> Isn't that fsetfilecon call useful, since your example must
> >> also handle an existing destination file?
> >
> >  Ahh, I see. I'd somehow missed the if check before the ifdef, or not
> > parsed what it meant.
> >
> >  Note that this does mean that:
> >
> > % echo me > uid
> > % sudo touch ouid_r ouid_w
> > % sudo chmod o+w ouid_w
> > % cp -a uid ouid_r
> > cp: cannot create regular file `ouid_r': Permission denied
> > zsh: 25994 exit 1     cp -a uid ouid_r
> > % cp -a uid ouid_w
> > zsh: 25995 exit 1     cp -a uid ouid_w
> 
> Thanks for the example.  I'll make that into a regression test.
> I need a lot more of those for the SELinux-related changes.
> 
> That demonstrates two problems in the Red Hat version of cp.
> Your first copy failed (as expected), because open-unreadable fails,
> The second failed (#1: but with no diagnostic!) due to failure
> to propagate the security context to the destination.  But imho, #2:
> cp -a must not try to apply any security context to the destination.
> 
> Over the weekend I nearly completed my own merge, and tried that.
> The latter "cp -a" still fails, but here because it fails to propagate
> the time stamps to a file not owned by me:
> 
>   $ cp -a uid ouid_r
>   cp: cannot create regular file `ouid_r': Permission denied
>   [Exit 1]
>   $ cp -a uid ouid_w
>   cp: preserving times for `ouid_w': Operation not permitted
>   [Exit 1]
> 
> With a little luck, I'll check it in and push today.
> 
> >> This is a good opportunity to ask about an old note I made:
> >>   - is it worthwhile to check for getfscreatecon failure?
> >>     The documentation says it can fail, but not how.
> >>     Even it's truly a "can't happen" condition now, that might change.
> >
> >  IIRC it:
> >
> >  open's /proc/self/task/$$/attr/fscreate
> >  reads the data
> >  malloc's the correct length for the result and copies.
> >
> > ...either of these three operations can fail.
> 
> Thanks.  I added code to detect the failures and to give diagnostics.
> 
> >  So any ideas on how to make cp do the right thing in all cases ?:)
> 
> Can you give more examples of how it doesn't work?

I'm not certain what the current state of cp is, but I think that we
want cp -a (or -p) to try to preserve security context but to gracefully
proceed (i.e. setfscreatecon(NULL) + retry if the open/mkdir/... failed
or ignore the failed fsetfilecon()) if that is not permitted, whereas we
want cp -c (or --preserve=context or --preserve=all) to treat such
errors as a fatal error.  That will enable contexts to be preserved when
permitted upon ordinary cp -a/-p copying, which is useful, while not
causing breakage when it is not permitted due to a mismatch between the
source and destination (e.g. cp -a /media/cdrom /etc) or due to
restrictions on the copying process (e.g. Secret process cannot
create/write to an Unclassified file but may read one and write it to a
Secret file; Unprivileged user process cannot create/write a system
file, but may read one and write a local copy).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Russell Coker]

On Saturday 13 January 2007 08:15, James Antill <james.antill@redhat.com> 
wrote:
> newcon = getfilecon("old")
> setfscreatecon(newcon);
> [...]
> fd = open("new");
> con = getfscreatecon();
> fsetfilecon(fd, con);
>
> ...the later of which, I think, does nothing useful atm.

touch /tmp/foo
cp -Z CONTEXT /tmp/bar /tmp/foo

Some time ago I reported a bug in cp -Z whereby setfscreatecon() was used but 
it didn't work when the file existed.  The above commands would not give the 
desired result for example.

I haven't looked at the cp source recently, might there be some similar corner 
case prevented by this double set?

-- 
russell@coker.com.au
http://etbe.blogspot.com/          My Blog

http://www.coker.com.au/sponsorship.html Sponsoring Free Software development

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:
>  I never saw the original discussion, but I'm 99% sure I complained to
> Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
> everything, when I was adding the xattr support.
>  Note that the changes in question are not really consistent:
> 
> 
>                FC-5           FC-6            rawhide          
> 
> mode = acls[1] yes            yes             no
> -p             trad.          trad.           trad.+acls+xattrs
> -a             -dpPR          -dpPRc          -dpPR
> --preserve=all trad.+context  trad.+context   trad.+context+acls+xattrs
> 
> ...and in FC-6 -Z also does the open+getfscreatecon+fsetfilecon dance.

Polite request:  When changes to the selinux functionality in packages
like coreutils are being proposed, please bring them to this list for
discussion so that we can (hopefully) catch these kinds of issues early.

Would it help to host the current selinux userland patches on the
sourceforge selinux project?  Digging a canonical set out of the Fedora
CVS might take a fair amount of work as one would need to coalesce the
multiple separate selinux-related patches that exist for some packages
and re-base them to a clean upstream package if possible.
 
-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Jim Meyering]

Stephen Smalley <sds@tycho.nsa.gov> wrote:

> On Fri, 2007-01-12 at 15:10 -0500, James Antill wrote:
>>  I never saw the original discussion, but I'm 99% sure I complained to
>> Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
>> everything, when I was adding the xattr support.
>>  Note that the changes in question are not really consistent:
>>
>>
>>                FC-5           FC-6            rawhide
>>
>> mode = acls[1] yes            yes             no
>> -p             trad.          trad.           trad.+acls+xattrs
>> -a             -dpPR          -dpPRc          -dpPR
>> --preserve=all trad.+context  trad.+context   trad.+context+acls+xattrs
>>
>> ...and in FC-6 -Z also does the open+getfscreatecon+fsetfilecon dance.
>
> Polite request:  When changes to the selinux functionality in packages
> like coreutils are being proposed, please bring them to this list for
> discussion so that we can (hopefully) catch these kinds of issues early.

Of course.

> Would it help to host the current selinux userland patches on the
> sourceforge selinux project?  Digging a canonical set out of the Fedora
> CVS might take a fair amount of work as one would need to coalesce the
> multiple separate selinux-related patches that exist for some packages
> and re-base them to a clean upstream package if possible.

I've done most of that merge for coreutils (but may lack some recent
patches) and the result will soon appear on a topic branch of the
upstream coreutils git repository:

  http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=summary

I've done this work independently of the coreutils-6.7 rawhide merge
that someone (Tim Waugh?) must have done recently, so it will be a good
cross-check to compare when I'm done.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Jim Meyering]

James Antill <james.antill@redhat.com> wrote:
> On Fri, 2007-01-12 at 13:55 -0500, Stephen Smalley wrote:
>> On Fri, 2007-01-12 at 13:46 -0500, Daniel J Walsh wrote:

Thanks for the Cc.

...
>> First, I seem to recall debates about whether -p (and thus -a) should
>> automatically enable preservation of SELinux context or whether that
>> should remain a separate option only.  Pros and cons to both sides.
>
>  I never saw the original discussion, but I'm 99% sure I complained to
> Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
> everything, when I was adding the xattr support.

If only "the right thing" worked in all cases.
FWIW, my cut at this[*] has cp -a still equivalent to cp -dpPR,
just like rawhide, below.

Do you know what motivated the "mode = acls[1] no" change below?
That introduces a new incompatibility with upstream.
Otherwise, upstream matches rawhide.

[*] I resumed work on my coreutils selinux merge recently, so you should
hear more about it soon -- on the bug-coreutils and bug-gnulib lists.

>  Note that the changes in question are not really consistent:
>
>                FC-5           FC-6            rawhide
>
> mode = acls[1] yes            yes             no
> -p             trad.          trad.           trad.+acls+xattrs
> -a             -dpPR          -dpPRc          -dpPR
> --preserve=all trad.+context  trad.+context   trad.+context+acls+xattrs
...
> [1] This means that preserving the "unix permission mode bits" implies
> preserving any ACLs.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 22:19 +0100, Jim Meyering wrote:
> James Antill <james.antill@redhat.com> wrote:
> > On Fri, 2007-01-12 at 13:55 -0500, Stephen Smalley wrote:
> >> On Fri, 2007-01-12 at 13:46 -0500, Daniel J Walsh wrote:
> 
> Thanks for the Cc.
> 
> ...
> >> First, I seem to recall debates about whether -p (and thus -a) should
> >> automatically enable preservation of SELinux context or whether that
> >> should remain a separate option only.  Pros and cons to both sides.
> >
> >  I never saw the original discussion, but I'm 99% sure I complained to
> > Jim and/or Dan that cp -a/-p should do "the right thing" and preserve
> > everything, when I was adding the xattr support.
> 
> If only "the right thing" worked in all cases.
> FWIW, my cut at this[*] has cp -a still equivalent to cp -dpPR,
> just like rawhide, below.
> 
> Do you know what motivated the "mode = acls[1] no" change below?
> That introduces a new incompatibility with upstream.
> Otherwise, upstream matches rawhide.
> 
> [*] I resumed work on my coreutils selinux merge recently, so you should
> hear more about it soon -- on the bug-coreutils and bug-gnulib lists.

It would be nice to have the actual diffs posted here.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Jim Meyering]

Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> [*] I resumed work on my coreutils selinux merge recently, so you should
>> hear more about it soon -- on the bug-coreutils and bug-gnulib lists.
>
> It would be nice to have the actual diffs posted here.

It won't be hard to remember to Cc this list :)

BTW, last week I thought of you guys (with a smile :-) when I noticed new
test failures and discovered that getfilecon and lgetfilecon now return
length upon success -- at least with rawhide, whereas they returned 0
a few months ago.  That shook out a few tests that were checking for
nonzero rather that negative.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: We currently have a problem with cp -a /media/cdrom /etc

[Stephen Smalley]

On Fri, 2007-01-12 at 22:47 +0100, Jim Meyering wrote:
> Stephen Smalley <sds@tycho.nsa.gov> wrote:
> >> [*] I resumed work on my coreutils selinux merge recently, so you should
> >> hear more about it soon -- on the bug-coreutils and bug-gnulib lists.
> >
> > It would be nice to have the actual diffs posted here.
> 
> It won't be hard to remember to Cc this list :)
> 
> BTW, last week I thought of you guys (with a smile :-) when I noticed new
> test failures and discovered that getfilecon and lgetfilecon now return
> length upon success -- at least with rawhide, whereas they returned 0
> a few months ago.  That shook out a few tests that were checking for
> nonzero rather that negative.

Returning the length was the original (and documented) behavior; the
returning 0 was a bug introduced by the MLS/MCS context translation
support in Fedora.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.