From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762513AbdEWTtC (ORCPT ); Tue, 23 May 2017 15:49:02 -0400 Received: from emsm-gh1-uea10.nsa.gov ([8.44.101.8]:18988 "EHLO emsm-gh1-uea10.nsa.gov" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760227AbdEWTtA (ORCPT ); Tue, 23 May 2017 15:49:00 -0400 X-IronPort-AV: E=Sophos;i="5.38,383,1491264000"; d="scan'208";a="7372580" IronPort-PHdr: =?us-ascii?q?9a23=3AXZn6khLriIp4I7KrmNmcpTZWNBhigK39O0sv0rFi?= =?us-ascii?q?tYgXK/vyrarrMEGX3/hxlliBBdydsKMbzbCH+Pm7CSQp2tWoiDg6aptCVhsI24?= =?us-ascii?q?09vjcLJ4q7M3D9N+PgdCcgHc5PBxdP9nC/NlVJSo6lPwWB6nK94iQPFRrhKAF7?= =?us-ascii?q?Ovr6GpLIj8Swyuu+54Dfbx9GiTe5Y75+Ngu6oAHeusULj4ZvKbs6xwfUrHdPZ+?= =?us-ascii?q?lY335jK0iJnxb76Mew/Zpj/DpVtvk86cNOUrj0crohQ7BAAzsoL2465MvwtRne?= =?us-ascii?q?VgSP/WcTUn8XkhVTHQfI6gzxU4rrvSv7sup93zSaPdHzQLspVzmu87tnRRn1gy?= =?us-ascii?q?ocKTU37H/YhdBxjKJDoRKuuRp/w5LPYIqIMPZyZ77Rcc8GSWZEWMteWTZBAoeh?= =?us-ascii?q?ZIURCeQPM/tTo43kq1cQsReyAA+hD/7txDBVnH/7xa403fkhHw/Y0gIvHdwOsH?= =?us-ascii?q?PIo9vyO6gcXvu4zLXKwDjZc/9bwyvx5YrOfxs8of+MR7Vwcc/JxEcyCwPKkE2Q?= =?us-ascii?q?qYz7MDOTy+8Drm2b4PBkVeKrlWEmqxx6rz+0xsgxkYnEnZ4Vy1DY+iV5x4Y5P9?= =?us-ascii?q?u4SFVhbtK+H5tQsD+aOpJwT8g/QG9ooD43xqAJtJO0ZiQHyIkrywTBZ/GIbYSE?= =?us-ascii?q?+A/vWeCMKjlinn1lYqiwhxOq/Eilze3zS9e73U5RripAjtnMrncN1wHP6sSfSv?= =?us-ascii?q?ty4EOh2TGX2gDP8O5EO0E0lbfAK5I73r4xloYcsUTEHiPsnkX5kLSWeVk+9uit?= =?us-ascii?q?6uTnZq3qpp6aN4BqlgHzKrkil8OwDOgiMgUCQnKX9fqz2bH950H1Xa1Gjvgsna?= =?us-ascii?q?nYtJDaK94bpqm8AwJNyYYs9g2/Aiy60NUYgXYHLFVFdAiBj4jyIV7COv/4DfCh?= =?us-ascii?q?g1i0ijdk2+jGPqH9ApXKNnXDkq3ufbNj5E5H0gYzycpT55dTCrEbOvLzW1Txuc?= =?us-ascii?q?ffDh8jKQO73+LnB8tn2owCXmKPB7eTMLnOvl+Q+uIvP+6MaZcOuDnmNvgl5uXu?= =?us-ascii?q?jWQ+mV8bZqSmwIYYaHaiEvt6JEWZZGLmgs0dHmcSogo+UOvqhUWGUT5SYXayQq?= =?us-ascii?q?096iggCI24EYjDW5qtgL2d3Ca7B5FWY2dGBU2REXfsaYqJQOkMaC2MLc97iDAE?= =?us-ascii?q?VqauS5Un1R6wsA/20b1nLvDb+icAr5LsyMB15/HPlRE17TF0C8Wd02eQT2B7h2?= =?us-ascii?q?8IRCE53Lp5oUNjzleOyrZ4g/NGGtxJ/f9JURk1NYTaz+NkD9D+QAXBfs2GSFy+?= =?us-ascii?q?WNWpHSkxTs4tw98Je0t9A8+tjg3H3yexG78ajaGLBJgt/qLZ2HjxINx9xGjc2K?= =?us-ascii?q?Y9iFkmR9NFNXe6ia5n6wjTG4nJnl2Cl6mxaKQc3TXN9HyEzWqIpk1XTRN/UaPe?= =?us-ascii?q?UHAQY0vZt9X55kfYQ7CyDrQnN1gJ9cnXEaZAY8b1jFhADN3+Oc/FZGT5z3y6GB?= =?us-ascii?q?eT3bSKKobmfU0S2SzcDA4PlAVFuT6+PBU6TgKmpHjTRGh2HE/rS1vl7O07rXS8?= =?us-ascii?q?VEJyxAaPOR5Pzb2wryUJiOScRvVb5bcNvCMsun0gB1qm987HANqH4Qx6deNTZs?= =?us-ascii?q?1rswQP7n7QqwEoZs/oFKtlnFNLNl0t504=3D?= X-IPAS-Result: =?us-ascii?q?A2EsAgA6kSRZ/wHyM5BcGgEBAQECAQEBAQgBAQEBFQEBAQE?= =?us-ascii?q?CAQEBAQgBAQEBgwEpgW6Db5pjBoEmmAaGJAKCX1cBAQEBAQEBAQIBAmgogjMkA?= =?us-ascii?q?YJBAQUjRBIQCw0BCgICERUCAlcGARIbh22CEQ2sdYImJgKLHgEBAQEBAQEDAQE?= =?us-ascii?q?BAQEBIoELhQ6FQIROVYJTgmAFnhuTJoIFiRmGVIkBi0pYgQomCQIeCCAPh1gkN?= =?us-ascii?q?okeAQEB?= Message-ID: <1495569200.8461.10.camel@tycho.nsa.gov> Subject: Re: [PATCH v6 1/2] selinux: add brief info to policydb From: Stephen Smalley To: Sebastien Buisson , Paul Moore Cc: selinux@tycho.nsa.gov, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, Sebastien Buisson , James Morris Date: Tue, 23 May 2017 15:53:20 -0400 In-Reply-To: References: <1495040944-11552-1-git-send-email-sbuisson@ddn.com> <1495116094.5475.4.camel@tycho.nsa.gov> <1495120065.5475.10.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: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2017-05-23 at 18:29 +0200, Sebastien Buisson wrote: > Hi, > > 2017-05-18 23:49 GMT+02:00 Paul Moore : > > My apologies to you and Sebastien for not reviewing these patches > > sooner. > > It is ok, no problem. > Thanks for all the advice from you and Stephen. I will try to take > all > this into account. > > As I understand it, I should not give the choice to allocate or not > the string returned by security_policydb_brief(). The initial reason > for this was that Lustre client code is expected to retrieve policy > brief info hundreds or thousands of times per second, so saving on > memory allocation would make sense. So if security_policydb_brief() > necessarily allocates memory for the string returned, and I > appreciate > it helps maintenance and avoids complexity, it should not be called > so > often. > One way to tackle this is to rely on the notification system: Lustre > client code would call security_policydb_brief() only when it gets a > change notification, and stores the current policy brief info > internally. > Another way could be to add another hook to check policy brief info > validity. It would take a string as an input parameter, and return 0 > if it matches the current policy. So Lustre client code would > systematically call this hook, and only call > security_policydb_brief() > when the policy has changed, to store the current value internally. > > I have recently identified a new need from Lustre client code. We > need > to protect against the case where the policy is changed or set in > permissive mode, and then set back to its previous state, to > workaround policy check as carried out on server side based on policy > brief info sent by client. In this scenario, the policy would only be > the expected one by the time the client sends a request to the server > (for instance a file open request), but not after that when SELinux > actually checks the permissions on the client (via > security_file_open() in this example). > A solution to address this could be to add a new parameter to > security_policydb_brief() hook, in the form of a pointer to an > integer > giving the current sequence number of the policy. That would > complement the policy brief info, with the notion of change to the > policy. I do not think it is desirable to include the sequence number > in the policy brief info, as it is not the essence of the policy. > Now with this sequence info in mind, the new hook to check policy > brief info validity would only need to check the sequence, instead of > the policy brief string. The current value of the sequence info > should > be stored by Lustre internally, and checked after SELinux permission > checks. If a change is detected, Lustre client must stop normal > processing and return an error for the current request. Not sure about your threat model but I think you are fighting a losing battle there. A malicious admin has too many ways to defeat your checking. Relying on the seqno also seems brittle; you could easily end up causing a client to fail just because a policy update happened to be installed at the same time, even though there was nothing wrong or malicious about the policy update itself.