Received: by 2002:ac0:a594:0:0:0:0:0 with SMTP id m20-v6csp4615396imm; Fri, 18 May 2018 07:59:56 -0700 (PDT) X-Google-Smtp-Source: AB8JxZo/vTQep197Cxw4g5kgCAwE21uINUhh0GnjJ4W8rXns0kcpUP5ls5a94ovAry5Z1PUQT7f0 X-Received: by 2002:a63:a51b:: with SMTP id n27-v6mr8016158pgf.47.1526655596420; Fri, 18 May 2018 07:59:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526655596; cv=none; d=google.com; s=arc-20160816; b=fPBL+vzXPxrm2yI85xYBeTdaT5hGUKjpQBGl1redXnWq7ggCuR2XTMZai47YdnZ7xJ XrKUb49bLH3L1I2ofzSj6a9tb9UyQUFcZw5lD1eMB3Clfyq50V1H7BN+TKTrnGF5WffR 2KTxrB6OQcwem/AfsipMUZLP4FyXmURxXABZTgjHMmgDDjiZRNwXhTvZrzYVDO4WlTOX NJG1aO1RaibFTB+UVeBmwUCGQzIcD7CG3IpxEJC/B5J7lcUsAdQbrqn4Au/hUjbCDsR5 +VaXfpHsnzo/H6WnOcIZEsnAgSjldfcxkagLWxZD8pc7+JDu2zTdYsuFD9IjQwSDUTp3 M1Ew== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-language :content-transfer-encoding:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:dkim-signature :arc-authentication-results; bh=o+rpF1WG9UDz4JlTtNGVzwEHdHWe2jyYCzNqnBxHagY=; b=adGmtAx8S/8z+LFgO2i02w91FXLx5FLa83rR7grbc8m8n1DghFdbhORtN03tasYLMM 4wfmc4rrcd2x1zY6wS5dxjlRveAGPKZw9uQI/uv9zSi/nKsXrq6oNhe9cLjNV1o9YnKp zAgKZsOCszf5ORuJ+GL5guTX8zITBmwCAJeCkiK/KasBbmLYnDlG4VAhVVU35Xi1ceUW lrE3h58mpfg4FtLoX1n6D43Dd31hunHFFKS64Q67tY/T+9PFp9iwauduUi1ajJqSNK6u heN5VVYQzq+OEev2f8DXC7bSiMyXC+4TbjWYLf1sdTHwXSUtMCxgtXrWxndnk4+PCLg2 speg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@yahoo.com header.s=s2048 header.b=hGgt5vpe; 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 y8-v6si4930185pgs.351.2018.05.18.07.59.18; Fri, 18 May 2018 07:59:56 -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=@yahoo.com header.s=s2048 header.b=hGgt5vpe; 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 S1751581AbeERO7E (ORCPT + 99 others); Fri, 18 May 2018 10:59:04 -0400 Received: from sonic306-27.consmr.mail.ne1.yahoo.com ([66.163.189.89]:41929 "EHLO sonic306-27.consmr.mail.ne1.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750977AbeERO67 (ORCPT ); Fri, 18 May 2018 10:58:59 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1526655538; bh=o+rpF1WG9UDz4JlTtNGVzwEHdHWe2jyYCzNqnBxHagY=; h=Subject:To:Cc:References:From:Date:In-Reply-To:From:Subject; b=hGgt5vpe6nRuteZwnBE2jAT27JTHKvTr/kY7xTryq1AMcQhwrn2JNYrM4WON/U2rP5N3T9cc9+BEEwLAvszqcQsEWuE4885Wodp1I7cxIs/F91jYhBZ9W5L3c4gOWi8wMXwwzAFR3e7HQfx7NFqH8HUwCGbhTPxhxIAhR5krmrqzOy/LFw7MeIGK2K+37StiY5IzTEM+cu/cq2S0qk9/ZUu6ciG6ECMoC/qJXn3Tov0qBAzww8nm334aLZe5B3zsBDI49RiFc19Nuo2bB+DmDa7V8Wyd5m+IICgdNmp8XwHdCedEX1vnbYNaYAtI+bQ/Co90z4/r3OPGzbUqLHlYbQ== X-YMail-OSG: tZig9hcVM1nVYkLuROamJO4GqOtLiHQIww9rbsBErU.YE1pfWBmXPTMOxDFyo8I R50IQ5QI2qoky1Wn9Cf_fIBXJZE0p_JZnZV1oqISKAcWoCItVvoKqxq3Nd89AZdP6ieydTDlhHD3 W818TxB9Mb1myMyzVg0USkUL7txUqclp5fp._A.VBYlwJ.2_T6ruU5xO_Ik4s4HQoRprCwHgFduu 6_uWhYqOVNVS3zmrIGlse4WLkxYZ4y4sYq6qfI4xl.Q2Ngus1RjAfOZZsDZOM980Gx2NZFNjGM7J I.nbpK_D7PAaXs5Jn33N.AnheXjtgkSrHbFVzl_04kY8iJBBUNkYa8OoTOsyQcevGhG6RjU6nrlj PLyUsesRwofu6lzruy3dIJYTvIuI1lC6ezcnxRoVaveXNr5ofgoEvEyiIiSp7yIoMnC.3gx0OYb8 zPSo4Srbwv02orrxXRCDm7bIOy9nl_HvD_pz9F_dFUTXc70dUdit8fKbNtTZQa628ZVtqr8_EZBz YQVs57fYSUh8ug3GUybxOZBxhv5ejlnn43cYAMkVTx.kXN8Modzw57gGJDUGVNm0tSS7gJZ8LPEr .0CTHSFttbevhpGJTvykrh3X4.uMskyMOX3yqBGsoXs3Mf2EEcdj_nIQOPbfg2A2HpmbqMFUlnnU xd1_AoOsNxg-- Received: from sonic.gate.mail.ne1.yahoo.com by sonic306.consmr.mail.ne1.yahoo.com with HTTP; Fri, 18 May 2018 14:58:58 +0000 Received: from c-67-169-65-224.hsd1.ca.comcast.net (EHLO [192.168.0.105]) ([67.169.65.224]) by smtp405.mail.ne1.yahoo.com (Oath Hermes SMTP Server) with ESMTPA ID e035e62cc670eb569f1a2c28617031c4; Fri, 18 May 2018 14:58:58 +0000 (UTC) Subject: Re: [PATCH v2 3/9] security: define security_kernel_read_blob() wrapper To: Mimi Zohar , "Eric W. Biederman" Cc: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, David Howells , "Luis R . Rodriguez" , kexec@lists.infradead.org, Andres Rodriguez , Greg Kroah-Hartman , Ard Biesheuvel , Kees Cook , Stephen Smalley References: <1526568530-9144-1-git-send-email-zohar@linux.vnet.ibm.com> <1526568530-9144-4-git-send-email-zohar@linux.vnet.ibm.com> <74c096ca-1ad1-799e-df3d-7b1b099333a7@schaufler-ca.com> <87y3ghhbws.fsf@xmission.com> <1526643050.3632.127.camel@linux.vnet.ibm.com> From: Casey Schaufler Message-ID: Date: Fri, 18 May 2018 07:58:55 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1526643050.3632.127.camel@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/18/2018 4:30 AM, Mimi Zohar wrote: > On Thu, 2018-05-17 at 22:37 -0500, Eric W. Biederman wrote: >> Casey Schaufler writes: >> >>> On 5/17/2018 7:48 AM, Mimi Zohar wrote: >>>> In order for LSMs and IMA-appraisal to differentiate between the original >>>> and new syscalls (eg. kexec, kernel modules, firmware), both the original >>>> and new syscalls must call an LSM hook. >>>> >>>> Commit 2e72d51b4ac3 ("security: introduce kernel_module_from_file hook") >>>> introduced calling security_kernel_module_from_file() in both the original >>>> and new syscalls. Commit a1db74209483 ("module: replace >>>> copy_module_from_fd with kernel version") replaced these LSM calls with >>>> security_kernel_read_file(). >>>> >>>> Commit e40ba6d56b41 ("firmware: replace call to fw_read_file_contents() >>>> with kernel version") and commit b804defe4297 ("kexec: replace call to >>>> copy_file_from_fd() with kernel version") replaced their own version of >>>> reading a file from the kernel with the generic >>>> kernel_read_file_from_path/fd() versions, which call the pre and post >>>> security_kernel_read_file LSM hooks. >>>> >>>> Missing are LSM calls in the original kexec syscall and firmware sysfs >>>> fallback method. From a technical perspective there is no justification >>>> for defining a new LSM hook, as the existing security_kernel_read_file() >>>> works just fine. The original syscalls, however, do not read a file, so >>>> the security hook name is inappropriate. Instead of defining a new LSM >>>> hook, this patch defines security_kernel_read_blob() as a wrapper for >>>> the existing LSM security_kernel_file_read() hook. >>> What a marvelous opportunity to bikeshed! >>> >>> I really dislike adding another security_ interface just because >>> the name isn't quite right. Especially a wrapper, which is just >>> code and execution overhead. Why not change security_kernel_read_file() >>> to security_kernel_read_blob() everywhere and be done? >> Nacked-by: "Eric W. Biederman" >> >> Nack on this sharing nonsense. These two interfaces do not share any >> code in their implementations other than the if statement to distinguish >> between the two cases. >> >> Casey you are wrong. We need something different here. >> >> Mimi a wrapper does not cut it. The code is not shared. Despite using >> a single function call today. >> >> If we want comprehensible and maintainable code in the security modules >> we need to split these two pieces of functionality apart. > kernel_read_file() is a common, generic method of reading a file from > the kernel, which is being called from a number of places.  The > kernel_read_file_id enumeration is needed to differentiate between the > callers.  The purpose of the new security_kernel_read_file() calls is > not for the kernel to read a file, but as a method of identifying the > original buffer based methods containing a file. > > Having to define a separate LSM hook for each of the original, non > kernel_read_file(), buffer based method callers, kind of makes sense, > as the callers themselves are specific, but is it really necessary? > Could we define a new, generic LSM hook named > security_kernel_buffer_data() for this purpose? If there are two disparate behaviors wrapped into kernel_read_file() Eric (bless him) is right. It should be broken into two hooks. I think that if we look (too) carefully we'll find other places where hooks should get broken up, or combined*. My question is just how important is it that this gets changed? --- * calling security_inode_secid() and then immediately security_secid_to_secctx() grates on my sensibilities.