(2010/02/06 20:20), Trond Norbye wrote:
>
> On Feb 5, 2010, at 3:28 AM, KaiGai Kohei wrote:
>>
>> I have one more point to be issued here.
>>
>> Memcached protects an item from concurrent updates using mutex lock
>> (cache_lock) in store_item. The store_item() acquires a mutex lock
>> during do_store_item(), so all the updates operations shall be
>> serialized.
>> However, it also means we cannot know whether the item to be updated
>> is already on the cache, or not, outside of the critical section.
>>
>
> That is an implementation detail in the default storage engine, and you
> are free to implement this the way you want in your own engine..
Yes, the author of engine module can choose its own locking mechanism.
The point is that I want to utilize the upcoming engine modules with
the security module. It maybe have the following structure, if it
is implemented as an engine module.
ENGINE_ERROR_CODE security_store(....) {
if (!permission_is_allowed(...))
return ENGINE_EACCESS;
return secondary_engine->store(....);
}
In this case, the item to be updated is correctly identical with the item
to be actually updated by the secondary engine module.
>> The SET command may work on either creation of a new item or updates
>> of an existing item, depending on existence of the specified item.
>> The REPLACE, APPEND and PREPEND command just returns an error if the
>> specified item does not exist on the cache.
>>
>> Even if the security stuff calls item_get() outside of the critical
>> section to check existence of the specified item, we have no guarantee
>> any other thread does not create/update the target item.
>> It is the reason why I put semcd_update_command() hook inside of the
>> do_store_item().
>>
>
> Not sure I understand what you are saying here.. do_store_item is called
> from store_item when we acquired the cache mutex. item_get will try to
> acquire the same mutex so it has to wait for do_store_item to return and
> release the mutex..
What I wanted say is....
For example, when we create a new item with SET command, a thread-A
calls settings.engine.v1->store() which checks permissions and calls
the secondary engine module which actually handles physical i/o.
In this case, a thread-B can execute the store() method between the
permission check and the secondary store.
See the following diagram,
Thread-A Thread-B
---o---------------------o-----
| |
V |
security_store(...) |
| |
V |
permission checks V
| security_store(...)
| |
| V
| permission checks
| |
| V
| store_item(...)
| - Lock mutex
| - do_store_item(...)
V - Unlock mutex
store_item(...) |
- Lock mutex |
- do_store_item(...) |
- Unlock mutex |
| |
V V
---------------------------------
In this case, the thread-A checks permission before store_item().
At that time, the specified item is not on the cached, it checks
permission to *create* a new item.
However, the thread-B may calls store_item() earlier than thread-A.
In this case, the thread-B also checks permission to *create* a new one.
In the result, the thread-A updates an existing item, although security
stuff allowed to create a new item.
(For example, thread-B assign different user-id from thread-A.)
But, after that, I found out that we can put a new mutex stuff in the
security_store() like:
security_store(...) {
pthread_mutex_lock(...);
if (!permission_is_allowed(...)) {
pthread_mutex_unlock(...);
return ENGINE_EACCESS;
}
retval = secondary_engine->store(...);
pthread_mutex_unlock(...);
return retval;
}
Right now, it seems to me this issues does not prevent us.
>> It is just an idea. If this efforts should be placed on the engine
>> project, it seems to me all the security hooks should be included
>> into server_interface_v1, rather than engine_interface_v1.
>> If I understand correctly, the server_interface_v1 structure is a set
>> of function pointers to be called from engine modules.
>> If we can make clear a set of interfaces that engine module to tell
>> an access control decision of security provider, we can implement
>> this feature much simpler. (Of course, the default security provider
>> should always return 'allowed' to keep backword compatibility.)
>>
>
> That would be possible, but I'm not sure if I see the big benefit of
> this. I have my doubts that the majority of engines will implement this
> kind of access control, and if we should create such interface exported
> from the memcached core it has to be designed in a way so that it works
> for other systems as well (such as Trusted Solaris). Personally I'm not
> convinced that this is a feature that a lot of people would use, that's
> why I believe that it would be great for a specialized engine (and by
> doing so, it can be tailored to the system you want instead of creating
> a generalized solution).
Yes, the assumption of reference monitor model is all the *internal* stuff
follows on the access control decision by the centralized security module.
If administrator loads an engine module which ignores the reference monitor,
it breaks the assumption.
> What kind of access control function did you
> want the core to provide, and what would be the required change in the
> core to support this?
At least, we needs three functionalities:
* Capability to load multiple (stackable) engine module.
I want the security feature to work independently from the type of
physical engine module. It may be possible to implement initialize()
method to load another module, but it is obviously reinvention of the
wheel. I think it should be a job of the core.
BTW, it will be valuable for non-security usage. We can expect a pseudo
engine module which distributes the given item into appropriate engine
module. For example, if the key is prefixed by "persistent.*", it is
stored in the module with backend storage, otherwise, the item is on
the default engine.
* Capability to store security attribute of the item and connection.
In SELinux model, any entities (such as items in memcached) have to be
labeled with security context, to identify them. It means the core stuff
has to provide a field to store a security context of item.
It may be a special purpose void * pointer, like as I did in my proof of
concept patch.
One other possibility is enlargement of the key/value field of hash_item
structure. For example, it may be possible to call the secondary allocate()
with enlarged nbytes, to allocate a region to store security attribute,
not only key/value pair.
The security attribute depends on the type of security model.
If it uses traditional user-id matching policy, each item will have user-id
which create itself.
* Hooks to make access control decision.
Right now, it is not clear whether the existing engine framework is enough
to host access control stuff. (Perhaps, the second proof of the concept
patch (on the engine branch) will make it clear.)
At least, it does not have an error code to notice "access denied".
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <[email protected]>
