Re: [RFC][PATCH] selinux: dynamic class/perm discovery

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2009-09-24 at 17:41 +0900, KaiGai Kohei wrote:
> Stephen Smalley wrote:
> > This needs more testing, but I thought I would circulate it for review.
> > 
> > Modify SELinux to dynamically discover class and permission values
> > upon policy load, based on the dynamic object class/perm discovery
> > logic from libselinux.  A mapping is created between kernel-private
> > class and permission indices used outside the security server and the
> > policy values used within the security server.
> 
> I'm grad to see this patch. I've been unable to pay my efforts for
> a few weeks due to the PostgreSQL works, although I mentioned this
> feature before. Thanks.
> 
> Some of random comments are bellow.
> 
> * It is confusable to use "tclass" always.
> When we get this patch, we need to handle the value stored within
> tclass carefully to distinguish whether it means the internal-code
> defined in the flask.h or the external-code defined in the current
> security policy.
> 
> In my opinion, we should not use the value of tclass to store the
> external-code, to avoid future possible confusion. For example,
> context_struct_compute_av() requires "tclass", but it implicitly
> assumes the external-code will be given.
> Is it possible to rename all the "tclass" which stores the external
> -code into "tclass_ex" instead?

context_struct_compute_av() only deals with the policy values.
security_compute_av() unmaps the kernel-private index value on entry and
maps the resulting decision on exit.  The functions that may require
mapping use orig_tclass vs tclass to distinguish them.

> * Audit denied messages due to the undefined permissions?
> When the loaded security policy does not have a definition of kernel
> object classes and its allow_unknown is false, the required kernel
> permission can be denied.
> In this case, context_struct_compute_av() initializes avd->allowed
> by 0UL and avd->auditdeny by 0xffffffffUL. Then, map_decision() drops
> bits from avd->auditdeny due to the undefined object class/access vector.
> This av_decision is inserted into the kernel AVC, then it will be refered
> at the avc_dump_av(). Finally, this policy will silently prevent kernel
> permissions being not defined in the security policy.
> What is the preferable behavior in this situation?

I've modified the logic to also add undefined permissions to the
auditdeny vector if !allow_unknown, so we should get audit messages for
their denials in that case.

> * What is the reason why "classmap.h", instead of "classmap.c"?
> The avc.c includes the classmap.h which defines secclass_map array.
> But it seems to me more straightforward manner to add classmap.c
> which provide only secclass_map[] array definition.

Needs to be included by mdp.c and genheaders.c as well.

> 
> * May be a short-tempered requirement.
> 
> It will be preferable, if userspace object manager can make a query
> using object class and access vectors with text representation, not
> the results of string_to_security_class(), because userspaces cannot
> make sure the string_to_security_class() and security_compute_av()
> are handled atomically.
> 
> The security policy may be reloaded between the string_to_security_class()
> and security_compute_av() in a corner case.
> BTW, SE-PostgreSQL checks sequencial number of security policy, and redo
> checks if the security policy reloaded. But it is not perfect. The netlink
> socket message can be delayed. :-(
>   http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.4.x/sepgsql/src/backend/security/sepgsql/avc.c#565
> 
> If the text -> code translation and lookups of security policy can be done
> within a single read_lock(&policy_rwlock) block, we can guarantee
> security_compute_av() is not invoked based on incorrect object class code.

We could either add a new node to selinuxfs that takes the string
representation, or just modify the existing handler functions to
automatically detect whether they were passed an integer or a string and
act accordingly.  But I'd view that as a separate follow-on patch.

-- 
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@xxxxxxxxxxxxx with
the words "unsubscribe selinux" without quotes as the message.

[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux