Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-02-02 Thread CAI Qian
Hi,


--- On Mon, 2/2/09, Subrata Modak subr...@linux.vnet.ibm.com wrote:

 From: Subrata Modak subr...@linux.vnet.ibm.com
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 To: Kamalesh Babulal kamal...@linux.vnet.ibm.com, Serge E. Hallyn 
 se...@us.ibm.com, CAI Qian caiq...@cclom.cn, s...@tycho.nsa.gov
 Cc: ltp-l...@lists.sf.net, aar...@linux.vnet.ibm.com
 Date: Monday, February 2, 2009, 9:24 PM
 Thanks Serge, Cai, Kamalesh  Stephen.


The following patch should not be merged given that it did not address the
problem completely that have been discussed in the previous mails. I'll
send a new patch soon.

CAI Qian
 
 Regards--
 Subrata

  
  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}
 };
 

--
Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM)
software. With Adobe AIR, Ajax developers can use existing skills and code to
build responsive, highly engaging applications that combine the power of local
resources and data with the reach of the web. Download the Adobe AIR SDK and
Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com
___
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-31 Thread CAI Qian
Hi,



- Original Message 
 From: Serge E. Hallyn se...@us.ibm.com
 To: Kamalesh Babulal kamal...@linux.vnet.ibm.com
 Cc: CAI Qian caiq...@cclom.cn; ltp-l...@lists.sf.net; s...@tycho.nsa.gov; 
 subr...@linux.vnet.ibm.com; aar...@linux.vnet.ibm.com
 Sent: Saturday, January 31, 2009 12:54:42 AM
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 
 Quoting Kamalesh Babulal (kamal...@linux.vnet.ibm.com):
  selinux enabled
  
  proc01  0  INFO  :  /proc/acpi/event: open: known issue: Device or 
 resource busy
  proc01  0  INFO  :  /proc/sys/net/ipv6/route/flush: is write-only.
  proc01  0  INFO  :  /proc/sys/net/ipv4/route/flush: is write-only.
  proc01  0  INFO  :  /proc/sys/fs/binfmt_misc/register: is write-only.
  proc01  0  INFO  :  /proc/sysrq-trigger: is write-only.
  proc01  0  INFO  :  /proc/kmsg: read: known issue: Resource temporarily 
 unavailable
  proc01  0  INFO  :  /proc/self/task/2875/mem: read: known issue: 
 Input/output error
  proc01  0  INFO  :  /proc/self/mem: read: known issue: Input/output 
  error
  proc01  1  PASS  :  readproc() completed successfully, total read: 
  1096865 
 bytes, 885 objs
  
  the EINVAL is returned only when the LSM is does not support the
  interface, and found_errno() checks for the know return value or else
  it handled the way the unknow error is hanlded.
 
 Right, but I think CAI is concerned that if there is a regression with
 selinux enabled and it mistakenly returns -EINVAL this won't catch it.
 

That is correct. In addition, if it happens, there will be some false positive
and bad messages like,

proc01 0 INFO: /proc/self/attr/exec: read: known issue: Invalid argument

 As Stephen pointed out, if that happens then you likely won't get a
 successful boot to begin with...
 

Perhaps,  but by testing those entries the same way as other procfs entries
here,  we can catch EINVAL in same scenarios and facilities the test
provided. For example, using different sizes of read buffers .

CAI Qian

 thanks,
 -serge


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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-30 Thread Kamalesh Babulal
* Serge E. Hallyn se...@us.ibm.com [2009-01-29 12:23:49]:

 Quoting CAI Qian (caiq...@cclom.cn):
  Hi,
to me the test here has a different testing focus -- try to read every 
entry in  /proc
filesystem. Those entries belong to this filesystem, so we'll read them 
the same
way. Who knows if those entries will not give us a hang or panic or 
behaving
badly with certain read buffers? SELinux test suite may or may not 
catch those
kind of bugs.
   
   Then add tests in there...
   
  
  No, they do not belong there, because we are focus on testing of proc
  filesystem here. The test case here should not care about the content it
  read, but rather what the kernel behaves when to read those files.
 
 Then the dummy lsm I suggested is the only way to really test what you
 want :)
 
   In fact, the logical course given your concern would be to write a
   kernel module defining an LSM allowing random long write to and reads
   from /proc/$$/attr/ so you can test the procfs bits of that API (if
   you could still write an LSM as a module :).  It should be doable to
   push a 'debug' LSM into the upstream kernel which just serves to
   facilitate such testing.
   
   Anyway I've made my own position clear, and I think Kamalesh's patch
   implements precisely what Stephen suggested.
   
  
  Stephen's suggestion is something we should take. I agree Kamalesh's
  patch has included his suggestion. Unfortunately, there are other issues
  with Kamalesh's patch that have been pointed out in the last email.
 
 Sorry, I've looked back through the thread, but don't see the other
 issues you're talking about.
 
 -serge

Please find the results after applying the patch 

selinux disabled
-
proc01  0  INFO  :  /proc/acpi/event: open: known issue: Device or resource 
busy
proc01  0  INFO  :  /proc/sys/net/ipv6/route/flush: is write-only.
proc01  0  INFO  :  /proc/sys/net/ipv4/route/flush: is write-only.
proc01  0  INFO  :  /proc/sys/fs/binfmt_misc/register: is write-only.
proc01  0  INFO  :  /proc/sysrq-trigger: is write-only.
proc01  0  INFO  :  /proc/kmsg: read: known issue: Resource temporarily 
unavailable
proc01  0  INFO  :  /proc/self/task/2893/mem: read: known issue: 
Input/output error
proc01  0  INFO  :  /proc/self/task/2893/attr/current: read: known issue: 
Invalid argument
proc01  0  INFO  :  /proc/self/task/2893/attr/prev: read: known issue: 
Invalid argument
proc01  0  INFO  :  /proc/self/task/2893/attr/exec: read: known issue: 
Invalid argument
proc01  0  INFO  :  /proc/self/task/2893/attr/fscreate: read: known issue: 
Invalid argument
proc01  0  INFO  :  /proc/self/task/2893/attr/keycreate: read: known issue: 
Invalid argument
proc01  0  INFO  :  /proc/self/task/2893/attr/sockcreate: read: known 
issue: Invalid argument
proc01  0  INFO  :  /proc/self/mem: read: known issue: Input/output error
proc01  0  INFO  :  /proc/self/attr/current: read: known issue: Invalid 
argument
proc01  0  INFO  :  /proc/self/attr/prev: read: known issue: Invalid 
argument
proc01  0  INFO  :  /proc/self/attr/exec: read: known issue: Invalid 
argument
proc01  0  INFO  :  /proc/self/attr/fscreate: read: known issue: Invalid 
argument
proc01  0  INFO  :  /proc/self/attr/keycreate: read: known issue: Invalid 
argument
proc01  0  INFO  :  /proc/self/attr/sockcreate: read: known issue: Invalid 
argument
proc01  1  PASS  :  readproc() completed successfully, total read: 1095917 
bytes, 885 objs

selinux disabled (changing {read, /proc/self/task/[0-9]*/attr/*, EIO and 
{read, /proc/self/task/[0-9]*/attr/*, EIO})

proc01  0  INFO  :  /proc/acpi/event: open: known issue: Device or resource 
busy
proc01  0  INFO  :  /proc/sys/net/ipv6/route/flush: is write-only.
proc01  0  INFO  :  /proc/sys/net/ipv4/route/flush: is write-only.
proc01  0  INFO  :  /proc/sys/fs/binfmt_misc/register: is write-only.
proc01  0  INFO  :  /proc/sysrq-trigger: is write-only.
proc01  0  INFO  :  /proc/kmsg: read: known issue: Resource temporarily 
unavailable
proc01  0  INFO  :  /proc/self/task/2991/mem: read: known issue: 
Input/output error
proc01  1  FAIL  :  /proc/self/task/2991/attr/current: read: Invalid 
argument
proc01  2  FAIL  :  /proc/self/task/2991/attr/prev: read: Invalid argument
proc01  3  FAIL  :  /proc/self/task/2991/attr/exec: read: Invalid argument
proc01  4  FAIL  :  /proc/self/task/2991/attr/fscreate: read: Invalid 
argument
proc01  5  FAIL  :  /proc/self/task/2991/attr/keycreate: read: Invalid 
argument
proc01  6  FAIL  :  /proc/self/task/2991/attr/sockcreate: read: Invalid 
argument
proc01  0  INFO  :  /proc/self/mem: read: known issue: Input/output error
proc01  7  FAIL  :  /proc/self/attr/current: read: Invalid argument
proc01  8  FAIL  :  /proc/self/attr/prev: read: Invalid argument
proc01  9  FAIL  :  /proc/self/attr/exec: read: 

Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-30 Thread Serge E. Hallyn
Quoting Kamalesh Babulal (kamal...@linux.vnet.ibm.com):
 selinux enabled
 
 proc01  0  INFO  :  /proc/acpi/event: open: known issue: Device or 
 resource busy
 proc01  0  INFO  :  /proc/sys/net/ipv6/route/flush: is write-only.
 proc01  0  INFO  :  /proc/sys/net/ipv4/route/flush: is write-only.
 proc01  0  INFO  :  /proc/sys/fs/binfmt_misc/register: is write-only.
 proc01  0  INFO  :  /proc/sysrq-trigger: is write-only.
 proc01  0  INFO  :  /proc/kmsg: read: known issue: Resource temporarily 
 unavailable
 proc01  0  INFO  :  /proc/self/task/2875/mem: read: known issue: 
 Input/output error
 proc01  0  INFO  :  /proc/self/mem: read: known issue: Input/output error
 proc01  1  PASS  :  readproc() completed successfully, total read: 
 1096865 bytes, 885 objs
 
 the EINVAL is returned only when the LSM is does not support the
 interface, and found_errno() checks for the know return value or else
 it handled the way the unknow error is hanlded.

Right, but I think CAI is concerned that if there is a regression with
selinux enabled and it mistakenly returns -EINVAL this won't catch it.

As Stephen pointed out, if that happens then you likely won't get a
successful boot to begin with...

thanks,
-serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-29 Thread Serge E. Hallyn
Quoting Kamalesh Babulal (kamal...@linux.vnet.ibm.com):

 Sorry send the wrong patch

Aaah.

   
   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},

Can't speak to the nfs parts, but putting the attr/* files
there looks just right.

thanks,
-serge

  {, , 0}
};
 
 -- 
 Thanks  Regards,
 Kamalesh Babulal,
 Linux Technology Center,
 IBM, ISTL.

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-29 Thread CAI Qian
Hi,



- Original Message 
 From: Kamalesh Babulal kamal...@linux.vnet.ibm.com
 To: Serge E. Hallyn se...@us.ibm.com
 Cc: CAI Qian caiq...@cclom.cn; ltp-l...@lists.sf.net; s...@tycho.nsa.gov; 
 subr...@linux.vnet.ibm.com; aar...@linux.vnet.ibm.com
 Sent: Thursday, January 29, 2009 6:21:49 PM
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 
 
 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.

I am afraid this patch is incorrect. It is total valid that people may want to 
run this
test in a SELinux enabled environment. For example, after applied the patch, we
might see some day,

INFO: /proc/self/attr/exec: EINVAL: known failure

That is just incorrect, because this entry should return successfully in such a 
testing
environment setup.

It is also fairly normal that in order to support a different testing 
environment, we
may add some conditional compilation code for then.

In addition, for people like me have used the previous version of the test in a
SELinux enabled environment, they will lost that kind of testing coverage for
those entries (only return successfully), which is bad from the testing point of
view.

You can argue that this has already been testing in SELinux test suite, but it 
looks
to me the test here has a different testing focus -- try to read every entry in 
 /proc
filesystem. Those entries belong to this filesystem, so we'll read them the same
way. Who knows if those entries will not give us a hang or panic or behaving
badly with certain read buffers? SELinux test suite may or may not catch those
kind of bugs.

CAI Qian

 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}
};
 
 -- 
 Thanks  Regards,
 Kamalesh Babulal,
 Linux Technology Center,
 IBM, ISTL.


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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-29 Thread CAI Qian
Hi,



- Original Message 
 From: Serge E. Hallyn se...@us.ibm.com
 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
 Sent: Friday, January 30, 2009 12:52:52 AM
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 
  You can argue that this has already been testing in SELinux test suite, but 
  it 
 looks
 
 Yup.
 
  to me the test here has a different testing focus -- try to read every 
  entry in  /proc
  filesystem. Those entries belong to this filesystem, so we'll read them the 
  same
  way. Who knows if those entries will not give us a hang or panic or behaving
  badly with certain read buffers? SELinux test suite may or may not catch 
  those
  kind of bugs.
 
 Then add tests in there...
 

No, they do not belong there, because we are focus on testing of proc
filesystem here. The test case here should not care about the content it
read, but rather what the kernel behaves when to read those files.

 In fact, the logical course given your concern would be to write a
 kernel module defining an LSM allowing random long write to and reads
 from /proc/$$/attr/ so you can test the procfs bits of that API (if
 you could still write an LSM as a module :).  It should be doable to
 push a 'debug' LSM into the upstream kernel which just serves to
 facilitate such testing.
 
 Anyway I've made my own position clear, and I think Kamalesh's patch
 implements precisely what Stephen suggested.
 

Stephen's suggestion is something we should take. I agree Kamalesh's
patch has included his suggestion. Unfortunately, there are other issues
with Kamalesh's patch that have been pointed out in the last email.

CAI Qian

 thanks,
 -serge


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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-29 Thread Stephen Smalley
On Thu, 2009-01-29 at 12:23 -0600, Serge E. Hallyn wrote:
 Quoting CAI Qian (caiq...@cclom.cn):
  Hi,
to me the test here has a different testing focus -- try to read every 
entry in  /proc
filesystem. Those entries belong to this filesystem, so we'll read them 
the same
way. Who knows if those entries will not give us a hang or panic or 
behaving
badly with certain read buffers? SELinux test suite may or may not 
catch those
kind of bugs.
   
   Then add tests in there...
   
  
  No, they do not belong there, because we are focus on testing of proc
  filesystem here. The test case here should not care about the content it
  read, but rather what the kernel behaves when to read those files.
 
 Then the dummy lsm I suggested is the only way to really test what you
 want :)

That's not really a serious suggestion, right?  If the proc01 test
cannot be dependent on the presence/absence of any given security module
(like SELinux), then it cannot be dependent on a fake test security
module either as that would preclude running the test while SELinux was
in fact enabled.

 
   In fact, the logical course given your concern would be to write a
   kernel module defining an LSM allowing random long write to and reads
   from /proc/$$/attr/ so you can test the procfs bits of that API (if
   you could still write an LSM as a module :).  It should be doable to
   push a 'debug' LSM into the upstream kernel which just serves to
   facilitate such testing.
   
   Anyway I've made my own position clear, and I think Kamalesh's patch
   implements precisely what Stephen suggested.
   
  
  Stephen's suggestion is something we should take. I agree Kamalesh's
  patch has included his suggestion. Unfortunately, there are other issues
  with Kamalesh's patch that have been pointed out in the last email.
 
 Sorry, I've looked back through the thread, but don't see the other
 issues you're talking about.

I think his concern is that it will obscure an actual bug in SELinux in
the future.

I don't care strongly about it either way - the selinux testsuite is
what you should be using to exercise SELinux, and just booting and
logging into a SELinux-enabled system will exercise many if not all of
those interfaces.

-- 
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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-29 Thread CAI Qian
Hi,



- Original Message 
 From: Stephen Smalley s...@tycho.nsa.gov
 To: Serge E. Hallyn se...@us.ibm.com
 Cc: CAI Qian caiq...@cclom.cn; ltp-l...@lists.sf.net; 
 aar...@linux.vnet.ibm.com
 Sent: Friday, January 30, 2009 2:58:48 AM
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 
   
   No, they do not belong there, because we are focus on testing of proc
   filesystem here. The test case here should not care about the content it
   read, but rather what the kernel behaves when to read those files.
  
  Then the dummy lsm I suggested is the only way to really test what you
  want :)
 
 That's not really a serious suggestion, right?  If the proc01 test
 cannot be dependent on the presence/absence of any given security module
 (like SELinux), then it cannot be dependent on a fake test security
 module either as that would preclude running the test while SELinux was
 in fact enabled.
 

Exactly. The test should be able to run in different testing environment if
possible, and expect correct results according to that particular environment
if they differ. The test code just need a little bit careful to make it not
overkill to maintain.

  
In fact, the logical course given your concern would be to write a
kernel module defining an LSM allowing random long write to and reads
from /proc/$$/attr/ so you can test the procfs bits of that API (if
you could still write an LSM as a module :).  It should be doable to
push a 'debug' LSM into the upstream kernel which just serves to
facilitate such testing.

Anyway I've made my own position clear, and I think Kamalesh's patch
implements precisely what Stephen suggested.

   
   Stephen's suggestion is something we should take. I agree Kamalesh's
   patch has included his suggestion. Unfortunately, there are other issues
   with Kamalesh's patch that have been pointed out in the last email.
  
  Sorry, I've looked back through the thread, but don't see the other
  issues you're talking about.
 
 I think his concern is that it will obscure an actual bug in SELinux in
 the future.
 

My main concerns of Kamalesh's patch are,

* reduce testing coverage of the test with SELinux enabled testing
   environment, because all reads of attr/* files are not flagged
   EINVAL as errors any more.

* false positive and misleading messages may happen on
  SELinux-enabled machine.

If attr/* files returns EINVAL while SELinux is enabled happens
in the future, it can be a bug in kernel or SELinux or both.

 I don't care strongly about it either way - the selinux testsuite is
 what you should be using to exercise SELinux, and just booting and
 logging into a SELinux-enabled system will exercise many if not all of
 those interfaces.
 

I agree. I suppose that proc01 is mainly to test how kernel behaves
when to read those entries -- may use various length read buffers
etc. Of course, people might use it differently. Someone may use
it as a latency kernel for realtime kernel testing. It would be nice
to keep that flexibiltiy other than saying that this test must be run
with a particular kernel or LSM configuration.

CAI Qian

 -- 
 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
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread CAI Qian
Hi,


--- On Wed, 1/28/09, Kamalesh Babulal kamal...@linux.vnet.ibm.com wrote:

 From: Kamalesh Babulal kamal...@linux.vnet.ibm.com
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 To: CAI Qian caiq...@cclom.cn
 Cc: ltp-l...@lists.sf.net, s...@tycho.nsa.gov, se...@us.ibm.com, 
 subr...@linux.vnet.ibm.com, aar...@linux.vnet.ibm.com
 Date: Wednesday, January 28, 2009, 2:31 PM
 * CAI Qian caiq...@cclom.cn [2009-01-24 10:10:52]:
 
  Hi,
  
  The following patch addes checking for SELinux. If it
 is enabled, the
  following entries are expected to be read
 successfully,
  
  /proc/self/attr/*
  /proc/self/task/[0-9]*/attr/*
  
  Otherwise, expecting read(2) return -1 with -EINVAL.
  
  It can be applied on the top of previously sent patch
 with the title,
  
  [PATCH] proc01: /proc/ppc64/rtas/error_log: read:
 Invalid argument
  
  Version 1 is broken.
  
  Signed-off-by: CAI Qian caiq...@cclom.cn
  
  --- testcases/kernel/fs/proc/proc01.c.p12009-01-24
 19:08:51.843650731 +0800
  +++ testcases/kernel/fs/proc/proc01.c   2009-01-25
 02:06:00.001650743 +0800
  @@ -25,6 +25,8 @@
* 
*/
  
  +#include config.h
  +
   #include errno.h /* for errno */
   #include stdio.h /* for NULL */
   #include stdlib.h/* for malloc() */
  @@ -37,6 +39,10 @@
   #include fcntl.h
   #include fnmatch.h
  
  +#ifdef HAVE_SELINUX_SELINUX_H
  +#include selinux/selinux.h
  +#endif
  +
   #include test.h
   #include usctest.h
  
  @@ -89,9 +95,23 @@
   {read, /proc/self/mem,
 EIO},
   {read,
 /proc/self/task/[0-9]*/mem, EIO},
   {read,
 /proc/ppc64/rtas/error_log, EINVAL},
  +{read, /proc/self/attr/*,
 EINVAL},
  +{read,
 /proc/self/task/[0-9]*/attr/*, EINVAL},
   {, , 0}
 };
  
  +#ifdef HAVE_SELINUX_SELINUX_H
  +/* If SELinux is enabled, the following entries
 should be read
  +   successfully. Note: SELinux libraries and headers
 should be installed
  +   for the test to read those files. Otherwise, they
 will be skipped! */
  +const char selinux_should_work[][PATH_MAX] =
  +  {
  +/proc/self/attr/*,
  +/proc/self/task/[0-9]*/attr/*,
  +
  +  };
  +#endif
  +
   /* Known files that does not honor O_NONBLOCK, so
 they will hang
  the test while being read.*/
   const char error_nonblock[][PATH_MAX] =
  @@ -105,6 +125,19 @@
   {
 int i;
  
  +/* Should not see any error for certain entries if
 SELinux is enabled. */
  +#ifdef HAVE_SELINUX_SELINUX_H
  +  if (is_selinux_enabled())
  +{
  +  for (i = 0; selinux_should_work[i][0] !=
 '\0'; i++)
  +{
  +  if (!strcmp(obj, selinux_should_work[i])
  +  || !fnmatch(selinux_should_work[i],
 obj, FNM_PATHNAME))
  +return 0;
  +}
  +}
  +#endif
  +
 for (i = 0; known_issues[i].err != 0; i++)
   if (tmperr == known_issues[i].err
(!strcmp(obj,
 known_issues[i].file)
  @@ -143,6 +176,16 @@
  TEST_PAUSE;
  
  tst_tmpdir();
  +
  +#ifdef HAVE_SELINUX_SELINUX_H
  +   if (is_selinux_enabled())
  +   tst_resm(TINFO, SELinux is enabled.);
  +   else
  +   tst_resm(TINFO, SELinux is disabled.);
  +#else
  +   tst_resm(TINFO,
  +   unable to determine if SELinux is disabled or
 not.);
  +#endif
   }
  
   void help()
  
  --- /dev/null   2009-01-24 15:26:18.326002642 +0800
  +++ m4/ltp-selinux.m4   2009-01-24 19:56:54.660651164
 +0800
  @@ -0,0 +1,29 @@
  +dnl
  +dnl Copyright (c) Red Hat Inc., 2009
  +dnl
  +dnl This program is free software;  you can
 redistribute it and/or
  +dnl modify it under the terms of the GNU General
 Public License as
  +dnl published by the Free Software Foundation; either
 version 2 of
  +dnl the License, or (at your option) any later
 version.
  +dnl
  +dnl This program is distributed in the hope that it
 will be useful,
  +dnl but WITHOUT ANY WARRANTY;  without even the
 implied warranty of
  +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR
 PURPOSE.  See
  +dnl the GNU General Public License for more details.
  +dnl
  +dnl You should have received a copy of the GNU
 General Public License
  +dnl along with this program;  if not, write to the
 Free Software
  +dnl Foundation, Inc., 59 Temple Place, Suite 330,
 Boston, MA 02111-1307
  +dnl USA
  +
  +dnl
  +dnl LTP_CHECK_SELINUX
  +dnl 
  +dnl
  +AC_DEFUN([LTP_CHECK_SELINUX],
  +[dnl
  +AC_CHECK_HEADERS(selinux/selinux.h,[
  +SELINUX_LIBS=-lselinux],[
  +SELINUX_LIBS=])
  +AC_SUBST(SELINUX_LIBS)
  +])
  
  --- testcases/kernel/fs/proc/Makefile.orig  2009-01-24
 18:56:50.064650109 +0800
  +++ testcases/kernel/fs/proc/Makefile   2009-01-25
 02:00:24.316649805 +0800
  @@ -16,12 +16,10 @@
   #  Foundation, Inc., 59 Temple Place, Suite 330,
 Boston, MA 02111-1307 USA
   #
  
 
 -###
  -# name of file : Makefile

Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread Serge E. Hallyn
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/* 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?

If that is not a situation we care about, then we should simply
ignore the files if selinux is disabled.  If selinux is enabled,
the user can run the selinux testsuite and it can test for proper
return values.

-serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread CAI Qian
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/* 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.

CAI Qian

 If that is not a situation we care about, then we should
 simply
 ignore the files if selinux is disabled.  If selinux is
 enabled,
 the user can run the selinux testsuite and it can test for
 proper
 return values.
 
 -serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread Serge E. Hallyn
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/* 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.

-serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread CAI Qian



--- On Wed, 1/28/09, CAI Qian caiq...@cclom.cn wrote:

 From: CAI Qian caiq...@cclom.cn
 Subject: Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2
 To: Serge E. Hallyn se...@us.ibm.com
 Cc: ltp-l...@lists.sf.net, aar...@linux.vnet.ibm.com, s...@tycho.nsa.gov
 Date: Wednesday, January 28, 2009, 11:25 PM
 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/* 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.
 

Here is the link for the email from Stephen Smalley that I was refer
to,
http://article.gmane.org/gmane.linux.ltp/7324

 CAI Qian
 
  If that is not a situation we care about, then we
 should
  simply
  ignore the files if selinux is disabled.  If selinux
 is
  enabled,
  the user can run the selinux testsuite and it can test
 for
  proper
  return values.
  
  -serge
 
 --
 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

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread Serge E. Hallyn
Quoting CAI Qian (caiq...@cclom.cn):
 --- On Wed, 1/28/09, CAI Qian caiq...@cclom.cn wrote:
 Here is the link for the email from Stephen Smalley that I was refer
 to,
 http://article.gmane.org/gmane.linux.ltp/7324

The patch you sent doesn't do what he suggests though.  He is saying
to ignore the case where the files return data, warn and then ignore
the case where it returns -EINVAL, and return a fatal error if
another error is returned.  Notice that should involve no checks
for whether selinux is enabled, of which your patch had many.

The only potential problem with Stephen's suggestion that I see would be
that an LSM may return -EPERM or some other error as part of its
implementation.  Not sure if that would become a problem in practice
or not.

So I would still suggest ignoring these files in proc01.c altogether,
and starting with a simple test under testcases/kernel/security.  If
that test becomes more baroque over time to reflect smack/tomoyo/etc
implementation details, then at least it's in the right place.

But I objected to your last patch because of all of the selinux-specific
code in what should be a simple procfs functionality test.  If you
implement precisely what Stephen suggested then I'll certainly ack
it.

thanks,
-serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread CAI Qian
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/*
 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.

CAI Qian

 
 -serge

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


Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-28 Thread Stephen Smalley
 On Wed, 2009-01-28 at 09:00 -0800, CAI Qian wrote: 
 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/*
  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

Re: [LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-27 Thread Kamalesh Babulal
* CAI Qian caiq...@cclom.cn [2009-01-24 10:10:52]:

 Hi,
 
 The following patch addes checking for SELinux. If it is enabled, the
 following entries are expected to be read successfully,
 
 /proc/self/attr/*
 /proc/self/task/[0-9]*/attr/*
 
 Otherwise, expecting read(2) return -1 with -EINVAL.
 
 It can be applied on the top of previously sent patch with the title,
 
 [PATCH] proc01: /proc/ppc64/rtas/error_log: read: Invalid argument
 
 Version 1 is broken.
 
 Signed-off-by: CAI Qian caiq...@cclom.cn
 
 --- testcases/kernel/fs/proc/proc01.c.p1  2009-01-24 19:08:51.843650731 
 +0800
 +++ testcases/kernel/fs/proc/proc01.c 2009-01-25 02:06:00.001650743 +0800
 @@ -25,6 +25,8 @@
   * 
   */
 
 +#include config.h
 +
  #include errno.h   /* for errno */
  #include stdio.h   /* for NULL */
  #include stdlib.h  /* for malloc() */
 @@ -37,6 +39,10 @@
  #include fcntl.h
  #include fnmatch.h
 
 +#ifdef HAVE_SELINUX_SELINUX_H
 +#include selinux/selinux.h
 +#endif
 +
  #include test.h
  #include usctest.h
 
 @@ -89,9 +95,23 @@
  {read, /proc/self/mem, EIO},
  {read, /proc/self/task/[0-9]*/mem, EIO},
  {read, /proc/ppc64/rtas/error_log, EINVAL},
 +{read, /proc/self/attr/*, EINVAL},
 +{read, /proc/self/task/[0-9]*/attr/*, EINVAL},
  {, , 0}
};
 
 +#ifdef HAVE_SELINUX_SELINUX_H
 +/* If SELinux is enabled, the following entries should be read
 +   successfully. Note: SELinux libraries and headers should be installed
 +   for the test to read those files. Otherwise, they will be skipped! */
 +const char selinux_should_work[][PATH_MAX] =
 +  {
 +/proc/self/attr/*,
 +/proc/self/task/[0-9]*/attr/*,
 +
 +  };
 +#endif
 +
  /* Known files that does not honor O_NONBLOCK, so they will hang
 the test while being read.*/
  const char error_nonblock[][PATH_MAX] =
 @@ -105,6 +125,19 @@
  {
int i;
 
 +/* Should not see any error for certain entries if SELinux is enabled. */
 +#ifdef HAVE_SELINUX_SELINUX_H
 +  if (is_selinux_enabled())
 +{
 +  for (i = 0; selinux_should_work[i][0] != '\0'; i++)
 +{
 +  if (!strcmp(obj, selinux_should_work[i])
 +  || !fnmatch(selinux_should_work[i], obj, FNM_PATHNAME))
 +return 0;
 +}
 +}
 +#endif
 +
for (i = 0; known_issues[i].err != 0; i++)
  if (tmperr == known_issues[i].err
   (!strcmp(obj, known_issues[i].file)
 @@ -143,6 +176,16 @@
   TEST_PAUSE;
 
   tst_tmpdir();
 +
 +#ifdef HAVE_SELINUX_SELINUX_H
 + if (is_selinux_enabled())
 + tst_resm(TINFO, SELinux is enabled.);
 + else
 + tst_resm(TINFO, SELinux is disabled.);
 +#else
 + tst_resm(TINFO,
 + unable to determine if SELinux is disabled or not.);
 +#endif
  }
 
  void help()
 
 --- /dev/null 2009-01-24 15:26:18.326002642 +0800
 +++ m4/ltp-selinux.m4 2009-01-24 19:56:54.660651164 +0800
 @@ -0,0 +1,29 @@
 +dnl
 +dnl Copyright (c) Red Hat Inc., 2009
 +dnl
 +dnl This program is free software;  you can redistribute it and/or
 +dnl modify it under the terms of the GNU General Public License as
 +dnl published by the Free Software Foundation; either version 2 of
 +dnl the License, or (at your option) any later version.
 +dnl
 +dnl This program is distributed in the hope that it will be useful,
 +dnl but WITHOUT ANY WARRANTY;  without even the implied warranty of
 +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
 +dnl the GNU General Public License for more details.
 +dnl
 +dnl You should have received a copy of the GNU General Public License
 +dnl along with this program;  if not, write to the Free Software
 +dnl Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 +dnl USA
 +
 +dnl
 +dnl LTP_CHECK_SELINUX
 +dnl 
 +dnl
 +AC_DEFUN([LTP_CHECK_SELINUX],
 +[dnl
 +AC_CHECK_HEADERS(selinux/selinux.h,[
 +SELINUX_LIBS=-lselinux],[
 +SELINUX_LIBS=])
 +AC_SUBST(SELINUX_LIBS)
 +])
 
 --- testcases/kernel/fs/proc/Makefile.orig2009-01-24 18:56:50.064650109 
 +0800
 +++ testcases/kernel/fs/proc/Makefile 2009-01-25 02:00:24.316649805 +0800
 @@ -16,12 +16,10 @@
  #  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  #
 
 -###
 -# name of file   : Makefile  
   #
 -# description: make(1) description file for the send(2) tests.   
   #
 -###
 -CFLAGS+= -I../../../../include
 -LOADLIBES+=  -L../../../../lib -lltp
 +include ../../../../config.mk
 +
 +CFLAGS+= -I../../../../include -Wall
 +LDLIBS+= -L../../../../lib -lltp $(SELINUX_LIBS)
 
  SRCS=$(wildcard *.c)
  TARGETS=$(patsubst %.c,%,$(SRCS))
 @@ -33,5 +31,3 @@
 
  clean:
   rm -f $(TARGETS)
 -
 -
 
 --- configure.ac.orig 2009-01-24 16:41:35.894653037 +0800
 +++ configure.ac  2009-01-24 

[LTP] [PATCH] proc01: SELinux with attr/* Interface - version 2

2009-01-24 Thread CAI Qian
Hi,

The following patch addes checking for SELinux. If it is enabled, the
following entries are expected to be read successfully,

/proc/self/attr/*
/proc/self/task/[0-9]*/attr/*

Otherwise, expecting read(2) return -1 with -EINVAL.

It can be applied on the top of previously sent patch with the title,

[PATCH] proc01: /proc/ppc64/rtas/error_log: read: Invalid argument

Version 1 is broken.

Signed-off-by: CAI Qian caiq...@cclom.cn

--- testcases/kernel/fs/proc/proc01.c.p12009-01-24 19:08:51.843650731 
+0800
+++ testcases/kernel/fs/proc/proc01.c   2009-01-25 02:06:00.001650743 +0800
@@ -25,6 +25,8 @@
  * 
  */
 
+#include config.h
+
 #include errno.h /* for errno */
 #include stdio.h /* for NULL */
 #include stdlib.h/* for malloc() */
@@ -37,6 +39,10 @@
 #include fcntl.h
 #include fnmatch.h
 
+#ifdef HAVE_SELINUX_SELINUX_H
+#include selinux/selinux.h
+#endif
+
 #include test.h
 #include usctest.h
 
@@ -89,9 +95,23 @@
 {read, /proc/self/mem, EIO},
 {read, /proc/self/task/[0-9]*/mem, EIO},
 {read, /proc/ppc64/rtas/error_log, EINVAL},
+{read, /proc/self/attr/*, EINVAL},
+{read, /proc/self/task/[0-9]*/attr/*, EINVAL},
 {, , 0}
   };
 
+#ifdef HAVE_SELINUX_SELINUX_H
+/* If SELinux is enabled, the following entries should be read
+   successfully. Note: SELinux libraries and headers should be installed
+   for the test to read those files. Otherwise, they will be skipped! */
+const char selinux_should_work[][PATH_MAX] =
+  {
+/proc/self/attr/*,
+/proc/self/task/[0-9]*/attr/*,
+
+  };
+#endif
+
 /* Known files that does not honor O_NONBLOCK, so they will hang
the test while being read.*/
 const char error_nonblock[][PATH_MAX] =
@@ -105,6 +125,19 @@
 {
   int i;
 
+/* Should not see any error for certain entries if SELinux is enabled. */
+#ifdef HAVE_SELINUX_SELINUX_H
+  if (is_selinux_enabled())
+{
+  for (i = 0; selinux_should_work[i][0] != '\0'; i++)
+{
+  if (!strcmp(obj, selinux_should_work[i])
+  || !fnmatch(selinux_should_work[i], obj, FNM_PATHNAME))
+return 0;
+}
+}
+#endif
+
   for (i = 0; known_issues[i].err != 0; i++)
 if (tmperr == known_issues[i].err
  (!strcmp(obj, known_issues[i].file)
@@ -143,6 +176,16 @@
TEST_PAUSE;
 
tst_tmpdir();
+
+#ifdef HAVE_SELINUX_SELINUX_H
+   if (is_selinux_enabled())
+   tst_resm(TINFO, SELinux is enabled.);
+   else
+   tst_resm(TINFO, SELinux is disabled.);
+#else
+   tst_resm(TINFO,
+   unable to determine if SELinux is disabled or not.);
+#endif
 }
 
 void help()

--- /dev/null   2009-01-24 15:26:18.326002642 +0800
+++ m4/ltp-selinux.m4   2009-01-24 19:56:54.660651164 +0800
@@ -0,0 +1,29 @@
+dnl
+dnl Copyright (c) Red Hat Inc., 2009
+dnl
+dnl This program is free software;  you can redistribute it and/or
+dnl modify it under the terms of the GNU General Public License as
+dnl published by the Free Software Foundation; either version 2 of
+dnl the License, or (at your option) any later version.
+dnl
+dnl This program is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY;  without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See
+dnl the GNU General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU General Public License
+dnl along with this program;  if not, write to the Free Software
+dnl Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+dnl USA
+
+dnl
+dnl LTP_CHECK_SELINUX
+dnl 
+dnl
+AC_DEFUN([LTP_CHECK_SELINUX],
+[dnl
+AC_CHECK_HEADERS(selinux/selinux.h,[
+SELINUX_LIBS=-lselinux],[
+SELINUX_LIBS=])
+AC_SUBST(SELINUX_LIBS)
+])

--- testcases/kernel/fs/proc/Makefile.orig  2009-01-24 18:56:50.064650109 
+0800
+++ testcases/kernel/fs/proc/Makefile   2009-01-25 02:00:24.316649805 +0800
@@ -16,12 +16,10 @@
 #  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-###
-# name of file : Makefile#
-# description  : make(1) description file for the send(2) tests. #
-###
-CFLAGS+=   -I../../../../include
-LOADLIBES+=-L../../../../lib -lltp
+include ../../../../config.mk
+
+CFLAGS+=   -I../../../../include -Wall
+LDLIBS+=   -L../../../../lib -lltp $(SELINUX_LIBS)
 
 SRCS=$(wildcard *.c)
 TARGETS=$(patsubst %.c,%,$(SRCS))
@@ -33,5 +31,3 @@
 
 clean:
rm -f $(TARGETS)
-
-

--- configure.ac.orig   2009-01-24 16:41:35.894653037 +0800
+++ configure.ac2009-01-24 16:43:14.064654299 +0800
@@ -18,5 +18,6 @@
 LTP_CHECK_SYSCALL_EVENTFD
 LTP_CHECK_SYSCALL_MODIFY_LDT
 LTP_CHECK_SYSCALL_SIGNALFD
+LTP_CHECK_SELINUX
 
 AC_OUTPUT

--- config.mk.in.orig