linux-kernel - Re: [PATCH v1] ksmbd: Fix user namespace mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75d077ca-4f1d-50c4-10d2-0fb31fcd0c86@digikod.net>
Date:   Thu, 29 Sep 2022 14:18:43 +0200
From:   Mickaël Salaün <mic@...ikod.net>
To:     Christian Brauner <brauner@...nel.org>
Cc:     Hyunchul Lee <hyc.lee@...il.com>,
        Namjae Jeon <linkinjeon@...nel.org>,
        Steve French <smfrench@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>, linux-cifs@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v1] ksmbd: Fix user namespace mapping


On 29/09/2022 13:37, Christian Brauner wrote:
> On Thu, Sep 29, 2022 at 12:04:47PM +0200, Mickaël Salaün wrote:
>> A kernel daemon should not rely on the current thread, which is unknown
>> and might be malicious.  Before this security fix,
>> ksmbd_override_fsids() didn't correctly override FS UID/GID which means
>> that arbitrary user space threads could trick the kernel to impersonate
>> arbitrary users or groups for file system access checks, leading to
>> file system access bypass.
>>
>> This was found while investigating truncate support for Landlock:
>> https://lore.kernel.org/r/CAKYAXd8fpMJ7guizOjHgxEyyjoUwPsx3jLOPZP=wPYcbhkVXqA@mail.gmail.com
>>
>> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
>> Cc: Hyunchul Lee <hyc.lee@...il.com>
>> Cc: Namjae Jeon <linkinjeon@...nel.org>
>> Cc: Steve French <smfrench@...il.com>
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Mickaël Salaün <mic@...ikod.net>
>> Link: https://lore.kernel.org/r/20220929100447.108468-1-mic@digikod.net
>> ---
> 
> I think this is ok. The alternative would probably be to somehow use a
> relevant userns when struct ksmbd_user is created when the session is
> established. But these are deeper ksmbd design questions. The fix
> proposed here itself seems good.

That would be better indeed. I guess ksmbd works whenever the netlink 
peer is not in a user namespace with mapped UID/GID, but it should 
result in obvious access bugs otherwise (which is already the case 
anyway). It seems that the netlink peer must be trusted because it is 
the source of truth for account/user mapping anyway. This change fixes 
the more critical side of the issue and it should fit well for backports.