"Serge E. Hallyn" <serge@xxxxxxxxxx> writes:
> On Mon, Oct 02, 2017 at 10:30:43PM -0500, Eric W. Biederman wrote:
>> Dimitri John Ledkov <xnox@xxxxxxxxxx> writes:
>>
>> > Currently, changing key ownership from one namespaced uid/gid to
>> > another namespaced uid/gid is only allowed by processes that have
>> > CAP_SYS_ADMIN in the intial namespace. Fix the capability check to
>> > also check the capability in the current capability.
>>
>> Nacked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
>>
>> I won't deny the issue, but unless I am misreading something this
>> will allow me to change the the uid of any key simply by unsharing
>> a user namespace. At which point there is no point in having a
>> permission check at all.
>
> Right so without having looked closely, at the very least you need to
> verify that the ucrrent user is privileged over key->{uid,gid} and
> over @user and @group. Now the latter is I *think* being done
> implicitly by the make_kuid(current_user_ns, user) at the top. So
> you need to further verify that key->uid and key->gid are mapped into
> current_user_ns.
>
> That *may* be sufficient.
Yes. It sounds like either we need to change something in the
implementation of keys so they have a clear user namespace owner
or implement capable_wrt_key_uidgid.
The latter is tricky so at the very least I would prefer it have a
function of it's own. Just so people don't handroll the necessary
pattern incorrectly at different places.
Eric
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
