Thanks Serge, Cai, Kamalesh & Stephen.
Regards--
Subrata
On Thu, 2009-01-29 at 15:51 +0530, Kamalesh Babulal wrote:
> * Serge E. Hallyn <se...@us.ibm.com> [2009-01-28 11:13:05]:
>
> > Quoting CAI Qian (caiq...@cclom.cn):
> > > Hi,
> > >
> > >
> > > --- On Wed, 1/28/09, Serge E. Hallyn <se...@us.ibm.com> wrote:
> > >
> > > > From: Serge E. Hallyn <se...@us.ibm.com>
> > > > Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface -
> > > > version 2
> > > > To: "CAI Qian" <caiq...@cclom.cn>
> > > > Cc: "Kamalesh Babulal" <kamal...@linux.vnet.ibm.com>,
> > > > ltp-l...@lists.sf.net, s...@tycho.nsa.gov, subr...@linux.vnet.ibm.com,
> > > > aar...@linux.vnet.ibm.com
> > > > Date: Wednesday, January 28, 2009, 11:50 PM
> > > > Quoting CAI Qian (caiq...@cclom.cn):
> > > > > Hi,
> > > > >
> > > > >
> > > > > --- On Wed, 1/28/09, Serge E. Hallyn
> > > > <se...@us.ibm.com> wrote:
> > > > >
> > > > > > From: Serge E. Hallyn <se...@us.ibm.com>
> > > > > > Subject: Re: [LTP] [PATCH] proc01: SELinux with
> > > > attr/* Interface - version 2
> > > > > > To: "CAI Qian" <caiq...@cclom.cn>
> > > > > > Cc: "Kamalesh Babulal"
> > > > <kamal...@linux.vnet.ibm.com>, ltp-l...@lists.sf.net,
> > > > s...@tycho.nsa.gov, subr...@linux.vnet.ibm.com,
> > > > aar...@linux.vnet.ibm.com
> > > > > > Date: Wednesday, January 28, 2009, 10:57 PM
> > > > > > Quoting CAI Qian (caiq...@cclom.cn):
> > > > > > > Kamalesh Babulal, well, my approach is that
> > > > anyone who
> > > > > > cares about
> > > > > > > AppArmor can add a list of files should work
> > > > to the
> > > > > > code. it is fair that if different LSMs behave
> > > > differently,
> > > > > > we'll need different lists
> > > > > > > (selinux_should_work and
> > > > apparmor_should_work) to deal
> > > > > > with them.
> > > > > > >
> > > > > > > > To make it
> > > > > > > > generic can we
> > > > > > > > just skip reading the list of files, if
> > > > they
> > > > > > return EINVAL
> > > > > > > > or else we
> > > > > > > > have to support checking of different
> > > > LSM's
> > > > > > and add
> > > > > > > > support for each of
> > > > > > > > them individually.
> > > > > > > >
> > > > > > >
> > > > > > > Yes, but then you will still need to treat
> > > > different
> > > > > > LSMs differently.
> > > > > > >
> > > > > > > > Agree that the coverage of the
> > > > testcase is going
> > > > > > to be
> > > > > > > > reduced. It will be
> > > > > > > > reduced more because the list which we
> > > > are taking
> > > > > > care is
> > > > > > > > incomplete,
> > > > > > >
> > > > > > > Which ones are missing -- should return
> > > > EINVAL with
> > > > > > SELinux
> > > > > > > disabled?
> > > > > > >
> > > > > > > > we could need to add other files to the
> > > > list like
> > > > > > nfs to be
> > > > > > > > skipped.
> > > > > > > > Sending another patch which will ignore
> > > > the file
> > > > > > if it
> > > > > > > > returns EINVAL or else
> > > > > > > > throw warning.
> > > > > > >
> > > > > > > This patch won't able to catch attr/*
>
> Sorry send the wrong patch
>
> > > > entries
> > > > > > return
> > > > > > > EINVAL while SELinux is enabled. It does not
> > > > look like
> > > > > > a good
> > > > > > > approach to me, because it is a test
> > > > coverage
> > > > > > regression.
> > > > > > >
> > > > > > > CAI Qian
> > > > > >
> > > > > > So, just to try and think through this... If no
> > > > LSM is
> > > > > > enabled,
> > > > > > the files should return -EINVAL. If they
> > > > don't return
> > > > > > -EINVAL,
> > > > > > is that a situation we care about? What would it
> > > > mean?
> > > > > >
> > > > >
> > > > > Yes, Stephen Smalley from National Security Agency of
> > > > U.S. told it
> > > > > means "security modules (e.g. capability)
> > > > don't support any of those
> > > > > interfaces", so if another errno is returned, it
> > > > should be brought up
> > > > > to attention.
> > > >
> > > > Obviously the correct behavior depends upon the security
> > > > subsystem,
> > > > but you're finagling the checks into fs-specific
> > > > checks.
> > > >
> > > > It seems to me that if you're going to check for
> > > > correct return
> > > > values from these functions, you should do so under
> > > > testcases/kernel/security.
> > >
> > > Well, it is fine to put some of those checks under another separate
> > > test, but it looks to me that it is probably easier and simpler here
> > > to consider those two tightly coupled component all together in this
> > > case. Because the test should not simple blindly skip those entries
> > > due to the reasons I have stated before, and people may run this test
> > > with SELinux on/off or AppArmor on/off (not consider other Linux
> > > security modules) as those are all valid test environment setup, we'll
> > > then have 3 different unique conditions:
> > >
> > > 1. SELinux on (attr/* read successfuly)
> > > 2. AppArmor on (???)
> > > 3. SELinux off and AppArmor off (attr/* read with -EINVAL)
> > 4. TOMOYO on
> > 5. Smack on
> > ...
> >
> > > As the result, the above checking code will need to be present in both
> > > proc01 and a new test.
> >
> > Then please stick to the simple suggestion from Stephen, keeping
> > any selinux- (or any other lsm-)specific code out of proc01.c.
> >
> > Which may be what you're suggesting :)
> >
> > -serge
>
> We can just add the files related to LSM, to known failure list. We already
> check
> for their return value, if not EINVAL report test failure or else skip.
> Added the nfsd files to the list.
>
> ---
> testcases/kernel/fs/proc/proc01.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> Index: b/testcases/kernel/fs/proc/proc01.c
> ===================================================================
> --- a/testcases/kernel/fs/proc/proc01.c
> +++ b/testcases/kernel/fs/proc/proc01.c
> @@ -88,6 +88,13 @@ const Mapping known_issues[] =
> {"read", "/proc/xen/privcmd", EINVAL},
> {"read", "/proc/self/mem", EIO},
> {"read", "/proc/self/task/[0-9]*/mem", EIO},
> + {"read", "/proc/self/attr/*", EINVAL},
> + {"read", "/proc/self/task/[0-9]*/attr/*", EINVAL},
> + {"read", "/proc/fs/nfsd/unlock_filesystem", EINVAL},
> + {"read", "/proc/fs/nfsd/unlock_ip", EINVAL},
> + {"read", "/proc/fs/nfsd/filehandle", EINVAL},
> + {"read", "/proc/fs/nfsd/.getfs", EINVAL},
> + {"read", "/proc/fs/nfsd/.getfd", EINVAL},
> {"", "", 0}
> };
>
------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list
