From patchwork Tue Aug 2 19:26:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 9260261 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AC5796077C for ; Tue, 2 Aug 2016 19:39:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9C28727F99 for ; Tue, 2 Aug 2016 19:39:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 90407284F2; Tue, 2 Aug 2016 19:39:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BA2BF27F99 for ; Tue, 2 Aug 2016 19:39:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756142AbcHBTjt (ORCPT ); Tue, 2 Aug 2016 15:39:49 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:38041 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755193AbcHBTjs (ORCPT ); Tue, 2 Aug 2016 15:39:48 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.82) (envelope-from ) id 1bUfXS-0005dd-68; Tue, 02 Aug 2016 13:39:42 -0600 Received: from 67-3-204-119.omah.qwest.net ([67.3.204.119] helo=x220.int.ebiederm.org.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.87) (envelope-from ) id 1bUfXQ-0000p7-Ba; Tue, 02 Aug 2016 13:39:41 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Mimi Zohar Cc: Seth Forshee , Dmitry Kasatkin , linux-ima-devel@lists.sourceforge.net, linux-ima-user@lists.sourceforge.net, linux-security-module@vger.kernel.org References: <1470057550-58098-1-git-send-email-seth.forshee@canonical.com> <1470080186.23563.427.camel@linux.vnet.ibm.com> <871t28w7h1.fsf@x220.int.ebiederm.org> <1470106743.23563.531.camel@linux.vnet.ibm.com> <87shuntg88.fsf@x220.int.ebiederm.org> <1470163748.23563.570.camel@linux.vnet.ibm.com> Date: Tue, 02 Aug 2016 14:26:29 -0500 In-Reply-To: <1470163748.23563.570.camel@linux.vnet.ibm.com> (Mimi Zohar's message of "Tue, 02 Aug 2016 14:49:08 -0400") Message-ID: <87a8gvotui.fsf@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1bUfXQ-0000p7-Ba; ; ; mid=<87a8gvotui.fsf@x220.int.ebiederm.org>; ; ; hst=in02.mta.xmission.com; ; ; ip=67.3.204.119; ; ; frm=ebiederm@xmission.com; ; ; spf=neutral X-XM-AID: U2FsdGVkX19u1V/cScygSeHAUetnKbZzx12mfXNMHG0= X-SA-Exim-Connect-IP: 67.3.204.119 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH 0/1] EVM/IMA xattr hardening and unprivileged user ns mounts X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP Mimi Zohar writes: > On Di, 2016-08-02 at 09:09 -0500, Eric W. Biederman wrote: >> Mimi Zohar writes: >> >> > On Mo, 2016-08-01 at 15:38 -0500, Eric W. Biederman wrote: >> >> Mimi Zohar writes: >> >> >> >> > On Mo, 2016-08-01 at 08:19 -0500, Seth Forshee wrote: >> >> >> I was taking a look at IMA and EVM in the context of allowing mounts in >> >> >> user namespaces from users with only capabilities in that namespace >> >> >> (i.e. no global capabilities). As a result of that I found just a >> >> >> handful of places where the size of an xattr or the values it contains >> >> >> lack sufficient validation to protect against malicious xattrs. The >> >> >> fixes for these are worthwhile on their own, so I'm sending them in the >> >> >> following patch. >> >> >> >> >> >> More generally though I'd like to request some guidance on how IMA and >> >> >> EVM should handle xattrs in mounts from globally-unprivileged users. For >> >> >> context, the intended use case for this is allowing fuse mounts in user >> >> >> namespace containers, and possibly mounts with a limited set of other >> >> >> filesystems in the future. All that would actually be needed to mount >> >> >> though are private user and mount namespaces. >> >> >> >> >> >> Based on what I see, I suspect that we can treat the user ns mounts like >> >> >> any other mount. I don't see that this would cause harm as long as the >> >> >> filesystem can't trigger bugs in the xattr parsing, and if the system is >> >> >> configured for IMA enforcment this policy should also be applied to >> >> >> these mounts. >> >> > >> >> > "security.evm" is written as a result of one of the protected security >> >> > xattrs being written. From your question, I assume that in this use >> >> > case scenario there are two concerns: >> >> > - parsing improper security.evm xattrs >> >> > - updating the protected security xattrs, causing the EVM xattr to >> >> > "bless" the change. >> >> > >> >> > The first issue you're addressing with the patch. As for the second >> >> > issue, as long as root in the namespace can not write security.xattrs, >> >> > there shouldn't be a problem. >> >> >> >> Assume that it is allowed that root in a namespace will be allowed to >> >> write security xattrs for a filesystem it has mounted. Certainly for >> >> the capability xattr this makes sense. >> > >> > Yeah, I remember. Having one capability xattr sounded interesting. >> > Have the capability patches been upstreamed? To label files in a >> > container, root in the namespace will need to be able to write an IMA >> > xattr. Whether it is possible to use a single xattr, is yet to be >> > determined. > >> For the most part yes. As part of this merge window Linus >> merged the changes to store s_user_ns on a super block and the code to >> use that. > > Great! I'll take a look. > >> I have kept back the code to relax the permission checks in the obvious >> cases. There is little immediate gain from that code and I noticed a >> few places where s_user_ns is not being handled properly. Primarily >> IMA and EVM (so this disucssion). > > Ok > >> >> If understand you correctly is that the key used to sign the HMAC in >> >> security.evm is machine local. As such I expect the proper semantics >> >> here will be to not update the security.evm xattrs on write of a >> >> filesystem with s_user_ns != &init_user_ns. >> > >> > The EVM xattr is verified before it can be changed, to prevent making an >> > existing invalid hmac, valid. If the namespace has the ability to >> > change any of the metadata included in the hmac, the EVM hmac will need >> > to be updated with the file metadata, otherwise nobody will be able to >> > access the file. >> >> To be clear with evm enabled and security.evm attributes present if >> the hmac is wrong access to the file is denied? > > Yes, if the file is included in the IMA policy and the EVM > hmac/signature verification fails, then access to the file is denied. Makes sense. >> What happens in the case of new files and in the case of file without a >> security.evm xattr? > > New files are a special case. The first EVM protected xattr* written > will cause the security.evm xattr to be written, without first > validating the existing xattr. > > *evm_config_xattrnames[] contains the list of protected xattrs. Thank you. >> >> On read I am not certain what the right thing to do is, ignore the >> >> attribute or do what happens on validation failure. >> > >> >> > Any changes made offline/elsewhere, would >> >> > result in the security.evm HMAC not verifying, causing a denial of >> >> > service. Unless of course the EVM HMAC key has somehow been >> >> > leaked/exfiltrated from the kernel. >> >> >> >> One scenario where this is likely to come up, and the kind of scenario >> >> we are planning for is a filesystem on a usb stick is transported from >> >> one machine to another. One the second system it is mounted directly >> >> (ideally) or through fuse running a copy of it's filesystem driver in >> >> userspace to guard against filesystem implementation bugs. >> >> >> >> So if that filesystem has security.evm xattrs if it appears things will >> >> work very poorly. >> > >> > It could work for files with EVM signatures, obviously not with an HMAC. >> >> Fair. >> >> What policy would you recommend for (evm and ima) dealing with >> filesystems that are mounted by a user namespace root and thus have >> sb->s_user_ns != &init_user_ns? > > I think as long as it can not escape the mount namespace, allowing it to > write both the EVM and IMA xattrs should be fine. In terms of reading > the xattrs, real root would need to be able to differentiate the xattrs' > origin. It is a little more subtle than that, but essentially it is true that a filesystem will not propogate into a mount namespace owned by root in more privileged user namespace. The piece of orgin that is retained in the kernel for a filesystem is sb->s_user_ns. So the information is there. It is not currently exported explicitly to userspace, but who owns the mount namespace a filesystem is in, is a big clue. But yes any reader knows which filesystem an xattr is coming from so this should not be a point of confusion. >> Like Seth I am not much of an expert in the semantics of IMA and EVM. >> I just know that ima calcualtes a hash of a file and does something with >> it. EVM caculcates a hash of other xattrs and the classic file >> attributes. > > EVM protects file metadata. For immutable files, such as ELF > executables, the security.evm should contain a file signature. For > mutable files, security.evm should contain an hmac. You know I am not certain what the distinction between a file signature and a hmac is in this context. >> That last hash as part of the work just merged I have updated to use >> the uid and gid in s_user_ns instead of &init_user_ns, as on disk that >> is the namespace the uid and gid are expected to be encoded in. > > Ok. I need to take a look. It is just the very obvious patch below. commit 0b3c9761d1e405514a551ed24d3ea89aea26ce14 Author: Seth Forshee Date: Thu Feb 5 10:44:50 2015 -0600 evm: Translate user/group ids relative to s_user_ns when computing HMAC The EVM HMAC should be calculated using the on disk user and group ids, so the k[ug]ids in the inode must be translated relative to the s_user_ns of the inode's super block. Signed-off-by: Seth Forshee Signed-off-by: Eric W. Biederman Eric --- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 30b6b7d0429f..11c1d30bd705 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -151,8 +151,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, memset(&hmac_misc, 0, sizeof(hmac_misc)); hmac_misc.ino = inode->i_ino; hmac_misc.generation = inode->i_generation; - hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); - hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); + hmac_misc.uid = from_kuid(inode->i_sb->s_user_ns, inode->i_uid); + hmac_misc.gid = from_kgid(inode->i_sb->s_user_ns, inode->i_gid); hmac_misc.mode = inode->i_mode; crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); if (evm_hmac_attrs & EVM_ATTR_FSUUID)