From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2998848AbdD1Pqo (ORCPT ); Fri, 28 Apr 2017 11:46:44 -0400 Received: from smtp.nsa.gov ([8.44.101.8]:53869 "EHLO emsm-gh1-uea10.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1424379AbdD1Pqe (ORCPT ); Fri, 28 Apr 2017 11:46:34 -0400 X-IronPort-AV: E=Sophos;i="5.37,388,1488844800"; d="scan'208";a="6459834" IronPort-PHdr: =?us-ascii?q?9a23=3AYyThhxTmpNjCek0JIlceewHD5tpsv+yvbD5Q0YIu?= =?us-ascii?q?jvd0So/mwa67ZxSDt8tkgFKBZ4jH8fUM07OQ6PG+HzBQqsfb+Fk5M7V0Hycfjs?= =?us-ascii?q?sXmwFySOWkMmbcaMDQUiohAc5ZX0Vk9XzoeWJcGcL5ekGA6ibqtW1aFRrwLxd6?= =?us-ascii?q?KfroEYDOkcu3y/qy+5rOaAlUmTaxe71/IRG3oAnLucQbgoRuJ6IvxhDUvnZGZu?= =?us-ascii?q?NayH9yK1mOhRj8/MCw/JBi8yRUpf0s8tNLXLv5caolU7FWFSwqPG8p6sLlsxnD?= =?us-ascii?q?VhaP6WAHUmoKiBpIAhPK4w/8U5zsryb1rOt92C2dPc3rUbA5XCmp4ql3RBP0ji?= =?us-ascii?q?oMKiU0+3/LhMNukK1boQqhpx1hzI7SfIGVL+d1cqfEcd8HWWZNQsNdWipPDYOm?= =?us-ascii?q?a4sEEvQPM+BWoYLgo1cCtAWyCRWpCO7p1zRGhGL53bci3uoiDA/I3BIuEdwMv3?= =?us-ascii?q?Taq9X6KKAcXu+6wqTT0TXObOlb1Svn5YTUcB0sp+yHU7JqccrWzEkiDx7LjkmO?= =?us-ascii?q?poz9PzOayOINuHWG4eplT+2vj2onpB9xozOywcoskZTGhpkOx1DY9SR23IY1Jd?= =?us-ascii?q?qiRE59et6rCoFcty6dN4toW84vRXxjtiUiyrAepJK2cycHxI4nyhLCcfCLbYeF?= =?us-ascii?q?7gz5WOqMJzpzmWhrd6ilhxmo9Eit0uj8Vs6p31lUtidFidzMtmwV1xzU98iHVu?= =?us-ascii?q?Nx/ke/1jaL0ADe8v1ELloularaNp4h2aQ8loYTsEvfHi/2n1/6jKmKeUU/5uek?= =?us-ascii?q?8eHnYrTippOENo90jB/xMrg2l8CiDuk1PRICUmiG9eimyrHu8lP1TK9XgvEul6?= =?us-ascii?q?nWqpHaJcAVpq6jBA9V154u6w2iADe9y9kYgXkGI05FeBKAlYTpPUrOL+riAfew?= =?us-ascii?q?hFSsji9nx+raMb35HpXNMn/Dna/6fblm9k5cyREzzctY55JSEbwOPe/8WknruN?= =?us-ascii?q?PECR85NhS+w/z7B9VlyoMeRWWPD7eEP6zIt1+I5/wgI+2OZIIOvTbyNfwl5/r0?= =?us-ascii?q?gn8/nl8ccrOl0ocQaHC9Bv5mOVmWYWLwgtcdFmcHphI+Q/b3iF2GSjNTf2y9X7?= =?us-ascii?q?845j0iDYKmCoDDRpqzj7CbwCi7GZhWbHhcCl+QCXfoa5mEW/AUZS2MOs9uiCYE?= =?us-ascii?q?Vbm6S4I6zRGhrhX6y7t8LurM/i0Xr47s28Zv6+3UjxEy+iR+D96B3GGVU2F0gm?= =?us-ascii?q?QISic03K9lpExy1EyD3bJ8g/BCENxT4OlJUh07NZ7H1OF6DMryVRjdcdeNVlmq?= =?us-ascii?q?WMmpATY0Ttgp2d8Bf159G8m+jhDExyeqGKEal6aEBJMq6a/c32L+J8J5y3fG0q?= =?us-ascii?q?ktlUUpQsxKNWe+nK5w6xDTB5LVk0Wej6uqcaUc3CjQ9GaM1GaOv19XUBR2Uarb?= =?us-ascii?q?WXASfVXWrdvn6UPYVbOuCqooMhFHycGcLqtGcNrpjU9JRP37ItTRf3qxm3usBR?= =?us-ascii?q?aP3r6MaIvqe2MA3CTSEUQEiB4c8mqbNQgkByehv2LfACVrFVLofkzs7O1+p22g?= =?us-ascii?q?Q08qwAGFcVdh26C2+hELn/ycTe0c3rYetCcmsTV0E06338jKBNqYuwphYKJcbM?= =?us-ascii?q?sm4FhcyGLZthd9PoenL6BknFIRbhl4v0X12hV4D4VPi8kqrHcwwAVuLqKY1QAJ?= =?us-ascii?q?SzTN5p36M6bLK2T0tDqyarXN3VeWhM2c56YU8/M+7VnvuimmE0Mj9zNs1NwDgF?= =?us-ascii?q?WG4ZCfNxYfSZL8VA4M8hF+o7zLKn0m65j8yWxnMa7ytCTLnd0uGr12mV6bY95D?= =?us-ascii?q?PfbcR0fJGMoACp3rcbZylg=3D=3D?= X-IPAS-Result: =?us-ascii?q?A2EaBAAfYwNZ/wHyM5BUChwBAQQBAQoBARcBAQQBAQoBAYM?= =?us-ascii?q?BKYFtg2iaMwEBAQEBAQaBJpd8hiQChDVXAQEBAQEBAQECAQJoKIIzIgGCPwEBA?= =?us-ascii?q?QECASMECwFGBQsLDQEKAgImAgJXBhOICIIKBQivLoFsOiYCimIBAQEBAQUBAQE?= =?us-ascii?q?BASOBC4UOhQo0hDcSgxyCXwWdUYpMiEKCAokQDIZAlClYgQolCQIeCB8PhTQdg?= =?us-ascii?q?X8kNYdsAQEB?= Message-ID: <1493394641.6177.8.camel@tycho.nsa.gov> Subject: Re: [PATCH 2/3] selinux: add checksum to policydb From: Stephen Smalley To: Sebastien Buisson Cc: linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, selinux@tycho.nsa.gov, serge@hallyn.com, james.l.morris@oracle.com, Eric Paris , Paul Moore , Daniel Jurgens , Sebastien Buisson Date: Fri, 28 Apr 2017 11:50:41 -0400 In-Reply-To: References: <1493218936-18522-1-git-send-email-sbuisson@ddn.com> <1493218936-18522-2-git-send-email-sbuisson@ddn.com> <1493231426.32540.11.camel@tycho.nsa.gov> <1493306283.2524.17.camel@tycho.nsa.gov> <1493318826.2524.21.camel@tycho.nsa.gov> Organization: National Security Agency Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.22.6 (3.22.6-2.fc25) Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2017-04-28 at 17:16 +0200, Sebastien Buisson wrote: > 2017-04-27 20:47 GMT+02:00 Stephen Smalley : > > > I just checked, with the method of computing the checksum on a > > > (data, > > > len) pair on entry to security_load_policy() the checksum does > > > not > > > change after using setsebool. So it seems I would need to call > > > security_read_policy() to retrieve the binary representation of > > > the > > > policy as currently enforced by the kernel. Unless you can see > > > another > > > way? > > > > I don't think that's a viable option, since security_read_policy() > > is > > going to be expensive in order to generate a full policy image, > > while > > security_set_bools() is supposed to be substantially cheaper than a > > full policy load. > > > > Also, the advantage of taking the hash of the original input file > > is > > that you can independently compute a reference hash offline or on > > the > > server from the same policy file and compare them and you can > > identify > > which policy file was loaded based on the hash. > > > > If you care about the active boolean state, then I'd suggest > > hashing > > the active boolean state separately and storing that after the > > policy > > hash.  You can do that in both security_load_policy() and > > security_set_bools().  Just iterate through the bools like > > security_set_bools() does, write the ->state of each boolean into a > > buffer, and then hash that buffer. > > I just noticed another issue: with the method of computing the > checksum on a (data, len) pair on entry to security_load_policy(), > the > checksum does not change after inserting a new module with semodule. > It is a problem as a module can allow actions by certain users on > some > file contexts. So not detecting that kind of policy tampering defeats > the purpose of the checksum as I imagine it. You seem to be conflating kernel policy with userspace policy. security_load_policy() is provided with the kernel policy image, which is the result of linking the kernel-relevant portions of all policy modules together. A hash of that image will change if you insert a policy module that affects the kernel policy in any way. But a change that only affects userspace policy isn't ever going to be reflected in the kernel. It doesn't matter where or when you compute your checksum within the kernel; it isn't ever going to reflect those userspace policy changes. > To address this I propose to come back to the idea of the notifier. > The checksum would not be stored inside the struct policydb. The > checksum would be computed on a (data, len) pair got from > security_read_policy() every time someone is asking for it through > the > security_policy_cksum() hook. The ones that would potentially call > security_policy_cksum() are those that would register a callback on > lsm_notifier, and the userspace processes reading > /sys/fs/selinux/policycksum. So no matter if computing the checksum > gets expensive, that would be the caller's responsibility to use it > with care. Just like with /sys/fs/selinux/policy today in fact. This won't detect changes to userspace policy configurations either, and it is less efficient than just computing/updating the checksum in security_load_policy() and security_set_bools(). Also, if all you want is a hash of /sys/fs/selinux/policy, then userspace can already read and hash that itself at any time. You aren't really providing any additional information that way. In contrast, saving and providing a hash of the policy image that was loaded is not something that is currently available, and could be useful in checking against a reference hash of the policy file or in identifying which policy file was loaded. > > > > You needed to get (global) enforcing mode too, didn't > > > > you?  That's > > > > separate from the policy. > > > > > > Exactly, I also need to rework the patch I proposed about this, > > > in > > > light of the comments I received. > > > > So perhaps what you really want is a hook interface and a selinuxfs > > interface that returns a single string that encodes all of the > > policy > > properties that you care about?  Rather than separate hooks and > > interfaces?  You could embed the enforcing status in the string > > too. > > Should probably include checkreqprot as well since that affects > > enforcement of mmap/mprotect checks. > > True, I should build a string of the form: > <0 or 1 for enforce>:<0 or 1 for checkreqprot>:= checksum> > I should probably rename it 'policybrief' instead of 'policycksum'. > > I realize that the 'SELinux user to UNIX user' assignments are > important as well. If for instance a regular user on a given cluster > node is mapped to unconfined_u instead of user_u, this user would > erroneously have major privileges. I do not know where I should look > for this information, and possibly compute another checksum. As above, that's userspace policy configuration, and not something that kernel can or should deal with.