On Thu, Feb 8, 2018 at 10:02 AM, Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> On Wed, 2018-02-07 at 14:56 -0500, Paul Moore wrote:
>> On Wed, Feb 7, 2018 at 12:48 PM, Stephen Smalley <sds@xxxxxxxxxxxxx>
>> wrote:
>> > On Tue, 2018-02-06 at 17:18 -0500, Paul Moore wrote:
>>
>> ...
>>
>> > > While I don't think we need to tackle this as part of the
>> > > encapsulation work, this is another reminder that we should look
>> > > into
>> > > breaking the separation between the security server and the
>> > > Linux/hooks code. I understand there were historical reasons for
>> > > this
>> > > split, but I think all of those reasons are now gone, and further
>> > > I
>> > > think enough Linux'isms have crept into the security server that
>> > > the
>> > > separation is no longer as meaningful as it may have been in the
>> > > past.
>> >
>> > I think we want to retain some degree of encapsulation of the
>> > policy
>> > logic and data structures, which is what the security server is
>> > supposed to provide. That allows us to evolve that logic and
>> > structures without impacting the object label management and
>> > permission
>> > enforcement code. Separation of policy from mechanism. I don't
>> > mind
>> > nativizing the security server for Linux, and where appropriate,
>> > allowing some optimization of the interfaces between it and the
>> > rest of
>> > the SELinux code, but I wouldn't want to e.g. directly expose the
>> > policydb to the rest of the code. There has been some leakage of
>> > policy awareness outside the security server in the past but I view
>> > that as a mistake that ought to be corrected over time.
>>
>> I agree that a level of abstraction between the policydb code and the
>> enforcement code is a good thing, but I think there are some
>> boundaries between the hook code and the security server that we
>> could
>> do without. Once again, not really part of this work, just popped up
>> in my head again while looking at these patches.
>
> I wanted to clarify that point because it motivates keeping selinux_ss
> as a separate struct from selinux_state (previously selinux_ns), and
> not exposing the selinux_ss struct definition outside of the security
> server. So the selinux_state struct would still be something like:
> struct selinux_state {
> bool disabled;
> #ifdef CONFIG_SECURITY_SELINUX_DEVELOP
> bool enforcing;
> #endif
> bool checkreqprot;
> bool initialized;
> bool policycap[__POLICYDB_CAPABILITY_MAX];
> struct selinux_avc *avc;
> struct selinux_ss *ss;
> }
>
> (dropping the reference count, work struct, and parent pointers that
> were part of selinux_ns as being specific to the namespace work).
I think keeping the selinux_avc and selinux_ss structs separate is
fine right now, as I mentioned earlier, I really don't want to
entangle the encapsulation work with any potential hooks/ss
consolidation work.
Directly embedding the selinux_avc and selinux_ss structures inside
selinux_state would be nice as we could just do one allocation, but
that isn't worth breaking the abstraction at this point. Maybe we
will get there at some point, but I think it's important to stay
focused on the task at hand.
> hooks.c would declare struct selinux_state selinux_state;.
> ss/services.c would declare static struct selinux_ss selinux_ss; and
> provide a selinux_ss_init() function (instead of selinux_ss_create)
> that would set the ->ss field to &selinux_ss. Likewise for avc.c and
> the selinux_avc. hooks.c:selinux_init() would call
> selinux_ss_init(&selinux_state.ss) and
> selinux_avc_init(&selinux_state.avc) to set those pointers to the
> appropriate structures private to the ss and avc code. The _create and
> _free functions would go away and none of the structures would be
> dynamically allocated/freed.
>
> This is in contrast to exposing the selinux_ss struct definition
> outside the security server and directly embedding it in the
> selinux_state, since that would require exposing the policydb and
> sidtab structures as well (unless we were to make those opaque pointers
> within the selinux_ss; currently the policydb and sidtab are directly
> embedded within it). Likewise for the selinux_avc and its embedded
> avc_cache structure.
>
> Trying to make sure we're in agreement on the data structures before I
> rewrite since I don't want to have to rewrite it twice ;)
--
paul moore
www.paul-moore.com
