[Keyrings] Re: keyutils observations/suggestions

[David Howells]

Alexander Zangerl <az at bond.edu.au> wrote:

> i've played around with your userland (v.0.3) for the 2.6 keystorage 
> and liked it a lot; i've come across some minor issues i'd like to
> note and get your opinion on.

Thanks.

> * keyctl wants the payload given on the command line. having secret material
> show up in ps isn't really nice (and having to deal with shell quoting for
> arbitrary key material is not fun either); a mode where keyctl reads from an
> fd of the caller's choice would be good and shouldn't be too hard to
> implement.

Good idea; thanks. It is possible to write some other client that'll call the
key management syscall directly, and this I'd expect to be the normal mode of
operation, but you're right: this is important.

I think that limiting it to stdout should be fine.

> * i think your request-key program provides a too little support to external
> query programs: just handing the whole task of instantiating a key to the
> query program defeats the purpose of the request-key wrapper a bit imho. why
> not allow generic query programs that just produce data on stdout and have
> request-key instantiate the key from that?

There are a number of problems with this:

 (1) This doubles the number of tasks involved in the creation of a key, and
     so requires extra forking.

 (2) This requires the data to be passed from the slave process to request-key
     along a pipe which adds more latency and consumes more resources.

 (3) It makes the negative instantiation of keys more difficult.

/sbin/request-key is meant to be as thin a layer as I can get away with. I
could make it do what you're suggesting in addition to how it currently works,
and I could also make it run libraries. Note that it's also going to be made
possible to skip forking off /sbin/request-key entirely, and have the key type
communicate with some already running process (such as gssapid or whatever
it's called).

> i think the query program shouldn't have to know anything about the
> keystorage kernel service but rather be dumb and generic.  (i've coded a
> simplified version of request-key that just runs whatever data is attached
> to key _queryprogram for this user and then instantiates the key from the
> program's stdout.)

That's not necessarily a good idea. Some query programs need access to the
parent process's keyrings, such as for obtaining Kerberos TGTs or eCryptFS
doing whatever it does.

> * not being able to set expiration times for user defined keys is a bit
> of a limitation; a keyctl op to update expiration times would be useful.
> is there a reason why you didn't implement this?

I didn't think of it at the time for some reason. It certainly needs adding.

> * if one makes the mistake of handing keyctl_instantiate the same destination 
> keyring as the key is in already, then you get two copies of the same key 
> in this keyring.

Yeah. Don't do that then:-)

This is there for a reason: so that an instantiator process can maintain its
own copy of a key if it wants to.

> this is, of course, the linking stuff working as designed; personally i
> think the kernel should either barf on such a misuse or silently strip
> multiple links in the same keyring.

Overlapping key handling needs a bit more work. The example you gave is an
extreme example. I should probably be more proactive in discarding expired,
negative and revoked keys from a keyring when a new key of the same type and
description is installed in the same keyring.

I'm not sure whether I should also discard matching valid keys; possibly I
should.

David