Re: [Keyrings] [PATCH] Keys: Add LSM hooks for key management

[James Morris <jmorris@namei.org>]

On Wed, 5 Oct 2005, Chris Wright wrote:

> What case causes context != current?

Indeed, this is critical: we always need to know which task initiated the 
current action.  If it's not current, then we need the calling task struct 
passed into the security hook.

> > +	/* do a final security check before publishing the key */
> > +	ret = security_key_alloc(key);
> 
> This may simply be allocating space for the label (and possibly labelling)
> not necessarily a security check.

Agree, in fact, I think we should always aim to keep housekeeping hooks 
separate from access control hooks.

Access checks seem to be usually done before this point via 
lookup_user_key(), which is ideal.

> > - error:
> > +	/* let the security module know the key has been published */
> > +	security_key_post_alloc(key);
> 
> This is odd, esp since nothing could have failed between alloc and
> publish.  Only state change is serial number.  Would you expect the
> security module to update a label based on serial number?

I don't think SELinux would care about this yet.  If so, the hook can be 
added later.

> > +	/* if we're not the sysadmin, we can only change a key that we own */
> > +	if (capable(CAP_SYS_ADMIN) || key->uid == current->fsuid)
> > +		ret = security_key_set_security(key, name, data, dlen);
> 
> Are you sure this is right?  Normally I'd expect users can _not_ set the
> security labels of their own keys.  But perhaps I've missed the point
> of this one, could you give a use case?

I think this is like xattrs on files, where the user can set and view 
security attributes.

In any case, I don't see why you'd use a DAC check here at all, as this is 
a complete passthrough to the security module.

key_get_security() has no DAC check.


> This would be a whole lot easier if keys were available in keyfs ;-)

Yes, then standard setxattr() getxattr() syscalls could be used, and we 
can avoid two new multiplexed syscalls.

David, admit it, this key stuff is all really a filesystem :-)


- James
-- 
James Morris
<jmorris@namei.org>