Re: [PATCH 2/2] KEYS: Add per-user_namespace registers for persistent per-UID kerberos caches

[ebiederm@xmission.com (Eric W. Biederman)]

David Howells <dhowells@redhat.com> writes:

> Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>> > The cache returned is a keyring named "_krb.<uid>" that the possessor can
>> > read, search, clear, invalidate, unlink from and add links to.  SELinux
>> > and co. get a say as to whether this call will succeed as the caller must
>> > have LINK permission on the cache keyring.
>> 
>> I think it would be more accurate to say you use the existing LSM
>> security hooks for security keys.
>
> Yes.
>
>> Calling out SELinux in particular just seems odd as there is absolutely
>> nothing SELinux specific in this patch.
>
> Sorry, I normally think of SELinux as that's what I usually deal with.  Yes,
> any and all LSMs.
>
>> > +		    !nsown_capable(CAP_SETUID))
>>
>> You you make this ns_capable(ns, CAP_SETUID);
>> 
>> nsown_capable is the right thing here but I am trying to remove the
>> function because it makes it too easy to not think about which
>> user namespace you are in.
>
> Okay.
>
>> > +	index_key.desc_len = sprintf(buf, "_krb.%u", __kuid_val(uid));
>> 
>> Please don't use the implementation detail __kuid_val.  Please use
>> from_kuid(&init_user_ns, uid) instead so it is explicitly documented
>> which user namespace you are using.
>
> Actually, I don't want that either.  I want the user-visible UID from the
> namespace.

Which is a definite reason to use from_kuid().  So you think about which
user namespace you want this to be seen in.

I guess if this is all in the user namespace from_kuid(ns, uid) is what
you are after.  I was thinking this was heading to the upcall which only
runs in the initial user namespace.  When passing things to the upcall
it makes sense to use values in the initial user namespace.

Eric