* Type boundaries: questions on the semantics / is the enforcement correct ?
@ 2009-11-08 19:24 Jacques Thomas
2009-11-18 2:44 ` KaiGai Kohei
0 siblings, 1 reply; 39+ messages in thread
From: Jacques Thomas @ 2009-11-08 19:24 UTC (permalink / raw)
To: SE Linux
Hi All,
I am trying to document the semantics of bounded types, and I am puzzled
by two things:
1/ The rationale for type boundaries on *target* types
I understand type boundaries on source types: the idea is that, if typeA
is bounded by typeB, then typeA can not exert more permissions than
typeB. This would be declared with the following statement in the source
policy:
TYPEBOUND typeB typeA
What is a good use case for type boundaries on target types ?
2/ The logic of type_attribute_bounds_av in ss/services.c (kernel side)
This method is invoked to filter/limit an access vector according to the
type boundaries. Its logic goes as follows:
if(source type has a bound){ // SOURCE
../..
if(the bound is not violated)
return
../..
}
if(target type has a bound){ // TARGET
../..
if(the bound is not violated)
return
../..
}
if(target and source types have a bound){ // BOTH
../..
if(these bounds are not violated)
return
../..
}
if(there was a violation){
send audit message to user space
}
The early return statements seem to violate the intent of the code. For
instance, suppose that the source type is bounded and no access bits
violates these bounds, then the code will never evaluate whether bounds
on the target type are violated.
It seems to me that the logic should be:
// BOTH
else // SOURCE
else // TARGET
Instead of:
// SOURCE
// TARGET
// BOTH
I am looking forward to your replies.
Best,
Jacques
--
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.
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-11-08 19:24 Type boundaries: questions on the semantics / is the enforcement correct ? Jacques Thomas @ 2009-11-18 2:44 ` KaiGai Kohei 2009-11-19 16:07 ` Jacques Thomas 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2009-11-18 2:44 UTC (permalink / raw) To: Jacques Thomas; +Cc: SE Linux, method Sorry for the late response due to my extreme busy days on the last week. Jacques Thomas wrote: > Hi All, > > I am trying to document the semantics of bounded types, and I am puzzled > by two things: > > > 1/ The rationale for type boundaries on *target* types > I understand type boundaries on source types: the idea is that, if typeA > is bounded by typeB, then typeA can not exert more permissions than > typeB. This would be declared with the following statement in the source > policy: > TYPEBOUND typeB typeA > > What is a good use case for type boundaries on target types ? If a target type (parent) has a few bounded types (children), we can use the parent as a shared resource, and children as private types for individual source types. For example, please assume httpd_user_content_t and httpd_guest_content_t are bounded by httpd_sys_content_t. In this example, a domain which tries to access to the httpd_user_content_t must have a set of equal or wider permissions on the httpd_sys_content_t. Ditto for httpd_guest_content_t. If here are two domains ( user_webapp_t and guest_webapp_t ) with its own private types individually, we can use the boundaries to restrict permissions to be allowed on the child types. However, we have to admit it is a bit hard to image actual use cases. In fact, we can apply same restriction using regular types. Joshua, do you have any idea for use cases of boundaries in targets? > 2/ The logic of type_attribute_bounds_av in ss/services.c (kernel side) > This method is invoked to filter/limit an access vector according to the > type boundaries. Its logic goes as follows: > > if(source type has a bound){ // SOURCE > ../.. > if(the bound is not violated) > return > ../.. > } > > if(target type has a bound){ // TARGET > ../.. > if(the bound is not violated) > return > ../.. > } > > if(target and source types have a bound){ // BOTH > ../.. > if(these bounds are not violated) > return > ../.. > } > > if(there was a violation){ > send audit message to user space > } > > The early return statements seem to violate the intent of the code. For > instance, suppose that the source type is bounded and no access bits > violates these bounds, then the code will never evaluate whether bounds > on the target type are violated. > > It seems to me that the logic should be: > > // BOTH > else // SOURCE > else // TARGET > > Instead of: > > // SOURCE > // TARGET > // BOTH Assume P(s,t) means a set of permissions allowed between 's' and 't'. Assume 'Sc' is bounded by 'Sp', and 'Tc' is bounded by 'Tp'. In this case, - P(Sc,Tp) should be equal or less than P(Sp,Tp) due to the SOURCE check. - P(Sp,Tc) should be equal or less than P(Sp,Tp) due to the TARGET check. - P(Sc,Tc) should be equal or less than P(Sp,Tp) due to the BOTH check, and should P(Sc,Tc) equal to (P(Sc,Tp) AND P(Sp,Tc)). However, the current implementation immediately returns when no violations between P(Sc,Tp) and P(Sp,Tp) on the SOURCE check, even if both of types have its boundaries. The P(Sc,Tc) should be also masked by P(Sc,Tp), but it seems to me being bypassed. If my logic is not incorrect, the implementation should be fixed as follows: type_attribute_bounds_av(Sc,Tc, ...) { masked = 0; if (Sc has its bounds) masked |= P(Sc,Tc) & ~P(Sp,Tc); if (Tc has its bounds) masked |= P(Sc,Tc) & ~P(Sc,Tp); avd->allowed &= ~masked; } Any comments? I also think we have one other a rough option. It simply applies type boundaries on only sources to restrict its privileges, and it does not apply any restrictions on target types. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-11-18 2:44 ` KaiGai Kohei @ 2009-11-19 16:07 ` Jacques Thomas 2009-11-30 18:40 ` Jacques Thomas 0 siblings, 1 reply; 39+ messages in thread From: Jacques Thomas @ 2009-11-19 16:07 UTC (permalink / raw) To: KaiGai Kohei; +Cc: SE Linux, method KaiGai Kohei wrote: > Sorry for the late response due to my extreme busy days on the last week. > No problem, and thanks for your reply. I should have sent a patch as a proposal. I was too busy to make (and test) a patch. > Jacques Thomas wrote: > >> Hi All, >> >> I am trying to document the semantics of bounded types, and I am puzzled >> by two things: >> >> >> 1/ The rationale for type boundaries on *target* types >> I understand type boundaries on source types: the idea is that, if typeA >> is bounded by typeB, then typeA can not exert more permissions than >> typeB. This would be declared with the following statement in the source >> policy: >> TYPEBOUND typeB typeA >> >> What is a good use case for type boundaries on target types ? >> > > If a target type (parent) has a few bounded types (children), we can use > the parent as a shared resource, and children as private types for individual > source types. > > For example, please assume httpd_user_content_t and httpd_guest_content_t > are bounded by httpd_sys_content_t. In this example, a domain which tries > to access to the httpd_user_content_t must have a set of equal or wider > permissions on the httpd_sys_content_t. Ditto for httpd_guest_content_t. > If here are two domains ( user_webapp_t and guest_webapp_t ) with its own > private types individually, we can use the boundaries to restrict permissions > to be allowed on the child types. > > However, we have to admit it is a bit hard to image actual use cases. > In fact, we can apply same restriction using regular types. > Almost everything can be expressed just in terms of types. The TYPEBOUND extension, however, makes it much easier to express the containment relationship that you want to establish between the http server *domain* and the *domains* of its worker threads (for Apache/SELinux plus ). Going with your example, what bothers me is that, if I wanted to give guest_webapp_t read/write/create access to httpd_guest_content_t, I would then have to give guest_webapp_t read/write/create access on httpd_sys_content_t. I think the containment relationship should be in the other direction: guest_webapp_t can not have more permissions on httpd_sys_content_t than it has on httpd_guest_content_t. This reminds me of what is called contravariance in type theory. I think it would be less confusing if boundaries were expressed using different keywords for boundaries on domains and boundaries on "data" types, even if the boundaries can be enforced using the same infrastructure code (which is clearly demonstrated by the current code). Keeping only the code that enforces boundaries on domains (subjects) is the simplest fix, and is adequate if there is no compelling example for why we need boundaries on subjects. > Joshua, do you have any idea for use cases of boundaries in targets? > > > >> 2/ The logic of type_attribute_bounds_av in ss/services.c (kernel side) >> This method is invoked to filter/limit an access vector according to the >> type boundaries. Its logic goes as follows: >> >> if(source type has a bound){ // SOURCE >> ../.. >> if(the bound is not violated) >> return >> ../.. >> } >> >> if(target type has a bound){ // TARGET >> ../.. >> if(the bound is not violated) >> return >> ../.. >> } >> >> if(target and source types have a bound){ // BOTH >> ../.. >> if(these bounds are not violated) >> return >> ../.. >> } >> >> if(there was a violation){ >> send audit message to user space >> } >> >> The early return statements seem to violate the intent of the code. For >> instance, suppose that the source type is bounded and no access bits >> violates these bounds, then the code will never evaluate whether bounds >> on the target type are violated. >> >> It seems to me that the logic should be: >> >> // BOTH >> else // SOURCE >> else // TARGET >> >> Instead of: >> >> // SOURCE >> // TARGET >> // BOTH >> > > Assume P(s,t) means a set of permissions allowed between 's' and 't'. > Assume 'Sc' is bounded by 'Sp', and 'Tc' is bounded by 'Tp'. > > In this case, > - P(Sc,Tp) should be equal or less than P(Sp,Tp) due to the SOURCE check. > - P(Sp,Tc) should be equal or less than P(Sp,Tp) due to the TARGET check. > - P(Sc,Tc) should be equal or less than P(Sp,Tp) due to the BOTH check, > and should P(Sc,Tc) equal to (P(Sc,Tp) AND P(Sp,Tc)). > > However, the current implementation immediately returns when no violations > between P(Sc,Tp) and P(Sp,Tp) on the SOURCE check, even if both of types > have its boundaries. The P(Sc,Tc) should be also masked by P(Sc,Tp), but > it seems to me being bypassed. > This is how I read this code too: if the first mask (SOURCE) is empty, the other masks are not evaluated. > If my logic is not incorrect, the implementation should be fixed as follows: > > type_attribute_bounds_av(Sc,Tc, ...) > { > masked = 0; > > if (Sc has its bounds) > masked |= P(Sc,Tc) & ~P(Sp,Tc); > > if (Tc has its bounds) > masked |= P(Sc,Tc) & ~P(Sc,Tp); > > avd->allowed &= ~masked; > } > > Any comments? > It makes sense. In the current code, there is also a check that (following your style) goes as follows: if ((Sc has its bounds) and (Tc has its bounds)) masked |= P(Sc,Tc) & ~P(Sp,Tp); I am not sure of the meaning of this check. > I also think we have one other a rough option. > It simply applies type boundaries on only sources to restrict its privileges, > and it does not apply any restrictions on target types. > Unless there is a clear use for bounds on targets, I would favor this option. (The "rough" one :-) ) I see mostly room for confusion with the bounds on target types, because of the contravariance issue. Thanks a lot for your time, Jacques -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-11-19 16:07 ` Jacques Thomas @ 2009-11-30 18:40 ` Jacques Thomas 2009-12-01 1:02 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Jacques Thomas @ 2009-11-30 18:40 UTC (permalink / raw) To: KaiGai Kohei; +Cc: SE Linux, method Jacques Thomas wrote: > KaiGai Kohei wrote: > > <snip> </snip> >> I also think we have one other a rough option. >> It simply applies type boundaries on only sources to restrict its privileges, >> and it does not apply any restrictions on target types. >> >> > > Unless there is a clear use for bounds on targets, I would favor this > option. (The "rough" one :-) ) > I see mostly room for confusion with the bounds on target types, because > of the contravariance issue. > I can write and submit a patch along these lines. The patch is straightforward: I just have to remove the "dead" code. However, could someone please indicate me how I am supposed to test the patch ? In other words, is there a standardized testing procedure that I am unaware of ? Thank you, Jacques -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-11-30 18:40 ` Jacques Thomas @ 2009-12-01 1:02 ` KaiGai Kohei 2009-12-01 3:53 ` Jacques Thomas 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2009-12-01 1:02 UTC (permalink / raw) To: Jacques Thomas; +Cc: SE Linux, method >>> I also think we have one other a rough option. >>> It simply applies type boundaries on only sources to restrict its privileges, >>> and it does not apply any restrictions on target types. >>> >>> >> Unless there is a clear use for bounds on targets, I would favor this >> option. (The "rough" one :-) ) >> I see mostly room for confusion with the bounds on target types, because >> of the contravariance issue. >> > > I can write and submit a patch along these lines. The patch is > straightforward: I just have to remove the "dead" code. Note that libsepol has an option which test type-boundary violations in usermode just before policy load. Also check check_avtab_hierarchy_callback() in libsepol/src/hierarchy.c. (It is called when ) Historically, this code delivered from hierarchy namespace support by Joshua Brindle. I'd like to ask him what about this change. MEMO: The hierarchy namespace support implicitly set up type-boundary on a couple of types. For example, if we defined httpd_t.cgi type, it is implicitly bounded by httpd_t type without TYPEBOUNDS. I also have not seen any case example which restrict target types by the hierarchy namespace support. So, it seems to me we have no matter to remove the "dead" code. Joshua, what's your opinion? > However, could someone please indicate me how I am supposed to test the > patch ? In other words, is there a standardized testing procedure that I > am unaware of ? http://ltp.sourceforge.net/ It also contains SELinux testcases including type boundary, but it also does not contains a case of type boundary on target types. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-12-01 1:02 ` KaiGai Kohei @ 2009-12-01 3:53 ` Jacques Thomas 2009-12-08 19:46 ` Joshua Brindle 2010-01-15 15:51 ` Stephen Smalley 0 siblings, 2 replies; 39+ messages in thread From: Jacques Thomas @ 2009-12-01 3:53 UTC (permalink / raw) To: KaiGai Kohei; +Cc: SE Linux, method KaiGai Kohei wrote: >>>> I also think we have one other a rough option. >>>> It simply applies type boundaries on only sources to restrict its privileges, >>>> and it does not apply any restrictions on target types. >>>> >>>> >>>> >>> Unless there is a clear use for bounds on targets, I would favor this >>> option. (The "rough" one :-) ) >>> I see mostly room for confusion with the bounds on target types, because >>> of the contravariance issue. >>> >>> >> I can write and submit a patch along these lines. The patch is >> straightforward: I just have to remove the "dead" code. >> > > Note that libsepol has an option which test type-boundary violations > in usermode just before policy load. > Also check check_avtab_hierarchy_callback() in libsepol/src/hierarchy.c. > (It is called when ) > > Historically, this code delivered from hierarchy namespace support by > Joshua Brindle. I'd like to ask him what about this change. > > MEMO: The hierarchy namespace support implicitly set up type-boundary > on a couple of types. For example, if we defined httpd_t.cgi type, > it is implicitly bounded by httpd_t type without TYPEBOUNDS. > > I also have not seen any case example which restrict target types by > the hierarchy namespace support. So, it seems to me we have no matter > to remove the "dead" code. > > Joshua, what's your opinion? > > > >> However, could someone please indicate me how I am supposed to test the >> patch ? In other words, is there a standardized testing procedure that I >> am unaware of ? >> > > http://ltp.sourceforge.net/ > > It also contains SELinux testcases including type boundary, but it also > does not contains a case of type boundary on target types. > Kaigai, thank you for you detailed answer. Joshua, I am waiting for your opinion. Thank you both, Jacques -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-12-01 3:53 ` Jacques Thomas @ 2009-12-08 19:46 ` Joshua Brindle 2010-01-15 15:51 ` Stephen Smalley 1 sibling, 0 replies; 39+ messages in thread From: Joshua Brindle @ 2009-12-08 19:46 UTC (permalink / raw) To: Jacques Thomas; +Cc: KaiGai Kohei, SE Linux Jacques Thomas wrote: > KaiGai Kohei wrote: >>>>> I also think we have one other a rough option. >>>>> It simply applies type boundaries on only sources to restrict its privileges, >>>>> and it does not apply any restrictions on target types. >>>>> >>>>> >>>>> >>>> Unless there is a clear use for bounds on targets, I would favor this >>>> option. (The "rough" one :-) ) >>>> I see mostly room for confusion with the bounds on target types, because >>>> of the contravariance issue. >>>> >>>> >>> I can write and submit a patch along these lines. The patch is >>> straightforward: I just have to remove the "dead" code. >>> >> Note that libsepol has an option which test type-boundary violations >> in usermode just before policy load. >> Also check check_avtab_hierarchy_callback() in libsepol/src/hierarchy.c. >> (It is called when ) >> >> Historically, this code delivered from hierarchy namespace support by >> Joshua Brindle. I'd like to ask him what about this change. >> >> MEMO: The hierarchy namespace support implicitly set up type-boundary >> on a couple of types. For example, if we defined httpd_t.cgi type, >> it is implicitly bounded by httpd_t type without TYPEBOUNDS. >> >> I also have not seen any case example which restrict target types by >> the hierarchy namespace support. So, it seems to me we have no matter >> to remove the "dead" code. >> >> Joshua, what's your opinion? I agree. The purpose of the type hierarchy was to confine children domains to a subset of their parents (or a subset of the parent domain -> parent target). For example, this is allowed: allow subject object: file read; allow subject.child object.child: file read; This was so that the parent didn't specifically have to have access to the child, but could allow children to access it based on the fact that the targets parent was accessible by the subjects parent. This was mostly for 'container' types where a subject isn't really used on the system and is only used as a container. We thought about adding a container keyword to remove those container types from the policy before it was built but after the hierarchy checks were done but that wouldn't work anymore due to the hierarchy checks in the kernel. >> >> >> >>> However, could someone please indicate me how I am supposed to test the >>> patch ? In other words, is there a standardized testing procedure that I >>> am unaware of ? >>> >> http://ltp.sourceforge.net/ >> >> It also contains SELinux testcases including type boundary, but it also >> does not contains a case of type boundary on target types. >> > > Kaigai, thank you for you detailed answer. > > Joshua, I am waiting for your opinion. > > Thank you both, > Jacques > > -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2009-12-01 3:53 ` Jacques Thomas 2009-12-08 19:46 ` Joshua Brindle @ 2010-01-15 15:51 ` Stephen Smalley 2010-01-18 10:10 ` KaiGai Kohei 1 sibling, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-01-15 15:51 UTC (permalink / raw) To: Jacques Thomas; +Cc: KaiGai Kohei, SE Linux, method On Mon, 2009-11-30 at 22:53 -0500, Jacques Thomas wrote: > KaiGai Kohei wrote: > >>>> I also think we have one other a rough option. > >>>> It simply applies type boundaries on only sources to restrict its privileges, > >>>> and it does not apply any restrictions on target types. > >>>> > >>>> > >>>> > >>> Unless there is a clear use for bounds on targets, I would favor this > >>> option. (The "rough" one :-) ) > >>> I see mostly room for confusion with the bounds on target types, because > >>> of the contravariance issue. > >>> > >>> > >> I can write and submit a patch along these lines. The patch is > >> straightforward: I just have to remove the "dead" code. > >> > > > > Note that libsepol has an option which test type-boundary violations > > in usermode just before policy load. > > Also check check_avtab_hierarchy_callback() in libsepol/src/hierarchy.c. > > (It is called when ) > > > > Historically, this code delivered from hierarchy namespace support by > > Joshua Brindle. I'd like to ask him what about this change. > > > > MEMO: The hierarchy namespace support implicitly set up type-boundary > > on a couple of types. For example, if we defined httpd_t.cgi type, > > it is implicitly bounded by httpd_t type without TYPEBOUNDS. > > > > I also have not seen any case example which restrict target types by > > the hierarchy namespace support. So, it seems to me we have no matter > > to remove the "dead" code. > > > > Joshua, what's your opinion? > > > > > > > >> However, could someone please indicate me how I am supposed to test the > >> patch ? In other words, is there a standardized testing procedure that I > >> am unaware of ? > >> > > > > http://ltp.sourceforge.net/ > > > > It also contains SELinux testcases including type boundary, but it also > > does not contains a case of type boundary on target types. > > Where does this stand? IIUC, we are going to just remove the dead code from type_attribute_bounds_av() in the kernel and check_avtab_hierarchy_callback() in libsepol? With regard to the ltp, note that the last version of the ltp with a working selinux testsuite was ltp-full-20090930. I am still trying to work with the ltp maintainers to fix it in cvs head, but that is still work in progress. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Type boundaries: questions on the semantics / is the enforcement correct ? 2010-01-15 15:51 ` Stephen Smalley @ 2010-01-18 10:10 ` KaiGai Kohei 2010-01-20 4:25 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 2010-01-20 4:26 ` [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() KaiGai Kohei 0 siblings, 2 replies; 39+ messages in thread From: KaiGai Kohei @ 2010-01-18 10:10 UTC (permalink / raw) To: Stephen Smalley; +Cc: Jacques Thomas, SE Linux, method (2010/01/16 0:51), Stephen Smalley wrote: > On Mon, 2009-11-30 at 22:53 -0500, Jacques Thomas wrote: >> KaiGai Kohei wrote: >>>>>> I also think we have one other a rough option. >>>>>> It simply applies type boundaries on only sources to restrict its privileges, >>>>>> and it does not apply any restrictions on target types. >>>>>> >>>>>> >>>>>> >>>>> Unless there is a clear use for bounds on targets, I would favor this >>>>> option. (The "rough" one :-) ) >>>>> I see mostly room for confusion with the bounds on target types, because >>>>> of the contravariance issue. >>>>> >>>>> >>>> I can write and submit a patch along these lines. The patch is >>>> straightforward: I just have to remove the "dead" code. >>>> >>> >>> Note that libsepol has an option which test type-boundary violations >>> in usermode just before policy load. >>> Also check check_avtab_hierarchy_callback() in libsepol/src/hierarchy.c. >>> (It is called when ) >>> >>> Historically, this code delivered from hierarchy namespace support by >>> Joshua Brindle. I'd like to ask him what about this change. >>> >>> MEMO: The hierarchy namespace support implicitly set up type-boundary >>> on a couple of types. For example, if we defined httpd_t.cgi type, >>> it is implicitly bounded by httpd_t type without TYPEBOUNDS. >>> >>> I also have not seen any case example which restrict target types by >>> the hierarchy namespace support. So, it seems to me we have no matter >>> to remove the "dead" code. >>> >>> Joshua, what's your opinion? >>> >>> >>> >>>> However, could someone please indicate me how I am supposed to test the >>>> patch ? In other words, is there a standardized testing procedure that I >>>> am unaware of ? >>>> >>> >>> http://ltp.sourceforge.net/ >>> >>> It also contains SELinux testcases including type boundary, but it also >>> does not contains a case of type boundary on target types. >>> > > Where does this stand? IIUC, we are going to just remove the dead code > from type_attribute_bounds_av() in the kernel and > check_avtab_hierarchy_callback() in libsepol? If Jacques is not available right now, I'll submit a patch to remove the dead code within this week. Please wait for a while. > With regard to the ltp, note that the last version of the ltp with a > working selinux testsuite was ltp-full-20090930. I am still trying to > work with the ltp maintainers to fix it in cvs head, but that is still > work in progress. > -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() 2010-01-18 10:10 ` KaiGai Kohei @ 2010-01-20 4:25 ` KaiGai Kohei 2010-01-20 13:33 ` Stephen Smalley 2010-01-20 4:26 ` [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() KaiGai Kohei 1 sibling, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-01-20 4:25 UTC (permalink / raw) To: Stephen Smalley; +Cc: Jacques Thomas, SE Linux, method This patch removes dead code in type_attribute_bounds_av(). Due to the historical reason, the type boundary feature is delivered from hierarchical types in libsepol, it has supported boundary features both of subject type (domain; in most cases) and target type. However, we don't have any actual use cases in bounded target types, and it tended to make conceptual confusion. So, this patch removes the dead code to apply boundary checks on the target types. I makes clear the TYPEBOUNDS restricts privileges of a certain domain bounded to any other domain. Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> -- security/selinux/ss/services.c | 36 ++---------------------------------- 1 files changed, 2 insertions(+), 34 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 3b42b15..a63593e 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -523,9 +523,10 @@ static void type_attribute_bounds_av(struct context *scontext, = policydb.type_val_to_struct[scontext->type - 1]; struct type_datum *target = policydb.type_val_to_struct[tcontext->type - 1]; - u32 masked = 0; if (source->bounds) { + u32 masked; + memset(&lo_avd, 0, sizeof(lo_avd)); memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); @@ -538,40 +539,7 @@ static void type_attribute_bounds_av(struct context *scontext, if ((lo_avd.allowed & avd->allowed) == avd->allowed) return; /* no masked permission */ masked = ~lo_avd.allowed & avd->allowed; - } - - if (target->bounds) { - memset(&lo_avd, 0, sizeof(lo_avd)); - - memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); - lo_tcontext.type = target->bounds; - - context_struct_compute_av(scontext, - &lo_tcontext, - tclass, - &lo_avd); - if ((lo_avd.allowed & avd->allowed) == avd->allowed) - return; /* no masked permission */ - masked = ~lo_avd.allowed & avd->allowed; - } - - if (source->bounds && target->bounds) { - memset(&lo_avd, 0, sizeof(lo_avd)); - /* - * lo_scontext and lo_tcontext are already - * set up. - */ - - context_struct_compute_av(&lo_scontext, - &lo_tcontext, - tclass, - &lo_avd); - if ((lo_avd.allowed & avd->allowed) == avd->allowed) - return; /* no masked permission */ - masked = ~lo_avd.allowed & avd->allowed; - } - if (masked) { /* mask violated permissions */ avd->allowed &= ~masked; -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() 2010-01-20 4:25 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei @ 2010-01-20 13:33 ` Stephen Smalley 2010-01-20 16:29 ` contents of /etc/selinux/<type>/contexts/users/* Hasan Rezaul-CHR010 2010-01-21 6:00 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 0 siblings, 2 replies; 39+ messages in thread From: Stephen Smalley @ 2010-01-20 13:33 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Jacques Thomas, SE Linux, method On Wed, 2010-01-20 at 13:25 +0900, KaiGai Kohei wrote: > This patch removes dead code in type_attribute_bounds_av(). > > Due to the historical reason, the type boundary feature is delivered > from hierarchical types in libsepol, it has supported boundary features > both of subject type (domain; in most cases) and target type. > > However, we don't have any actual use cases in bounded target types, > and it tended to make conceptual confusion. > So, this patch removes the dead code to apply boundary checks on the > target types. I makes clear the TYPEBOUNDS restricts privileges of > a certain domain bounded to any other domain. > > Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> > -- > security/selinux/ss/services.c | 36 ++---------------------------------- > 1 files changed, 2 insertions(+), 34 deletions(-) $ make C=2 security/selinux/ss/services.o security/selinux/ss/services.c: In function ‘type_attribute_bounds_av’: security/selinux/ss/services.c:524: warning: unused variable ‘target’ security/selinux/ss/services.c:520: warning: unused variable ‘lo_tcontext’ > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 3b42b15..a63593e 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -523,9 +523,10 @@ static void type_attribute_bounds_av(struct context *scontext, > = policydb.type_val_to_struct[scontext->type - 1]; > struct type_datum *target > = policydb.type_val_to_struct[tcontext->type - 1]; > - u32 masked = 0; > > if (source->bounds) { > + u32 masked; > + > memset(&lo_avd, 0, sizeof(lo_avd)); > > memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); > @@ -538,40 +539,7 @@ static void type_attribute_bounds_av(struct context *scontext, > if ((lo_avd.allowed & avd->allowed) == avd->allowed) > return; /* no masked permission */ > masked = ~lo_avd.allowed & avd->allowed; > - } > - > - if (target->bounds) { > - memset(&lo_avd, 0, sizeof(lo_avd)); > - > - memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); > - lo_tcontext.type = target->bounds; > - > - context_struct_compute_av(scontext, > - &lo_tcontext, > - tclass, > - &lo_avd); > - if ((lo_avd.allowed & avd->allowed) == avd->allowed) > - return; /* no masked permission */ > - masked = ~lo_avd.allowed & avd->allowed; > - } > - > - if (source->bounds && target->bounds) { > - memset(&lo_avd, 0, sizeof(lo_avd)); > - /* > - * lo_scontext and lo_tcontext are already > - * set up. > - */ > - > - context_struct_compute_av(&lo_scontext, > - &lo_tcontext, > - tclass, > - &lo_avd); > - if ((lo_avd.allowed & avd->allowed) == avd->allowed) > - return; /* no masked permission */ > - masked = ~lo_avd.allowed & avd->allowed; > - } > > - if (masked) { > /* mask violated permissions */ > avd->allowed &= ~masked; > -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* contents of /etc/selinux/<type>/contexts/users/* 2010-01-20 13:33 ` Stephen Smalley @ 2010-01-20 16:29 ` Hasan Rezaul-CHR010 2010-01-20 17:33 ` Stephen Smalley 2010-01-21 6:00 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 1 sibling, 1 reply; 39+ messages in thread From: Hasan Rezaul-CHR010 @ 2010-01-20 16:29 UTC (permalink / raw) To: Stephen Smalley; +Cc: SE Linux Hi All, Can you please point me to documentation that describes the syntax details of the files found in the directory: /etc/selinux/<type>/contexts/users/ local.users root staff_u system.users unconfined_u user_u These files are already created for me, with several lines in them. I would like to better understand what the content of these files really mean and what its really doing... This way, I could perhaps make some adjustments to customize them for my specific needs. Thanks in advance. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: contents of /etc/selinux/<type>/contexts/users/* 2010-01-20 16:29 ` contents of /etc/selinux/<type>/contexts/users/* Hasan Rezaul-CHR010 @ 2010-01-20 17:33 ` Stephen Smalley 0 siblings, 0 replies; 39+ messages in thread From: Stephen Smalley @ 2010-01-20 17:33 UTC (permalink / raw) To: Hasan Rezaul-CHR010; +Cc: SE Linux On Wed, 2010-01-20 at 11:29 -0500, Hasan Rezaul-CHR010 wrote: > Hi All, > > Can you please point me to documentation that describes the syntax > details of the files found in the directory: > > /etc/selinux/<type>/contexts/users/ > local.users > root > staff_u > system.users > unconfined_u > user_u > > These files are already created for me, with several lines in them. I > would like to better understand what the content of these files really > mean and what its really doing... This way, I could perhaps make some > adjustments to customize them for my specific needs. Thanks in advance. These are just per-user versions of the default_contexts file, see: http://www.nsa.gov/research/_files/selinux/papers/policy2/x724.shtml -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() 2010-01-20 13:33 ` Stephen Smalley 2010-01-20 16:29 ` contents of /etc/selinux/<type>/contexts/users/* Hasan Rezaul-CHR010 @ 2010-01-21 6:00 ` KaiGai Kohei 2010-01-21 14:08 ` Stephen Smalley 2010-01-24 22:24 ` James Morris 1 sibling, 2 replies; 39+ messages in thread From: KaiGai Kohei @ 2010-01-21 6:00 UTC (permalink / raw) To: Stephen Smalley; +Cc: Jacques Thomas, SE Linux, method (2010/01/20 22:33), Stephen Smalley wrote: > $ make C=2 security/selinux/ss/services.o > security/selinux/ss/services.c: In function ‘type_attribute_bounds_av’: > security/selinux/ss/services.c:524: warning: unused variable ‘target’ > security/selinux/ss/services.c:520: warning: unused variable ‘lo_tcontext’ Sorry, it was fixed. -------- This patch removes dead code in type_attribute_bounds_av(). Due to the historical reason, the type boundary feature is delivered from hierarchical types in libsepol, it has supported boundary features both of subject type (domain; in most cases) and target type. However, we don't have any actual use cases in bounded target types, and it tended to make conceptual confusion. So, this patch removes the dead code to apply boundary checks on the target types. I makes clear the TYPEBOUNDS restricts privileges of a certain domain bounded to any other domain. Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> -- security/selinux/ss/services.c | 43 +++------------------------------------ 1 files changed, 4 insertions(+), 39 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 3b42b15..4a2bf21 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -516,16 +516,14 @@ static void type_attribute_bounds_av(struct context *scontext, u16 tclass, struct av_decision *avd) { - struct context lo_scontext; - struct context lo_tcontext; - struct av_decision lo_avd; struct type_datum *source = policydb.type_val_to_struct[scontext->type - 1]; - struct type_datum *target - = policydb.type_val_to_struct[tcontext->type - 1]; - u32 masked = 0; if (source->bounds) { + struct context lo_scontext; + struct av_decision lo_avd; + u32 masked; + memset(&lo_avd, 0, sizeof(lo_avd)); memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); @@ -538,40 +536,7 @@ static void type_attribute_bounds_av(struct context *scontext, if ((lo_avd.allowed & avd->allowed) == avd->allowed) return; /* no masked permission */ masked = ~lo_avd.allowed & avd->allowed; - } - - if (target->bounds) { - memset(&lo_avd, 0, sizeof(lo_avd)); - - memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); - lo_tcontext.type = target->bounds; - - context_struct_compute_av(scontext, - &lo_tcontext, - tclass, - &lo_avd); - if ((lo_avd.allowed & avd->allowed) == avd->allowed) - return; /* no masked permission */ - masked = ~lo_avd.allowed & avd->allowed; - } - - if (source->bounds && target->bounds) { - memset(&lo_avd, 0, sizeof(lo_avd)); - /* - * lo_scontext and lo_tcontext are already - * set up. - */ - - context_struct_compute_av(&lo_scontext, - &lo_tcontext, - tclass, - &lo_avd); - if ((lo_avd.allowed & avd->allowed) == avd->allowed) - return; /* no masked permission */ - masked = ~lo_avd.allowed & avd->allowed; - } - if (masked) { /* mask violated permissions */ avd->allowed &= ~masked; -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() 2010-01-21 6:00 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei @ 2010-01-21 14:08 ` Stephen Smalley 2010-01-24 22:24 ` James Morris 1 sibling, 0 replies; 39+ messages in thread From: Stephen Smalley @ 2010-01-21 14:08 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Jacques Thomas, SE Linux, method On Thu, 2010-01-21 at 15:00 +0900, KaiGai Kohei wrote: > (2010/01/20 22:33), Stephen Smalley wrote: > > $ make C=2 security/selinux/ss/services.o > > security/selinux/ss/services.c: In function ‘type_attribute_bounds_av’: > > security/selinux/ss/services.c:524: warning: unused variable ‘target’ > > security/selinux/ss/services.c:520: warning: unused variable ‘lo_tcontext’ > > Sorry, it was fixed. > > -------- > This patch removes dead code in type_attribute_bounds_av(). > > Due to the historical reason, the type boundary feature is delivered > from hierarchical types in libsepol, it has supported boundary features > both of subject type (domain; in most cases) and target type. > > However, we don't have any actual use cases in bounded target types, > and it tended to make conceptual confusion. > So, this patch removes the dead code to apply boundary checks on the > target types. I makes clear the TYPEBOUNDS restricts privileges of > a certain domain bounded to any other domain. > > Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > -- > security/selinux/ss/services.c | 43 +++------------------------------------ > 1 files changed, 4 insertions(+), 39 deletions(-) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 3b42b15..4a2bf21 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -516,16 +516,14 @@ static void type_attribute_bounds_av(struct context *scontext, > u16 tclass, > struct av_decision *avd) > { > - struct context lo_scontext; > - struct context lo_tcontext; > - struct av_decision lo_avd; > struct type_datum *source > = policydb.type_val_to_struct[scontext->type - 1]; > - struct type_datum *target > - = policydb.type_val_to_struct[tcontext->type - 1]; > - u32 masked = 0; > > if (source->bounds) { > + struct context lo_scontext; > + struct av_decision lo_avd; > + u32 masked; > + > memset(&lo_avd, 0, sizeof(lo_avd)); > > memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); > @@ -538,40 +536,7 @@ static void type_attribute_bounds_av(struct context *scontext, > if ((lo_avd.allowed & avd->allowed) == avd->allowed) > return; /* no masked permission */ > masked = ~lo_avd.allowed & avd->allowed; > - } > - > - if (target->bounds) { > - memset(&lo_avd, 0, sizeof(lo_avd)); > - > - memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); > - lo_tcontext.type = target->bounds; > - > - context_struct_compute_av(scontext, > - &lo_tcontext, > - tclass, > - &lo_avd); > - if ((lo_avd.allowed & avd->allowed) == avd->allowed) > - return; /* no masked permission */ > - masked = ~lo_avd.allowed & avd->allowed; > - } > - > - if (source->bounds && target->bounds) { > - memset(&lo_avd, 0, sizeof(lo_avd)); > - /* > - * lo_scontext and lo_tcontext are already > - * set up. > - */ > - > - context_struct_compute_av(&lo_scontext, > - &lo_tcontext, > - tclass, > - &lo_avd); > - if ((lo_avd.allowed & avd->allowed) == avd->allowed) > - return; /* no masked permission */ > - masked = ~lo_avd.allowed & avd->allowed; > - } > > - if (masked) { > /* mask violated permissions */ > avd->allowed &= ~masked; > > -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() 2010-01-21 6:00 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 2010-01-21 14:08 ` Stephen Smalley @ 2010-01-24 22:24 ` James Morris 1 sibling, 0 replies; 39+ messages in thread From: James Morris @ 2010-01-24 22:24 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, method On Thu, 21 Jan 2010, KaiGai Kohei wrote: > (2010/01/20 22:33), Stephen Smalley wrote: > > $ make C=2 security/selinux/ss/services.o > > security/selinux/ss/services.c: In function ?type_attribute_bounds_av?: > > security/selinux/ss/services.c:524: warning: unused variable ?target? > > security/selinux/ss/services.c:520: warning: unused variable ?lo_tcontext? > > Sorry, it was fixed. > > -------- > This patch removes dead code in type_attribute_bounds_av(). Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next -- James Morris <jmorris@namei.org> -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-01-18 10:10 ` KaiGai Kohei 2010-01-20 4:25 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei @ 2010-01-20 4:26 ` KaiGai Kohei 2010-02-05 5:42 ` KaiGai Kohei 1 sibling, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-01-20 4:26 UTC (permalink / raw) To: method; +Cc: Stephen Smalley, Jacques Thomas, SE Linux This patch removes dead code in check_avtab_hierarchy_callback(). Due to the historical reason, the type boundary feature is delivered from hierarchical types in libsepol, it has supported boundary features both of subject type (domain; in most cases) and target type. However, we don't have any actual use cases in bounded target types, and it tended to make conceptual confusion. So, this patch removes the dead code to apply boundary checks on the target types in libsepol (when expand-check=1). I makes clear the TYPEBOUNDS restricts privileges of a certain domain bounded to any other domain. Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> -- libsepol/src/hierarchy.c | 71 ++++++++++++--------------------------------- 1 files changed, 19 insertions(+), 52 deletions(-) diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c index e2df5a4..87a9d9c 100644 --- a/libsepol/src/hierarchy.c +++ b/libsepol/src/hierarchy.c @@ -159,7 +159,7 @@ static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d, { avtab_key_t key; hierarchy_args_t *a = (hierarchy_args_t *) args; - type_datum_t *s, *t1 = NULL, *t2 = NULL; + type_datum_t *s, *t; avtab_datum_t av; if (!(k->specified & AVTAB_ALLOWED)) { @@ -169,62 +169,29 @@ static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d, /* search for parent first */ s = a->p->type_val_to_struct[k->source_type - 1]; - if (find_parent_type(a, s, &t1) < 0) + if (find_parent_type(a, s, &t) < 0) return -1; - if (t1) { - /* - * search for access allowed between type 1's - * parent and type 2. - */ - key.source_type = t1->s.value; - key.target_type = k->target_type; - key.target_class = k->target_class; - key.specified = AVTAB_ALLOWED; - compute_avtab_datum(a, &key, &av); - - if ((av.data & d->data) == d->data) - return 0; - } - - /* next we try type 1 and type 2's parent */ - s = a->p->type_val_to_struct[k->target_type - 1]; - if (find_parent_type(a, s, &t2) < 0) - return -1; - if (t2) { - /* - * search for access allowed between type 1 and - * type 2's parent. - */ - key.source_type = k->source_type; - key.target_type = t2->s.value; - key.target_class = k->target_class; - key.specified = AVTAB_ALLOWED; - compute_avtab_datum(a, &key, &av); - - if ((av.data & d->data) == d->data) - return 0; - } - if (t1 && t2) { - /* - * search for access allowed between type 1's parent - * and type 2's parent. - */ - key.source_type = t1->s.value; - key.target_type = t2->s.value; - key.target_class = k->target_class; - key.specified = AVTAB_ALLOWED; - compute_avtab_datum(a, &key, &av); - - if ((av.data & d->data) == d->data) - return 0; - } + /* + * If the given subject security context does not have any + * parent domain, we don't need to apply sanity checks on + * the type boundary constraint. + */ + if (!t) + return 0; /* - * Neither one of these types have parents and - * therefore the hierarchical constraint does not apply + * Search for access allowed between the parent domain and + * the type. All the permissions with the child domain have + * to be allowed to the parent domain also. */ - if (!t1 && !t2) + key.source_type = t->s.value; + key.target_type = k->target_type; + key.target_class = k->target_class; + key.specified = AVTAB_ALLOWED; + compute_avtab_datum(a, &key, &av); + + if ((av.data & d->data) == d->data) return 0; /* -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-01-20 4:26 ` [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() KaiGai Kohei @ 2010-02-05 5:42 ` KaiGai Kohei 2010-02-05 14:50 ` Stephen Smalley 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-02-05 5:42 UTC (permalink / raw) To: method; +Cc: Stephen Smalley, Jacques Thomas, SE Linux What is the current status of this patch? Its kernel side patch has been already merged into James's -next tree. http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=7d52a155e38d5a165759dbbee656455861bf7801 Thanks, (2010/01/20 13:26), KaiGai Kohei wrote: > This patch removes dead code in check_avtab_hierarchy_callback(). > > Due to the historical reason, the type boundary feature is delivered > from hierarchical types in libsepol, it has supported boundary features > both of subject type (domain; in most cases) and target type. > > However, we don't have any actual use cases in bounded target types, > and it tended to make conceptual confusion. > So, this patch removes the dead code to apply boundary checks on the > target types in libsepol (when expand-check=1). I makes clear the TYPEBOUNDS > restricts privileges of a certain domain bounded to any other domain. > > Signed-off-by: KaiGai Kohei<kaigai@ak.jp.nec.com> > -- > libsepol/src/hierarchy.c | 71 ++++++++++++--------------------------------- > 1 files changed, 19 insertions(+), 52 deletions(-) > > diff --git a/libsepol/src/hierarchy.c b/libsepol/src/hierarchy.c > index e2df5a4..87a9d9c 100644 > --- a/libsepol/src/hierarchy.c > +++ b/libsepol/src/hierarchy.c > @@ -159,7 +159,7 @@ static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d, > { > avtab_key_t key; > hierarchy_args_t *a = (hierarchy_args_t *) args; > - type_datum_t *s, *t1 = NULL, *t2 = NULL; > + type_datum_t *s, *t; > avtab_datum_t av; > > if (!(k->specified& AVTAB_ALLOWED)) { > @@ -169,62 +169,29 @@ static int check_avtab_hierarchy_callback(avtab_key_t * k, avtab_datum_t * d, > > /* search for parent first */ > s = a->p->type_val_to_struct[k->source_type - 1]; > - if (find_parent_type(a, s,&t1)< 0) > + if (find_parent_type(a, s,&t)< 0) > return -1; > - if (t1) { > - /* > - * search for access allowed between type 1's > - * parent and type 2. > - */ > - key.source_type = t1->s.value; > - key.target_type = k->target_type; > - key.target_class = k->target_class; > - key.specified = AVTAB_ALLOWED; > - compute_avtab_datum(a,&key,&av); > - > - if ((av.data& d->data) == d->data) > - return 0; > - } > - > - /* next we try type 1 and type 2's parent */ > - s = a->p->type_val_to_struct[k->target_type - 1]; > - if (find_parent_type(a, s,&t2)< 0) > - return -1; > - if (t2) { > - /* > - * search for access allowed between type 1 and > - * type 2's parent. > - */ > - key.source_type = k->source_type; > - key.target_type = t2->s.value; > - key.target_class = k->target_class; > - key.specified = AVTAB_ALLOWED; > - compute_avtab_datum(a,&key,&av); > - > - if ((av.data& d->data) == d->data) > - return 0; > - } > > - if (t1&& t2) { > - /* > - * search for access allowed between type 1's parent > - * and type 2's parent. > - */ > - key.source_type = t1->s.value; > - key.target_type = t2->s.value; > - key.target_class = k->target_class; > - key.specified = AVTAB_ALLOWED; > - compute_avtab_datum(a,&key,&av); > - > - if ((av.data& d->data) == d->data) > - return 0; > - } > + /* > + * If the given subject security context does not have any > + * parent domain, we don't need to apply sanity checks on > + * the type boundary constraint. > + */ > + if (!t) > + return 0; > > /* > - * Neither one of these types have parents and > - * therefore the hierarchical constraint does not apply > + * Search for access allowed between the parent domain and > + * the type. All the permissions with the child domain have > + * to be allowed to the parent domain also. > */ > - if (!t1&& !t2) > + key.source_type = t->s.value; > + key.target_type = k->target_type; > + key.target_class = k->target_class; > + key.specified = AVTAB_ALLOWED; > + compute_avtab_datum(a,&key,&av); > + > + if ((av.data& d->data) == d->data) > return 0; > > /* > -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-05 5:42 ` KaiGai Kohei @ 2010-02-05 14:50 ` Stephen Smalley 2010-02-09 6:46 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-02-05 14:50 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux On Fri, 2010-02-05 at 14:42 +0900, KaiGai Kohei wrote: > What is the current status of this patch? > Its kernel side patch has been already merged into James's -next tree. > > http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=7d52a155e38d5a165759dbbee656455861bf7801 I had to use -l to get it to apply (whitespace mangled). Before applying it, when trying to install the test_policy.pp module from the selinux testsuite with expand-check=1 in /etc/selinux/semanage.conf, I get: libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } libsepol.hierarchy_check_constraints: 2 total errors found during hierarchy check And after applying it, I get the following: libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : process { fork transition sigchld sigkill sigstop signull signal getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh rlimitinh setcurrent setkeycreate setsockcreate } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : capability { dac_override dac_read_search } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : file { ioctl read write getattr lock append open } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : dir { ioctl read getattr lock search open } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fd { use } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : lnk_file { ioctl read getattr lock } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fifo_file { ioctl read write getattr lock append open } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_stream_socket { ioctl read write create getattr setattr append bind connect listen accept getopt setopt shutdown } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_dgram_socket { ioctl read write create getattr setattr append bind connect getopt setopt shutdown sendto } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : association { sendto } libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } libsepol.hierarchy_check_constraints: 12 total errors found during hierarchy check -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-05 14:50 ` Stephen Smalley @ 2010-02-09 6:46 ` KaiGai Kohei 2010-02-16 2:36 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-02-09 6:46 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux (2010/02/05 23:50), Stephen Smalley wrote: > On Fri, 2010-02-05 at 14:42 +0900, KaiGai Kohei wrote: >> What is the current status of this patch? >> Its kernel side patch has been already merged into James's -next tree. >> >> http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=7d52a155e38d5a165759dbbee656455861bf7801 > > I had to use -l to get it to apply (whitespace mangled). > > Before applying it, when trying to install the test_policy.pp module > from the selinux testsuite with expand-check=1 > in /etc/selinux/semanage.conf, I get: > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } > libsepol.hierarchy_check_constraints: 2 total errors found during hierarchy check > > And after applying it, I get the following: > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : process { fork transition sigchld sigkill sigstop signull signal getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh rlimitinh setcurrent setkeycreate setsockcreate } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : capability { dac_override dac_read_search } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : file { ioctl read write getattr lock append open } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : dir { ioctl read getattr lock search open } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fd { use } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : lnk_file { ioctl read getattr lock } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fifo_file { ioctl read write getattr lock append open } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_stream_socket { ioctl read write create getattr setattr append bind connect listen accept getopt setopt shutdown } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_dgram_socket { ioctl read write create getattr setattr append bind connect getopt setopt shutdown sendto } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : association { sendto } > libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } > libsepol.hierarchy_check_constraints: 12 total errors found during hierarchy check > We might overlook something in the case when both of source and target are bounded. When test_bounds_child_t tries to access a pseudo file, such as /proc/<PID>, these entries are labeled to domain of the corresponding process. The test_bounds_child_t is bounded to test_bounds_parent_t, and it is also a case when both of source and target are bounded to its super domain. This test policy indeed violated to the first check that ensures the permissions between source (test_bounds_child_t) and target (test_bounds_child_t) are enclosed by the permissions between parent of the source (test_bounds_parent_t) and the target, because nothing are allowed between test_bounds_parent_t and test_bounds_child_t as a filesystem object. However, the older implementation had also ensured the permissions between the source and the target are enclosed by the permissions between parent of the source and parent of the target, then, if these are enclosed, this check allows the permissions between both of children, even if the first checks were violated. In other words, we have to allow the test_bounds_parent_t to performs something on the test_bounds_child_t explicitly, in this case. At first, I replied to Jacques Thomas that I could not find any actual use cases in the target side boundary, but it might be misjudge. If I vote it again, I'll support an idea to revert my patch. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-09 6:46 ` KaiGai Kohei @ 2010-02-16 2:36 ` KaiGai Kohei 2010-02-16 15:25 ` Stephen Smalley 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-02-16 2:36 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux (2010/02/09 15:46), KaiGai Kohei wrote: > (2010/02/05 23:50), Stephen Smalley wrote: >> On Fri, 2010-02-05 at 14:42 +0900, KaiGai Kohei wrote: >>> What is the current status of this patch? >>> Its kernel side patch has been already merged into James's -next tree. >>> >>> http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=7d52a155e38d5a165759dbbee656455861bf7801 >> >> I had to use -l to get it to apply (whitespace mangled). >> >> Before applying it, when trying to install the test_policy.pp module >> from the selinux testsuite with expand-check=1 >> in /etc/selinux/semanage.conf, I get: >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } >> libsepol.hierarchy_check_constraints: 2 total errors found during hierarchy check >> >> And after applying it, I get the following: >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : process { fork transition sigchld sigkill sigstop signull signal getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh rlimitinh setcurrent setkeycreate setsockcreate } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : capability { dac_override dac_read_search } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : file { ioctl read write getattr lock append open } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : dir { ioctl read getattr lock search open } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fd { use } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : lnk_file { ioctl read getattr lock } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fifo_file { ioctl read write getattr lock append open } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_stream_socket { ioctl read write create getattr setattr append bind connect listen accept getopt setopt shutdown } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_dgram_socket { ioctl read write create getattr setattr append bind connect getopt setopt shutdown sendto } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : association { sendto } >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } >> libsepol.hierarchy_check_constraints: 12 total errors found during hierarchy check >> > > We might overlook something in the case when both of source and target are bounded. > > When test_bounds_child_t tries to access a pseudo file, such as /proc/<PID>, these > entries are labeled to domain of the corresponding process. > The test_bounds_child_t is bounded to test_bounds_parent_t, and it is also a case > when both of source and target are bounded to its super domain. > > This test policy indeed violated to the first check that ensures the permissions > between source (test_bounds_child_t) and target (test_bounds_child_t) are enclosed > by the permissions between parent of the source (test_bounds_parent_t) and the target, > because nothing are allowed between test_bounds_parent_t and test_bounds_child_t as > a filesystem object. > > However, the older implementation had also ensured the permissions between the source > and the target are enclosed by the permissions between parent of the source and parent > of the target, then, if these are enclosed, this check allows the permissions between > both of children, even if the first checks were violated. > > In other words, we have to allow the test_bounds_parent_t to performs something on > the test_bounds_child_t explicitly, in this case. > > At first, I replied to Jacques Thomas that I could not find any actual use cases in > the target side boundary, but it might be misjudge. > If I vote it again, I'll support an idea to revert my patch. What is your opinion for this topic? At first, I didn't find out the case when a bounded domain accesses /proc/self/* entries with its parent's security context. In this case, the policy writer has to allow parent domain to access all the pseudo file entries of possible child domains. The previous boundary code on the target side well handled it, but I removed it as a dead code. I think we have two options. The one is simple. It is to revert this commit: 7d52a155e38d5a165759dbbee656455861bf7801 The other is to have a convention when we write policies using bounds domain. For example, if all the domain bounded to httpd_t must have httpd_child_domain attribute, we can allow httpd_t domain to access file entries of possible child domains in apache.te. Right now, my preference is the later option. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-16 2:36 ` KaiGai Kohei @ 2010-02-16 15:25 ` Stephen Smalley 2010-02-16 23:49 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-02-16 15:25 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux On Tue, 2010-02-16 at 11:36 +0900, KaiGai Kohei wrote: > (2010/02/09 15:46), KaiGai Kohei wrote: > > (2010/02/05 23:50), Stephen Smalley wrote: > >> On Fri, 2010-02-05 at 14:42 +0900, KaiGai Kohei wrote: > >>> What is the current status of this patch? > >>> Its kernel side patch has been already merged into James's -next tree. > >>> > >>> http://git.kernel.org/?p=linux/kernel/git/jmorris/security-testing-2.6.git;a=commit;h=7d52a155e38d5a165759dbbee656455861bf7801 > >> > >> I had to use -l to get it to apply (whitespace mangled). > >> > >> Before applying it, when trying to install the test_policy.pp module > >> from the selinux testsuite with expand-check=1 > >> in /etc/selinux/semanage.conf, I get: > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } > >> libsepol.hierarchy_check_constraints: 2 total errors found during hierarchy check > >> > >> And after applying it, I get the following: > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_blue_t : file { ioctl read getattr lock open } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : process { fork transition sigchld sigkill sigstop signull signal getsched setsched getsession getpgid setpgid getcap setcap share getattr setexec setfscreate noatsecure siginh rlimitinh setcurrent setkeycreate setsockcreate } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : capability { dac_override dac_read_search } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : file { ioctl read write getattr lock append open } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : dir { ioctl read getattr lock search open } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fd { use } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : lnk_file { ioctl read getattr lock } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : fifo_file { ioctl read write getattr lock append open } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_stream_socket { ioctl read write create getattr setattr append bind connect listen accept getopt setopt shutdown } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : unix_dgram_socket { ioctl read write create getattr setattr append bind connect getopt setopt shutdown sendto } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_child_t : association { sendto } > >> libsepol.check_avtab_hierarchy_callback: hierarchy violation between types test_bounds_child_t and test_bounds_file_red_t : file { write append } > >> libsepol.hierarchy_check_constraints: 12 total errors found during hierarchy check > >> > > > > We might overlook something in the case when both of source and target are bounded. > > > > When test_bounds_child_t tries to access a pseudo file, such as /proc/<PID>, these > > entries are labeled to domain of the corresponding process. > > The test_bounds_child_t is bounded to test_bounds_parent_t, and it is also a case > > when both of source and target are bounded to its super domain. > > > > This test policy indeed violated to the first check that ensures the permissions > > between source (test_bounds_child_t) and target (test_bounds_child_t) are enclosed > > by the permissions between parent of the source (test_bounds_parent_t) and the target, > > because nothing are allowed between test_bounds_parent_t and test_bounds_child_t as > > a filesystem object. > > > > However, the older implementation had also ensured the permissions between the source > > and the target are enclosed by the permissions between parent of the source and parent > > of the target, then, if these are enclosed, this check allows the permissions between > > both of children, even if the first checks were violated. > > > > In other words, we have to allow the test_bounds_parent_t to performs something on > > the test_bounds_child_t explicitly, in this case. > > > > At first, I replied to Jacques Thomas that I could not find any actual use cases in > > the target side boundary, but it might be misjudge. > > If I vote it again, I'll support an idea to revert my patch. > > What is your opinion for this topic? > > At first, I didn't find out the case when a bounded domain > accesses /proc/self/* entries with its parent's security context. > In this case, the policy writer has to allow parent domain to > access all the pseudo file entries of possible child domains. > > The previous boundary code on the target side well handled it, > but I removed it as a dead code. > > I think we have two options. > > The one is simple. It is to revert this commit: > 7d52a155e38d5a165759dbbee656455861bf7801 > > The other is to have a convention when we write policies using > bounds domain. > For example, if all the domain bounded to httpd_t must have > httpd_child_domain attribute, we can allow httpd_t domain to > access file entries of possible child domains in apache.te. > > Right now, my preference is the later option. I'd say we revert the changeset and restore the prior behavior. I don't think we should impose the latter convention on policy writers. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-16 15:25 ` Stephen Smalley @ 2010-02-16 23:49 ` KaiGai Kohei 2010-02-17 13:51 ` Stephen Smalley ` (2 more replies) 0 siblings, 3 replies; 39+ messages in thread From: KaiGai Kohei @ 2010-02-16 23:49 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux > I'd say we revert the changeset and restore the prior behavior. > I don't think we should impose the latter convention on policy writers. OK, fair enough for me. This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 which removed a part of type_attribute_bounds_av as a dead code. However, at that time, we didn't find out the target side boundary allows to handle some of pseudo /proc/<pid>/* entries with its process's security context well. Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> -- security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++++++++--- 1 files changed, 39 insertions(+), 4 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 4e976f5..42d423c 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -524,14 +524,16 @@ static void type_attribute_bounds_av(struct context *scontext, u16 tclass, struct av_decision *avd) { + struct context lo_scontext; + struct context lo_tcontext; + struct av_decision lo_avd; struct type_datum *source = policydb.type_val_to_struct[scontext->type - 1]; + struct type_datum *target + = policydb.type_val_to_struct[tcontext->type - 1]; + u32 masked = 0; if (source->bounds) { - struct context lo_scontext; - struct av_decision lo_avd; - u32 masked; - memset(&lo_avd, 0, sizeof(lo_avd)); memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); @@ -544,7 +546,40 @@ static void type_attribute_bounds_av(struct context *scontext, if ((lo_avd.allowed & avd->allowed) == avd->allowed) return; /* no masked permission */ masked = ~lo_avd.allowed & avd->allowed; + } + + if (target->bounds) { + memset(&lo_avd, 0, sizeof(lo_avd)); + + memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); + lo_tcontext.type = target->bounds; + + context_struct_compute_av(scontext, + &lo_tcontext, + tclass, + &lo_avd); + if ((lo_avd.allowed & avd->allowed) == avd->allowed) + return; /* no masked permission */ + masked = ~lo_avd.allowed & avd->allowed; + } + + if (source->bounds && target->bounds) { + memset(&lo_avd, 0, sizeof(lo_avd)); + /* + * lo_scontext and lo_tcontext are already + * set up. + */ + + context_struct_compute_av(&lo_scontext, + &lo_tcontext, + tclass, + &lo_avd); + if ((lo_avd.allowed & avd->allowed) == avd->allowed) + return; /* no masked permission */ + masked = ~lo_avd.allowed & avd->allowed; + } + if (masked) { /* mask violated permissions */ avd->allowed &= ~masked; -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-16 23:49 ` KaiGai Kohei @ 2010-02-17 13:51 ` Stephen Smalley 2010-02-19 7:33 ` KaiGai Kohei 2010-02-19 17:31 ` Stephen Smalley 2010-02-21 22:09 ` James Morris 2 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-02-17 13:51 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: > > I'd say we revert the changeset and restore the prior behavior. > > I don't think we should impose the latter convention on policy writers. > > OK, fair enough for me. > > This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 > which removed a part of type_attribute_bounds_av as a dead code. > However, at that time, we didn't find out the target side boundary allows > to handle some of pseudo /proc/<pid>/* entries with its process's security > context well. Does Jacques' original concern about the code still hold true? http://marc.info/?l=selinux&m=125770868309928&w=2 http://marc.info/?l=selinux&m=125851264424682&w=2 > > Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> > -- > security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++++++++--- > 1 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 4e976f5..42d423c 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -524,14 +524,16 @@ static void type_attribute_bounds_av(struct context *scontext, > u16 tclass, > struct av_decision *avd) > { > + struct context lo_scontext; > + struct context lo_tcontext; > + struct av_decision lo_avd; > struct type_datum *source > = policydb.type_val_to_struct[scontext->type - 1]; > + struct type_datum *target > + = policydb.type_val_to_struct[tcontext->type - 1]; > + u32 masked = 0; > > if (source->bounds) { > - struct context lo_scontext; > - struct av_decision lo_avd; > - u32 masked; > - > memset(&lo_avd, 0, sizeof(lo_avd)); > > memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); > @@ -544,7 +546,40 @@ static void type_attribute_bounds_av(struct context *scontext, > if ((lo_avd.allowed & avd->allowed) == avd->allowed) > return; /* no masked permission */ > masked = ~lo_avd.allowed & avd->allowed; > + } > + > + if (target->bounds) { > + memset(&lo_avd, 0, sizeof(lo_avd)); > + > + memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); > + lo_tcontext.type = target->bounds; > + > + context_struct_compute_av(scontext, > + &lo_tcontext, > + tclass, > + &lo_avd); > + if ((lo_avd.allowed & avd->allowed) == avd->allowed) > + return; /* no masked permission */ > + masked = ~lo_avd.allowed & avd->allowed; > + } > + > + if (source->bounds && target->bounds) { > + memset(&lo_avd, 0, sizeof(lo_avd)); > + /* > + * lo_scontext and lo_tcontext are already > + * set up. > + */ > + > + context_struct_compute_av(&lo_scontext, > + &lo_tcontext, > + tclass, > + &lo_avd); > + if ((lo_avd.allowed & avd->allowed) == avd->allowed) > + return; /* no masked permission */ > + masked = ~lo_avd.allowed & avd->allowed; > + } > > + if (masked) { > /* mask violated permissions */ > avd->allowed &= ~masked; > -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-17 13:51 ` Stephen Smalley @ 2010-02-19 7:33 ` KaiGai Kohei 2010-02-19 15:20 ` Stephen Smalley 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-02-19 7:33 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux, KaiGai Kohei (2010/02/17 22:51), Stephen Smalley wrote: > On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>> I'd say we revert the changeset and restore the prior behavior. >>> I don't think we should impose the latter convention on policy writers. >> >> OK, fair enough for me. >> >> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 >> which removed a part of type_attribute_bounds_av as a dead code. >> However, at that time, we didn't find out the target side boundary allows >> to handle some of pseudo /proc/<pid>/* entries with its process's security >> context well. > > Does Jacques' original concern about the code still hold true? > http://marc.info/?l=selinux&m=125770868309928&w=2 > http://marc.info/?l=selinux&m=125851264424682&w=2 This patch just tries to revert the changes by previous my patch, and returns to the start point, so it also reverts the Jacques' original concern. At that time, IIRC, Jacques concerned about the logic being unclear. Then, I introduced two options. The one is rough; that removes boundary checks in the target side. The other option tried to mask union bits of both of violated permissions on subject and target side boundaries (*1). (*1) type_attribute_bounds_av(Sc,Tc, ...) { masked = 0; if (Sc has its bounds) masked |= P(Sc,Tc) & ~P(Sp,Tc); if (Tc has its bounds) masked |= P(Sc,Tc) & ~P(Sc,Tp); avd->allowed &= ~masked; } However, the later option also requires policy writers special treatments to handle pseudo file entries labeled with parent's domain. For example, when web server (httpd_t) launches a thread and assign an individual bounded security context (webapp_t), we don't need to take a special treatment to access pseudo files labeled as webapp_t in the original logic. If we adopt the logic introduced at (*1), when we write webapp_t's policy, we have to allow webapp_t domain to access files labeled as httpd_t, not only webapp_t, because permissions between webapp_t and webapp_t will be eventually masked by one's between httpd_t domain and webapp_t type or webapp_t domain and httpd_t type. However, it also seems to me it is right manner to allow them explicitly in the security policy, although I sent a patch to revert the changes. In the original logic, when httpd_t domain is allowed to access httpd_t type, webapp_t domain is also allowed to access webapp_t type, although httpd_t domain is not allowed to access webapp_t type. It seems to me it is a case when child domain has permissions which are not allowed to the parent domain. I reconsidered that it is a case when we should write security policy explicitly. What do you think about it? Thanks, >> >> Signed-off-by: KaiGai Kohei<kaigai@ak.jp.nec.com> >> -- >> security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++++++++--- >> 1 files changed, 39 insertions(+), 4 deletions(-) >> >> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c >> index 4e976f5..42d423c 100644 >> --- a/security/selinux/ss/services.c >> +++ b/security/selinux/ss/services.c >> @@ -524,14 +524,16 @@ static void type_attribute_bounds_av(struct context *scontext, >> u16 tclass, >> struct av_decision *avd) >> { >> + struct context lo_scontext; >> + struct context lo_tcontext; >> + struct av_decision lo_avd; >> struct type_datum *source >> = policydb.type_val_to_struct[scontext->type - 1]; >> + struct type_datum *target >> + = policydb.type_val_to_struct[tcontext->type - 1]; >> + u32 masked = 0; >> >> if (source->bounds) { >> - struct context lo_scontext; >> - struct av_decision lo_avd; >> - u32 masked; >> - >> memset(&lo_avd, 0, sizeof(lo_avd)); >> >> memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); >> @@ -544,7 +546,40 @@ static void type_attribute_bounds_av(struct context *scontext, >> if ((lo_avd.allowed& avd->allowed) == avd->allowed) >> return; /* no masked permission */ >> masked = ~lo_avd.allowed& avd->allowed; >> + } >> + >> + if (target->bounds) { >> + memset(&lo_avd, 0, sizeof(lo_avd)); >> + >> + memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); >> + lo_tcontext.type = target->bounds; >> + >> + context_struct_compute_av(scontext, >> + &lo_tcontext, >> + tclass, >> + &lo_avd); >> + if ((lo_avd.allowed& avd->allowed) == avd->allowed) >> + return; /* no masked permission */ >> + masked = ~lo_avd.allowed& avd->allowed; >> + } >> + >> + if (source->bounds&& target->bounds) { >> + memset(&lo_avd, 0, sizeof(lo_avd)); >> + /* >> + * lo_scontext and lo_tcontext are already >> + * set up. >> + */ >> + >> + context_struct_compute_av(&lo_scontext, >> + &lo_tcontext, >> + tclass, >> + &lo_avd); >> + if ((lo_avd.allowed& avd->allowed) == avd->allowed) >> + return; /* no masked permission */ >> + masked = ~lo_avd.allowed& avd->allowed; >> + } >> >> + if (masked) { >> /* mask violated permissions */ >> avd->allowed&= ~masked; >> -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-19 7:33 ` KaiGai Kohei @ 2010-02-19 15:20 ` Stephen Smalley 2010-03-01 2:43 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-02-19 15:20 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux, KaiGai Kohei On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: > (2010/02/17 22:51), Stephen Smalley wrote: > > On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: > >>> I'd say we revert the changeset and restore the prior behavior. > >>> I don't think we should impose the latter convention on policy writers. > >> > >> OK, fair enough for me. > >> > >> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 > >> which removed a part of type_attribute_bounds_av as a dead code. > >> However, at that time, we didn't find out the target side boundary allows > >> to handle some of pseudo /proc/<pid>/* entries with its process's security > >> context well. > > > > Does Jacques' original concern about the code still hold true? > > http://marc.info/?l=selinux&m=125770868309928&w=2 > > http://marc.info/?l=selinux&m=125851264424682&w=2 > > This patch just tries to revert the changes by previous my patch, > and returns to the start point, so it also reverts the Jacques' > original concern. > > At that time, IIRC, Jacques concerned about the logic being unclear. > Then, I introduced two options. The one is rough; that removes boundary > checks in the target side. The other option tried to mask union bits of > both of violated permissions on subject and target side boundaries (*1). > > (*1) type_attribute_bounds_av(Sc,Tc, ...) > { > masked = 0; > > if (Sc has its bounds) > masked |= P(Sc,Tc) & ~P(Sp,Tc); > > if (Tc has its bounds) > masked |= P(Sc,Tc) & ~P(Sc,Tp); > > avd->allowed &= ~masked; > } > > However, the later option also requires policy writers special treatments > to handle pseudo file entries labeled with parent's domain. > > For example, when web server (httpd_t) launches a thread and assign an > individual bounded security context (webapp_t), we don't need to take > a special treatment to access pseudo files labeled as webapp_t in the > original logic. > > If we adopt the logic introduced at (*1), when we write webapp_t's policy, > we have to allow webapp_t domain to access files labeled as httpd_t, not > only webapp_t, because permissions between webapp_t and webapp_t will be > eventually masked by one's between httpd_t domain and webapp_t type or > webapp_t domain and httpd_t type. That seems wrong to me - we don't want webapp_t to be able to access the /proc/pid entries of other tasks running in httpd_t. We only want it to be able to access its own /proc/pid entries in webapp_t. Yes? > However, it also seems to me it is right manner to allow them explicitly > in the security policy, although I sent a patch to revert the changes. > > In the original logic, when httpd_t domain is allowed to access httpd_t > type, webapp_t domain is also allowed to access webapp_t type, although > httpd_t domain is not allowed to access webapp_t type. > It seems to me it is a case when child domain has permissions which are > not allowed to the parent domain. > > I reconsidered that it is a case when we should write security policy > explicitly. What do you think about it? -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-19 15:20 ` Stephen Smalley @ 2010-03-01 2:43 ` KaiGai Kohei 2010-03-01 14:34 ` Stephen Smalley 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-03-01 2:43 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux, KaiGai Kohei (2010/02/20 0:20), Stephen Smalley wrote: > On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >> (2010/02/17 22:51), Stephen Smalley wrote: >>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>> I'd say we revert the changeset and restore the prior behavior. >>>>> I don't think we should impose the latter convention on policy writers. >>>> >>>> OK, fair enough for me. >>>> >>>> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 >>>> which removed a part of type_attribute_bounds_av as a dead code. >>>> However, at that time, we didn't find out the target side boundary allows >>>> to handle some of pseudo /proc/<pid>/* entries with its process's security >>>> context well. >>> >>> Does Jacques' original concern about the code still hold true? >>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>> http://marc.info/?l=selinux&m=125851264424682&w=2 >> >> This patch just tries to revert the changes by previous my patch, >> and returns to the start point, so it also reverts the Jacques' >> original concern. >> >> At that time, IIRC, Jacques concerned about the logic being unclear. >> Then, I introduced two options. The one is rough; that removes boundary >> checks in the target side. The other option tried to mask union bits of >> both of violated permissions on subject and target side boundaries (*1). >> >> (*1) type_attribute_bounds_av(Sc,Tc, ...) >> { >> masked = 0; >> >> if (Sc has its bounds) >> masked |= P(Sc,Tc)& ~P(Sp,Tc); >> >> if (Tc has its bounds) >> masked |= P(Sc,Tc)& ~P(Sc,Tp); >> >> avd->allowed&= ~masked; >> } >> >> However, the later option also requires policy writers special treatments >> to handle pseudo file entries labeled with parent's domain. >> >> For example, when web server (httpd_t) launches a thread and assign an >> individual bounded security context (webapp_t), we don't need to take >> a special treatment to access pseudo files labeled as webapp_t in the >> original logic. >> >> If we adopt the logic introduced at (*1), when we write webapp_t's policy, >> we have to allow webapp_t domain to access files labeled as httpd_t, not >> only webapp_t, because permissions between webapp_t and webapp_t will be >> eventually masked by one's between httpd_t domain and webapp_t type or >> webapp_t domain and httpd_t type. > > That seems wrong to me - we don't want webapp_t to be able to access > the /proc/pid entries of other tasks running in httpd_t. We only want > it to be able to access its own /proc/pid entries in webapp_t. Yes? > Sorry for the late replying, because I've been unavailable last week. Yes, I also think it is unnatural to require webapp_t to have access rights to /proc/pid entries labeled as httpd_t, if and when we adopt the above logic. However, it does not solve the matter that Jacques pointed out the meaning of the original logic is unclear. In addition, I pointed out the original logic can allow webapp_t domain some permissions on the webapp_t type without permissions of httpd_t which bounds webapp_t. Example) allow httpd_t httpd_t : file { read }; allow webapp_t webapp_t : file { read }; In this case, webapp_t can read from files labeled as webapp_t, and it is not masked because httpd_t also has same permissions on itself. It seems to me httpd_t should have permissions on webapp_t types from the perspective of the definition of type boundary, even if we need to modify existing security policy a bit. (BTW, existing refpolicy does not use boundary right now.) I think we want webapp_t to have access rights (except for ones allowed explicitly) on the httpd_t, but it is not unnatural that httpd_t have access rights on webapp_t types. It performs boundary of the webapp_t's permissions as literal. >> However, it also seems to me it is right manner to allow them explicitly >> in the security policy, although I sent a patch to revert the changes. >> >> In the original logic, when httpd_t domain is allowed to access httpd_t >> type, webapp_t domain is also allowed to access webapp_t type, although >> httpd_t domain is not allowed to access webapp_t type. >> It seems to me it is a case when child domain has permissions which are >> not allowed to the parent domain. >> >> I reconsidered that it is a case when we should write security policy >> explicitly. What do you think about it? > -- KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-01 2:43 ` KaiGai Kohei @ 2010-03-01 14:34 ` Stephen Smalley 2010-03-03 8:09 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-03-01 14:34 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux, KaiGai Kohei On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: > (2010/02/20 0:20), Stephen Smalley wrote: > > On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: > >> (2010/02/17 22:51), Stephen Smalley wrote: > >>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: > >>>>> I'd say we revert the changeset and restore the prior behavior. > >>>>> I don't think we should impose the latter convention on policy writers. > >>>> > >>>> OK, fair enough for me. > >>>> > >>>> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 > >>>> which removed a part of type_attribute_bounds_av as a dead code. > >>>> However, at that time, we didn't find out the target side boundary allows > >>>> to handle some of pseudo /proc/<pid>/* entries with its process's security > >>>> context well. > >>> > >>> Does Jacques' original concern about the code still hold true? > >>> http://marc.info/?l=selinux&m=125770868309928&w=2 > >>> http://marc.info/?l=selinux&m=125851264424682&w=2 > >> > >> This patch just tries to revert the changes by previous my patch, > >> and returns to the start point, so it also reverts the Jacques' > >> original concern. > >> > >> At that time, IIRC, Jacques concerned about the logic being unclear. > >> Then, I introduced two options. The one is rough; that removes boundary > >> checks in the target side. The other option tried to mask union bits of > >> both of violated permissions on subject and target side boundaries (*1). > >> > >> (*1) type_attribute_bounds_av(Sc,Tc, ...) > >> { > >> masked = 0; > >> > >> if (Sc has its bounds) > >> masked |= P(Sc,Tc)& ~P(Sp,Tc); > >> > >> if (Tc has its bounds) > >> masked |= P(Sc,Tc)& ~P(Sc,Tp); > >> > >> avd->allowed&= ~masked; > >> } > >> > >> However, the later option also requires policy writers special treatments > >> to handle pseudo file entries labeled with parent's domain. > >> > >> For example, when web server (httpd_t) launches a thread and assign an > >> individual bounded security context (webapp_t), we don't need to take > >> a special treatment to access pseudo files labeled as webapp_t in the > >> original logic. > >> > >> If we adopt the logic introduced at (*1), when we write webapp_t's policy, > >> we have to allow webapp_t domain to access files labeled as httpd_t, not > >> only webapp_t, because permissions between webapp_t and webapp_t will be > >> eventually masked by one's between httpd_t domain and webapp_t type or > >> webapp_t domain and httpd_t type. > > > > That seems wrong to me - we don't want webapp_t to be able to access > > the /proc/pid entries of other tasks running in httpd_t. We only want > > it to be able to access its own /proc/pid entries in webapp_t. Yes? > > > > Sorry for the late replying, because I've been unavailable last week. > > Yes, I also think it is unnatural to require webapp_t to have access > rights to /proc/pid entries labeled as httpd_t, if and when we adopt > the above logic. > > However, it does not solve the matter that Jacques pointed out the > meaning of the original logic is unclear. > > In addition, I pointed out the original logic can allow webapp_t > domain some permissions on the webapp_t type without permissions > of httpd_t which bounds webapp_t. > > Example) > allow httpd_t httpd_t : file { read }; > allow webapp_t webapp_t : file { read }; > > In this case, webapp_t can read from files labeled as webapp_t, and > it is not masked because httpd_t also has same permissions on itself. > > It seems to me httpd_t should have permissions on webapp_t types from > the perspective of the definition of type boundary, even if we need to > modify existing security policy a bit. > (BTW, existing refpolicy does not use boundary right now.) > > I think we want webapp_t to have access rights (except for ones allowed > explicitly) on the httpd_t, but it is not unnatural that httpd_t have > access rights on webapp_t types. It performs boundary of the webapp_t's > permissions as literal. I think I need to revisit the original design of the hierarchical types support, and how it compares with the extension policy logic that inspired it (described in Section 4.2.2.3.2 on page 39 of: http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) Perhaps Joshua has a design tech note available on the hierarchical types design? -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-01 14:34 ` Stephen Smalley @ 2010-03-03 8:09 ` KaiGai Kohei 2010-03-04 16:01 ` Joshua Brindle 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-03-03 8:09 UTC (permalink / raw) To: Stephen Smalley; +Cc: method, Jacques Thomas, SE Linux, KaiGai Kohei (2010/03/01 23:34), Stephen Smalley wrote: > On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >> (2010/02/20 0:20), Stephen Smalley wrote: >>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>> I don't think we should impose the latter convention on policy writers. >>>>>> >>>>>> OK, fair enough for me. >>>>>> >>>>>> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>> However, at that time, we didn't find out the target side boundary allows >>>>>> to handle some of pseudo /proc/<pid>/* entries with its process's security >>>>>> context well. >>>>> >>>>> Does Jacques' original concern about the code still hold true? >>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>> >>>> This patch just tries to revert the changes by previous my patch, >>>> and returns to the start point, so it also reverts the Jacques' >>>> original concern. >>>> >>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>> Then, I introduced two options. The one is rough; that removes boundary >>>> checks in the target side. The other option tried to mask union bits of >>>> both of violated permissions on subject and target side boundaries (*1). >>>> >>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>> { >>>> masked = 0; >>>> >>>> if (Sc has its bounds) >>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>> >>>> if (Tc has its bounds) >>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>> >>>> avd->allowed&= ~masked; >>>> } >>>> >>>> However, the later option also requires policy writers special treatments >>>> to handle pseudo file entries labeled with parent's domain. >>>> >>>> For example, when web server (httpd_t) launches a thread and assign an >>>> individual bounded security context (webapp_t), we don't need to take >>>> a special treatment to access pseudo files labeled as webapp_t in the >>>> original logic. >>>> >>>> If we adopt the logic introduced at (*1), when we write webapp_t's policy, >>>> we have to allow webapp_t domain to access files labeled as httpd_t, not >>>> only webapp_t, because permissions between webapp_t and webapp_t will be >>>> eventually masked by one's between httpd_t domain and webapp_t type or >>>> webapp_t domain and httpd_t type. >>> >>> That seems wrong to me - we don't want webapp_t to be able to access >>> the /proc/pid entries of other tasks running in httpd_t. We only want >>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>> >> >> Sorry for the late replying, because I've been unavailable last week. >> >> Yes, I also think it is unnatural to require webapp_t to have access >> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >> the above logic. >> >> However, it does not solve the matter that Jacques pointed out the >> meaning of the original logic is unclear. >> >> In addition, I pointed out the original logic can allow webapp_t >> domain some permissions on the webapp_t type without permissions >> of httpd_t which bounds webapp_t. >> >> Example) >> allow httpd_t httpd_t : file { read }; >> allow webapp_t webapp_t : file { read }; >> >> In this case, webapp_t can read from files labeled as webapp_t, and >> it is not masked because httpd_t also has same permissions on itself. >> >> It seems to me httpd_t should have permissions on webapp_t types from >> the perspective of the definition of type boundary, even if we need to >> modify existing security policy a bit. >> (BTW, existing refpolicy does not use boundary right now.) >> >> I think we want webapp_t to have access rights (except for ones allowed >> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >> access rights on webapp_t types. It performs boundary of the webapp_t's >> permissions as literal. > > I think I need to revisit the original design of the hierarchical types > support, and how it compares with the extension policy logic that > inspired it (described in Section 4.2.2.3.2 on page 39 of: > http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) It seems to me this article does not mention about a case when source and target SIDs are in different parent-child-trees individually. For example, when webapp_t being a child of httpd_t tries to access files labeled as webapp_content_t being a child of httpd_sys_content_t, it was unclear for me what is an expected behavior. (Perhaps, other part of the article may introduce it, but the volume of the article is a bit large. :( ) > Perhaps Joshua has a design tech note available on the hierarchical > types design? If not available, it is good idea to make clear what is the expected behavior explicitly, on the wikipage for instance. Thanks, -- KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-03 8:09 ` KaiGai Kohei @ 2010-03-04 16:01 ` Joshua Brindle 2010-03-04 16:24 ` Joshua Brindle 2010-03-05 0:39 ` KaiGai Kohei 0 siblings, 2 replies; 39+ messages in thread From: Joshua Brindle @ 2010-03-04 16:01 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei KaiGai Kohei wrote: > (2010/03/01 23:34), Stephen Smalley wrote: >> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>> (2010/02/20 0:20), Stephen Smalley wrote: >>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>>> I don't think we should impose the latter convention on policy writers. >>>>>>> OK, fair enough for me. >>>>>>> >>>>>>> This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>>> However, at that time, we didn't find out the target side boundary allows >>>>>>> to handle some of pseudo /proc/<pid>/* entries with its process's security >>>>>>> context well. >>>>>> Does Jacques' original concern about the code still hold true? >>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>> This patch just tries to revert the changes by previous my patch, >>>>> and returns to the start point, so it also reverts the Jacques' >>>>> original concern. >>>>> >>>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>>> Then, I introduced two options. The one is rough; that removes boundary >>>>> checks in the target side. The other option tried to mask union bits of >>>>> both of violated permissions on subject and target side boundaries (*1). >>>>> >>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>> { >>>>> masked = 0; >>>>> >>>>> if (Sc has its bounds) >>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>> >>>>> if (Tc has its bounds) >>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>> >>>>> avd->allowed&= ~masked; >>>>> } >>>>> >>>>> However, the later option also requires policy writers special treatments >>>>> to handle pseudo file entries labeled with parent's domain. >>>>> >>>>> For example, when web server (httpd_t) launches a thread and assign an >>>>> individual bounded security context (webapp_t), we don't need to take >>>>> a special treatment to access pseudo files labeled as webapp_t in the >>>>> original logic. >>>>> >>>>> If we adopt the logic introduced at (*1), when we write webapp_t's policy, >>>>> we have to allow webapp_t domain to access files labeled as httpd_t, not >>>>> only webapp_t, because permissions between webapp_t and webapp_t will be >>>>> eventually masked by one's between httpd_t domain and webapp_t type or >>>>> webapp_t domain and httpd_t type. >>>> That seems wrong to me - we don't want webapp_t to be able to access >>>> the /proc/pid entries of other tasks running in httpd_t. We only want >>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>>> >>> Sorry for the late replying, because I've been unavailable last week. >>> >>> Yes, I also think it is unnatural to require webapp_t to have access >>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >>> the above logic. >>> >>> However, it does not solve the matter that Jacques pointed out the >>> meaning of the original logic is unclear. >>> >>> In addition, I pointed out the original logic can allow webapp_t >>> domain some permissions on the webapp_t type without permissions >>> of httpd_t which bounds webapp_t. >>> >>> Example) >>> allow httpd_t httpd_t : file { read }; >>> allow webapp_t webapp_t : file { read }; >>> >>> In this case, webapp_t can read from files labeled as webapp_t, and >>> it is not masked because httpd_t also has same permissions on itself. >>> >>> It seems to me httpd_t should have permissions on webapp_t types from >>> the perspective of the definition of type boundary, even if we need to >>> modify existing security policy a bit. >>> (BTW, existing refpolicy does not use boundary right now.) >>> >>> I think we want webapp_t to have access rights (except for ones allowed >>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >>> access rights on webapp_t types. It performs boundary of the webapp_t's >>> permissions as literal. >> I think I need to revisit the original design of the hierarchical types >> support, and how it compares with the extension policy logic that >> inspired it (described in Section 4.2.2.3.2 on page 39 of: >> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) > > It seems to me this article does not mention about a case when source and > target SIDs are in different parent-child-trees individually. > > For example, when webapp_t being a child of httpd_t tries to access files > labeled as webapp_content_t being a child of httpd_sys_content_t, it was > unclear for me what is an expected behavior. > (Perhaps, other part of the article may introduce it, but the volume of > the article is a bit large. :( ) > The original hierarchy specified that if httpd_t had e.g., write access to httpd_sys_content_t then webapp_t could be given write access to webapp_content_t without httpd_t having direct access to webapp_content_t. This was done so that, in policy access controls, parents could be decoupled from children while still allowing child subjects to access child objects. One application of this was to have parents that, themselves, did not have access to children objects (or were not active at all). >> Perhaps Joshua has a design tech note available on the hierarchical >> types design? > > If not available, it is good idea to make clear what is the expected > behavior explicitly, on the wikipage for instance. > The original paper talking about type hierarchies is at: http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf section 2.3.1 -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-04 16:01 ` Joshua Brindle @ 2010-03-04 16:24 ` Joshua Brindle 2010-03-05 0:39 ` KaiGai Kohei 1 sibling, 0 replies; 39+ messages in thread From: Joshua Brindle @ 2010-03-04 16:24 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei Joshua Brindle wrote: > KaiGai Kohei wrote: >> (2010/03/01 23:34), Stephen Smalley wrote: >>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>> writers. >>>>>>>> OK, fair enough for me. >>>>>>>> >>>>>>>> This patch revert the commit of >>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>>>> However, at that time, we didn't find out the target side >>>>>>>> boundary allows >>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>> process's security >>>>>>>> context well. >>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>> This patch just tries to revert the changes by previous my patch, >>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>> original concern. >>>>>> >>>>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>>>> Then, I introduced two options. The one is rough; that removes >>>>>> boundary >>>>>> checks in the target side. The other option tried to mask union >>>>>> bits of >>>>>> both of violated permissions on subject and target side boundaries >>>>>> (*1). >>>>>> >>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>> { >>>>>> masked = 0; >>>>>> >>>>>> if (Sc has its bounds) >>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>> >>>>>> if (Tc has its bounds) >>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>> >>>>>> avd->allowed&= ~masked; >>>>>> } >>>>>> >>>>>> However, the later option also requires policy writers special >>>>>> treatments >>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>> >>>>>> For example, when web server (httpd_t) launches a thread and >>>>>> assign an >>>>>> individual bounded security context (webapp_t), we don't need to take >>>>>> a special treatment to access pseudo files labeled as webapp_t in the >>>>>> original logic. >>>>>> >>>>>> If we adopt the logic introduced at (*1), when we write webapp_t's >>>>>> policy, >>>>>> we have to allow webapp_t domain to access files labeled as >>>>>> httpd_t, not >>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>> will be >>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>> type or >>>>>> webapp_t domain and httpd_t type. >>>>> That seems wrong to me - we don't want webapp_t to be able to access >>>>> the /proc/pid entries of other tasks running in httpd_t. We only want >>>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>>>> >>>> Sorry for the late replying, because I've been unavailable last week. >>>> >>>> Yes, I also think it is unnatural to require webapp_t to have access >>>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >>>> the above logic. >>>> >>>> However, it does not solve the matter that Jacques pointed out the >>>> meaning of the original logic is unclear. >>>> >>>> In addition, I pointed out the original logic can allow webapp_t >>>> domain some permissions on the webapp_t type without permissions >>>> of httpd_t which bounds webapp_t. >>>> >>>> Example) >>>> allow httpd_t httpd_t : file { read }; >>>> allow webapp_t webapp_t : file { read }; >>>> >>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>> it is not masked because httpd_t also has same permissions on itself. >>>> >>>> It seems to me httpd_t should have permissions on webapp_t types from >>>> the perspective of the definition of type boundary, even if we need to >>>> modify existing security policy a bit. >>>> (BTW, existing refpolicy does not use boundary right now.) >>>> >>>> I think we want webapp_t to have access rights (except for ones allowed >>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >>>> access rights on webapp_t types. It performs boundary of the webapp_t's >>>> permissions as literal. >>> I think I need to revisit the original design of the hierarchical types >>> support, and how it compares with the extension policy logic that >>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >> >> It seems to me this article does not mention about a case when source and >> target SIDs are in different parent-child-trees individually. >> >> For example, when webapp_t being a child of httpd_t tries to access files >> labeled as webapp_content_t being a child of httpd_sys_content_t, it was >> unclear for me what is an expected behavior. >> (Perhaps, other part of the article may introduce it, but the volume of >> the article is a bit large. :( ) >> > > The original hierarchy specified that if httpd_t had e.g., write access > to httpd_sys_content_t then webapp_t could be given write access to > webapp_content_t without httpd_t having direct access to webapp_content_t. > > This was done so that, in policy access controls, parents could be > decoupled from children while still allowing child subjects to access > child objects. One application of this was to have parents that, > themselves, did not have access to children objects (or were not active > at all). > Interestingly I can't find a complete description of the type hierarchy either in the tech notes or on the list. However, if you look at the original patch at http://marc.info/?l=selinux&m=111263146201885&w=2 in the check_avtab_hierarchy_callback() function you'll see: ... /* search for access allowed between type 1's parent and type 2 */ ... /* next we try type 1 and type 2's parent */ ... and last (t is the subject parent, t2 is the object parent) if (t && t2) { ... } I believe this is still correct behavior. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-04 16:01 ` Joshua Brindle 2010-03-04 16:24 ` Joshua Brindle @ 2010-03-05 0:39 ` KaiGai Kohei 2010-03-05 14:19 ` Joshua Brindle 1 sibling, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-03-05 0:39 UTC (permalink / raw) To: Joshua Brindle; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei (2010/03/05 1:01), Joshua Brindle wrote: > KaiGai Kohei wrote: >> (2010/03/01 23:34), Stephen Smalley wrote: >>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>> writers. >>>>>>>> OK, fair enough for me. >>>>>>>> >>>>>>>> This patch revert the commit of >>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>>>> However, at that time, we didn't find out the target side >>>>>>>> boundary allows >>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>> process's security >>>>>>>> context well. >>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>> This patch just tries to revert the changes by previous my patch, >>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>> original concern. >>>>>> >>>>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>>>> Then, I introduced two options. The one is rough; that removes >>>>>> boundary >>>>>> checks in the target side. The other option tried to mask union >>>>>> bits of >>>>>> both of violated permissions on subject and target side boundaries >>>>>> (*1). >>>>>> >>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>> { >>>>>> masked = 0; >>>>>> >>>>>> if (Sc has its bounds) >>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>> >>>>>> if (Tc has its bounds) >>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>> >>>>>> avd->allowed&= ~masked; >>>>>> } >>>>>> >>>>>> However, the later option also requires policy writers special >>>>>> treatments >>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>> >>>>>> For example, when web server (httpd_t) launches a thread and >>>>>> assign an >>>>>> individual bounded security context (webapp_t), we don't need to take >>>>>> a special treatment to access pseudo files labeled as webapp_t in the >>>>>> original logic. >>>>>> >>>>>> If we adopt the logic introduced at (*1), when we write webapp_t's >>>>>> policy, >>>>>> we have to allow webapp_t domain to access files labeled as >>>>>> httpd_t, not >>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>> will be >>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>> type or >>>>>> webapp_t domain and httpd_t type. >>>>> That seems wrong to me - we don't want webapp_t to be able to access >>>>> the /proc/pid entries of other tasks running in httpd_t. We only want >>>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>>>> >>>> Sorry for the late replying, because I've been unavailable last week. >>>> >>>> Yes, I also think it is unnatural to require webapp_t to have access >>>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >>>> the above logic. >>>> >>>> However, it does not solve the matter that Jacques pointed out the >>>> meaning of the original logic is unclear. >>>> >>>> In addition, I pointed out the original logic can allow webapp_t >>>> domain some permissions on the webapp_t type without permissions >>>> of httpd_t which bounds webapp_t. >>>> >>>> Example) >>>> allow httpd_t httpd_t : file { read }; >>>> allow webapp_t webapp_t : file { read }; >>>> >>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>> it is not masked because httpd_t also has same permissions on itself. >>>> >>>> It seems to me httpd_t should have permissions on webapp_t types from >>>> the perspective of the definition of type boundary, even if we need to >>>> modify existing security policy a bit. >>>> (BTW, existing refpolicy does not use boundary right now.) >>>> >>>> I think we want webapp_t to have access rights (except for ones allowed >>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >>>> access rights on webapp_t types. It performs boundary of the webapp_t's >>>> permissions as literal. >>> I think I need to revisit the original design of the hierarchical types >>> support, and how it compares with the extension policy logic that >>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >> >> It seems to me this article does not mention about a case when source and >> target SIDs are in different parent-child-trees individually. >> >> For example, when webapp_t being a child of httpd_t tries to access files >> labeled as webapp_content_t being a child of httpd_sys_content_t, it was >> unclear for me what is an expected behavior. >> (Perhaps, other part of the article may introduce it, but the volume of >> the article is a bit large. :( ) >> > > The original hierarchy specified that if httpd_t had e.g., write access > to httpd_sys_content_t then webapp_t could be given write access to > webapp_content_t without httpd_t having direct access to webapp_content_t. > > This was done so that, in policy access controls, parents could be > decoupled from children while still allowing child subjects to access > child objects. One application of this was to have parents that, > themselves, did not have access to children objects (or were not active > at all). Is this behavior really preferable? If my_app_t domain is allowed to access httpd_sys_content_t, and your_app_t domain allowed to access webapp_content_t, it looks like here is no data flows between two domains. However, in this example, webapp_t shares its process local memory with httpd_t, the content of httpd_sys_content_t will be visible webapp_t and vice versa. In other words, all webapp_t domain can do are also allowed to httpd_t. my_app_t <-----> httpd_sys_content_t <-----> httpd_t .... r/w r/w : process : local memory your_app_t <-----> webapp_content_t <-----> webapp_t ......: r/w r/w We shouldn't have reused idea of the type hierarchy when we implemented type boundary and multi-threading support? Or, is it really necessary for the policy management server the parent types to be decoupled from the children? >>> Perhaps Joshua has a design tech note available on the hierarchical >>> types design? >> >> If not available, it is good idea to make clear what is the expected >> behavior explicitly, on the wikipage for instance. >> > > The original paper talking about type hierarchies is at: > > http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf > > section 2.3.1 In this example, if we provide apache.content that is accessable from apache.cgi, the policy writer can allow apache.cgi.user to access apache.content.user. When he writes the policy between apache.cgi and apache.content, he does not need to know rest of the upcoming child policies. At least, it also seems to me reasonable. If apache.cgi could access apache.content.user and apache.content.main being children of the apache.content automatically, such as attribute, we don't need to worry about this behavior.... :( Thanks, -- KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-05 0:39 ` KaiGai Kohei @ 2010-03-05 14:19 ` Joshua Brindle 2010-03-05 14:25 ` Stephen Smalley 0 siblings, 1 reply; 39+ messages in thread From: Joshua Brindle @ 2010-03-05 14:19 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei KaiGai Kohei wrote: > (2010/03/05 1:01), Joshua Brindle wrote: >> KaiGai Kohei wrote: >>> (2010/03/01 23:34), Stephen Smalley wrote: >>>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>>> writers. >>>>>>>>> OK, fair enough for me. >>>>>>>>> >>>>>>>>> This patch revert the commit of >>>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>>>>> However, at that time, we didn't find out the target side >>>>>>>>> boundary allows >>>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>>> process's security >>>>>>>>> context well. >>>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>>> This patch just tries to revert the changes by previous my patch, >>>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>>> original concern. >>>>>>> >>>>>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>>>>> Then, I introduced two options. The one is rough; that removes >>>>>>> boundary >>>>>>> checks in the target side. The other option tried to mask union >>>>>>> bits of >>>>>>> both of violated permissions on subject and target side boundaries >>>>>>> (*1). >>>>>>> >>>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>>> { >>>>>>> masked = 0; >>>>>>> >>>>>>> if (Sc has its bounds) >>>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>>> >>>>>>> if (Tc has its bounds) >>>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>>> >>>>>>> avd->allowed&= ~masked; >>>>>>> } >>>>>>> >>>>>>> However, the later option also requires policy writers special >>>>>>> treatments >>>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>>> >>>>>>> For example, when web server (httpd_t) launches a thread and >>>>>>> assign an >>>>>>> individual bounded security context (webapp_t), we don't need to take >>>>>>> a special treatment to access pseudo files labeled as webapp_t in the >>>>>>> original logic. >>>>>>> >>>>>>> If we adopt the logic introduced at (*1), when we write webapp_t's >>>>>>> policy, >>>>>>> we have to allow webapp_t domain to access files labeled as >>>>>>> httpd_t, not >>>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>>> will be >>>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>>> type or >>>>>>> webapp_t domain and httpd_t type. >>>>>> That seems wrong to me - we don't want webapp_t to be able to access >>>>>> the /proc/pid entries of other tasks running in httpd_t. We only want >>>>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>>>>> >>>>> Sorry for the late replying, because I've been unavailable last week. >>>>> >>>>> Yes, I also think it is unnatural to require webapp_t to have access >>>>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >>>>> the above logic. >>>>> >>>>> However, it does not solve the matter that Jacques pointed out the >>>>> meaning of the original logic is unclear. >>>>> >>>>> In addition, I pointed out the original logic can allow webapp_t >>>>> domain some permissions on the webapp_t type without permissions >>>>> of httpd_t which bounds webapp_t. >>>>> >>>>> Example) >>>>> allow httpd_t httpd_t : file { read }; >>>>> allow webapp_t webapp_t : file { read }; >>>>> >>>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>>> it is not masked because httpd_t also has same permissions on itself. >>>>> >>>>> It seems to me httpd_t should have permissions on webapp_t types from >>>>> the perspective of the definition of type boundary, even if we need to >>>>> modify existing security policy a bit. >>>>> (BTW, existing refpolicy does not use boundary right now.) >>>>> >>>>> I think we want webapp_t to have access rights (except for ones allowed >>>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >>>>> access rights on webapp_t types. It performs boundary of the webapp_t's >>>>> permissions as literal. >>>> I think I need to revisit the original design of the hierarchical types >>>> support, and how it compares with the extension policy logic that >>>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >>> It seems to me this article does not mention about a case when source and >>> target SIDs are in different parent-child-trees individually. >>> >>> For example, when webapp_t being a child of httpd_t tries to access files >>> labeled as webapp_content_t being a child of httpd_sys_content_t, it was >>> unclear for me what is an expected behavior. >>> (Perhaps, other part of the article may introduce it, but the volume of >>> the article is a bit large. :( ) >>> >> The original hierarchy specified that if httpd_t had e.g., write access >> to httpd_sys_content_t then webapp_t could be given write access to >> webapp_content_t without httpd_t having direct access to webapp_content_t. >> >> This was done so that, in policy access controls, parents could be >> decoupled from children while still allowing child subjects to access >> child objects. One application of this was to have parents that, >> themselves, did not have access to children objects (or were not active >> at all). > > Is this behavior really preferable? > > If my_app_t domain is allowed to access httpd_sys_content_t, and your_app_t > domain allowed to access webapp_content_t, it looks like here is no data > flows between two domains. However, in this example, webapp_t shares its > process local memory with httpd_t, the content of httpd_sys_content_t will > be visible webapp_t and vice versa. > In other words, all webapp_t domain can do are also allowed to httpd_t. > > my_app_t<-----> httpd_sys_content_t<-----> httpd_t .... > r/w r/w : process > : local memory > your_app_t<-----> webapp_content_t<-----> webapp_t ......: > r/w r/w > > We shouldn't have reused idea of the type hierarchy when we implemented > type boundary and multi-threading support? > > Or, is it really necessary for the policy management server the parent > types to be decoupled from the children? > >>>> Perhaps Joshua has a design tech note available on the hierarchical >>>> types design? >>> If not available, it is good idea to make clear what is the expected >>> behavior explicitly, on the wikipage for instance. >>> >> The original paper talking about type hierarchies is at: >> >> http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf >> >> section 2.3.1 > > In this example, if we provide apache.content that is accessable from apache.cgi, > the policy writer can allow apache.cgi.user to access apache.content.user. > When he writes the policy between apache.cgi and apache.content, he does not need > to know rest of the upcoming child policies. > At least, it also seems to me reasonable. > > If apache.cgi could access apache.content.user and apache.content.main being > children of the apache.content automatically, such as attribute, we don't need > to worry about this behavior.... :( > Well, we certainly didn't want an inherit permissions kind of system for the type hierarchy. It was an implicit constraint system so that permissions got smaller and smaller as you went down. The purpose of the type hierarchy was to protect the policy intent. The new behavior wrt threads and hierarchy allows extra permissions (namely, access to the parent domains memory) that were never present before so policy analysis will likely have to assume that they are combined domains, unfortunately. Is a dyntrans rule still required for threads to setcon to a child domain? -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-05 14:19 ` Joshua Brindle @ 2010-03-05 14:25 ` Stephen Smalley 2010-03-05 14:31 ` Joshua Brindle 0 siblings, 1 reply; 39+ messages in thread From: Stephen Smalley @ 2010-03-05 14:25 UTC (permalink / raw) To: Joshua Brindle; +Cc: KaiGai Kohei, Jacques Thomas, SE Linux, KaiGai Kohei On Fri, 2010-03-05 at 09:19 -0500, Joshua Brindle wrote: > KaiGai Kohei wrote: > > (2010/03/05 1:01), Joshua Brindle wrote: > >> KaiGai Kohei wrote: > >>> (2010/03/01 23:34), Stephen Smalley wrote: > >>>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: > >>>>> (2010/02/20 0:20), Stephen Smalley wrote: > >>>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: > >>>>>>> (2010/02/17 22:51), Stephen Smalley wrote: > >>>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: > >>>>>>>>>> I'd say we revert the changeset and restore the prior behavior. > >>>>>>>>>> I don't think we should impose the latter convention on policy > >>>>>>>>>> writers. > >>>>>>>>> OK, fair enough for me. > >>>>>>>>> > >>>>>>>>> This patch revert the commit of > >>>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 > >>>>>>>>> which removed a part of type_attribute_bounds_av as a dead code. > >>>>>>>>> However, at that time, we didn't find out the target side > >>>>>>>>> boundary allows > >>>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its > >>>>>>>>> process's security > >>>>>>>>> context well. > >>>>>>>> Does Jacques' original concern about the code still hold true? > >>>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 > >>>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 > >>>>>>> This patch just tries to revert the changes by previous my patch, > >>>>>>> and returns to the start point, so it also reverts the Jacques' > >>>>>>> original concern. > >>>>>>> > >>>>>>> At that time, IIRC, Jacques concerned about the logic being unclear. > >>>>>>> Then, I introduced two options. The one is rough; that removes > >>>>>>> boundary > >>>>>>> checks in the target side. The other option tried to mask union > >>>>>>> bits of > >>>>>>> both of violated permissions on subject and target side boundaries > >>>>>>> (*1). > >>>>>>> > >>>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) > >>>>>>> { > >>>>>>> masked = 0; > >>>>>>> > >>>>>>> if (Sc has its bounds) > >>>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); > >>>>>>> > >>>>>>> if (Tc has its bounds) > >>>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); > >>>>>>> > >>>>>>> avd->allowed&= ~masked; > >>>>>>> } > >>>>>>> > >>>>>>> However, the later option also requires policy writers special > >>>>>>> treatments > >>>>>>> to handle pseudo file entries labeled with parent's domain. > >>>>>>> > >>>>>>> For example, when web server (httpd_t) launches a thread and > >>>>>>> assign an > >>>>>>> individual bounded security context (webapp_t), we don't need to take > >>>>>>> a special treatment to access pseudo files labeled as webapp_t in the > >>>>>>> original logic. > >>>>>>> > >>>>>>> If we adopt the logic introduced at (*1), when we write webapp_t's > >>>>>>> policy, > >>>>>>> we have to allow webapp_t domain to access files labeled as > >>>>>>> httpd_t, not > >>>>>>> only webapp_t, because permissions between webapp_t and webapp_t > >>>>>>> will be > >>>>>>> eventually masked by one's between httpd_t domain and webapp_t > >>>>>>> type or > >>>>>>> webapp_t domain and httpd_t type. > >>>>>> That seems wrong to me - we don't want webapp_t to be able to access > >>>>>> the /proc/pid entries of other tasks running in httpd_t. We only want > >>>>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? > >>>>>> > >>>>> Sorry for the late replying, because I've been unavailable last week. > >>>>> > >>>>> Yes, I also think it is unnatural to require webapp_t to have access > >>>>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt > >>>>> the above logic. > >>>>> > >>>>> However, it does not solve the matter that Jacques pointed out the > >>>>> meaning of the original logic is unclear. > >>>>> > >>>>> In addition, I pointed out the original logic can allow webapp_t > >>>>> domain some permissions on the webapp_t type without permissions > >>>>> of httpd_t which bounds webapp_t. > >>>>> > >>>>> Example) > >>>>> allow httpd_t httpd_t : file { read }; > >>>>> allow webapp_t webapp_t : file { read }; > >>>>> > >>>>> In this case, webapp_t can read from files labeled as webapp_t, and > >>>>> it is not masked because httpd_t also has same permissions on itself. > >>>>> > >>>>> It seems to me httpd_t should have permissions on webapp_t types from > >>>>> the perspective of the definition of type boundary, even if we need to > >>>>> modify existing security policy a bit. > >>>>> (BTW, existing refpolicy does not use boundary right now.) > >>>>> > >>>>> I think we want webapp_t to have access rights (except for ones allowed > >>>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have > >>>>> access rights on webapp_t types. It performs boundary of the webapp_t's > >>>>> permissions as literal. > >>>> I think I need to revisit the original design of the hierarchical types > >>>> support, and how it compares with the extension policy logic that > >>>> inspired it (described in Section 4.2.2.3.2 on page 39 of: > >>>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) > >>> It seems to me this article does not mention about a case when source and > >>> target SIDs are in different parent-child-trees individually. > >>> > >>> For example, when webapp_t being a child of httpd_t tries to access files > >>> labeled as webapp_content_t being a child of httpd_sys_content_t, it was > >>> unclear for me what is an expected behavior. > >>> (Perhaps, other part of the article may introduce it, but the volume of > >>> the article is a bit large. :( ) > >>> > >> The original hierarchy specified that if httpd_t had e.g., write access > >> to httpd_sys_content_t then webapp_t could be given write access to > >> webapp_content_t without httpd_t having direct access to webapp_content_t. > >> > >> This was done so that, in policy access controls, parents could be > >> decoupled from children while still allowing child subjects to access > >> child objects. One application of this was to have parents that, > >> themselves, did not have access to children objects (or were not active > >> at all). > > > > Is this behavior really preferable? > > > > If my_app_t domain is allowed to access httpd_sys_content_t, and your_app_t > > domain allowed to access webapp_content_t, it looks like here is no data > > flows between two domains. However, in this example, webapp_t shares its > > process local memory with httpd_t, the content of httpd_sys_content_t will > > be visible webapp_t and vice versa. > > In other words, all webapp_t domain can do are also allowed to httpd_t. > > > > my_app_t<-----> httpd_sys_content_t<-----> httpd_t .... > > r/w r/w : process > > : local memory > > your_app_t<-----> webapp_content_t<-----> webapp_t ......: > > r/w r/w > > > > We shouldn't have reused idea of the type hierarchy when we implemented > > type boundary and multi-threading support? > > > > Or, is it really necessary for the policy management server the parent > > types to be decoupled from the children? > > > >>>> Perhaps Joshua has a design tech note available on the hierarchical > >>>> types design? > >>> If not available, it is good idea to make clear what is the expected > >>> behavior explicitly, on the wikipage for instance. > >>> > >> The original paper talking about type hierarchies is at: > >> > >> http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf > >> > >> section 2.3.1 > > > > In this example, if we provide apache.content that is accessable from apache.cgi, > > the policy writer can allow apache.cgi.user to access apache.content.user. > > When he writes the policy between apache.cgi and apache.content, he does not need > > to know rest of the upcoming child policies. > > At least, it also seems to me reasonable. > > > > If apache.cgi could access apache.content.user and apache.content.main being > > children of the apache.content automatically, such as attribute, we don't need > > to worry about this behavior.... :( > > > > Well, we certainly didn't want an inherit permissions kind of system for > the type hierarchy. It was an implicit constraint system so that > permissions got smaller and smaller as you went down. The purpose of the > type hierarchy was to protect the policy intent. > > The new behavior wrt threads and hierarchy allows extra permissions > (namely, access to the parent domains memory) that were never present > before so policy analysis will likely have to assume that they are > combined domains, unfortunately. > > Is a dyntrans rule still required for threads to setcon to a child domain? Yes. And dyntransition already implies that you are trusting the program to maintain any desired separation between the two security contexts, as already documented in the setcon(3) man page. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-05 14:25 ` Stephen Smalley @ 2010-03-05 14:31 ` Joshua Brindle 2010-03-08 6:56 ` KaiGai Kohei 0 siblings, 1 reply; 39+ messages in thread From: Joshua Brindle @ 2010-03-05 14:31 UTC (permalink / raw) To: Stephen Smalley; +Cc: KaiGai Kohei, Jacques Thomas, SE Linux, KaiGai Kohei Stephen Smalley wrote: > On Fri, 2010-03-05 at 09:19 -0500, Joshua Brindle wrote: >> KaiGai Kohei wrote: >>> (2010/03/05 1:01), Joshua Brindle wrote: >>>> KaiGai Kohei wrote: >>>>> (2010/03/01 23:34), Stephen Smalley wrote: >>>>>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>>>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>>>>> I'd say we revert the changeset and restore the prior behavior. >>>>>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>>>>> writers. >>>>>>>>>>> OK, fair enough for me. >>>>>>>>>>> >>>>>>>>>>> This patch revert the commit of >>>>>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>>>>> which removed a part of type_attribute_bounds_av as a dead code. >>>>>>>>>>> However, at that time, we didn't find out the target side >>>>>>>>>>> boundary allows >>>>>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>>>>> process's security >>>>>>>>>>> context well. >>>>>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>>>>> This patch just tries to revert the changes by previous my patch, >>>>>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>>>>> original concern. >>>>>>>>> >>>>>>>>> At that time, IIRC, Jacques concerned about the logic being unclear. >>>>>>>>> Then, I introduced two options. The one is rough; that removes >>>>>>>>> boundary >>>>>>>>> checks in the target side. The other option tried to mask union >>>>>>>>> bits of >>>>>>>>> both of violated permissions on subject and target side boundaries >>>>>>>>> (*1). >>>>>>>>> >>>>>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>>>>> { >>>>>>>>> masked = 0; >>>>>>>>> >>>>>>>>> if (Sc has its bounds) >>>>>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>>>>> >>>>>>>>> if (Tc has its bounds) >>>>>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>>>>> >>>>>>>>> avd->allowed&= ~masked; >>>>>>>>> } >>>>>>>>> >>>>>>>>> However, the later option also requires policy writers special >>>>>>>>> treatments >>>>>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>>>>> >>>>>>>>> For example, when web server (httpd_t) launches a thread and >>>>>>>>> assign an >>>>>>>>> individual bounded security context (webapp_t), we don't need to take >>>>>>>>> a special treatment to access pseudo files labeled as webapp_t in the >>>>>>>>> original logic. >>>>>>>>> >>>>>>>>> If we adopt the logic introduced at (*1), when we write webapp_t's >>>>>>>>> policy, >>>>>>>>> we have to allow webapp_t domain to access files labeled as >>>>>>>>> httpd_t, not >>>>>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>>>>> will be >>>>>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>>>>> type or >>>>>>>>> webapp_t domain and httpd_t type. >>>>>>>> That seems wrong to me - we don't want webapp_t to be able to access >>>>>>>> the /proc/pid entries of other tasks running in httpd_t. We only want >>>>>>>> it to be able to access its own /proc/pid entries in webapp_t. Yes? >>>>>>>> >>>>>>> Sorry for the late replying, because I've been unavailable last week. >>>>>>> >>>>>>> Yes, I also think it is unnatural to require webapp_t to have access >>>>>>> rights to /proc/pid entries labeled as httpd_t, if and when we adopt >>>>>>> the above logic. >>>>>>> >>>>>>> However, it does not solve the matter that Jacques pointed out the >>>>>>> meaning of the original logic is unclear. >>>>>>> >>>>>>> In addition, I pointed out the original logic can allow webapp_t >>>>>>> domain some permissions on the webapp_t type without permissions >>>>>>> of httpd_t which bounds webapp_t. >>>>>>> >>>>>>> Example) >>>>>>> allow httpd_t httpd_t : file { read }; >>>>>>> allow webapp_t webapp_t : file { read }; >>>>>>> >>>>>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>>>>> it is not masked because httpd_t also has same permissions on itself. >>>>>>> >>>>>>> It seems to me httpd_t should have permissions on webapp_t types from >>>>>>> the perspective of the definition of type boundary, even if we need to >>>>>>> modify existing security policy a bit. >>>>>>> (BTW, existing refpolicy does not use boundary right now.) >>>>>>> >>>>>>> I think we want webapp_t to have access rights (except for ones allowed >>>>>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t have >>>>>>> access rights on webapp_t types. It performs boundary of the webapp_t's >>>>>>> permissions as literal. >>>>>> I think I need to revisit the original design of the hierarchical types >>>>>> support, and how it compares with the extension policy logic that >>>>>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>>>>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >>>>> It seems to me this article does not mention about a case when source and >>>>> target SIDs are in different parent-child-trees individually. >>>>> >>>>> For example, when webapp_t being a child of httpd_t tries to access files >>>>> labeled as webapp_content_t being a child of httpd_sys_content_t, it was >>>>> unclear for me what is an expected behavior. >>>>> (Perhaps, other part of the article may introduce it, but the volume of >>>>> the article is a bit large. :( ) >>>>> >>>> The original hierarchy specified that if httpd_t had e.g., write access >>>> to httpd_sys_content_t then webapp_t could be given write access to >>>> webapp_content_t without httpd_t having direct access to webapp_content_t. >>>> >>>> This was done so that, in policy access controls, parents could be >>>> decoupled from children while still allowing child subjects to access >>>> child objects. One application of this was to have parents that, >>>> themselves, did not have access to children objects (or were not active >>>> at all). >>> Is this behavior really preferable? >>> >>> If my_app_t domain is allowed to access httpd_sys_content_t, and your_app_t >>> domain allowed to access webapp_content_t, it looks like here is no data >>> flows between two domains. However, in this example, webapp_t shares its >>> process local memory with httpd_t, the content of httpd_sys_content_t will >>> be visible webapp_t and vice versa. >>> In other words, all webapp_t domain can do are also allowed to httpd_t. >>> >>> my_app_t<-----> httpd_sys_content_t<-----> httpd_t .... >>> r/w r/w : process >>> : local memory >>> your_app_t<-----> webapp_content_t<-----> webapp_t ......: >>> r/w r/w >>> >>> We shouldn't have reused idea of the type hierarchy when we implemented >>> type boundary and multi-threading support? >>> >>> Or, is it really necessary for the policy management server the parent >>> types to be decoupled from the children? >>> >>>>>> Perhaps Joshua has a design tech note available on the hierarchical >>>>>> types design? >>>>> If not available, it is good idea to make clear what is the expected >>>>> behavior explicitly, on the wikipage for instance. >>>>> >>>> The original paper talking about type hierarchies is at: >>>> >>>> http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf >>>> >>>> section 2.3.1 >>> In this example, if we provide apache.content that is accessable from apache.cgi, >>> the policy writer can allow apache.cgi.user to access apache.content.user. >>> When he writes the policy between apache.cgi and apache.content, he does not need >>> to know rest of the upcoming child policies. >>> At least, it also seems to me reasonable. >>> >>> If apache.cgi could access apache.content.user and apache.content.main being >>> children of the apache.content automatically, such as attribute, we don't need >>> to worry about this behavior.... :( >>> >> Well, we certainly didn't want an inherit permissions kind of system for >> the type hierarchy. It was an implicit constraint system so that >> permissions got smaller and smaller as you went down. The purpose of the >> type hierarchy was to protect the policy intent. >> >> The new behavior wrt threads and hierarchy allows extra permissions >> (namely, access to the parent domains memory) that were never present >> before so policy analysis will likely have to assume that they are >> combined domains, unfortunately. >> >> Is a dyntrans rule still required for threads to setcon to a child domain? > > Yes. And dyntransition already implies that you are trusting the > program to maintain any desired separation between the two security > contexts, as already documented in the setcon(3) man page. > If dyntrans is still required then I don't see any change in behavior here. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-05 14:31 ` Joshua Brindle @ 2010-03-08 6:56 ` KaiGai Kohei 2010-03-08 15:27 ` Joshua Brindle 0 siblings, 1 reply; 39+ messages in thread From: KaiGai Kohei @ 2010-03-08 6:56 UTC (permalink / raw) To: Joshua Brindle; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei (2010/03/05 23:31), Joshua Brindle wrote: > > > Stephen Smalley wrote: >> On Fri, 2010-03-05 at 09:19 -0500, Joshua Brindle wrote: >>> KaiGai Kohei wrote: >>>> (2010/03/05 1:01), Joshua Brindle wrote: >>>>> KaiGai Kohei wrote: >>>>>> (2010/03/01 23:34), Stephen Smalley wrote: >>>>>>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>>>>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>>>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>>>>>> I'd say we revert the changeset and restore the prior >>>>>>>>>>>>> behavior. >>>>>>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>>>>>> writers. >>>>>>>>>>>> OK, fair enough for me. >>>>>>>>>>>> >>>>>>>>>>>> This patch revert the commit of >>>>>>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>>>>>> which removed a part of type_attribute_bounds_av as a dead >>>>>>>>>>>> code. >>>>>>>>>>>> However, at that time, we didn't find out the target side >>>>>>>>>>>> boundary allows >>>>>>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>>>>>> process's security >>>>>>>>>>>> context well. >>>>>>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>>>>>> This patch just tries to revert the changes by previous my patch, >>>>>>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>>>>>> original concern. >>>>>>>>>> >>>>>>>>>> At that time, IIRC, Jacques concerned about the logic being >>>>>>>>>> unclear. >>>>>>>>>> Then, I introduced two options. The one is rough; that removes >>>>>>>>>> boundary >>>>>>>>>> checks in the target side. The other option tried to mask union >>>>>>>>>> bits of >>>>>>>>>> both of violated permissions on subject and target side >>>>>>>>>> boundaries >>>>>>>>>> (*1). >>>>>>>>>> >>>>>>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>>>>>> { >>>>>>>>>> masked = 0; >>>>>>>>>> >>>>>>>>>> if (Sc has its bounds) >>>>>>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>>>>>> >>>>>>>>>> if (Tc has its bounds) >>>>>>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>>>>>> >>>>>>>>>> avd->allowed&= ~masked; >>>>>>>>>> } >>>>>>>>>> >>>>>>>>>> However, the later option also requires policy writers special >>>>>>>>>> treatments >>>>>>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>>>>>> >>>>>>>>>> For example, when web server (httpd_t) launches a thread and >>>>>>>>>> assign an >>>>>>>>>> individual bounded security context (webapp_t), we don't need >>>>>>>>>> to take >>>>>>>>>> a special treatment to access pseudo files labeled as webapp_t >>>>>>>>>> in the >>>>>>>>>> original logic. >>>>>>>>>> >>>>>>>>>> If we adopt the logic introduced at (*1), when we write >>>>>>>>>> webapp_t's >>>>>>>>>> policy, >>>>>>>>>> we have to allow webapp_t domain to access files labeled as >>>>>>>>>> httpd_t, not >>>>>>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>>>>>> will be >>>>>>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>>>>>> type or >>>>>>>>>> webapp_t domain and httpd_t type. >>>>>>>>> That seems wrong to me - we don't want webapp_t to be able to >>>>>>>>> access >>>>>>>>> the /proc/pid entries of other tasks running in httpd_t. We >>>>>>>>> only want >>>>>>>>> it to be able to access its own /proc/pid entries in webapp_t. >>>>>>>>> Yes? >>>>>>>>> >>>>>>>> Sorry for the late replying, because I've been unavailable last >>>>>>>> week. >>>>>>>> >>>>>>>> Yes, I also think it is unnatural to require webapp_t to have >>>>>>>> access >>>>>>>> rights to /proc/pid entries labeled as httpd_t, if and when we >>>>>>>> adopt >>>>>>>> the above logic. >>>>>>>> >>>>>>>> However, it does not solve the matter that Jacques pointed out the >>>>>>>> meaning of the original logic is unclear. >>>>>>>> >>>>>>>> In addition, I pointed out the original logic can allow webapp_t >>>>>>>> domain some permissions on the webapp_t type without permissions >>>>>>>> of httpd_t which bounds webapp_t. >>>>>>>> >>>>>>>> Example) >>>>>>>> allow httpd_t httpd_t : file { read }; >>>>>>>> allow webapp_t webapp_t : file { read }; >>>>>>>> >>>>>>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>>>>>> it is not masked because httpd_t also has same permissions on >>>>>>>> itself. >>>>>>>> >>>>>>>> It seems to me httpd_t should have permissions on webapp_t types >>>>>>>> from >>>>>>>> the perspective of the definition of type boundary, even if we >>>>>>>> need to >>>>>>>> modify existing security policy a bit. >>>>>>>> (BTW, existing refpolicy does not use boundary right now.) >>>>>>>> >>>>>>>> I think we want webapp_t to have access rights (except for ones >>>>>>>> allowed >>>>>>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t >>>>>>>> have >>>>>>>> access rights on webapp_t types. It performs boundary of the >>>>>>>> webapp_t's >>>>>>>> permissions as literal. >>>>>>> I think I need to revisit the original design of the hierarchical >>>>>>> types >>>>>>> support, and how it compares with the extension policy logic that >>>>>>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>>>>>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >>>>>> It seems to me this article does not mention about a case when >>>>>> source and >>>>>> target SIDs are in different parent-child-trees individually. >>>>>> >>>>>> For example, when webapp_t being a child of httpd_t tries to >>>>>> access files >>>>>> labeled as webapp_content_t being a child of httpd_sys_content_t, >>>>>> it was >>>>>> unclear for me what is an expected behavior. >>>>>> (Perhaps, other part of the article may introduce it, but the >>>>>> volume of >>>>>> the article is a bit large. :( ) >>>>>> >>>>> The original hierarchy specified that if httpd_t had e.g., write >>>>> access >>>>> to httpd_sys_content_t then webapp_t could be given write access to >>>>> webapp_content_t without httpd_t having direct access to >>>>> webapp_content_t. >>>>> >>>>> This was done so that, in policy access controls, parents could be >>>>> decoupled from children while still allowing child subjects to access >>>>> child objects. One application of this was to have parents that, >>>>> themselves, did not have access to children objects (or were not >>>>> active >>>>> at all). >>>> Is this behavior really preferable? >>>> >>>> If my_app_t domain is allowed to access httpd_sys_content_t, and >>>> your_app_t >>>> domain allowed to access webapp_content_t, it looks like here is no >>>> data >>>> flows between two domains. However, in this example, webapp_t shares >>>> its >>>> process local memory with httpd_t, the content of >>>> httpd_sys_content_t will >>>> be visible webapp_t and vice versa. >>>> In other words, all webapp_t domain can do are also allowed to httpd_t. >>>> >>>> my_app_t<-----> httpd_sys_content_t<-----> httpd_t .... >>>> r/w r/w : process >>>> : local memory >>>> your_app_t<-----> webapp_content_t<-----> webapp_t ......: >>>> r/w r/w >>>> >>>> We shouldn't have reused idea of the type hierarchy when we implemented >>>> type boundary and multi-threading support? >>>> >>>> Or, is it really necessary for the policy management server the parent >>>> types to be decoupled from the children? >>>> >>>>>>> Perhaps Joshua has a design tech note available on the hierarchical >>>>>>> types design? >>>>>> If not available, it is good idea to make clear what is the expected >>>>>> behavior explicitly, on the wikipage for instance. >>>>>> >>>>> The original paper talking about type hierarchies is at: >>>>> >>>>> http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf >>>>> >>>>> >>>>> section 2.3.1 >>>> In this example, if we provide apache.content that is accessable >>>> from apache.cgi, >>>> the policy writer can allow apache.cgi.user to access >>>> apache.content.user. >>>> When he writes the policy between apache.cgi and apache.content, he >>>> does not need >>>> to know rest of the upcoming child policies. >>>> At least, it also seems to me reasonable. >>>> >>>> If apache.cgi could access apache.content.user and >>>> apache.content.main being >>>> children of the apache.content automatically, such as attribute, we >>>> don't need >>>> to worry about this behavior.... :( >>>> >>> Well, we certainly didn't want an inherit permissions kind of system for >>> the type hierarchy. It was an implicit constraint system so that >>> permissions got smaller and smaller as you went down. The purpose of the >>> type hierarchy was to protect the policy intent. >>> >>> The new behavior wrt threads and hierarchy allows extra permissions >>> (namely, access to the parent domains memory) that were never present >>> before so policy analysis will likely have to assume that they are >>> combined domains, unfortunately. >>> >>> Is a dyntrans rule still required for threads to setcon to a child >>> domain? >> >> Yes. And dyntransition already implies that you are trusting the >> program to maintain any desired separation between the two security >> contexts, as already documented in the setcon(3) man page. >> > > If dyntrans is still required then I don't see any change in behavior here. Unlike setcon(3) in a single-thread-process, a worker thread can see its process local memory which can contain the information to be invisible imported by other thread after setcon(3). Is there any fundamental differences? It seems me here is no fundamental differences, but I may miss something. Thanks, -- KaiGai Kohei <kaigai@ak.jp.nec.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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-03-08 6:56 ` KaiGai Kohei @ 2010-03-08 15:27 ` Joshua Brindle 0 siblings, 0 replies; 39+ messages in thread From: Joshua Brindle @ 2010-03-08 15:27 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, Jacques Thomas, SE Linux, KaiGai Kohei KaiGai Kohei wrote: > (2010/03/05 23:31), Joshua Brindle wrote: >> >> Stephen Smalley wrote: >>> On Fri, 2010-03-05 at 09:19 -0500, Joshua Brindle wrote: >>>> KaiGai Kohei wrote: >>>>> (2010/03/05 1:01), Joshua Brindle wrote: >>>>>> KaiGai Kohei wrote: >>>>>>> (2010/03/01 23:34), Stephen Smalley wrote: >>>>>>>> On Mon, 2010-03-01 at 11:43 +0900, KaiGai Kohei wrote: >>>>>>>>> (2010/02/20 0:20), Stephen Smalley wrote: >>>>>>>>>> On Fri, 2010-02-19 at 16:33 +0900, KaiGai Kohei wrote: >>>>>>>>>>> (2010/02/17 22:51), Stephen Smalley wrote: >>>>>>>>>>>> On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: >>>>>>>>>>>>>> I'd say we revert the changeset and restore the prior >>>>>>>>>>>>>> behavior. >>>>>>>>>>>>>> I don't think we should impose the latter convention on policy >>>>>>>>>>>>>> writers. >>>>>>>>>>>>> OK, fair enough for me. >>>>>>>>>>>>> >>>>>>>>>>>>> This patch revert the commit of >>>>>>>>>>>>> 7d52a155e38d5a165759dbbee656455861bf7801 >>>>>>>>>>>>> which removed a part of type_attribute_bounds_av as a dead >>>>>>>>>>>>> code. >>>>>>>>>>>>> However, at that time, we didn't find out the target side >>>>>>>>>>>>> boundary allows >>>>>>>>>>>>> to handle some of pseudo /proc/<pid>/* entries with its >>>>>>>>>>>>> process's security >>>>>>>>>>>>> context well. >>>>>>>>>>>> Does Jacques' original concern about the code still hold true? >>>>>>>>>>>> http://marc.info/?l=selinux&m=125770868309928&w=2 >>>>>>>>>>>> http://marc.info/?l=selinux&m=125851264424682&w=2 >>>>>>>>>>> This patch just tries to revert the changes by previous my patch, >>>>>>>>>>> and returns to the start point, so it also reverts the Jacques' >>>>>>>>>>> original concern. >>>>>>>>>>> >>>>>>>>>>> At that time, IIRC, Jacques concerned about the logic being >>>>>>>>>>> unclear. >>>>>>>>>>> Then, I introduced two options. The one is rough; that removes >>>>>>>>>>> boundary >>>>>>>>>>> checks in the target side. The other option tried to mask union >>>>>>>>>>> bits of >>>>>>>>>>> both of violated permissions on subject and target side >>>>>>>>>>> boundaries >>>>>>>>>>> (*1). >>>>>>>>>>> >>>>>>>>>>> (*1) type_attribute_bounds_av(Sc,Tc, ...) >>>>>>>>>>> { >>>>>>>>>>> masked = 0; >>>>>>>>>>> >>>>>>>>>>> if (Sc has its bounds) >>>>>>>>>>> masked |= P(Sc,Tc)& ~P(Sp,Tc); >>>>>>>>>>> >>>>>>>>>>> if (Tc has its bounds) >>>>>>>>>>> masked |= P(Sc,Tc)& ~P(Sc,Tp); >>>>>>>>>>> >>>>>>>>>>> avd->allowed&= ~masked; >>>>>>>>>>> } >>>>>>>>>>> >>>>>>>>>>> However, the later option also requires policy writers special >>>>>>>>>>> treatments >>>>>>>>>>> to handle pseudo file entries labeled with parent's domain. >>>>>>>>>>> >>>>>>>>>>> For example, when web server (httpd_t) launches a thread and >>>>>>>>>>> assign an >>>>>>>>>>> individual bounded security context (webapp_t), we don't need >>>>>>>>>>> to take >>>>>>>>>>> a special treatment to access pseudo files labeled as webapp_t >>>>>>>>>>> in the >>>>>>>>>>> original logic. >>>>>>>>>>> >>>>>>>>>>> If we adopt the logic introduced at (*1), when we write >>>>>>>>>>> webapp_t's >>>>>>>>>>> policy, >>>>>>>>>>> we have to allow webapp_t domain to access files labeled as >>>>>>>>>>> httpd_t, not >>>>>>>>>>> only webapp_t, because permissions between webapp_t and webapp_t >>>>>>>>>>> will be >>>>>>>>>>> eventually masked by one's between httpd_t domain and webapp_t >>>>>>>>>>> type or >>>>>>>>>>> webapp_t domain and httpd_t type. >>>>>>>>>> That seems wrong to me - we don't want webapp_t to be able to >>>>>>>>>> access >>>>>>>>>> the /proc/pid entries of other tasks running in httpd_t. We >>>>>>>>>> only want >>>>>>>>>> it to be able to access its own /proc/pid entries in webapp_t. >>>>>>>>>> Yes? >>>>>>>>>> >>>>>>>>> Sorry for the late replying, because I've been unavailable last >>>>>>>>> week. >>>>>>>>> >>>>>>>>> Yes, I also think it is unnatural to require webapp_t to have >>>>>>>>> access >>>>>>>>> rights to /proc/pid entries labeled as httpd_t, if and when we >>>>>>>>> adopt >>>>>>>>> the above logic. >>>>>>>>> >>>>>>>>> However, it does not solve the matter that Jacques pointed out the >>>>>>>>> meaning of the original logic is unclear. >>>>>>>>> >>>>>>>>> In addition, I pointed out the original logic can allow webapp_t >>>>>>>>> domain some permissions on the webapp_t type without permissions >>>>>>>>> of httpd_t which bounds webapp_t. >>>>>>>>> >>>>>>>>> Example) >>>>>>>>> allow httpd_t httpd_t : file { read }; >>>>>>>>> allow webapp_t webapp_t : file { read }; >>>>>>>>> >>>>>>>>> In this case, webapp_t can read from files labeled as webapp_t, and >>>>>>>>> it is not masked because httpd_t also has same permissions on >>>>>>>>> itself. >>>>>>>>> >>>>>>>>> It seems to me httpd_t should have permissions on webapp_t types >>>>>>>>> from >>>>>>>>> the perspective of the definition of type boundary, even if we >>>>>>>>> need to >>>>>>>>> modify existing security policy a bit. >>>>>>>>> (BTW, existing refpolicy does not use boundary right now.) >>>>>>>>> >>>>>>>>> I think we want webapp_t to have access rights (except for ones >>>>>>>>> allowed >>>>>>>>> explicitly) on the httpd_t, but it is not unnatural that httpd_t >>>>>>>>> have >>>>>>>>> access rights on webapp_t types. It performs boundary of the >>>>>>>>> webapp_t's >>>>>>>>> permissions as literal. >>>>>>>> I think I need to revisit the original design of the hierarchical >>>>>>>> types >>>>>>>> support, and how it compares with the extension policy logic that >>>>>>>> inspired it (described in Section 4.2.2.3.2 on page 39 of: >>>>>>>> http://www.cs.utah.edu/flux/fluke/html/ftls.ps.gz ) >>>>>>> It seems to me this article does not mention about a case when >>>>>>> source and >>>>>>> target SIDs are in different parent-child-trees individually. >>>>>>> >>>>>>> For example, when webapp_t being a child of httpd_t tries to >>>>>>> access files >>>>>>> labeled as webapp_content_t being a child of httpd_sys_content_t, >>>>>>> it was >>>>>>> unclear for me what is an expected behavior. >>>>>>> (Perhaps, other part of the article may introduce it, but the >>>>>>> volume of >>>>>>> the article is a bit large. :( ) >>>>>>> >>>>>> The original hierarchy specified that if httpd_t had e.g., write >>>>>> access >>>>>> to httpd_sys_content_t then webapp_t could be given write access to >>>>>> webapp_content_t without httpd_t having direct access to >>>>>> webapp_content_t. >>>>>> >>>>>> This was done so that, in policy access controls, parents could be >>>>>> decoupled from children while still allowing child subjects to access >>>>>> child objects. One application of this was to have parents that, >>>>>> themselves, did not have access to children objects (or were not >>>>>> active >>>>>> at all). >>>>> Is this behavior really preferable? >>>>> >>>>> If my_app_t domain is allowed to access httpd_sys_content_t, and >>>>> your_app_t >>>>> domain allowed to access webapp_content_t, it looks like here is no >>>>> data >>>>> flows between two domains. However, in this example, webapp_t shares >>>>> its >>>>> process local memory with httpd_t, the content of >>>>> httpd_sys_content_t will >>>>> be visible webapp_t and vice versa. >>>>> In other words, all webapp_t domain can do are also allowed to httpd_t. >>>>> >>>>> my_app_t<-----> httpd_sys_content_t<-----> httpd_t .... >>>>> r/w r/w : process >>>>> : local memory >>>>> your_app_t<-----> webapp_content_t<-----> webapp_t ......: >>>>> r/w r/w >>>>> >>>>> We shouldn't have reused idea of the type hierarchy when we implemented >>>>> type boundary and multi-threading support? >>>>> >>>>> Or, is it really necessary for the policy management server the parent >>>>> types to be decoupled from the children? >>>>> >>>>>>>> Perhaps Joshua has a design tech note available on the hierarchical >>>>>>>> types design? >>>>>>> If not available, it is good idea to make clear what is the expected >>>>>>> behavior explicitly, on the wikipage for instance. >>>>>>> >>>>>> The original paper talking about type hierarchies is at: >>>>>> >>>>>> http://selinux-symposium.org/2006/papers/01-policy-management-server.pdf >>>>>> >>>>>> >>>>>> section 2.3.1 >>>>> In this example, if we provide apache.content that is accessable >>>>> from apache.cgi, >>>>> the policy writer can allow apache.cgi.user to access >>>>> apache.content.user. >>>>> When he writes the policy between apache.cgi and apache.content, he >>>>> does not need >>>>> to know rest of the upcoming child policies. >>>>> At least, it also seems to me reasonable. >>>>> >>>>> If apache.cgi could access apache.content.user and >>>>> apache.content.main being >>>>> children of the apache.content automatically, such as attribute, we >>>>> don't need >>>>> to worry about this behavior.... :( >>>>> >>>> Well, we certainly didn't want an inherit permissions kind of system for >>>> the type hierarchy. It was an implicit constraint system so that >>>> permissions got smaller and smaller as you went down. The purpose of the >>>> type hierarchy was to protect the policy intent. >>>> >>>> The new behavior wrt threads and hierarchy allows extra permissions >>>> (namely, access to the parent domains memory) that were never present >>>> before so policy analysis will likely have to assume that they are >>>> combined domains, unfortunately. >>>> >>>> Is a dyntrans rule still required for threads to setcon to a child >>>> domain? >>> Yes. And dyntransition already implies that you are trusting the >>> program to maintain any desired separation between the two security >>> contexts, as already documented in the setcon(3) man page. >>> >> If dyntrans is still required then I don't see any change in behavior here. > > Unlike setcon(3) in a single-thread-process, a worker thread can see its > process local memory which can contain the information to be invisible > imported by other thread after setcon(3). > Is there any fundamental differences? > > It seems me here is no fundamental differences, but I may miss something. > Correct, there is no fundamental difference. It sounds like the hierarchy behavior is how we intended it originally. -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-16 23:49 ` KaiGai Kohei 2010-02-17 13:51 ` Stephen Smalley @ 2010-02-19 17:31 ` Stephen Smalley 2010-02-21 22:09 ` James Morris 2 siblings, 0 replies; 39+ messages in thread From: Stephen Smalley @ 2010-02-19 17:31 UTC (permalink / raw) To: KaiGai Kohei; +Cc: method, Jacques Thomas, SE Linux, James Morris On Wed, 2010-02-17 at 08:49 +0900, KaiGai Kohei wrote: > > I'd say we revert the changeset and restore the prior behavior. > > I don't think we should impose the latter convention on policy writers. > > OK, fair enough for me. > > This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 > which removed a part of type_attribute_bounds_av as a dead code. > However, at that time, we didn't find out the target side boundary allows > to handle some of pseudo /proc/<pid>/* entries with its process's security > context well. > > Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> James - this is just a revert of the prior commit. Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > -- > security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++++++++--- > 1 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index 4e976f5..42d423c 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -524,14 +524,16 @@ static void type_attribute_bounds_av(struct context *scontext, > u16 tclass, > struct av_decision *avd) > { > + struct context lo_scontext; > + struct context lo_tcontext; > + struct av_decision lo_avd; > struct type_datum *source > = policydb.type_val_to_struct[scontext->type - 1]; > + struct type_datum *target > + = policydb.type_val_to_struct[tcontext->type - 1]; > + u32 masked = 0; > > if (source->bounds) { > - struct context lo_scontext; > - struct av_decision lo_avd; > - u32 masked; > - > memset(&lo_avd, 0, sizeof(lo_avd)); > > memcpy(&lo_scontext, scontext, sizeof(lo_scontext)); > @@ -544,7 +546,40 @@ static void type_attribute_bounds_av(struct context *scontext, > if ((lo_avd.allowed & avd->allowed) == avd->allowed) > return; /* no masked permission */ > masked = ~lo_avd.allowed & avd->allowed; > + } > + > + if (target->bounds) { > + memset(&lo_avd, 0, sizeof(lo_avd)); > + > + memcpy(&lo_tcontext, tcontext, sizeof(lo_tcontext)); > + lo_tcontext.type = target->bounds; > + > + context_struct_compute_av(scontext, > + &lo_tcontext, > + tclass, > + &lo_avd); > + if ((lo_avd.allowed & avd->allowed) == avd->allowed) > + return; /* no masked permission */ > + masked = ~lo_avd.allowed & avd->allowed; > + } > + > + if (source->bounds && target->bounds) { > + memset(&lo_avd, 0, sizeof(lo_avd)); > + /* > + * lo_scontext and lo_tcontext are already > + * set up. > + */ > + > + context_struct_compute_av(&lo_scontext, > + &lo_tcontext, > + tclass, > + &lo_avd); > + if ((lo_avd.allowed & avd->allowed) == avd->allowed) > + return; /* no masked permission */ > + masked = ~lo_avd.allowed & avd->allowed; > + } > > + if (masked) { > /* mask violated permissions */ > avd->allowed &= ~masked; > -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() 2010-02-16 23:49 ` KaiGai Kohei 2010-02-17 13:51 ` Stephen Smalley 2010-02-19 17:31 ` Stephen Smalley @ 2010-02-21 22:09 ` James Morris 2 siblings, 0 replies; 39+ messages in thread From: James Morris @ 2010-02-21 22:09 UTC (permalink / raw) To: KaiGai Kohei; +Cc: Stephen Smalley, method, Jacques Thomas, SE Linux On Wed, 17 Feb 2010, KaiGai Kohei wrote: > > I'd say we revert the changeset and restore the prior behavior. > > I don't think we should impose the latter convention on policy writers. > > OK, fair enough for me. > > This patch revert the commit of 7d52a155e38d5a165759dbbee656455861bf7801 > which removed a part of type_attribute_bounds_av as a dead code. > However, at that time, we didn't find out the target side boundary allows > to handle some of pseudo /proc/<pid>/* entries with its process's security > context well. > > Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com> Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next -- James Morris <jmorris@namei.org> -- 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. ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2010-03-08 15:27 UTC | newest] Thread overview: 39+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-08 19:24 Type boundaries: questions on the semantics / is the enforcement correct ? Jacques Thomas 2009-11-18 2:44 ` KaiGai Kohei 2009-11-19 16:07 ` Jacques Thomas 2009-11-30 18:40 ` Jacques Thomas 2009-12-01 1:02 ` KaiGai Kohei 2009-12-01 3:53 ` Jacques Thomas 2009-12-08 19:46 ` Joshua Brindle 2010-01-15 15:51 ` Stephen Smalley 2010-01-18 10:10 ` KaiGai Kohei 2010-01-20 4:25 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 2010-01-20 13:33 ` Stephen Smalley 2010-01-20 16:29 ` contents of /etc/selinux/<type>/contexts/users/* Hasan Rezaul-CHR010 2010-01-20 17:33 ` Stephen Smalley 2010-01-21 6:00 ` [PATCH 1/2] selinux: remove dead code in type_attribute_bounds_av() KaiGai Kohei 2010-01-21 14:08 ` Stephen Smalley 2010-01-24 22:24 ` James Morris 2010-01-20 4:26 ` [PATCH 2/2] libsepol: remove dead code in check_avtab_hierarchy_callback() KaiGai Kohei 2010-02-05 5:42 ` KaiGai Kohei 2010-02-05 14:50 ` Stephen Smalley 2010-02-09 6:46 ` KaiGai Kohei 2010-02-16 2:36 ` KaiGai Kohei 2010-02-16 15:25 ` Stephen Smalley 2010-02-16 23:49 ` KaiGai Kohei 2010-02-17 13:51 ` Stephen Smalley 2010-02-19 7:33 ` KaiGai Kohei 2010-02-19 15:20 ` Stephen Smalley 2010-03-01 2:43 ` KaiGai Kohei 2010-03-01 14:34 ` Stephen Smalley 2010-03-03 8:09 ` KaiGai Kohei 2010-03-04 16:01 ` Joshua Brindle 2010-03-04 16:24 ` Joshua Brindle 2010-03-05 0:39 ` KaiGai Kohei 2010-03-05 14:19 ` Joshua Brindle 2010-03-05 14:25 ` Stephen Smalley 2010-03-05 14:31 ` Joshua Brindle 2010-03-08 6:56 ` KaiGai Kohei 2010-03-08 15:27 ` Joshua Brindle 2010-02-19 17:31 ` Stephen Smalley 2010-02-21 22:09 ` James Morris
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.