On Wed, 2009-01-28 at 09:00 -0800, CAI Qian wrote: > Hi, > > > --- On Wed, 1/28/09, Serge E. Hallyn <[email protected]> wrote: > > > From: Serge E. Hallyn <[email protected]> > > Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2 > > To: "CAI Qian" <[email protected]> > > Cc: "Kamalesh Babulal" <[email protected]>, > > [email protected], [email protected], [email protected], > > [email protected] > > Date: Wednesday, January 28, 2009, 11:50 PM > > Quoting CAI Qian ([email protected]): > > > Hi, > > > > > > > > > --- On Wed, 1/28/09, Serge E. Hallyn > > <[email protected]> wrote: > > > > > > > From: Serge E. Hallyn <[email protected]> > > > > Subject: Re: [LTP] [PATCH] proc01: SELinux with > > attr/* Interface - version 2 > > > > To: "CAI Qian" <[email protected]> > > > > Cc: "Kamalesh Babulal" > > <[email protected]>, [email protected], > > [email protected], [email protected], > > [email protected] > > > > Date: Wednesday, January 28, 2009, 10:57 PM > > > > Quoting CAI Qian ([email protected]): > > > > > 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/* > > 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) > > As the result, the above checking code will need to be present in both > proc01 and a new test. AppArmor (and Smack) support a subset of the attr/* files; in particular, they only support the attr/current file. The rest will return -1 with errno EINVAL upon read(2) calls.
CONFIG_SECURITY=n completely omits the proc/self/attr subdirectory. For CONFIG_SECURITY=y, it might be nice if modules could selectively register support for a subset of the files and only have those files appear in the subdirectory, but that doesn't exist today. That would avoid proc01 even trying to call open() on the unsupported files. Of course, even if someone made that change today, you'd still want the ltp to work on existing kernels. BTW, the proc01 test is fairly limited in what it can truly test, given that it knows nothing about the semantics of the individual proc nodes and thus cannot assess whether the result is correct (it can tell whether read() succeeded or failed, but not whether the returned value is what one expects - that becomes specific to the particular proc node, and in the case of /proc/self/attr, it becomes specific not only to the particular security module but even to the particular policy in use). The selinux tests naturally make use of /proc/self/attr as part of exercising SELinux, both via calls to the libselinux interfaces and via use of the SELinux options to coreutils. Those tests can leverage their knowledge of SELinux and of the test policy to make more meaningful decisions about the results. -- Stephen Smalley National Security Agency ------------------------------------------------------------------------------ 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 [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
