Received: by 2002:ac0:a5a6:0:0:0:0:0 with SMTP id m35-v6csp3735167imm; Mon, 1 Oct 2018 03:42:57 -0700 (PDT) X-Google-Smtp-Source: ACcGV63mRAf5EIHizXhongWlsAPBjMon+wDHgqGMaL2hQKh4S+KSSRvzzcFGAmlpkXPHFSDdSw7t X-Received: by 2002:a17:902:1ab:: with SMTP id b40-v6mr11306873plb.82.1538390577663; Mon, 01 Oct 2018 03:42:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1538390577; cv=none; d=google.com; s=arc-20160816; b=qBM2REr6a8SrPk0meey4ft5ierVviySXY0rPK1ikZQzCMw82nHkh8OtMW207f/gQzg zGhQqx60cIk861Yx1e5DQcpahSh+wK42/wP543eBSBq8DZOjrWF/7u2PqZHNPtROw5A9 BdnPCS9rUcCn9D31a3oSKw7SwrAA28vyNG1FD+a/GR2F0+Pz+zBU3euwEH4/HSXI6/6Y SzRVn8JuhG5fSdxKc6keqZC1wa+K8Q2YfDuqdlg7LhdapOLJqswX/49VO/iaI4nKCqmE L9yPsmR6mpaVg1EX4VZllws87mQnhZnUarItBcX/saBZUGXSi6PESN1XGELRrQ4tKJnP QOEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=KPUXJxFzfx17HCf3DduXRicvGC+b8niBfxP/R2v4zdU=; b=yeh4pCc7hWVSvQ684mfZ6sTmdn90UWrcvCvaVKBhIrHPYNSk8ikCRWZgHNtjCVqHtG CCzwepNyS+K4gLt87Rxc1Udf7nPYECN++zcMB5fiDbHDCrNPi5e8rBPuo30BJbRcIPD2 XBpu+Z4/AmJMxhFxkS9dKLW9pm6yOISiEspq/DK7Aji8CHhfLhBwdTXPYRa8Q6Xki3FA fLcZ69I3OSJ+w+LNr2E7M4vlFLp1n9dA3ic/KIz1KUGxAPglMH0wO1JLD0iAIjY8r8F2 hQQwM3TwZaw6VcEyrPAugpDwoT53hhhkr+RkIw5zjGPWIswQkO6TaWCDm00oZo09mS/4 tqgA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@brauner.io header.s=google header.b="OVYru40/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c12-v6si7760262pgj.70.2018.10.01.03.42.43; Mon, 01 Oct 2018 03:42:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@brauner.io header.s=google header.b="OVYru40/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729111AbeJARTW (ORCPT + 99 others); Mon, 1 Oct 2018 13:19:22 -0400 Received: from mail-wr1-f65.google.com ([209.85.221.65]:38669 "EHLO mail-wr1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729060AbeJARTW (ORCPT ); Mon, 1 Oct 2018 13:19:22 -0400 Received: by mail-wr1-f65.google.com with SMTP id a13-v6so2645190wrt.5 for ; Mon, 01 Oct 2018 03:42:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=brauner.io; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=KPUXJxFzfx17HCf3DduXRicvGC+b8niBfxP/R2v4zdU=; b=OVYru40/5Q55qASQI9RUW7wL48iiHx2GTaDvaFvSlYtgHRp4pJhLLMpv3hC4UR3maA sJsAR/UiP5ssGeljyApiV8N2hqleBw9FN2G+98nhaaRDQh9Ou9nN8xDY0EBDLv/fhmQq Fu8aOccpBXH3gYwqGkakiszDnoTR+hOL5TgaW7LpjuxNYI6SLXcsDNBpA6cEKl6I8a1c nz9kEbxxLqGfylNtZtsd3dJa8BshLA6vTu6L8Ep8OxKyXqXwSFDnhDUq2z1PdU2wsoTO Cb5E+muN6eIYsrJrNVyjeoq9QqOv2AcS9hsAGstt/xbrrDghHbu3zfYwOY+y+Xak5/KX yNiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=KPUXJxFzfx17HCf3DduXRicvGC+b8niBfxP/R2v4zdU=; b=b14/W4mer5o/isgDa4kQncvxodAHHPl24xCNNxjSCNpVf6JaTuqccZ+9uBqjIyelT+ fnVKjN5cdbQBHfhJJ4Ghbk9tqakkRNYecpfl5Pu6Rcrlyf7kOMvjs68k8Xj+RFzRD3+b TLFxAW284UgbQRXg3qOlrz0q8D7V+ZFyjp9EHk2tbZTuscCBCPE8WHg/60t+XpUQbSB4 3AabXTcacAKSRDBE1dfQ6ZECnuY880jtz0HdVzij3QDM1cKxFWsKxAEaSFhf+2P2hEjM sYAFrKEXNPHRQ8iakr2gY+SBpVONF2NhYSGLmp2sYguFNIRZqw0LY4FWnhAUHSLast09 kEiQ== X-Gm-Message-State: ABuFfoi1Razuc2fHIiT4uieaMOIk+i/gLy52dPuv2g2IzJxcjnOI/qm/ zRqUjb2VG3JypRXpWtaG9cZo3A== X-Received: by 2002:a5d:450d:: with SMTP id s13-v6mr6632763wrq.82.1538390530127; Mon, 01 Oct 2018 03:42:10 -0700 (PDT) Received: from brauner.io (u-087-c227.eap.uni-tuebingen.de. [134.2.87.227]) by smtp.gmail.com with ESMTPSA id t2-v6sm12555070wre.96.2018.10.01.03.42.08 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 01 Oct 2018 03:42:09 -0700 (PDT) Date: Mon, 1 Oct 2018 12:42:03 +0200 From: Christian Brauner To: Aleksa Sarai Cc: Jann Horn , "Eric W. Biederman" , jlayton@kernel.org, Bruce Fields , Al Viro , Arnd Bergmann , shuah@kernel.org, David Howells , Andy Lutomirski , Tycho Andersen , kernel list , linux-fsdevel@vger.kernel.org, linux-arch , linux-kselftest@vger.kernel.org, dev@opencontainers.org, containers@lists.linux-foundation.org, Linux API Subject: Re: [PATCH 2/3] namei: implement AT_THIS_ROOT chroot-like path resolution Message-ID: <20181001104202.f6tz54s3fbvld56g@brauner.io> References: <20180929103453.12025-1-cyphar@cyphar.com> <20180929131534.24472-1-cyphar@cyphar.com> <20181001054246.gfinmx3api7kjhmc@ryuk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20181001054246.gfinmx3api7kjhmc@ryuk> User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Oct 01, 2018 at 03:44:28PM +1000, Aleksa Sarai wrote: > On 2018-09-29, Jann Horn wrote: > > The problem is what happens if a folder you are walking through is > > concurrently moved out of the chroot. Consider the following scenario: > > > > You attempt to open "C/../../etc/passwd" under the root "/A/B". > > Something else concurrently moves /A/B/C to /A/C. This can result in > > the following: > > > > 1. You start the path walk and reach /A/B/C. > > 2. The other process moves /A/B/C to /A/C. Your path walk is now at /A/C. > > 3. Your path walk follows the first ".." up into /A. This is outside > > the process root, but you never actually encountered the process root, > > so you don't notice. > > 4. Your path walk follows the second ".." up to /. Again, this is > > outside the process root, but you don't notice. > > 5. Your path walk walks down to /etc/passwd, and the open completes > > successfully. You now have an fd pointing outside your chroot. > > > > If the root of your walk is below an attacker-controlled directory, > > this of course means that you lose instantly. If you point the root of > > the walk at a directory out of which a process in the container > > wouldn't be able to move the file, you're probably kinda mostly fine - > > as long as you know, for certain, that nothing else on the system > > would ever do that. But I still wouldn't feel good about that. > > Please correct me if I'm wrong here (this is the first patch I've > written for VFS). Isn't the retry/LOOKUP_REVAL code meant to handle this > -- or does that only handle if a particular path component changes > *while* it's being walked through? Is it possible for a path walk to > succeed after a path component was unmounted (obviously you can't delete > a directory path component since you'd get -ENOTEMPTY)? > > If this is an issue for AT_THIS_ROOT, I believe this might also be an > issue for AT_BENEATH since they are effectively both using the same > nd->root trick (so you could similarly trick AT_BENEATH to not error > out). So we'd need to figure out how to solve this problem in order for > AT_BENEATH to be safe. > > Speaking naively, doesn't it make sense to invalidate the walk if a path > component was modified? Or is this something that would be far too > costly with little benefit? What if we do more aggressive nd->root > checks when resolving with AT_BENEATH or AT_THIS_ROOT (or if nd->root != > current->mnt_ns->root)? > > Regarding chroot attacks, I was aware of the trivial > chroot-open-chroot-fchdir attack but I was not aware that there was a > rename attack for chroot. Thanks for bringing this up! > > > I believe that the only way to robustly use this would be to point the > > dirfd at a mount point, such that you know that being moved out of the > > chroot is impossible because the mount point limits movement of > > directories under it. (Well, technically, it doesn't, but it ensures > > that if a directory does dangerously move away, the syscall fails.) It > > might make sense to hardcode this constraint in the implementation of > > AT_THIS_ROOT, to keep people from shooting themselves in the foot. > > Unless I'm missing something, would this not also affect using a > mountpoint as a dirfd-root (with MS_MOVE of an already-walked-through > path component) -- or does MS_MOVE cause a rewalk in a way that rename > does not? > > I wouldn't mind tying AT_THIS_ROOT to only work on mountpoints (I > thought that bind-mounts would be an issue but you also get -EXDEV when > trying to rename across bind-mounts even if they are on the same > underlying filesystem). But AT_BENEATH might be a more bitter pill to > swallow. I'm not sure. > > In the usecase of container runtimes, we wouldn't generally be doing > resolution of attacker-controlled paths but it still definitely doesn't > hurt to consider this part of the threat model -- to avoid foot-gunning > as you've said. (There also might be some nested-container cases where > you might want to do that.) > > > > Currently most container runtimes try to do this resolution in > > > userspace[1], causing many potential race conditions. In addition, the > > > "obvious" alternative (actually performing a {ch,pivot_}root(2)) > > > requires a fork+exec which is *very* costly if necessary for every > > > filesystem operation involving a container. > > > > Wait. fork() I understand, but why exec? And actually, you don't need > > a full fork() either, clone() lets you do this with some process parts > > shared. And then you also shouldn't need to use SCM_RIGHTS, just keep > > the file descriptor table shared. And why chroot()/pivot_root(), > > wouldn't you want to use setns()? > > You're right about this -- for C runtimes. In Go we cannot do a raw > clone() or fork() (if you do it manually with RawSyscall you'll end with > broken runtime state). So you're forced to do fork+exec (which then > means that you can't use CLONE_FILES and must use SCM_RIGHTS). Same goes > for CLONE_VFORK. > > (It should be noted that multi-threaded C runtimes have somewhat similar > issues -- AFAIK you can technically only use AS-Safe glibc functions > after a fork() but that's more of a theoretical concern here. If you > just use raw syscalls there isn't an issue.) > > As for why use setns() rather than pivot_root(), there are cases where > you're operating on a container's image without a running container > (think image extraction or snapshotting tools). In those cases, you > would need to set up a dummy container process in order to setns() into > its namespaces. You are right that setns() would be a better option if > you want the truthful state of what mounts the container sees. > > [I also don't like the idea of joining the user namespace of a malicious > container unless it's necessary but that's probably just needless > paranoia more than anything -- since you're not joining the pidns you > aren't trivially addressable by a malicious container.] > > > // Ensure that we are non-dumpable. Together with > > // commit bfedb589252c, this ensures that container root > > // can't trace our child once it enters the container. > > // My patch > > // https://lore.kernel.org/lkml/1451098351-8917-1-git-send-email-jann@thejh.net/ > > // would make this unnecessary, but that patch didn't > > // land because Eric nacked it (for political reasons, > > // because people incorrectly claimed that this was a > > // security fix): > > Unless I'm very much mistaken this was fixed by bfedb589252c ("mm: Add a > user_ns owner to mm_struct and fix ptrace permission checks"). If you > join a user namespace then processes within that user namespace won't > have ptrace_may_access() permissions because your mm is owned by an > ancestor user namespace -- only after exec() will you be traceable. That is not _completely_ true. Iirc (Please someone do yell at me if I'm wrong!), this is as follows. You will in fact be dumpable as long as you don't set{g,u}id() to an effective uid that is different from the effective uid of the process that created the task. For example, if you clone(CLONE_NEWUSER) as an unprivileged user with uid and euid 1000 you are in fact dumpable and thus traceable *but* if you do a setuid(0) in the new task then you will end up with old->euid = 1000 and new->euid = 0 at which point the kernel will remove the dumpable flag and the creating process cannot trace you anymore (which has funny consequences for lsm isolation and sending fds around). Iiuc, The same logic applies when you do a setns() to another user namespace. /* dumpability changes */ if (!uid_eq(old->euid, new->euid) || !gid_eq(old->egid, new->egid) || !uid_eq(old->fsuid, new->fsuid) || !gid_eq(old->fsgid, new->fsgid) || !cred_cap_issubset(old, new)) { if (task->mm) set_dumpable(task->mm, suid_dumpable); <<< suid_dumpable == 0 at this point task->pdeath_signal = 0; smp_wmb(); } > > We still use PR_SET_DUMPABLE in runc but that's because we support older > kernels (and people don't use user namespaces under Docker) but with > user namespaces this should not be required anymore. > > -- > Aleksa Sarai > Senior Software Engineer (Containers) > SUSE Linux GmbH >