Hi! Thanks for your proof of concept.
>From observing your patch, you should be able to develop this as an engine once we make changes to the modular engine subsystem based on your feedback. Your feedback on get() being called inside another function is a good point (thanks!) and I believe some of us have come to an agreement to refactor that area. As for getting your hands on the connection socket, Trond is willing to add it to the API. Toru 2010/2/4 KaiGai Kohei <[email protected]>: > The attached patch is a proof of the concept. > > It enables to apply access controls on every client's request based on > the access control decision of SELinux. > In my plan, SELinux specific routines will be pluggable using modular > architecture, but, right now, this patch implements this feature as > a static logic to simplify the code. > > All the functions are prefixed as "semcd_*" to avoid namespace conflicts. > It means "secure memcached". > From viewpoint of the core memcached routines, these functions perform as > security provider. It anyway returns its access control decision (whether > it should be allowed, or not) based on its own security policy, and the > caller has to control the code path according to the decision. > > For example, process_get_command() calls semcd_get_command() before returning > data contents of the specified item, then it returns either true or false. > If false, the caller (process_get_command) returns error. It does not depend > on the way to make access control decision. (For example, we can assume > a security model based on the SASL authenticated user, not only SELinux.) > > @@ -2543,6 +2584,11 @@ static inline void process_get_command(conn *c, > token_t *tokens, size_t ntokens, > stats_prefix_record_get(key, nkey, NULL != it); > } > if (it) { > + if (!semcd_get_command(c, it)) { > + out_string(c, "SERVER_ERROR access denied"); > + item_remove(it); > + return; > + } > > This patch modified two data structure to contain security attributes. > The conn::semcd_private and item::semcd_private is used to store > security_id_t (which is a pointer for a pair of security context in text > representation and reference counter) object. > > And the following hooks allows to make access control decision at the > strategic points in memcached. > > extern bool semcd_get_command(conn *c, item *it); > extern bool semcd_update_command(conn *c, item *old_it, item *new_it, int > comm); > extern bool semcd_delta_command(conn *c, item *it, bool incr); > extern bool semcd_delete_command(conn *c, item *it); > extern bool semcd_flush_command(conn *c); > > When SELinux support is not enabled, these function calls are replaced by > (true). It means this code does not affect anything when security feature > is not enabled. > > The "test_policy.te" is a development purpose security policy. > It can be built and installed as follows: > > % make -f /usr/share/selinux/devel/Makefile test_policy.pp > # su > # semodule -i test_policy.pp > # setsebool semcd_enable_auditallow 1 > > It is a working example: > > [kai...@saba memcached]$ ./memcached -v -s /tmp/memcached.sock > selinux: semcd_initialize() done > selinux: new connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > avc: granted { create } for > scontext=unconfined_u:unconfined_r:unconfined_t:s0:c0 \ > tcontext=unconfined_u:object_r:semcd_item_t:s0:c0 tclass=db_blob > selinux: close connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > --> connection from "...:s0:c0", new item was also labeled as "...:s0:c0". > > selinux: new connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c1' > avc: granted { create } for > scontext=unconfined_u:unconfined_r:unconfined_t:s0:c1 \ > tcontext=unconfined_u:object_r:semcd_item_t:s0:c1 tclass=db_blob > selinux: close connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c1' > --> connection from "...:s0:c1", new item was also labeled as "...:s0:c1" > > selinux: new connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > avc: granted { read } for > scontext=unconfined_u:unconfined_r:unconfined_t:s0:c0 \ > tcontext=unconfined_u:object_r:semcd_item_t:s0:c0 tclass=db_blob > selinux: close connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > --> connection from "...:s0:c0" can read an item labeled as "...:s0:c0" > > selinux: new connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > avc: denied { read } for > scontext=unconfined_u:unconfined_r:unconfined_t:s0:c0 \ > tcontext=unconfined_u:object_r:semcd_item_t:s0:c1 tclass=db_blob > selinux: close connection from 'unconfined_u:unconfined_r:unconfined_t:s0:c0' > --> but connection from "...:s0:c0" cannot read an item labeled as > "...:s0:c1" > > Note that we recommend to use unix domain socket for development purpose, > because > it does not need to set up labeled network configuration between server and > client. > > Any comments are welcome. > > Thanks, > > (2010/01/22 16:10), KaiGai Kohei wrote: >> (2010/01/22 5:36), Dustin wrote: >>> >>> On Jan 21, 1:00 am, KaiGai Kohei<[email protected]> wrote: >>> >>>> * Again, the engine framework is not designed to host multiple modules >>>> in usual way. If a security module occupies the engine framework, it >>>> also has to load another module that provide physical storage engine, >>>> by itself. (Or, security module also has to provide i/o feature? it >>>> is quite nonsense.) >>> >>> It's not *entirely* clear what you mean by this. I do use the >>> engine to load multiple sub-engines that are separate and provide >>> security isolation. >> >> What I wanted to say is, it is not an essential job for the security >> module to load and manage sub-modules, although it is possible. >> >> Basically, I don't think the security module should touch something >> corresponding to physical i/o operations. As long as user have enough >> privileges, it should call the secondary engine module without any >> preventions. >> >> If it does not need any additional properties on the session and >> items to be accessed, the matter is not too hard. >> The security module just track the entry points of secondary module, >> and can call it on its invocations. >> >> However, the secondary module may want to use conn->engine_storage, >> although the security module also want to cache the user's privilege. >> In addition, format of the item object is depending on the secondary >> module, so the security module cannot modify it in discretionary. >> >> If we focus on the security design that enforces simple isolation >> between (authenticated) users, we don't need store any additional >> security properties in item objects. But, it is not flexible. >> >>>> * The engine framework does not provide a hook just after a connection >>>> from client is established. It is not a must-requirement. However, >>>> we want to obtain privilege of the client process, and cache it in >>>> the userspace. >>>> If it is not available, we have to invoke a system-call for each >>>> access control decision, but it will be expensive. >>> >>> It certainly does and I use it. You can see it here: >>> >>> >>> http://github.com/northscale/bucket_engine/blob/master/bucket_engine.c#L433 >> >> Oh, I could not find it. >> >>>> * The security module needs its private storage different from the storage >>>> engine module. >>>> Now, the conn structure has "void *engine_storage" to store an engine >>>> private information, but the right to use is closed for the second >>>> module. >>> >>> The engine should be aware of what you're doing with that data, so >>> it can supply the API to manipulate it. >> >> The matter is both of the security module and engine module want to >> use this private storage for their own purpose. :( >> >>>> In addition, the hash_item structure does not provide any private >>>> storage to store security context of the item (e.g this context will >>>> record what process create this object). >>> >>> Well, no. Expanding the item structure is *very* costly. That >>> should should be avoided where possible. >>> >>> That said, a given engine can define an item to look like whatever >>> it wants after the base stuff. You'll notice the base structure >>> doesn't even have a space for the key or data. >> >> Yes, do_item_alloc() allocates sizeof(hash_item) plus various length >> bytes of memory for each items. It is a well known technique to handle >> various kind of data objects, not only memcached. >> >> How much length do you expect for the security property of an item? >> I plan to add a "void *" pointer which indicates security_id object >> managed in libselinux. The security_id structure is a pair of >> security context (cstring) and reference counter. The number of >> security context is proportional to the number of users, not # of items, >> it does not cousume local memory so much. >> >>>> * The deployment of some hooks makes hard to decide what permission >>>> should be applied. For example, process_bin_delete() calls "get" >>>> method to confirm the object to be removed is existing. >>>> But the "get" method is only chance for the engine module to know >>>> a certain item is accessed in reader context. >>>> Of course, if we consider "delete" is a combination of "read" and >>>> "delete", it will be resolved. But it will damage the flexibility >>>> of security design. >>> >>> This is a good time to be talking about this, as the engine API >>> isn't dogma yet. >> >> Now I lean toward the security purpose framework which provides >> meaningful hooks for access controls, and capability to assign >> a security property on the session and items. >> >> Is it too early? >> >> Thanks, > > > -- > OSS Platform Development Division, NEC > KaiGai Kohei <[email protected]> > -- Toru Maesaka <[email protected]>
