Keyrings, user namespaces and the user_struct

[David Howells]

Eric W. Biederman <ebiederm at xmission.com> wrote:

> > Jann Horn <jann at thejh.net> wrote:
> >
> >> find_keyring_by_name() checks that the UID of the keyring's owner is
> >> mapped into the current user namespace. But that doesn't catch the
> >> scenario I described: The keyring is created in an attacker-created
> >> namespace and looked up from the init namespace, into which all kuids are
> >> mapped.
> >
> > Ah - gotcha.
> 
> Unless I am misreading something it actually gets worse.  You don't even
> need a user namespace.  You can just call keyctl_join_session_keyring
> and the named keyring of your choice will be created.

It's meant to do that - assuming a keyring of that name doesn't yet exist or
isn't accessible by you.

I should perhaps prohibit the creation of a _uid.N keyring.  However, I think
that conceptually, it should be possible to join the user-session keyring as
your session - or I should make it so that the user-session keyring doesn't
exist (but I'm pretty certain that will break userspace).

> Plus there are various really weird things in their where the keyring
> names of _tid, _pid, _ses, get reused over and over again.

True, however per-thread (_tid) and per-process(_pid) keyrings are always
allocated by key_alloc() and never looked up by name when being created.

Anonymous session (_ses) keyrings are also created by key_alloc() and not
looked up when created.  It's only when a named session keyring is requested
that a look up by name is done.

I could make the per-thread, per-process and anon-session keyrings nameless by
default, or prefix them with '.' and not permit joining of a keyring whose
name begins with a '.' (you aren't allowed to use add_key() to create a such
keyrings, so that really ought to be extended to here too).

> So it looks like there are some significant things to fix.

So:

 (1) Make the keyring name list per user_namespace.

 (2) Don't let _uid.N, _tid, _pid and _ses (anonymous session) keyrings be
     joined.

 (3) Don't let keyrings whose name begins '.' be joined.

 (4) Don't keep the user and user-session keyrings in the user_struct, but
     rather make them subordinates of the user namespace.

for starters.

David